On 08/17/2018 03:14 AM, Bernd Edlinger wrote: > Hi! > > > After the other patch has been applied, I re-based this patch accordingly. > > Except the mechanical changes, there are a few notable differences to the > previous version: > > In string_constant, I added a similar check for the STRING_CSTs > because when callers don't use mem_size, they assume to be > able to read "TREE_STRING_LENGTH (array)" bytes, but that is > not always the case, for languages that don't always use > zero-terminated strings, for instance hollerith strings in fortran. > > --- gcc/expr.c 2018-08-17 05:32:57.332211963 +0200 > +++ gcc/expr.c 2018-08-16 23:08:23.544940795 +0200 > @@ -11372,6 +11372,9 @@ string_constant (tree arg, tree *ptr_off > *ptr_offset = fold_convert (sizetype, offset); > if (mem_size) > *mem_size = TYPE_SIZE_UNIT (TREE_TYPE (array)); > + else if (compare_tree_int (TYPE_SIZE_UNIT (TREE_TYPE (array)), > + TREE_STRING_LENGTH (array)) < 0) > + return NULL_TREE; > return array; > } > Yes. I purposefully didn't include this change in its entirety as it was dependent upon your earlier patch. Instead I twiddled your patch so that it applied on the current trunk and didn't regress anything while keeping the core concept you were introducing.
I'm also confident that change breaks one or more tests in the testsuite. I'm not sure why you didn't see the regression. But I certainly saw it with the variant shown above. Anyway, the basic idea was to carve out the basic concept of your patch to allow callers to specify how to count without regressing the trunk. That allows us to carve out an issue, resolve it and move on. The interdependent and conflicting patches are a nightmare to sort out. > > The range check in c_getstr was refined as well: > > This I added, because vla arrays can be initialized with string constants, > especially since the 71625 patch was installed: > In this case we end up with mem_size that fails to be constant. > > > @@ -14606,25 +14603,17 @@ c_getstr (tree src, unsigned HOST_WIDE_I > offset = tree_to_uhwi (offset_node); > } > > + if (!tree_fits_uhwi_p (mem_size)) > + return NULL; > + > /* STRING_LENGTH is the size of the string literal, including any > embedded NULs. STRING_SIZE is the size of the array the string > literal is stored in. */ > > Also the rest of the string length checks are refined, to return > actually zero-terminated single byte strings when strlen is not given, > and return something not necessarily zero-terminated which is > suitable for memxxx-functions otherwise. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? Not yet. There's a lot to go over here and I haven't finished reviewing all the discussions around 86711/86714. Jeff
