On Fri, May 15, 2020 at 10:32:00AM -0600, Jeff Law via Gcc-patches wrote:
> > I wasn't sure if it wouldn't be safer to add some bool flag set to true by
> > the new code and then add gcc_assert in all the other paths, like following
> > incremental patch.  I believe none of the asserts can trigger right now,
> > but the code is still adjusting what it plans to use as source before 
> > actually
> > only copying fewer bytes from it, so if somebody changes it later...
> > 
> > Thoughts on that?
> Can't hurt, and debugging the assert tripping is likely a hell of a lot easier
> than debugging the resultant incorrect code.   So if it passes, then I'd say 
> go
> for it.

Testing passed, so I've committed it with those asserts (and thankfully I've
added them!) but it apparently broke Linux kernel build on arm.

The problem is that if the STRING_CST is very short, while the full object
has BLKmode, the short string could very well have
QImode/HImode/SImode/DImode and in that case it wouldn't take the path that
copies the string and then clears the remaining space, but different paths
in which it will ICE because of those asserts and without those it would
just emit wrong-code.

The following patch fixes it by enforcing BLKmode for the string MEM, even
if it is short, so that we copy it and memset the rest.

Ok for trunk if it passes bootstrap/regtest?

2020-05-29  Jakub Jelinek  <ja...@redhat.com>

        PR middle-end/95052
        * expr.c (store_expr): For shortedned_string_cst, ensure temp has
        BLKmode.

        * gcc.dg/pr95052.c: New test.

--- gcc/expr.c.jj       2020-05-29 10:42:26.000000000 +0200
+++ gcc/expr.c  2020-05-29 21:49:11.421646101 +0200
@@ -5779,6 +5779,11 @@ store_expr (tree exp, rtx target, int ca
                               (call_param_p
                                ? EXPAND_STACK_PARM : EXPAND_NORMAL),
                               &alt_rtl, false);
+      if (shortened_string_cst)
+       {
+         gcc_assert (MEM_P (temp));
+         temp = change_address (temp, BLKmode, NULL_RTX);
+       }
     }
 
   /* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
--- gcc/testsuite/gcc.dg/pr95052.c.jj   2020-05-29 21:56:15.139426809 +0200
+++ gcc/testsuite/gcc.dg/pr95052.c      2020-05-29 21:55:51.919767620 +0200
@@ -0,0 +1,12 @@
+/* PR middle-end/95052 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fconserve-stack" } */
+
+void bar (char *);
+
+void
+foo (void)
+{
+  char buf[70] = "";
+  bar (buf);
+}


        Jakub

Reply via email to