Bug 82353 - [8 Regression] runtime ubsan crash
Summary: [8 Regression] runtime ubsan crash
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 8.0
: P1 normal
Target Milestone: 8.0
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2017-09-28 17:31 UTC by Dmitry Babokin
Modified: 2021-11-01 23:07 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-09-29 00:00:00


Attachments
reduced test case (759 bytes, application/x-xz)
2017-09-28 17:31 UTC, Dmitry Babokin
Details
original test case (184.25 KB, application/x-xz)
2017-09-28 17:33 UTC, Dmitry Babokin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Babokin 2017-09-28 17:31:28 UTC
Created attachment 42255 [details]
reduced test case

GCC trunk, rev 253244, x86_64.

> g++ -std=c++11 -fsanitize=undefined -fno-sanitize-recover=undefined -w -O2 ff.cpp dd.cpp -o out
> ./out
ff.cpp:20:27: runtime error: pointer index expression with base 0x0000006022f0 overflowed to 0x000000602300

Test case was reduced from a larger UB-free program.

I failed to reduce test case to a single file reproducer.
Comment 1 Dmitry Babokin 2017-09-28 17:33:05 UTC
Created attachment 42256 [details]
original test case

I'm also attaching original test case, just in case. For the bug to reproduce it's important to compile with -std=c++11 switch.

> g++ -std=c++11 -fsanitize=undefined -fno-sanitize-recover=undefined -w -O2 func.cpp driver.cpp -o out
> ./out
func.cpp:2448:96: runtime error: pointer index expression with base 0x00000063a3a0 overflowed to 0x00000063a3b8
Comment 2 Martin Liška 2017-09-29 08:23:19 UTC
Confirmed.
Comment 3 Jakub Jelinek 2017-09-29 11:48:48 UTC
Confirmed, but it is actually a RTL optimization bug, not a problem with the sanitization.
We have:
(insn 202 201 203 29 (set (reg:DI 98 [ _12 ])
        (symbol_ref:DI ("tf_2_struct_obj_2") [flags 0x40] <var_decl 0x7f6c98ff7480 tf_2_struct_obj_2>)) "pr82353-2.C":19 -1
     (nil))
(insn 203 202 204 29 (parallel [
            (set (reg:DI 143 [ _60 ])
                (plus:DI (reg:DI 98 [ _12 ])
                    (const_int 16 [0x10])))
            (clobber (reg:CC 17 flags))
        ]) "pr82353-2.C":19 -1
     (nil))
(insn 204 203 205 29 (set (reg:CC 17 flags)
        (compare:CC (reg:DI 98 [ _12 ])
            (const_int -16 [0xfffffffffffffff0]))) "pr82353-2.C":19 -1
     (nil))
(jump_insn 205 204 206 29 (set (pc)
        (if_then_else (ltu (reg:CC 17 flags)
                (const_int 0 [0]))
            (label_ref 212)
            (pc))) "pr82353-2.C":19 -1
     (int_list:REG_BR_PROB 39986 (nil))
 -> 212)
...
(code_label 212 211 213 31 22 (nil) [1 uses])
(note 213 212 214 31 [bb 31] NOTE_INSN_BASIC_BLOCK)
(insn 214 213 215 31 (set (mem/c:SI (symbol_ref:DI ("tf_2_var_233") [flags 0x40] <var_decl 0x7f6c98ff72d0 tf_2_var_233>) [9 tf_2_var_233+0 S4 A32])
        (reg:SI 92 [ _6 ])) "pr82353-2.C":19 -1
     (nil))
(insn 215 214 216 31 (set (reg:DI 114 [ _29 ])
        (symbol_ref:DI ("tf_2_struct_obj_2") [flags 0x40] <var_decl 0x7f6c98ff7480 tf_2_struct_obj_2>)) "pr82353-2.C":20 -1
     (nil))
(insn 216 215 217 31 (parallel [
            (set (reg:DI 118 [ _33 ])
                (plus:DI (reg:DI 114 [ _29 ])
                    (const_int 16 [0x10])))
            (clobber (reg:CC 17 flags))
        ]) "pr82353-2.C":20 -1
     (nil))
(insn 217 216 218 31 (set (reg:CC 17 flags)
        (compare:CC (reg:DI 114 [ _29 ])
            (const_int -16 [0xfffffffffffffff0]))) "pr82353-2.C":20 -1
     (nil))
(jump_insn 218 217 219 31 (set (pc)
        (if_then_else (ltu (reg:CC 17 flags)
                (const_int 0 [0]))
            (label_ref 225)
            (pc))) "pr82353-2.C":20 -1
     (int_list:REG_BR_PROB 39986 (nil))
 -> 225)

at expansion time (the reason for that is that sanopt pass doesn't perform the optimization of redundant pointer overflow checks yet (Martin, do you have it still on your todo list or shall I have a look?)).
Then fwprop1 figures out that insn 215 and 216 are redundant and removes them.
Then cse2 figures out the insn 217 is redundant and removes it.
This is still valid, there is a comparison that sets flags, then conditional jump to code_label 212 (fallthrough into __ubsan_pointer_overflow_abort call), there the store in insn 214 followed by a conditional jump which uses the still computed flags, though of course if we were smarter we could have changed it into unconditional jump at that point.
*.ira still looks correct, but then something in LRA isn't aware that the flags register is live on the edge from bb29 to bb31 and inside of bb31 until jump_insn 218, and rematerializes? there pseudo 92 which is stored in insn 214
and we end up with invalid:
(code_label 212 211 213 33 22 (nil) [1 uses])
(note 213 212 470 33 [bb 33] NOTE_INSN_BASIC_BLOCK)
(insn 470 213 466 33 (parallel [
            (set (reg:SI 0 ax [orig:92 _6 ] [92])
                (mult:SI (reg:SI 40 r11 [orig:91 _5 ] [91])
                    (const_int 89 [0x59])))
            (clobber (reg:CC 17 flags))
        ]) "pr82353-2.C":19 328 {*mulsi3_1}
     (nil))
(note 466 470 214 33 NOTE_INSN_DELETED)
(insn 214 466 218 33 (set (mem/c:SI (symbol_ref:DI ("tf_2_var_233") [flags 0x40] <var_decl 0x7f6c98ff72d0 tf_2_var_233>) [9 tf_2_var_233+0 S4 A32])
        (reg:SI 0 ax [orig:92 _6 ] [92])) "pr82353-2.C":19 82 {*movsi_internal}
     (nil))
(jump_insn 218 214 219 33 (set (pc)
        (if_then_else (ltu (reg:CC 17 flags)
                (const_int 0 [0]))
            (label_ref 225)
            (pc))) "pr82353-2.C":20 632 {*jcc_1}
     (int_list:REG_BR_PROB 39986 (nil))
 -> 225)
where the multiplication clobbers the flags and then we use them in insn 218.

Vlad, can you please have a look?  I'll still try to see if we can have a single file testcase here.
Comment 4 Martin Liška 2017-09-29 11:52:50 UTC
> at expansion time (the reason for that is that sanopt pass doesn't perform
> the optimization of redundant pointer overflow checks yet (Martin, do you
> have it still on your todo list or shall I have a look?)).

Yes I have, just finishing switch expansion and will return to that hopefully next week.

Martin
Comment 5 Vladimir Makarov 2017-09-29 19:41:24 UTC
(In reply to Jakub Jelinek from comment #3)

> Vlad, can you please have a look?  I'll still try to see if we can have a
> single file testcase here.

Yes, it seems as LRA rematerialization bug.  I'll work on it but the patch will be ready only in a week.
Comment 6 Vladimir Makarov 2017-10-11 19:36:19 UTC
Author: vmakarov
Date: Wed Oct 11 19:35:48 2017
New Revision: 253656

URL: https://gcc.gnu.org/viewcvs?rev=253656&root=gcc&view=rev
Log:
2017-10-11  Vladimir Makarov  <vmakarov@redhat.com>

	PR sanitizer/82353
	* lra.c (collect_non_operand_hard_regs): Don't ignore operator
	locations.
	* lra-lives.c (bb_killed_pseudos, bb_gen_pseudos): Move up.
	(make_hard_regno_born, make_hard_regno_dead): Update
	bb_killed_pseudos and bb_gen_pseudos.

2017-10-11  Vladimir Makarov  <vmakarov@redhat.com>

	PR sanitizer/82353
	* gcc.target/i386/i386.exp (tests): Permit '.C' extension.
	* gcc.target/i386/pr82353.C: New.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr82353.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lra-lives.c
    trunk/gcc/lra.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/i386/i386.exp
Comment 7 Jakub Jelinek 2017-10-12 07:22:43 UTC
Author: jakub
Date: Thu Oct 12 07:22:12 2017
New Revision: 253672

URL: https://gcc.gnu.org/viewcvs?rev=253672&root=gcc&view=rev
Log:
	PR target/82353
	* gcc.target/i386/i386.exp (tests): Revert the '.C' extension change.
	* gcc.target/i386/pr82353.C: Moved to ...
	* g++.dg/ubsan/pr82353.C: ... here.  Restrict to i?86/x86_64 && lp64.

Added:
    trunk/gcc/testsuite/g++.dg/ubsan/pr82353.C
      - copied, changed from r253671, trunk/gcc/testsuite/gcc.target/i386/pr82353.C
Removed:
    trunk/gcc/testsuite/gcc.target/i386/pr82353.C
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/i386/i386.exp
Comment 8 Vladimir Makarov 2017-10-12 17:07:01 UTC
Author: vmakarov
Date: Thu Oct 12 17:06:29 2017
New Revision: 253685

URL: https://gcc.gnu.org/viewcvs?rev=253685&root=gcc&view=rev
Log:
2017-10-12  Vladimir Makarov  <vmakarov@redhat.com>

	Revert
	2017-10-11  Vladimir Makarov  <vmakarov@redhat.com>
	PR sanitizer/82353
	* lra.c (collect_non_operand_hard_regs): Don't ignore operator
	locations.
	* lra-lives.c (bb_killed_pseudos, bb_gen_pseudos): Move up.
	(make_hard_regno_born, make_hard_regno_dead): Update
	bb_killed_pseudos and bb_gen_pseudos.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lra-lives.c
    trunk/gcc/lra.c
Comment 9 Jakub Jelinek 2017-10-13 20:19:48 UTC
Author: jakub
Date: Fri Oct 13 20:19:17 2017
New Revision: 253744

URL: https://gcc.gnu.org/viewcvs?rev=253744&root=gcc&view=rev
Log:
	PR sanitizer/82353
	* g++.dg/ubsan/pr82353-2.C: New test.
	* g++.dg/ubsan/pr82353-2-aux.cc: New file.
	* g++.dg/ubsan/pr82353-2.h: New file.

Added:
    trunk/gcc/testsuite/g++.dg/ubsan/pr82353-2-aux.cc
    trunk/gcc/testsuite/g++.dg/ubsan/pr82353-2.C
    trunk/gcc/testsuite/g++.dg/ubsan/pr82353-2.h
Modified:
    trunk/gcc/testsuite/ChangeLog
Comment 10 Vladimir Makarov 2017-10-16 20:35:24 UTC
Author: vmakarov
Date: Mon Oct 16 20:34:53 2017
New Revision: 253796

URL: https://gcc.gnu.org/viewcvs?rev=253796&root=gcc&view=rev
Log:
2017-10-16  Vladimir Makarov  <vmakarov@redhat.com>

	PR sanitizer/82353
	* lra.c (collect_non_operand_hard_regs): Don't ignore operator
	locations.
	* lra-lives.c (bb_killed_pseudos, bb_gen_pseudos): Move up.
	(make_hard_regno_born, make_hard_regno_dead): Update
	bb_killed_pseudos and bb_gen_pseudos for fixed regs.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lra-lives.c
    trunk/gcc/lra.c
Comment 11 Jakub Jelinek 2017-10-20 08:15:01 UTC
Fixed.