GCC 7 and prior eliminate both strlen() calls in the program below. GCC 8 only eliminates the first call, emitting worse code than prior versions. $ cat c.c && gcc -O2 -S -Wall -Wextra -fdump-tree-optimized=/dev/stdout c.c char a[3]; int main () { int n[2]; __builtin_strcpy (a, "12"); n[0] = __builtin_strlen (a); __builtin_strcpy (a, "12"); n[1] = __builtin_strlen (a); __builtin_printf ("%i %i\n", n[0], n[1]); } ;; Function main (main, funcdef_no=0, decl_uid=1956, cgraph_uid=0, symbol_order=1) (executed once) main () { long unsigned int _3; int _4; <bb 2> [local count: 1073741825]: MEM[(char * {ref-all})&a] = MEM[(char * {ref-all})"12"]; _3 = __builtin_strlen (&a); _4 = (int) _3; __builtin_printf ("%i %i\n", 2, _4); return 0; }
Bisection points to r255197 (see also bug 83501 for another regression related to this change): 2017-11-28 Richard Biener <rguenther@suse.de> PR middle-end/83141 * gimple-fold.c (gimple_fold_builtin_memory_op): For aggregate copies generated from memcpy use a character array as reference type.
Confirmed. Note this was a correctness fix so we properly copy padding given aggregate assignment in GIMPLE is not a block copy if done for a structure type. For this case in question it shouldn't have made a difference though. We seem to invalidate the string info for some reason after visiting the second store.
The strcpy() calls are first transformed into MEM[(char * {ref-all})&a] = MEM[(char * {ref-all})"12"]; In GCC 7, the above is then transformed into MEM[(char * {ref-all})&a] = "12"; (I'm not sure what the difference is). In GCC 7, the second instance of the above is then removed in fre1. In GCC 8, the second instance makes it all the way to the strlen pass where handle_char_store() isn't prepared to deal with it if a length record exists for the destination. I think the strlen() limitation can be handled by the same solution as bug 86043: i.e., have handle_char_store() handle cases where substrings of any length is overwritten without changing their length, not just those of length one by plain character assignment. I don't know why the duplicate MEM assignment above isn't eliminated earlier (that may be a separate bug).
On Tue, 5 Jun 2018, msebor at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86042 > > --- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> --- > The strcpy() calls are first transformed into > > MEM[(char * {ref-all})&a] = MEM[(char * {ref-all})"12"]; > > In GCC 7, the above is then transformed into > > MEM[(char * {ref-all})&a] = "12"; > > (I'm not sure what the difference is). "12" is considered a constant while MEM[(char * {ref-all})"12"] is considered a read from the constant pool. It's not simplified to that because GCC 8 uses 'unsigned char[]' as access type while GCC 7 used char[] and those are not compatible. I suspect if we change /* We get an aggregate copy. Use an unsigned char[] type to perform the copying to preserve padding and to avoid any issues with TREE_ADDRESSABLE types or float modes behavior on copying. */ desttype = build_array_type_nelts (unsigned_char_type_node, tree_to_uhwi (len)); in gimple_fold_builtin_memory_op to use char_type_node then we'll get back GCC 7 behavior for this case. (I chose unsigned char type to not change IL based on -f[un]signed-char) I suspect that with -funsigned-char the testcase already works with GCC8? " In GCC 7, the second instance of the > above is then removed in fre1. By means of redundant store removal which ATM only handles stores from constants and registers but not aggregate copies. > In GCC 8, the second instance makes it all the way to the strlen pass where > handle_char_store() isn't prepared to deal with it if a length record exists > for the destination. I think the strlen() limitation can be handled by the > same solution as bug 86043: i.e., have handle_char_store() handle cases where > substrings of any length is overwritten without changing their length, not just > those of length one by plain character assignment. Yes, that's a good enhancement independent of this bug. > I don't know why the duplicate MEM assignment above isn't eliminated earlier > (that may be a separate bug). See above - redundant store removal doesn't handle aggregate copies.
Patch: https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00403.html
GCC 8.2 has been released.
Author: msebor Date: Thu Jul 26 16:45:43 2018 New Revision: 263018 URL: https://gcc.gnu.org/viewcvs?rev=263018&root=gcc&view=rev Log: PR tree-optimization/86043 - strlen after memcpy partially overwriting a string not optimized PR tree-optimization/86042 - missing strlen optimization after second strcpy gcc/ChangeLog: PR tree-optimization/86043 PR tree-optimization/86042 * tree-ssa-strlen.c (handle_builtin_memcpy): Handle strict overlaps. (get_string_cst_length): Rename... (get_min_string_length): ...to this. Add argument. (handle_char_store): Extend to handle multi-character stores by MEM_REF. * tree.c (initializer_zerop): Use new argument. Handle MEM_REF. * tree.h (initializer_zerop): Add argument. gcc/testsuite/ChangeLog: PR tree-optimization/86043 PR tree-optimization/86042 * gcc/testsuite/gcc.dg/attr-nonstring-2.c: Xfail test cases due to pr86688. * gcc.dg/strlenopt-44.c: New test. Added: trunk/gcc/testsuite/gcc.dg/strlenopt-54.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/attr-nonstring-2.c trunk/gcc/tree-ssa-strlen.c trunk/gcc/tree.c trunk/gcc/tree.h
Adjusted patch committed in r263018.