On Thu, 2020-05-14 at 18:51 +0200, Jakub Jelinek wrote: > On Thu, May 14, 2020 at 10:10:55AM -0600, Jeff Law wrote: > > On Tue, 2020-05-12 at 10:12 +0200, Jakub Jelinek wrote: > > > Hi! > > > > > > In the following testcase, store_expr of e.g. 97 bytes long string literal > > > into 1MB long array is implemented by copying the 97 bytes from .rodata > > > section, followed by clearing the remaining bytes. But, as the STRING_CST > > > has type char[1024*1024], we actually allocate whole 1MB in .rodata > > > section > > > for it, even when we only use the first 97 bytes from that. > > > > > > The following patch tweaks it so that if we are going to initialize only > > > the > > > small part from it, we don't emit all the zeros that we never use after > > > it. > > > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > > > > > 2020-05-12 Jakub Jelinek <ja...@redhat.com> > > > > > > PR middle-end/95052 > > > * expr.c (store_expr): If expr_size is constant and significantly > > > larger than TREE_STRING_LENGTH, set temp to just the > > > TREE_STRING_LENGTH portion of the STRING_CST. > > > > > > * gcc.target/i386/pr95052.c: New test. > > OK. Initially I had a number of concerns, but this only affects the .rodata > > that's used to initialize the real object. The .rodata bits have no other > > purpose and aren't otherwise accessable. > > 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.
jeff >
