[Bug rtl-optimization/104985] New: [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math

[Bug rtl-optimization/104985] New: [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math

[zsojka at seznam dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

            Bug ID: 104985
           Summary: [12 Regression] ICE: SIGSEGV in undo_to_marker /
                    adjust_reg_mode with -Os -frounding-math
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu
            Target: mips64el-unknown-linux-gnuabi64

Created attachment 52651
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52651&action=edit
reduced testcase

Compiler output:
$ mips64el-unknown-linux-gnuabi64-gcc -Os -frounding-math testcase.c -wrapper
valgrind,-q,--track-origins=yes
testcase.c: In function 'main':
testcase.c:87:14: warning: integer constant is so large that it is unsigned
   87 |              14417272188526493017, 18446744073709551615, 0, 0,
      |              ^~~~~~~~~~~~~~~~~~~~
testcase.c:87:36: warning: integer constant is so large that it is unsigned
   87 |              14417272188526493017, 18446744073709551615, 0, 0,
      |                                    ^~~~~~~~~~~~~~~~~~~~
testcase.c:88:14: warning: integer constant is so large that it is unsigned
   88 |              13365776591774966959,
      |              ^~~~~~~~~~~~~~~~~~~~
testcase.c:89:14: warning: integer constant is so large that it is unsigned
   89 |              18446744073709551615, (C) { 526289, 88628 }, (C) { 5 },
      |              ^~~~~~~~~~~~~~~~~~~~
==21781== Invalid read of size 8
==21781==    at 0x17325FF: undo_to_marker(void*) (combine.cc:4758)
==21781==    by 0x1751586: undo_all (combine.cc:4779)
==21781==    by 0x1751586: try_combine(rtx_insn*, rtx_insn*, rtx_insn*,
rtx_insn*, int*, rtx_insn*) (combine.cc:3353)
==21781==    by 0x175B056: combine_instructions (combine.cc:1288)
==21781==    by 0x175B056: rest_of_handle_combine (combine.cc:14931)
==21781==    by 0x175B056: (anonymous
namespace)::pass_combine::execute(function*) (combine.cc:14976)
==21781==    by 0xE28999: execute_one_pass(opt_pass*) (passes.cc:2637)
==21781==    by 0xE2925F: execute_pass_list_1(opt_pass*) (passes.cc:2737)
==21781==    by 0xE29271: execute_pass_list_1(opt_pass*) (passes.cc:2738)
==21781==    by 0xE29298: execute_pass_list(function*, opt_pass*)
(passes.cc:2748)
==21781==    by 0xA52AC5: expand (cgraphunit.cc:1834)
==21781==    by 0xA52AC5: cgraph_node::expand() (cgraphunit.cc:1787)
==21781==    by 0xA5400F: expand_all_functions (cgraphunit.cc:1998)
==21781==    by 0xA5400F: symbol_table::compile() [clone .part.0]
(cgraphunit.cc:2348)
==21781==    by 0xA56C47: compile (cgraphunit.cc:2261)
==21781==    by 0xA56C47: symbol_table::finalize_compilation_unit()
(cgraphunit.cc:2529)
==21781==    by 0xF3858F: compile_file() (toplev.cc:479)
==21781==    by 0x89CFEA: do_compile (toplev.cc:2168)
==21781==    by 0x89CFEA: toplev::main(int, char**) (toplev.cc:2320)
==21781==  Address 0x4865c48 is in a rw- anonymous segment
==21781== 
==21781== Invalid read of size 1
==21781==    at 0xB159DB: adjust_reg_mode(rtx_def*, machine_mode)
(emit-rtl.cc:1294)
==21781==    by 0x1732606: undo_to_marker(void*) (combine.cc:4758)
==21781==    by 0x1751586: undo_all (combine.cc:4779)
==21781==    by 0x1751586: try_combine(rtx_insn*, rtx_insn*, rtx_insn*,
rtx_insn*, int*, rtx_insn*) (combine.cc:3353)
==21781==    by 0x175B056: combine_instructions (combine.cc:1288)
==21781==    by 0x175B056: rest_of_handle_combine (combine.cc:14931)
==21781==    by 0x175B056: (anonymous
namespace)::pass_combine::execute(function*) (combine.cc:14976)
==21781==    by 0xE28999: execute_one_pass(opt_pass*) (passes.cc:2637)
==21781==    by 0xE2925F: execute_pass_list_1(opt_pass*) (passes.cc:2737)
==21781==    by 0xE29271: execute_pass_list_1(opt_pass*) (passes.cc:2738)
==21781==    by 0xE29298: execute_pass_list(function*, opt_pass*)
(passes.cc:2748)
==21781==    by 0xA52AC5: expand (cgraphunit.cc:1834)
==21781==    by 0xA52AC5: cgraph_node::expand() (cgraphunit.cc:1787)
==21781==    by 0xA5400F: expand_all_functions (cgraphunit.cc:1998)
==21781==    by 0xA5400F: symbol_table::compile() [clone .part.0]
(cgraphunit.cc:2348)
==21781==    by 0xA56C47: compile (cgraphunit.cc:2261)
==21781==    by 0xA56C47: symbol_table::finalize_compilation_unit()
(cgraphunit.cc:2529)
==21781==    by 0xF3858F: compile_file() (toplev.cc:479)
==21781==  Address 0xa5a5a5a5a5a5a5a7 is not stack'd, malloc'd or (recently)
free'd
==21781== 
during RTL pass: combine
testcase.c:107:1: internal compiler error: Segmentation fault
  107 | }
      | ^
0xf382cf crash_signal
        /repo/gcc-trunk/gcc/toplev.cc:322
0xb159db adjust_reg_mode(rtx_def*, machine_mode)
        /repo/gcc-trunk/gcc/emit-rtl.cc:1294
0x1732606 undo_to_marker
        /repo/gcc-trunk/gcc/combine.cc:4758
0x1751586 undo_all
        /repo/gcc-trunk/gcc/combine.cc:4779
0x1751586 try_combine
        /repo/gcc-trunk/gcc/combine.cc:3353
0x175b056 combine_instructions
        /repo/gcc-trunk/gcc/combine.cc:1288
0x175b056 rest_of_handle_combine
        /repo/gcc-trunk/gcc/combine.cc:14931
0x175b056 execute
        /repo/gcc-trunk/gcc/combine.cc:14976
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

$ mips64el-unknown-linux-gnuabi64-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-mips64el/bin/mips64el-unknown-linux-gnuabi64-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r12-7721-20220319135312-gb60bc913cca-checking-yes-rtl-df-extra-mips64el/bin/../libexec/gcc/mips64el-unknown-linux-gnuabi64/12.0.1/lto-wrapper
Target: mips64el-unknown-linux-gnuabi64
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl
--with-sysroot=/usr/mips64el-unknown-linux-gnuabi64 --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=mips64el-unknown-linux-gnuabi64
--with-ld=/usr/bin/mips64el-unknown-linux-gnuabi64-ld
--with-as=/usr/bin/mips64el-unknown-linux-gnuabi64-as --disable-multilib
--with-abi=64 --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r12-7721-20220319135312-gb60bc913cca-checking-yes-rtl-df-extra-mips64el
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.0.1 20220319 (experimental) (GCC)

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12 Regression] ICE:        |[12 Regression] ICE:
                   |SIGSEGV in undo_to_marker / |SIGSEGV in undo_to_marker /
                   |adjust_reg_mode with -Os    |adjust_reg_mode with -Os
                   |-frounding-math             |-frounding-math since
                   |                            |r12-4767-g81342e95827f77
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
           Keywords|needs-bisection             |
   Last reconfirmed|                            |2022-03-21
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-4767-g81342e95827f77.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I can't reproduce this, maybe it's fixed?

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
No, it's still there and you will likely need cross GAS to reproduce it:

~/Programming/gcc/configure --enable-languages=c,c++
--prefix=/home/marxin/bin/gcc --disable-multilib --enable-host-shared
--disable-libsanitizer --enable-valgrind-annotations --disable-bootstrap
--target=mips64el-unknown-linux-gnuabi64
--with-as=/home/marxin/Programming/binutils/objdir/gas/as-new
--enable-checking=yes,rtl,df,extra

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I can't reproduce it either.  Can you perhaps attach auto-host.h with which you
can reproduce it?

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
Created attachment 52673
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52673&action=edit
auto-host.h

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Still can't reproduce.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
With the following debugging patch:

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 8f06ee0e54f..150cc6fae1b 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -4755,6 +4755,7 @@ undo_to_marker (void *marker)
          *undo->where.i = undo->old_contents.i;
          break;
        case UNDO_MODE:
+         fprintf (stderr, "undo called: %p\n", undo->where.r);
          adjust_reg_mode (*undo->where.r, undo->old_contents.m);
          break;
        case UNDO_LINKS:
diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
index f4404d7abe3..f98e5eb46f7 100644
--- a/gcc/emit-rtl.cc
+++ b/gcc/emit-rtl.cc
@@ -1226,6 +1226,8 @@ emit_status::ensure_regno_capacity ()
   regno_pointer_align = (unsigned char *) tmp;

   rtx *new1 = GGC_RESIZEVEC (rtx, regno_reg_rtx, new_size);
+  fprintf (stderr, "emit_status::ensure_regno_capacity: %p-%p\n", new1, new1
+          + (new_size * sizeof(rtx)));
   memset (new1 + old_size, 0, (new_size - old_size) * sizeof (rtx));
   regno_reg_rtx = new1;


emit_status::ensure_regno_capacity: 0x7ffff75fc000-0x7ffff7605300
emit_status::ensure_regno_capacity: 0x7ffff75f8000-0x7ffff760a600
undo called: 0x7ffff75f87e8
undo called: 0x7ffff75f87e8
...
emit_status::ensure_regno_capacity: 0x7ffff75ea000-0x7ffff760ec00
undo called: 0x7ffff75f8c48

As seen the last *undo->where.r access a memory that is GGC freed and that was
previously allocated by emit_status::ensure_regno_capacity.

The last emit_status::ensure_regno_capacity that reallocates is called here:

#0  emit_status::ensure_regno_capacity (this=0x3082010 <x_rtl+48>) at
/home/marxin/Programming/gcc/gcc/emit-rtl.cc:1229
#1  0x0000000000e2fa3f in gen_reg_rtx (mode=E_DImode) at
/home/marxin/Programming/gcc/gcc/emit-rtl.cc:1201
#2  0x00000000012f088d in maybe_legitimize_operand (icode=CODE_FOR_ashldi3,
opno=0, op=0x7fffffff9f90) at /home/marxin/Programming/gcc/gcc/optabs.cc:7791
#3  0x00000000012f126d in maybe_legitimize_operands (icode=CODE_FOR_ashldi3,
opno=0, nops=3, ops=0x7fffffff9f90) at
/home/marxin/Programming/gcc/gcc/optabs.cc:7935
#4  0x00000000012f1326 in maybe_gen_insn (icode=CODE_FOR_ashldi3, nops=3,
ops=0x7fffffff9f90) at /home/marxin/Programming/gcc/gcc/optabs.cc:7954
#5  0x00000000012dadde in expand_binop_directly (icode=CODE_FOR_ashldi3,
mode=E_DImode, binoptab=ashl_optab, op0=0x7ffff72cb420, op1=0x7ffff760f5c0,
target=0x0, unsignedp=1, methods=OPTAB_LIB_WIDEN, last=0x7ffff72c78c0) at
/home/marxin/Programming/gcc/gcc/optabs.cc:1442
#6  0x00000000012db335 in expand_binop (mode=E_DImode, binoptab=ashl_optab,
op0=0x7ffff72cb420, op1=0x7ffff760f5c0, target=0x0, unsignedp=1,
methods=OPTAB_LIB_WIDEN) at /home/marxin/Programming/gcc/gcc/optabs.cc:1529
#7  0x00000000012da55c in expand_simple_binop (mode=E_DImode, code=ASHIFT,
op0=0x7ffff72cb420, op1=0x7ffff760f5c0, target=0x0, unsignedp=1,
methods=OPTAB_LIB_WIDEN) at /home/marxin/Programming/gcc/gcc/optabs.cc:1261
#8  0x0000000000eba222 in force_operand (value=0x7ffff72cb450, target=0x0) at
/home/marxin/Programming/gcc/gcc/expr.cc:7957
#9  0x0000000000e68948 in force_reg (mode=E_DImode, x=0x7ffff72cb450) at
/home/marxin/Programming/gcc/gcc/explow.cc:682
#10 0x0000000001ac255f in mips_move_integer (temp=0x7ffff7769bd0,
dest=0x7ffff7769c60, value=4294167595) at
/home/marxin/Programming/gcc/gcc/config/mips/mips.cc:3662
#11 0x0000000002172535 in gen_split_51 (curr_insn=0x7ffff7768840,
operands=0x3168f20 <recog_data>) at
/home/marxin/Programming/gcc/gcc/config/mips/mips.md:4699
#12 0x000000000231ce2f in split_6 (x1=0x7ffff72c9760, insn=0x7ffff7768840) at
/home/marxin/Programming/gcc/gcc/config/mips/mips.md:796
#13 0x000000000232072d in split_7 (x1=0x7ffff72c9760, insn=0x7ffff7768840) at
/home/marxin/Programming/gcc/gcc/config/mips/mips.md:7077
#14 0x00000000023220a6 in split_insns (x1=0x7ffff72c9760, insn=0x7ffff7768840)
at /home/marxin/Programming/gcc/gcc/config/mips/mips.md:7060
#15 0x00000000023608c9 in combine_split_insns (pattern=0x7ffff72c9760,
insn=0x7ffff7768840) at /home/marxin/Programming/gcc/gcc/combine.cc:530
#16 0x0000000002373b1d in try_combine (i3=0x7ffff7768840, i2=0x7ffff7768800,
i1=0x7ffff77687c0, i0=0x0, new_direct_jump_p=0x7fffffffd5f8,
last_combined_insn=0x7ffff7768840) at
/home/marxin/Programming/gcc/gcc/combine.cc:3588

and the crashing undo with:

#0  0x0000000000e2ff70 in adjust_reg_mode (reg=0xa5a5a5a5a5a5a5a5,
mode=E_SImode) at /home/marxin/Programming/gcc/gcc/emit-rtl.cc:1296
#1  0x00000000023814f1 in undo_to_marker (marker=0x0) at
/home/marxin/Programming/gcc/gcc/combine.cc:4759
#2  0x0000000002381571 in undo_all () at
/home/marxin/Programming/gcc/gcc/combine.cc:4780
#3  0x000000000237c2e4 in try_combine (i3=0x7ffff7768840, i2=0x7ffff7768800,
i1=0x7ffff77687c0, i0=0x0, new_direct_jump_p=0x7fffffffd5f8,
last_combined_insn=0x7ffff7768840) at
/home/marxin/Programming/gcc/gcc/combine.cc:4050

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52695
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52695&action=edit
gcc12-pr104985.patch

As I can't reproduce, just an untested patch, which will not keep around
addresses of regno_reg_rtx array elements, but the regnos.
Yet another option would be to make where.m a rtx and save there the REGs.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #8)
> Created attachment 52695 [details]
> gcc12-pr104985.patch
> 
> As I can't reproduce, just an untested patch, which will not keep around
> addresses of regno_reg_rtx array elements, but the regnos.
> Yet another option would be to make where.m a rtx and save there the REGs.

I think that's a good cleanup!  Martin - does this fix the issue?

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52696
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52696&action=edit
gcc12-pr104985-2.patch

A variant patch, also untested, but much smaller.
Yet another way would be to change do_SUBST_MODE into SUBST_MODE and just pass
the rtx instead of rtx *.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52697
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52697&action=edit
gcc12-pr104985-3.patch

And 3rd untested version, keep SUBST_MODE users as they are, but change
SUBST_MODE implementation and UNDO_MODE users.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think once gen_reg_rtx is called, we no longer overwrite regno_reg_rtx[regno]
but just modify it in place to change mode (except that we throw it away at the
end of RTL passes for each function), so I think all 3 patches should be safe.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW

--- Comment #13 from Martin Liška <marxin at gcc dot gnu.org> ---
I can confirm all the patches handle the crash. Thanks for it.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #14 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Are you sure this only ever handles pseudos?  It is completely broken if not.

Changing the mode of regno_reg_rtx[...] is always wrong, too.

Patches 2 and 3 look better, but need a lot more explanation.  What does "m"
mean, to start with?

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #14)
> Are you sure this only ever handles pseudos?  It is completely broken if not.

Yes, I am.
All the 4 uses of SUBST_MODE look like:
                  if (regno < FIRST_PSEUDO_REGISTER)
                    newpat_dest = gen_rtx_REG (compare_mode, regno);
                  else
                    {
                      SUBST_MODE (regno_reg_rtx[regno], compare_mode);
                      newpat_dest = regno_reg_rtx[regno];
                    }

> Changing the mode of regno_reg_rtx[...] is always wrong, too.
> 
> Patches 2 and 3 look better, but need a lot more explanation.  What does "m"
> mean, to start with?

m stands for mode.
enum undo_kind { UNDO_RTX, UNDO_INT, UNDO_MODE, UNDO_LINKS };
...
  union { rtx r; int i; machine_mode m; struct insn_link *l; } old_contents;
  union { rtx *r; int *i; struct insn_link **l; } where;
There are 2 ways to look at those names, one is that they somehow describe
their type (but for where what they point to), the other is that it
sign the undo_kind (first letter after UNDO_ in lowercase).
In the latter case, UNDO_MODE was the only outlier that used where.r instead of
where.m.

Note, I've posted the patch (last version) to gcc-patches, so patch review can
happen there:
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592450.html

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #16 from Segher Boessenkool <segher at gcc dot gnu.org> ---
"machine_mode m" I understand of course.  "rtx m" is something different :-)

I didn't see the patch yet, sorry, will get to it later today.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:61bee6aed26eb30b798c75b9a595c9d51e080442

commit r12-8030-g61bee6aed26eb30b798c75b9a595c9d51e080442
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 6 18:42:52 2022 +0200

    combine: Don't record for UNDO_MODE pointers into regno_reg_rtx array
[PR104985]

    The testcase in the PR fails under valgrind on mips64 (but only Martin
    can reproduce, I couldn't).
    But the problem reported there is that SUBST_MODE remembers addresses
    into the regno_reg_rtx array, then some splitter needs a new pseudo
    and calls gen_reg_rtx, which reallocates the regno_reg_rtx array
    and finally undo operation is done and dereferences the old regno_reg_rtx
    entry.
    The rtx values stored in regno_reg_rtx array seems to be created
    by gen_reg_rtx only and since then aren't modified, all we do for it
    is adjusting its fields (e.g. adjust_reg_mode that SUBST_MODE does).

    So, I think it is useless to use where.r for UNDO_MODE and store
    &regno_reg_rtx[regno] in struct undo, we can store just
    regno_reg_rtx[regno] (i.e. pointer to the REG itself instead of
    pointer to pointer to REG) or could also store just the regno.

    The following patch does the latter, and because SUBST_MODE no longer
    needs to be a macro, changes all SUBST_MODE uses to subst_mode.

    2022-04-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/104985
            * combine.cc (struct undo): Add where.regno member.
            (do_SUBST_MODE): Rename to ...
            (subst_mode): ... this.  Change first argument from rtx * into int,
            operate on regno_reg_rtx[regno] and save regno into where.regno.
            (SUBST_MODE): Remove.
            (try_combine): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.  For UNDO_MODE,
use
            regno_reg_rtx[undo->where.regno] instead of *undo->where.r.
            (undo_to_marker): For UNDO_MODE, use
regno_reg_rtx[undo->where.regno]
            instead of *undo->where.r.
            (simplify_set): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:a487dbd802d71bcea8feaa41defde92a6848d0c6

commit r11-9845-ga487dbd802d71bcea8feaa41defde92a6848d0c6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 6 18:42:52 2022 +0200

    combine: Don't record for UNDO_MODE pointers into regno_reg_rtx array
[PR104985]

    The testcase in the PR fails under valgrind on mips64 (but only Martin
    can reproduce, I couldn't).
    But the problem reported there is that SUBST_MODE remembers addresses
    into the regno_reg_rtx array, then some splitter needs a new pseudo
    and calls gen_reg_rtx, which reallocates the regno_reg_rtx array
    and finally undo operation is done and dereferences the old regno_reg_rtx
    entry.
    The rtx values stored in regno_reg_rtx array seems to be created
    by gen_reg_rtx only and since then aren't modified, all we do for it
    is adjusting its fields (e.g. adjust_reg_mode that SUBST_MODE does).

    So, I think it is useless to use where.r for UNDO_MODE and store
    &regno_reg_rtx[regno] in struct undo, we can store just
    regno_reg_rtx[regno] (i.e. pointer to the REG itself instead of
    pointer to pointer to REG) or could also store just the regno.

    The following patch does the latter, and because SUBST_MODE no longer
    needs to be a macro, changes all SUBST_MODE uses to subst_mode.

    2022-04-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/104985
            * combine.c (struct undo): Add where.regno member.
            (do_SUBST_MODE): Rename to ...
            (subst_mode): ... this.  Change first argument from rtx * into int,
            operate on regno_reg_rtx[regno] and save regno into where.regno.
            (SUBST_MODE): Remove.
            (try_combine): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.  For UNDO_MODE,
use
            regno_reg_rtx[undo->where.regno] instead of *undo->where.r.
            (undo_to_marker): For UNDO_MODE, use
regno_reg_rtx[undo->where.regno]
            instead of *undo->where.r.
            (simplify_set): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.

    (cherry picked from commit 61bee6aed26eb30b798c75b9a595c9d51e080442)

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:6c081dee437170a34416c40411000ec33230409f

commit r10-10704-g6c081dee437170a34416c40411000ec33230409f
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 6 18:42:52 2022 +0200

    combine: Don't record for UNDO_MODE pointers into regno_reg_rtx array
[PR104985]

    The testcase in the PR fails under valgrind on mips64 (but only Martin
    can reproduce, I couldn't).
    But the problem reported there is that SUBST_MODE remembers addresses
    into the regno_reg_rtx array, then some splitter needs a new pseudo
    and calls gen_reg_rtx, which reallocates the regno_reg_rtx array
    and finally undo operation is done and dereferences the old regno_reg_rtx
    entry.
    The rtx values stored in regno_reg_rtx array seems to be created
    by gen_reg_rtx only and since then aren't modified, all we do for it
    is adjusting its fields (e.g. adjust_reg_mode that SUBST_MODE does).

    So, I think it is useless to use where.r for UNDO_MODE and store
    &regno_reg_rtx[regno] in struct undo, we can store just
    regno_reg_rtx[regno] (i.e. pointer to the REG itself instead of
    pointer to pointer to REG) or could also store just the regno.

    The following patch does the latter, and because SUBST_MODE no longer
    needs to be a macro, changes all SUBST_MODE uses to subst_mode.

    2022-04-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/104985
            * combine.c (struct undo): Add where.regno member.
            (do_SUBST_MODE): Rename to ...
            (subst_mode): ... this.  Change first argument from rtx * into int,
            operate on regno_reg_rtx[regno] and save regno into where.regno.
            (SUBST_MODE): Remove.
            (try_combine): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.  For UNDO_MODE,
use
            regno_reg_rtx[undo->where.regno] instead of *undo->where.r.
            (undo_to_marker): For UNDO_MODE, use
regno_reg_rtx[undo->where.regno]
            instead of *undo->where.r.
            (simplify_set): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.

    (cherry picked from commit 61bee6aed26eb30b798c75b9a595c9d51e080442)

[Bug rtl-optimization/104985] [12 Regression] ICE: SIGSEGV in undo_to_marker / adjust_reg_mode with -Os -frounding-math since r12-4767-g81342e95827f77

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104985

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:5169f5756e26feac7042a6046e974dd4b650b6ed

commit r9-10145-g5169f5756e26feac7042a6046e974dd4b650b6ed
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 6 18:42:52 2022 +0200

    combine: Don't record for UNDO_MODE pointers into regno_reg_rtx array
[PR104985]

    The testcase in the PR fails under valgrind on mips64 (but only Martin
    can reproduce, I couldn't).
    But the problem reported there is that SUBST_MODE remembers addresses
    into the regno_reg_rtx array, then some splitter needs a new pseudo
    and calls gen_reg_rtx, which reallocates the regno_reg_rtx array
    and finally undo operation is done and dereferences the old regno_reg_rtx
    entry.
    The rtx values stored in regno_reg_rtx array seems to be created
    by gen_reg_rtx only and since then aren't modified, all we do for it
    is adjusting its fields (e.g. adjust_reg_mode that SUBST_MODE does).

    So, I think it is useless to use where.r for UNDO_MODE and store
    &regno_reg_rtx[regno] in struct undo, we can store just
    regno_reg_rtx[regno] (i.e. pointer to the REG itself instead of
    pointer to pointer to REG) or could also store just the regno.

    The following patch does the latter, and because SUBST_MODE no longer
    needs to be a macro, changes all SUBST_MODE uses to subst_mode.

    2022-04-06  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/104985
            * combine.c (struct undo): Add where.regno member.
            (do_SUBST_MODE): Rename to ...
            (subst_mode): ... this.  Change first argument from rtx * into int,
            operate on regno_reg_rtx[regno] and save regno into where.regno.
            (SUBST_MODE): Remove.
            (try_combine): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.  For UNDO_MODE,
use
            regno_reg_rtx[undo->where.regno] instead of *undo->where.r.
            (undo_to_marker): For UNDO_MODE, use
regno_reg_rtx[undo->where.regno]
            instead of *undo->where.r.
            (simplify_set): Use subst_mode instead of SUBST_MODE, change first
            argument from regno_reg_rtx[whatever] to whatever.

    (cherry picked from commit 61bee6aed26eb30b798c75b9a595c9d51e080442)