[PATCH] use string length to relax -Wstringop-overflow for nonstrings (PR 85623)

[Martin Sebor]

On 05/14/2018 10:43 AM, Franz Sirl wrote:
> Am 2018-05-10 um 21:26 schrieb Martin Sebor:
>> GCC 8.1 warns for unbounded (and some bounded) string comparisons
>> involving arrays declared attribute nonstring (i.e., char arrays
>> that need not be nul-terminated).  For instance:
>>
>>    extern __attribute__((nonstring)) char a[4];
>>
>>    int f (void)
>>    {
>>      return strncmp (a, "123", sizeof a);
>>    }
>>
>>    warning: ‘strcmp’ argument 1 declared attribute ‘nonstring’
>>
>> Note that the warning refers to strcmp even though the call in
>> the source is to strncmp, because prior passes transform one to
>> the other.
>>
>> The warning above is unnecessary (for strcmp) and incorrect for
>> strncmp because the call reads exactly four bytes from the non-
>> string array a regardless of the bound and so there is no risk
>> that it will read past the end of the array.
>>
>> The attached change enhances the warning to use the length of
>> the string argument to suppress some of these needless warnings
>> for both bounded and unbounded string comparison functions.
>> When the length of the string is unknown, the warning uses its
>> size (when possible) as the upper bound on the number of accessed
>> bytes.  The change adds no new warnings.
>>
>> I'm looking for approval to commit it to both trunk and 8-branch.
>
> Hi Martin,
>
> this patch is a nice improvement and makes "nonstring" a lot more
> useable. But shouldn't the attribute also handle these cases (similar to
> PR 85602):
>
> #include <string.h>
>
> typedef struct {
>         char segname[16] __attribute__((__nonstring__));
>         char sectname[16] __attribute__((__nonstring__));
> } MachO_header;
>
> const char *get_seg_sect(MachO_header *hdr)
> {
>         static char segname_sectname[sizeof(hdr->segname) +
> sizeof(hdr->sectname) + 2];
>
>         memset(segname_sectname, 0, sizeof(segname_sectname));
>         strncpy(segname_sectname, hdr->segname, sizeof(hdr->segname));
>         strcat(segname_sectname, "@");
>         strncat(segname_sectname, hdr->sectname, sizeof(hdr->sectname));
>
>         return segname_sectname;
> }
>
> $ gcc-8 -c -O2 -W -Wall test-macho.c
> In file included from /usr/include/string.h:630,
>                  from test-macho.c:1:
> test-macho.c: In function 'get_seg_sect':
> test-macho.c:14:48: warning: argument to 'sizeof' in '__builtin_strncpy'
> call is the same expression as the source; did you mean to use the size
> of the destination? [-Wsizeof-pointer-memaccess]
>   strncpy(segname_sectname, hdr->segname, sizeof(hdr->segname));
>                                                 ^
> test-macho.c:16:49: warning: argument to 'sizeof' in '__builtin_strncat'
> call is the same expression as the source; did you mean to use the size
> of the destination? [-Wsizeof-pointer-memaccess]
>   strncat(segname_sectname, hdr->sectname, sizeof(hdr->sectname));
>                                                  ^
>
> As you can see __attribute__((__nonstring__)) doesn't silence the warning.

Thanks for the heads up.  I've added my comments to the bug.
I will see if I can enhance the warning to detect the attribute
and suppress the warning for the use case above.  I think I'd
prefer to do that separately from this bug fix since the two
affect different warnings and will likely need changes to
different areas of the compiler (front-end vs middle-end).

Martin