On Tue, 9 Nov 2021, Jakub Jelinek wrote:
> On Tue, Nov 09, 2021 at 10:44:45AM +0100, Jakub Jelinek via Gcc-patches wrote:
> > On Tue, Nov 09, 2021 at 08:12:05AM +0100, Richard Biener wrote:
> > > > So, here is 1), 2), 3) implemented. With this patch alone,
> > > > g++.dg/pch/system-2.C test ICEs. This is because GCC 12 has added
> > > > function::x_range_query member, which is set to &global_ranges on
> > > > cfun creation and is:
> > > > range_query * GTY ((skip)) x_range_query;
> > > > which means when a PIE binary writes PCH and a PIE binary loaded
> > > > at a different address loads it, cfun->x_range_query might be a garbage
> > > > pointer. We can either apply a patch like the attached one after
> > > > this inline patch, but then probably callback is misnamed and we should
> > > > rename it to relocate_and_skip or something similar. Or we could
> > > > e.g. during gimplification overwrite cfun->x_range_query =
> > > > &global_ranges.
> > >
> > > I think struct function allocation should initialize it to NULL and
> > > the init to &global_ranges be done only when we do init_tree_ssa?
> > > In fact x_range_query could be moved to the gimple_df substructure
> > > to make that clear.
> >
> > Agreed, Andrew/Aldy, what do you think?
> >
> > > Hopefully PCH happens before init_tree_ssa.
> >
> > I think it does.
>
> Unfortunately, seems cfun->x_range_query is used already in the FEs :(.
>
> I was trying:
>
> --- gcc/function.h.jj 2021-08-31 22:55:23.072795814 +0200
> +++ gcc/function.h 2021-11-09 11:33:22.656779018 +0100
> @@ -312,8 +312,9 @@ struct GTY(()) function {
>
> /* Range query mechanism for functions. The default is to pick up
> global ranges. If a pass wants on-demand ranges OTOH, it must
> - call enable/disable_ranger(). The pointer is never null. It
> - should be queried by calling get_range_query(). */
> + call enable/disable_ranger(). The pointer is never null in between
> + init_tree_ssa and delete_tree_ssa. It should be queried by calling
> + get_range_query(). */
> range_query * GTY ((skip)) x_range_query;
>
> /* Last statement uid. */
> --- gcc/function.c.jj 2021-07-20 22:31:11.088835781 +0200
> +++ gcc/function.c 2021-11-09 11:33:47.695424319 +0100
> @@ -4873,8 +4873,6 @@ allocate_struct_function (tree fndecl, b
> binding annotations among them. */
> cfun->debug_nonbind_markers = lang_hooks.emits_begin_stmt
> && MAY_HAVE_DEBUG_MARKER_STMTS;
> -
> - cfun->x_range_query = &global_ranges;
> }
>
> /* This is like allocate_struct_function, but pushes a new cfun for FNDECL
> --- gcc/tree-ssa.c.jj 2021-11-03 23:02:44.367985554 +0100
> +++ gcc/tree-ssa.c 2021-11-09 12:02:07.095351378 +0100
> @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "attribs.h"
> #include "asan.h"
> +#include "value-query.h"
>
> /* Pointer map of variable mappings, keyed by edge. */
> static hash_map<edge, auto_vec<edge_var_map> > *edge_var_maps;
> @@ -1224,6 +1225,7 @@ init_tree_ssa (struct function *fn, int
> {
> fn->gimple_df = ggc_cleared_alloc<gimple_df> ();
> fn->gimple_df->default_defs = hash_table<ssa_name_hasher>::create_ggc (20);
> + fn->x_range_query = &global_ranges;
> pt_solution_reset (&fn->gimple_df->escaped);
> init_ssanames (fn, size);
> }
> @@ -1246,6 +1248,7 @@ delete_tree_ssa (struct function *fn)
> delete fn->gimple_df->decls_to_pointers;
> fn->gimple_df->decls_to_pointers = NULL;
> fn->gimple_df = NULL;
> + fn->x_range_query = NULL;
>
> /* We no longer need the edge variable maps. */
> redirect_edge_var_map_empty ();
>
> but that ICEs with:
> #0 0x0000555556d27348 in get_range (val=val@entry=0x7fffe9f8c2d0,
> stmt=0x7fffffffbb80, stmt@entry=0x0, minmax=minmax@entry=0x7fffffffbc10,
> rvals=0x0)
> at ../../gcc/tree-ssa-strlen.c:217
> #1 0x0000555556a2fe73 in get_offset_range (x=0x7fffe9f8c2d0, stmt=0x0,
> r=0x7fffffffbd70, rvals=<optimized out>) at ../../gcc/pointer-query.cc:92
> #2 0x0000555556a33d3e in handle_array_ref (aref=0x7fffe7e17620,
> addr=<optimized out>, ostype=1, pref=0x7fffffffc000, snlim=...,
> qry=<optimized out>, stmt=<optimized out>)
> at ../../gcc/pointer-query.cc:1621
> #3 0x0000555556a3669d in compute_objsize (ptr=0x7fffe81b3100,
> stmt=<optimized out>, ostype=1, pref=0x7fffffffc000, ptr_qry=0x7fffffffbf00)
> at ../../gcc/pointer-query.cc:2154
> #4 0x0000555556a368e4 in compute_objsize (ptr=ptr@entry=0x7fffe81b3100,
> stmt=stmt@entry=0x0, ostype=ostype@entry=1, pref=pref@entry=0x7fffffffc000,
> rvals=rvals@entry=0x0)
> at ../../gcc/pointer-query.cc:2172
> #5 0x0000555556383f09 in compute_objsize (pref=0x7fffffffc000, ostype=1,
> ptr=0x7fffe81b3100) at ../../gcc/pointer-query.h:262
> #6 warn_placement_new_too_small (type=0x7fffe9f8a3f0, nelts=0x7fffe81b3160,
> size=0x7fffe9f8c108, oper=0x7fffe81b3100) at ../../gcc/cp/init.c:2621
> #7 0x000055555638cf9e in build_new_1 (placement=<optimized out>,
> type=0x7fffe9f8a3f0, nelts=<optimized out>, init=0x7fffffffc3d0,
> globally_qualified_p=<optimized out>, complain=3)
> at ../../gcc/cp/init.c:3287
> #8 0x000055555638dd92 in build_new (loc=<optimized out>,
> placement=placement@entry=0x7fffffffc3c8, type=<optimized out>,
> type@entry=0x7fffe9f8a3f0, nelts=0x7fffe81b3160,
> nelts@entry=0x7fffe81b3120, init=init@entry=0x7fffffffc3d0,
> use_global_new=use_global_new@entry=0, complain=3) at ../../gcc/cp/init.c:3838
>
> Apparently the range_of_expr can handle some tree cases through
> range_query::get_tree_range, like INTEGER_CSTs, ADDR_EXPRs,
> and some binary and unary ops.
But that shouldn't need a range query object ... this was all
available pre-ranger and just got stuffed there for no good reason?
Richard.
