[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410 Roman Zhuykov changed: What|Removed |Added CC||zhroma at gcc dot gnu.org --- Comment #10 from Roman Zhuykov --- (In reply to Martin Liška from comment #3) > Confirmed also with r247015 (GCC 7 branch point). (In reply to Martin Liška from comment #9) > I would close this as fixed. Asked about backporting, see https://gcc.gnu.org/ml/gcc-patches/2020-02/msg01523.html
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410 Martin Liška changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #9 from Martin Liška --- I would close this as fixed.
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410 --- Comment #8 from Matthew Malcomson --- Author: matmal01 Date: Mon Dec 9 12:03:53 2019 New Revision: 279124 URL: https://gcc.gnu.org/viewcvs?rev=279124=gcc=rev Log: [mid-end] Add notes to dataflow insn info when re-emitting (PR92410) In scheduling passes, notes are removed with `remove_notes` before the scheduling is done, and added back in with `reemit_notes` once the scheduling has been decided. This process leaves the notes in the RTL chain with different insn uid's than were there before. Having different UID's (larger than the previous ones) means that DF_INSN_INFO_GET(insn) will access outside of the allocated array. This has been seen in the `regstat_bb_compute_calls_crossed` function. This patch adds an assert to the `regstat_bb_compute_calls_crossed` function so that bad accesses here are caught instead of going unnoticed, and then avoids the problem. We avoid the problem by ensuring that new notes added by `reemit_notes` have an insn record given to them. This is done by adding a call to `df_insn_create_insn_record` on each note added in `reemit_notes`. `df_insn_create_insn_record` leaves this new record zeroed out, which appears to be fine for notes (e.g. `df_bb_refs_record` already does not set anything except the luid for notes, and notes have no dataflow information to record). We add the testcase that Martin found here https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410#c2 . This testcase fails with the "regstat.c" change, and then succeeds with the "haifa-sched.c" change. There is a similar problem with labels, that the `gcc_assert` catches when running regression tests in gcc.dg/fold-eqandshift-1.c and gcc.c-torture/compile/pr32482.c. This is due to the `cfg_layout_finalize` call in `bb-reorder.c` emitting new labels, and these labels not having a dataflow df_insn_info member. We solve this by manually calling `df_recompute_luids` on each basic block once this pass has finished. Testing done: Ran regression tests on aarch64-none-linux-gnu cross compiler. Bootstrapped and ran tests on aarch64-none-linux-gnu native. gcc/ChangeLog: 2019-12-09 Matthew Malcomson PR middle-end/92410 * bb-reorder.c (pass_reorder_blocks::execute): Recompute dataflow luids once basic blocks have been reordered. * haifa-sched.c (reemit_notes): Create df insn record for each new note. * regstat.c (regstat_bb_compute_calls_crossed): Assert every insn has an insn record before trying to use it. gcc/testsuite/ChangeLog: 2019-12-09 Matthew Malcomson PR middle-end/92410 * gcc.dg/torture/pr92410.c: New test. Added: trunk/gcc/testsuite/gcc.dg/torture/pr92410.c Modified: trunk/gcc/ChangeLog trunk/gcc/bb-reorder.c trunk/gcc/haifa-sched.c trunk/gcc/regstat.c trunk/gcc/testsuite/ChangeLog
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410
--- Comment #7 from Matthew Malcomson ---
(In reply to Matthew Malcomson from comment #6)
> I'm not sure whether there's any pre-existing "should not use dataflow
> queries on notes" rule. If there is, then the
> regstat_bb_compute_calls_crossed function should be modified to check for
> NONDEBUG_INSN_P and continue earlier on its loop.
I now see that `df_bb_refs_record` generates insn info for notes (but leaves it
mostly zeroed out). I figure doing the same for the notes emitted by
`reemit_notes` seems reasonable.
Currently bootstrapping and regtesting (both with HWASAN and without) the
following patch.
diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 41cf1f3..2e1a84f 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -5433,6 +5433,7 @@ reemit_notes (rtx_insn *insn)
last = emit_note_before (note_type, last);
remove_note (insn, note);
+ df_insn_create_insn_record (last);
}
}
}
diff --git a/gcc/regstat.c b/gcc/regstat.c
index 4da9b7c..c6cefb11 100644
--- a/gcc/regstat.c
+++ b/gcc/regstat.c
@@ -324,6 +324,7 @@ regstat_bb_compute_calls_crossed (unsigned int bb_index,
bitmap live)
FOR_BB_INSNS_REVERSE (bb, insn)
{
+ gcc_assert (INSN_UID (insn) < DF_INSN_SIZE ());
struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
unsigned int regno;
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410 --- Comment #6 from Matthew Malcomson --- I believe the problem is that `remove_notes` followed by `reemit_notes` can generate these notes with a different UID. When `reemit_notes` adds the new note, the dataflow information is not updated automatically because `add_insn_before` only updates the information for INSN_P(insn). Hence the later lookup of this dataflow information is problematic. I'm not sure whether there's any pre-existing "should not use dataflow queries on notes" rule. If there is, then the regstat_bb_compute_calls_crossed function should be modified to check for NONDEBUG_INSN_P and continue earlier on its loop. If there isn't such a rule then I guess the best approach would be to ensure we call `df_insn_create_insn_record` whenever calling `emit_note_before` or `emit_note_after` once the dataflow information has been created. (assuming that notes don't need the information to be populated since `df_insn_rescan` seems to ignore notes). I've tried both moving the check for NONDEBUG_INSN_P in `regstat_bb_compute_calls_crossed` and adding a call to `df_insn_create_insn_record` into `reemit_notes` on a cross-compiler and both pass the testcase Martin found.
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410
--- Comment #5 from Matthew Malcomson ---
I've had a little look into it, and the below seems promising:
Based on a comment in haifa-sched.c, notes are removed before scheduling and
added back in.
Since the insn that is larger than the df buffer is a note, and I saw in gdb
that it's added during `reemit_notes`, I figure the root problem might be that
the notes are removed, then the df->insns array is calculated, then notes are
added back in.
I hence tested the below patch, and the testcase that Martin found no longer
crashes.
I have not yet looked into whether `df_recompute_luids` is the correct function
to call or if there's a better approach. Just sharing an update.
diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 41cf1f3..564a358 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -6231,6 +6231,7 @@ commit_schedule (rtx_insn *prev_head, rtx_insn *tail,
basic_block *target_bb)
reemit_notes (insn);
last_scheduled_insn = insn;
}
+ df_recompute_luids(*target_bb);
scheduled_insns.truncate (0);
}
diff --git a/gcc/regstat.c b/gcc/regstat.c
index 4da9b7c..c6cefb11 100644
--- a/gcc/regstat.c
+++ b/gcc/regstat.c
@@ -324,6 +324,7 @@ regstat_bb_compute_calls_crossed (unsigned int bb_index,
bitmap live)
FOR_BB_INSNS_REVERSE (bb, insn)
{
+ gcc_assert (INSN_UID (insn) < DF_INSN_SIZE ());
struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
unsigned int regno;
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 8a1d414..5d8 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -4673,6 +4673,7 @@ sel_restore_notes (void)
if (NONDEBUG_INSN_P (insn))
reemit_notes (insn);
+ df_recompute_luids (first);
first = first->next_bb;
}
while (first != last);
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410 --- Comment #4 from Martin Liška --- Can be reproduced with a aarch64 cross compiler on x86-64-linux-gnu.
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410 Martin Liška changed: What|Removed |Added CC||vmakarov at gcc dot gnu.org Known to fail||10.0, 7.4.0 --- Comment #3 from Martin Liška --- Confirmed also with r247015 (GCC 7 branch point). @Vladimir: Can you please take a look?
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410
--- Comment #2 from Martin Liška ---
One can see it with the following patch:
diff --git a/gcc/regstat.c b/gcc/regstat.c
index 4da9b7cc523..c6cefb117d7 100644
--- a/gcc/regstat.c
+++ b/gcc/regstat.c
@@ -324,6 +324,7 @@ regstat_bb_compute_calls_crossed (unsigned int bb_index,
bitmap live)
FOR_BB_INSNS_REVERSE (bb, insn)
{
+ gcc_assert (INSN_UID (insn) < DF_INSN_SIZE ());
struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
unsigned int regno;
and with the following test-case:
$ cat /tmp/ice.i
int v;
int a() {
;
return v;
}
$ ./xgcc -B. /tmp/ice.i -O2 -c -g
during RTL pass: sched2
/tmp/ice.i: In function ‘a’:
/tmp/ice.i:6:1: internal compiler error: in regstat_bb_compute_calls_crossed,
at regstat.c:327
6 | }
| ^
0x10519d1 regstat_bb_compute_calls_crossed
../../gcc/regstat.c:327
0x1051c0e regstat_compute_calls_crossed()
../../gcc/regstat.c:380
0x1d30bbd sched_init()
../../gcc/haifa-sched.c:7337
0x1d30c21 haifa_sched_init()
../../gcc/haifa-sched.c:7354
0x10acbac schedule_insns()
../../gcc/sched-rgn.c:3514
0x10ad507 rest_of_handle_sched2
../../gcc/sched-rgn.c:3746
0x10ad6ce execute
../../gcc/sched-rgn.c:3882
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See
[Bug middle-end/92410] Invalid access to df->insns[] in regstat_bb_compute_calls_crossed (caught by hwasan)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410 Martin Liška changed: What|Removed |Added Keywords||needs-bisection Status|UNCONFIRMED |NEW Last reconfirmed||2019-11-08 CC||marxin at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from Martin Liška --- I can confirm the issue with ASAN: ==10808==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xaaf536f0 at pc 0x01cc3bdc bp 0xcfc5bb40 sp 0xcfc5bb58 READ of size 8 at 0xaaf536f0 thread T0 #0 0x1cc3bd8 in regstat_bb_compute_calls_crossed ../../gcc/regstat.c:327 #1 0x1cc43dc in regstat_compute_calls_crossed() ../../gcc/regstat.c:379 #2 0x3d89080 in sched_init() ../../gcc/haifa-sched.c:7335 #3 0x3d891cc in haifa_sched_init() ../../gcc/haifa-sched.c:7352 #4 0x1de31a8 in schedule_insns() ../../gcc/sched-rgn.c:3514 #5 0x1de5508 in rest_of_handle_sched2 ../../gcc/sched-rgn.c:3746 #6 0x1de5954 in execute ../../gcc/sched-rgn.c:3882 #7 0x1b8b500 in execute_one_pass(opt_pass*) ../../gcc/passes.c:2494 #8 0x1b8be8c in execute_pass_list_1 ../../gcc/passes.c:2580 #9 0x1b8bf10 in execute_pass_list_1 ../../gcc/passes.c:2581 #10 0x1b8bf10 in execute_pass_list_1 ../../gcc/passes.c:2581 #11 0x1b8bfcc in execute_pass_list(function*, opt_pass*) ../../gcc/passes.c:2591 #12 0xe9bb6c in cgraph_node::expand() ../../gcc/cgraphunit.c:2194 #13 0xe9cda4 in expand_all_functions ../../gcc/cgraphunit.c:2332 #14 0xe9f684 in symbol_table::compile() ../../gcc/cgraphunit.c:2688 #15 0xea0198 in symbol_table::finalize_compilation_unit() ../../gcc/cgraphunit.c:2868 #16 0x1f6a6f4 in compile_file ../../gcc/toplev.c:481 #17 0x1f74fd4 in do_compile ../../gcc/toplev.c:2166 #18 0x1f75a1c in toplev::main(int, char**) ../../gcc/toplev.c:2301 #19 0x407f270 in main ../../gcc/main.c:39 #20 0xaea6a3e8 in __libc_start_main (/lib64/libc.so.6+0x243e8) #21 0x87c324 (/home/marxin/Programming/gcc/objdir/gcc/cc1+0x87c324) 0xaaf536f0 is located 0 bytes to the right of 240-byte region [0xaaf53600,0xaaf536f0) allocated by thread T0 here: #0 0xaeeb9918 in malloc (/usr/lib64/libasan.so.5+0xe8918) #1 0x42a95bc in xrealloc ../../libiberty/xmalloc.c:177 #2 0xf26e1c in df_grow_insn_info() ../../gcc/df-scan.c:544 #3 0xf242bc in df_scan_alloc(bitmap_head*) ../../gcc/df-scan.c:262 #4 0x177e470 in do_reload ../../gcc/ira.c:5558 #5 0x177eeb0 in execute ../../gcc/ira.c:5681 #6 0x1b8b500 in execute_one_pass(opt_pass*) ../../gcc/passes.c:2494 #7 0x1b8be8c in execute_pass_list_1 ../../gcc/passes.c:2580 #8 0x1b8bf10 in execute_pass_list_1 ../../gcc/passes.c:2581 #9 0x1b8bfcc in execute_pass_list(function*, opt_pass*) ../../gcc/passes.c:2591 #10 0xe9bb6c in cgraph_node::expand() ../../gcc/cgraphunit.c:2194 #11 0xe9cda4 in expand_all_functions ../../gcc/cgraphunit.c:2332 #12 0xe9f684 in symbol_table::compile() ../../gcc/cgraphunit.c:2688 #13 0xea0198 in symbol_table::finalize_compilation_unit() ../../gcc/cgraphunit.c:2868 #14 0x1f6a6f4 in compile_file ../../gcc/toplev.c:481 #15 0x1f74fd4 in do_compile ../../gcc/toplev.c:2166 #16 0x1f75a1c in toplev::main(int, char**) ../../gcc/toplev.c:2301 #17 0x407f270 in main ../../gcc/main.c:39 #18 0xaea6a3e8 in __libc_start_main (/lib64/libc.so.6+0x243e8) #19 0x87c324 (/home/marxin/Programming/gcc/objdir/gcc/cc1+0x87c324) SUMMARY: AddressSanitizer: heap-buffer-overflow ../../gcc/regstat.c:327 in regstat_bb_compute_calls_crossed Shadow bytes around the buggy address: 0x200ff55ea680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa 0x200ff55ea690: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x200ff55ea6a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200ff55ea6b0: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa 0x200ff55ea6c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x200ff55ea6d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00[fa]fa 0x200ff55ea6e0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x200ff55ea6f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200ff55ea700: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa 0x200ff55ea710: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x200ff55ea720: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6
