| Message ID | 20200529200339.GS8462@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | expr: Fix fallout from optimize store_expr from STRING_CST [PR95052] | expand |
On Fri, 2020-05-29 at 22:03 +0200, Jakub Jelinek wrote: > 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. Mips trips over this as well. > > 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 <jakub@redhat.com> > > PR middle-end/95052 > * expr.c (store_expr): For shortedned_string_cst, ensure temp has > BLKmode. > > * gcc.dg/pr95052.c: New test. OK jeff >
--- 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); +}