public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-7196] combine: Fix ICE with substitution of CONST_INT into PRE_DEC argument [PR104446]
@ 2022-02-11 12:52 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2022-02-11 12:52 UTC (permalink / raw)
  To: gcc-cvs

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

commit r12-7196-gfb76c0ad35f96505ecd9213849ebc3df6163a0f7
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Feb 11 11:34:46 2022 +0100

    combine: Fix ICE with substitution of CONST_INT into PRE_DEC argument [PR104446]
    
    The following testcase ICEs, because combine substitutes
    (insn 10 9 11 2 (set (reg/v:SI 7 sp [ a ])
            (const_int 0 [0])) "pr104446.c":9:5 81 {*movsi_internal}
         (nil))
    (insn 13 11 14 2 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [0  S4 A32])
            (reg:SI 85)) "pr104446.c":10:3 56 {*pushsi2}
         (expr_list:REG_DEAD (reg:SI 85)
            (expr_list:REG_ARGS_SIZE (const_int 16 [0x10])
                (nil))))
    forming
    (insn 13 11 14 2 (set (mem/f:SI (pre_dec:SI (const_int 0 [0])) [0  S4 A32])
            (reg:SI 85)) "pr104446.c":10:3 56 {*pushsi2}
         (expr_list:REG_DEAD (reg:SI 85)
            (expr_list:REG_ARGS_SIZE (const_int 16 [0x10])
                (nil))))
    which is invalid RTL (pre_dec's argument must be a REG).
    I know substitution creates various forms of invalid RTL and hopes that
    invalid RTL just won't recog.
    But unfortunately in this case we ICE before we get to recog, as
    try_combine does:
      if (n_auto_inc)
        {
          int new_n_auto_inc = 0;
          for_each_inc_dec (newpat, count_auto_inc, &new_n_auto_inc);
    
          if (n_auto_inc != new_n_auto_inc)
            {
              if (dump_file && (dump_flags & TDF_DETAILS))
                fprintf (dump_file, "Number of auto_inc expressions changed\n");
              undo_all ();
              return 0;
            }
        }
    and for_each_inc_dec under the hood will do e.g. for the PRE_DEC case:
        case PRE_DEC:
        case POST_DEC:
          {
            poly_int64 size = GET_MODE_SIZE (GET_MODE (mem));
            rtx r1 = XEXP (x, 0);
            rtx c = gen_int_mode (-size, GET_MODE (r1));
            return fn (mem, x, r1, r1, c, data);
          }
    and that code rightfully expects that the PRE_DEC operand has non-VOIDmode
    (as it needs to be a REG) - gen_int_mode for VOIDmode results in ICE.
    I think it is better not to emit the clearly invalid RTL during substitution
    like we do for other cases, than to adding workarounds for invalid IL
    created by combine to rtlanal.cc and perhaps elsewhere.
    As for the testcase, of course it is UB at runtime to modify sp that way,
    but if such code is never reached, we must compile it, not to ICE on it.
    And I don't see why on other targets which use the autoinc rtxes much more
    it couldn't happen with other registers.
    
    2022-02-11  Jakub Jelinek  <jakub@redhat.com>
    
            PR middle-end/104446
            * combine.cc (subst): Don't substitute CONST_INTs into RTX_AUTOINC
            operands.
    
            * gcc.target/i386/pr104446.c: New test.

Diff:
---
 gcc/combine.cc                           |  6 ++++++
 gcc/testsuite/gcc.target/i386/pr104446.c | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/gcc/combine.cc b/gcc/combine.cc
index c264ededf20..c4c5444038e 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -5534,6 +5534,12 @@ subst (rtx x, rtx from, rtx to, int in_dest, int in_cond, int unique_copy)
 		  if (!x)
 		    return gen_rtx_CLOBBER (VOIDmode, const0_rtx);
 		}
+	      /* CONST_INTs shouldn't be substituted into PRE_DEC, PRE_MODIFY
+		 etc. arguments, otherwise we can ICE before trying to recog
+		 it.  See PR104446.  */
+	      else if (CONST_SCALAR_INT_P (new_rtx)
+		       && GET_RTX_CLASS (GET_CODE (x)) == RTX_AUTOINC)
+		return gen_rtx_CLOBBER (VOIDmode, const0_rtx);
 	      else
 		SUBST (XEXP (x, i), new_rtx);
 	    }
diff --git a/gcc/testsuite/gcc.target/i386/pr104446.c b/gcc/testsuite/gcc.target/i386/pr104446.c
new file mode 100644
index 00000000000..510cb69cfb6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr104446.c
@@ -0,0 +1,15 @@
+/* PR middle-end/104446 */
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-O2 -mrtd" } */
+
+register volatile int a __asm__("%esp");
+void foo (void *);
+void bar (void *);
+
+void
+baz (void)
+{
+  foo (__builtin_return_address (0));
+  a = 0;
+  bar (__builtin_return_address (0));
+}


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-02-11 12:52 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-11 12:52 [gcc r12-7196] combine: Fix ICE with substitution of CONST_INT into PRE_DEC argument [PR104446] Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).