RE: [RFC] Sanitize rtx_addr_can_trap_p_1

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

Hi,

I have here an updated patch, which improves two things:

First it moves debug code out to an extra patch, as suggested by Jakub.

Secondly, it fixes the checks on STACK_GROWS_DOWNWARD and
ARGS_GROW_DOWNWARD.  Previously these used to be conditionally defined
symbols, but recently they were changed to be always defined, but with 0 or 1.

That made all #ifndef checks on those two flags work the wrong way, and it caused
most of the false positives in the previous version.

Now the number of false positives in the stage2 drops significantly to only 4 new ones:

*** argp can trap: function=uw_init_context_1, pass=reload, offset=236, size=4, low_bound=-16, high_bound=16
*** argp can trap: function=uw_init_context_1, pass=reload, offset=236, size=4, low_bound=-16, high_bound=16
*** argp can trap: function=uw_init_context_1, pass=reload, offset=440, size=8, low_bound=-16, high_bound=16
*** argp can trap: function=uw_init_context_1, pass=reload, offset=440, size=8, low_bound=-16, high_bound=16

These came from libgcc/unwind-dw2.c

They seem to have the same reason as the 2930 "frame can trap" warnings that were already
there before the patch.


All these seem to happen due to some hidden bug.  In the debugger the call stack looks always like this:

#0  rtx_addr_can_trap_p_1 (x=0x7ffff6ecb378, offset=440, size=8, mode=DImode, unaligned_mems=false) at ../../gcc-trunk/gcc/rtlanal.c:671
#1  0x0000000000d90f4a in rtx_addr_can_trap_p_1 (x=0x7ffff67ac7f8, offset=0, size=8, mode=DImode, unaligned_mems=false) at ../../gcc-trunk/gcc/rtlanal.c:699
#2  0x0000000000d953c4 in may_trap_p_1 (x=0x7ffff67ac810, flags=0) at ../../gcc-trunk/gcc/rtlanal.c:2760
#3  0x0000000000d95619 in may_trap_p_1 (x=0x7ffff671ac90, flags=0) at ../../gcc-trunk/gcc/rtlanal.c:2838
#4  0x0000000000d956cc in may_trap_p (x=0x7ffff671ac90) at ../../gcc-trunk/gcc/rtlanal.c:2857
#5  0x0000000000c6b2ab in process_bb_lives (bb=0x7ffff6c3a958, curr_point=@0x7fffffffd8e4: 23, dead_insn_p=true) at ../../gcc-trunk/gcc/lra-lives.c:698
#6  0x0000000000c6d19a in lra_create_live_ranges_1 (all_p=true, dead_insn_p=true) at ../../gcc-trunk/gcc/lra-lives.c:1262
#7  0x0000000000c6d47c in lra_create_live_ranges (all_p=true, dead_insn_p=true) at ../../gcc-trunk/gcc/lra-lives.c:1327
#8  0x0000000000c4a253 in lra (f=0x24f7940) at ../../gcc-trunk/gcc/lra.c:2309
#9  0x0000000000bf3d25 in do_reload () at ../../gcc-trunk/gcc/ira.c:5401
#10 0x0000000000bf40d3 in (anonymous namespace)::pass_reload::execute (this=0x23fce20) at ../../gcc-trunk/gcc/ira.c:5572
#11 0x0000000000d08e37 in execute_one_pass (pass=0x23fce20) at ../../gcc-trunk/gcc/passes.c:2359
#12 0x0000000000d09081 in execute_pass_list_1 (pass=0x23fce20) at ../../gcc-trunk/gcc/passes.c:2412
#13 0x0000000000d090b2 in execute_pass_list_1 (pass=0x23fbda0) at ../../gcc-trunk/gcc/passes.c:2413
#14 0x0000000000d090ef in execute_pass_list (fn=0x7ffff7044c78, pass=0x23f8bc0) at ../../gcc-trunk/gcc/passes.c:2423
#15 0x00000000008de80c in cgraph_node::expand (this=0x7ffff6c09498) at ../../gcc-trunk/gcc/cgraphunit.c:1937
#16 0x00000000008dee3d in expand_all_functions () at ../../gcc-trunk/gcc/cgraphunit.c:2073
#17 0x00000000008df954 in symbol_table::compile (this=0x7ffff6ecf000) at ../../gcc-trunk/gcc/cgraphunit.c:2426
#18 0x00000000008dfb68 in symbol_table::finalize_compilation_unit (this=0x7ffff6ecf000) at ../../gcc-trunk/gcc/cgraphunit.c:2513
#19 0x0000000000e094ba in compile_file () at ../../gcc-trunk/gcc/toplev.c:580
#20 0x0000000000e0b9fa in do_compile () at ../../gcc-trunk/gcc/toplev.c:2070
#21 0x0000000000e0bc46 in toplev::main (this=0x7fffffffdc50, argc=35, argv=0x7fffffffdd58) at ../../gcc-trunk/gcc/toplev.c:2171
#22 0x00000000016b656d in main (argc=35, argv=0x7fffffffdd58) at ../../gcc-trunk/gcc/main.c:39

I cannot find any instruction in the rtl dumps that corresponds to this large argp offset.  So I think there must be
something wrong along the call stack above, which looks identically even on the bogus frame pointer references.


Is this patch OK for trunk now?

At least Jakub and I are in favour of it, Eric is against it.
That makes 2:1 ...


Thanks
Bernd.

Attachment: changelog-pr60147.txt
Description: Text document

Attachment: patch-pr61047.diff
Description: Binary data

Attachment: patch-pr61047-debug.diff
Description: Binary data