[PATCH] set range for strlen(array) to avoid spurious -Wstringop-overflow (PR 83373 , PR 78450)

[Jeff Law]

On 12/12/2017 08:47 PM, Martin Sebor wrote:
>>
>> struct fu {
>> Â  char x1[10];
>> Â  char x2[10];
>> Â  int avoid_trailing_array;
>> }
>>
>> Where objects stored in x1 are not null terminated.  Are we in the realm
>> of undefined behavior at that point (I hope so)?
> 
> Yes, this is undefined.  Pointer arithmetic (either direct or
> via standard library functions) is only defined for pointers
> to the same object or subobject.  So even something like
That's what I expected -- I just wanted to be sure there wasn't some
kind of escape clause that would allow one to compose an object of that
nature, then use pfu->x1 to read/write pfu->x2.


> 
> Â memcpy (pfu->x1, pfu->x1 + 10, 10);
> 
> is undefined.  _FORTIFY_SOURCE allows it for raw memory functions
> because some low level code copies regions of the same object that
> span two or more members (I've seen an example or two in the Linux
> kernel but, IIRC, nowhere else).  With the patch, using memchr
> would be the only way to get away with it.
Presumably the right way to go about things in this situation is to use
a pointer to the outer object.

> 
> Other than that, the new -Wstringop-truncation warning is designed
> to prevent creating unterminated character arrays and the manual
> suggests using attribute nonstring when it's necessary.  The
> -Wrestrict warning I'm about to check in also complains about
> forming invalid pointers by built-ins with restrict-qualified
> arguments (although it won't diagnose the above).
Right.  The concern is that it's an new option and folks likely haven't
cleaned up their codebases for these kinds of issues.

> 
> Although I would prefer not to, I suppose if letting strlen cross
> the boundaries of subobjects was considered an important use to
> accommodate in limited cases the optimization could be disabled
> for member arrays declared with the new nonstring attribute (while
> still issuing a warning for it as GCC does today).
> 
> Another alternative (if the above use case is considered important
> enough) might be to suppress the optimization for member character
> arrays that are immediately followed by another such array.
History tells us that there will be someone out there that does this
kind of thing -- the question is how pervasive is it.  My suspicion is
that it is not common.

Given that I don't expect those uses to be common, the only thing that
should break is non-conforming code and we have a (new) warning for such
code my inclination is to go forward.

So I'm OK with the patch.  I'd give folks till Monday to chime in with
dissenting opinions.

Jeff