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 <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. OK jeff >
