| Message ID | bd26cb89-86a6-0d53-4f41-2516125af938@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | use string length to relax -Wstringop-overflow for nonstrings (PR 85623) | expand |
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. Franz.
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
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg00509.html On 05/10/2018 01:26 PM, Martin Sebor wrote: > 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. > > Martin
On 05/10/2018 01:26 PM, Martin Sebor wrote: > 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. > > Martin > > gcc-85623.diff > > > PR c/85623 - strncmp() warns about attribute 'nonstring' incorrectly in -Wstringop-overflow > > gcc/ChangeLog: > > PR c/85623 > * calls.c (maybe_warn_nonstring_arg): Use string length to set > or ajust the presumed bound on an operation to avoid unnecessary > warnings. s/ajust/adjust/ > > gcc/testsuite/ChangeLog: > > PR c/85623 > * c-c++-common/attr-nonstring-3.c: Adjust. > * c-c++-common/attr-nonstring-4.c: Adjust. > * c-c++-common/attr-nonstring-6.c: New test. > > diff --git a/gcc/calls.c b/gcc/calls.c > index 9eb0467..f5c8ad4 100644 > --- a/gcc/calls.c > +++ b/gcc/calls.c > @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3. If not see > #include "stringpool.h" > #include "attribs.h" > #include "builtins.h" > +#include "gimple-fold.h" > > /* Like PREFERRED_STACK_BOUNDARY but in units of bytes, not bits. */ > #define STACK_BYTES (PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT) > @@ -1612,15 +1613,36 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp) > /* The bound argument to a bounded string function like strncpy. */ > tree bound = NULL_TREE; > > + /* The range of lengths of a string argument to one of the comparison > + functions. If the length is less than the bound it is used instead. */ > + tree lenrng[2] = { NULL_TREE, NULL_TREE }; > + > /* It's safe to call "bounded" string functions with a non-string > argument since the functions provide an explicit bound for this > purpose. */ > switch (DECL_FUNCTION_CODE (fndecl)) > { > - case BUILT_IN_STPNCPY: > - case BUILT_IN_STPNCPY_CHK: > + case BUILT_IN_STRCMP: > case BUILT_IN_STRNCMP: > case BUILT_IN_STRNCASECMP: > + { > + /* For these, if one argument refers to one or more of a set > + of string constants or arrays of known size, determine > + the range of their known or possible lengths and use it > + conservatively as the bound for the unbounded function, > + and to adjust the range of the bound of the bounded ones. */ > + unsigned stride = with_bounds ? 2 : 1; > + for (unsigned argno = 0; argno < nargs && !*lenrng; argno += stride) > + { > + tree arg = CALL_EXPR_ARG (exp, argno); > + if (!get_attr_nonstring_decl (arg)) > + get_range_strlen (arg, lenrng); > + } > + } > + /* Fall through. */ > + > + case BUILT_IN_STPNCPY: > + case BUILT_IN_STPNCPY_CHK: > case BUILT_IN_STRNCPY: > case BUILT_IN_STRNCPY_CHK: > { > @@ -1647,6 +1669,33 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp) > + { > + /* Replace the bound on the oparation with the upper bound s/oparation/operation/ OK for the trunk with the nits fixed. Also note that I've acked a patch from Martin L (I believe) that removes the chkp/bounds checking bits that were deprecated in gcc-8. So there's some chance the bounds-related bits will need to be updated depending on whether or not Martin's L's patch has been committed. This isn't strictly a regression. So unless this is affecting some critical code (ie glibc, kernel or something similar) this probably would require an explicit OK from Jakub or Richi to be eligible for the gcc-8 branch. jeff
On 05/21/2018 05:02 PM, Jeff Law wrote: > On 05/10/2018 01:26 PM, Martin Sebor wrote: >> 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. >> >> Martin >> >> gcc-85623.diff >> >> >> PR c/85623 - strncmp() warns about attribute 'nonstring' incorrectly in -Wstringop-overflow >> >> gcc/ChangeLog: >> >> PR c/85623 >> * calls.c (maybe_warn_nonstring_arg): Use string length to set >> or ajust the presumed bound on an operation to avoid unnecessary >> warnings. > s/ajust/adjust/ > >> >> gcc/testsuite/ChangeLog: >> >> PR c/85623 >> * c-c++-common/attr-nonstring-3.c: Adjust. >> * c-c++-common/attr-nonstring-4.c: Adjust. >> * c-c++-common/attr-nonstring-6.c: New test. >> >> diff --git a/gcc/calls.c b/gcc/calls.c >> index 9eb0467..f5c8ad4 100644 >> --- a/gcc/calls.c >> +++ b/gcc/calls.c >> @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3. If not see >> #include "stringpool.h" >> #include "attribs.h" >> #include "builtins.h" >> +#include "gimple-fold.h" >> >> /* Like PREFERRED_STACK_BOUNDARY but in units of bytes, not bits. */ >> #define STACK_BYTES (PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT) >> @@ -1612,15 +1613,36 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp) >> /* The bound argument to a bounded string function like strncpy. */ >> tree bound = NULL_TREE; >> >> + /* The range of lengths of a string argument to one of the comparison >> + functions. If the length is less than the bound it is used instead. */ >> + tree lenrng[2] = { NULL_TREE, NULL_TREE }; >> + >> /* It's safe to call "bounded" string functions with a non-string >> argument since the functions provide an explicit bound for this >> purpose. */ >> switch (DECL_FUNCTION_CODE (fndecl)) >> { >> - case BUILT_IN_STPNCPY: >> - case BUILT_IN_STPNCPY_CHK: >> + case BUILT_IN_STRCMP: >> case BUILT_IN_STRNCMP: >> case BUILT_IN_STRNCASECMP: >> + { >> + /* For these, if one argument refers to one or more of a set >> + of string constants or arrays of known size, determine >> + the range of their known or possible lengths and use it >> + conservatively as the bound for the unbounded function, >> + and to adjust the range of the bound of the bounded ones. */ >> + unsigned stride = with_bounds ? 2 : 1; >> + for (unsigned argno = 0; argno < nargs && !*lenrng; argno += stride) >> + { >> + tree arg = CALL_EXPR_ARG (exp, argno); >> + if (!get_attr_nonstring_decl (arg)) >> + get_range_strlen (arg, lenrng); >> + } >> + } >> + /* Fall through. */ >> + >> + case BUILT_IN_STPNCPY: >> + case BUILT_IN_STPNCPY_CHK: >> case BUILT_IN_STRNCPY: >> case BUILT_IN_STRNCPY_CHK: >> { >> @@ -1647,6 +1669,33 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp) >> + { >> + /* Replace the bound on the oparation with the upper bound > s/oparation/operation/ > > OK for the trunk with the nits fixed. > > Also note that I've acked a patch from Martin L (I believe) that removes > the chkp/bounds checking bits that were deprecated in gcc-8. So there's > some chance the bounds-related bits will need to be updated depending on > whether or not Martin's L's patch has been committed. > > This isn't strictly a regression. So unless this is affecting some > critical code (ie glibc, kernel or something similar) this probably > would require an explicit OK from Jakub or Richi to be eligible for the > gcc-8 branch. There are a number of warnings in Binutils/GDB that people have been suppressing by pragmas because the attribute isn't always effective, most due to bug 85643: https://sourceware.org/ml/binutils/2018-05/msg00212.html I don't know if this bug is also among those instances (there are several threads on the mailing lists and I may have missed some). If the warning for the strncmp() test case above isn't one of them it certainly is a bug/oversight in the warning that makes the attribute less useful than it's meant to be and the warning more noisy. Jakub/Richard, can you please review the bug and the patch and decide if it's also appropriate for GCC 8? Martin
PR c/85623 - strncmp() warns about attribute 'nonstring' incorrectly in -Wstringop-overflow gcc/ChangeLog: PR c/85623 * calls.c (maybe_warn_nonstring_arg): Use string length to set or ajust the presumed bound on an operation to avoid unnecessary warnings. gcc/testsuite/ChangeLog: PR c/85623 * c-c++-common/attr-nonstring-3.c: Adjust. * c-c++-common/attr-nonstring-4.c: Adjust. * c-c++-common/attr-nonstring-6.c: New test. diff --git a/gcc/calls.c b/gcc/calls.c index 9eb0467..f5c8ad4 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3. If not see #include "stringpool.h" #include "attribs.h" #include "builtins.h" +#include "gimple-fold.h" /* Like PREFERRED_STACK_BOUNDARY but in units of bytes, not bits. */ #define STACK_BYTES (PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT) @@ -1612,15 +1613,36 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp) /* The bound argument to a bounded string function like strncpy. */ tree bound = NULL_TREE; + /* The range of lengths of a string argument to one of the comparison + functions. If the length is less than the bound it is used instead. */ + tree lenrng[2] = { NULL_TREE, NULL_TREE }; + /* It's safe to call "bounded" string functions with a non-string argument since the functions provide an explicit bound for this purpose. */ switch (DECL_FUNCTION_CODE (fndecl)) { - case BUILT_IN_STPNCPY: - case BUILT_IN_STPNCPY_CHK: + case BUILT_IN_STRCMP: case BUILT_IN_STRNCMP: case BUILT_IN_STRNCASECMP: + { + /* For these, if one argument refers to one or more of a set + of string constants or arrays of known size, determine + the range of their known or possible lengths and use it + conservatively as the bound for the unbounded function, + and to adjust the range of the bound of the bounded ones. */ + unsigned stride = with_bounds ? 2 : 1; + for (unsigned argno = 0; argno < nargs && !*lenrng; argno += stride) + { + tree arg = CALL_EXPR_ARG (exp, argno); + if (!get_attr_nonstring_decl (arg)) + get_range_strlen (arg, lenrng); + } + } + /* Fall through. */ + + case BUILT_IN_STPNCPY: + case BUILT_IN_STPNCPY_CHK: case BUILT_IN_STRNCPY: case BUILT_IN_STRNCPY_CHK: { @@ -1647,6 +1669,33 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp) if (bound) get_size_range (bound, bndrng); + if (*lenrng) + { + /* Add one for the nul. */ + lenrng[0] = const_binop (PLUS_EXPR, TREE_TYPE (lenrng[0]), + lenrng[0], size_one_node); + lenrng[1] = const_binop (PLUS_EXPR, TREE_TYPE (lenrng[1]), + lenrng[1], size_one_node); + + if (!bndrng[0]) + { + /* Conservatively use the upper bound of the lengths for + both the lower and the upper bound of the operation. */ + bndrng[0] = lenrng[1]; + bndrng[1] = lenrng[1]; + bound = void_type_node; + } + else + { + /* Replace the bound on the oparation with the upper bound + of the length of the string if the latter is smaller. */ + if (tree_int_cst_lt (lenrng[1], bndrng[0])) + bndrng[0] = lenrng[1]; + else if (tree_int_cst_lt (lenrng[1], bndrng[1])) + bndrng[1] = lenrng[1]; + } + } + /* Iterate over the built-in function's formal arguments and check each const char* against the actual argument. If the actual argument is declared attribute non-string issue a warning unless @@ -1689,18 +1738,28 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp) tree type = TREE_TYPE (decl); + /* The maximum number of array elements accessed. */ offset_int wibnd = 0; if (bndrng[0]) wibnd = wi::to_offset (bndrng[0]); + /* Size of the array. */ offset_int asize = wibnd; + /* Determine the array size. For arrays of unknown bound and + pointers reset BOUND to trigger the appropriate warning. */ if (TREE_CODE (type) == ARRAY_TYPE) - if (tree arrbnd = TYPE_DOMAIN (type)) - { - if ((arrbnd = TYPE_MAX_VALUE (arrbnd))) - asize = wi::to_offset (arrbnd) + 1; - } + { + if (tree arrbnd = TYPE_DOMAIN (type)) + { + if ((arrbnd = TYPE_MAX_VALUE (arrbnd))) + asize = wi::to_offset (arrbnd) + 1; + } + else if (bound == void_type_node) + bound = NULL_TREE; + } + else if (bound == void_type_node) + bound = NULL_TREE; location_t loc = EXPR_LOCATION (exp); diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-3.c b/gcc/testsuite/c-c++-common/attr-nonstring-3.c index 1c50e0d..a54e73a 100644 --- a/gcc/testsuite/c-c++-common/attr-nonstring-3.c +++ b/gcc/testsuite/c-c++-common/attr-nonstring-3.c @@ -47,7 +47,10 @@ char* strndup (const char*, size_t); #define NONSTRING __attribute__ ((nonstring)) -char str[4]; +/* STR needs to be bigger than ARR to trigger warnings, otherwise + since STR must be a string, using both in a string function + can be assumed to be safe even if ARR isn't nul-terminated. */ +char str[5]; char arr[4] NONSTRING; char *ptr; @@ -55,7 +58,7 @@ char *parr NONSTRING; struct MemArrays { - char str[4]; + char str[5]; char arr[4] NONSTRING; char *parr NONSTRING; }; diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-4.c b/gcc/testsuite/c-c++-common/attr-nonstring-4.c index 0571e46..597bbb3 100644 --- a/gcc/testsuite/c-c++-common/attr-nonstring-4.c +++ b/gcc/testsuite/c-c++-common/attr-nonstring-4.c @@ -37,23 +37,23 @@ int warn_strcmp_cst_2 (void) int warn_strncmp_cst_1 (void) { - return strncmp ("bar", ar5, X); /* { dg-warning "argument 2 declared attribute .nonstring." } */ + return strncmp ("12345", ar5, X); /* { dg-warning "argument 2 declared attribute .nonstring." } */ } int warn_strncmp_cst_2 (void) { - return strncmp (ar5, "foo", X); /* { dg-warning "argument 1 declared attribute .nonstring." } */ + return strncmp (ar5, "12345", X); /* { dg-warning "argument 1 declared attribute .nonstring." } */ } int nowarn_strncmp_cst_1 (void) { - return strncmp ("bar", ar5, N); + return strncmp ("12345", ar5, N); } int nowarn_strncmp_cst_2 (void) { - return strncmp (ar5, "foo", N); + return strncmp (ar5, "12345", N); } diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-6.c b/gcc/testsuite/c-c++-common/attr-nonstring-6.c new file mode 100644 index 0000000..19ceaac --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-nonstring-6.c @@ -0,0 +1,185 @@ +/* PR 85623 - strncmp() warns about attribute 'nonstring' incorrectly + in -Wstringop-overflow + { dg-do compile } + { dg-options "-O2 -Wstringop-overflow -ftrack-macro-expansion=0" } */ + +#include "../gcc.dg/range.h" + +#if __cplusplus +extern "C" { +#endif + +extern int strcmp (const char*, const char*); +extern int strncmp (const char*, const char*, size_t); +extern int strncasecmp (const char*, const char*, size_t); + +extern size_t strspn (const char*, const char*); +extern size_t strcspn (const char*, const char*); + +#if __cplusplus +} +#endif + +#define S26 "0123456789abcdefghijklmnopqrstuvwxyz" +#define S(n) (S26 + sizeof S26 - 1 - (n)) + +char __attribute__ ((nonstring)) a3[3]; +char __attribute__ ((nonstring)) a5[5]; + +void sink (int); + +#define T(call) sink (call) + +void test_strcmp_cst (void) +{ + /* Verify that no warning is issued for strcmp() calls with a non-string + array argument when the other argument is a string whose length is + less than the size of the array. Because the function stops reading + at the first nul character there is no chance that it will read past + the end of the array. */ + T (strcmp (S (0), a3)); + T (strcmp (S (1), a3)); + T (strcmp (S (2), a3)); + /* The following reads a3[3]. */ + T (strcmp (S (3), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + /* The following also reads past the end of a3. */ + T (strcmp (S (9), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strcmp (a3, S (0))); + T (strcmp (a3, S (1))); + T (strcmp (a3, S (2))); + T (strcmp (a3, S (3))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcmp (a3, S (9))); /* { dg-warning "\\\[-Wstringop-overflow" } */ +} + + +void test_strcmp_range (const char *s) +{ + s = signed_value () < 0 ? S (0) : S (1); + T (strcmp (a3, s)); + + s = signed_value () < 0 ? S (0) : S (2); + T (strcmp (a3, s)); + + s = signed_value () < 0 ? S (0) : S (3); + T (strcmp (a3, s)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + s = signed_value () < 0 ? S (1) : S (2); + T (strcmp (a3, s)); + + s = signed_value () < 0 ? S (1) : S (3); + T (strcmp (a3, s)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + s = signed_value () < 0 ? S (3) : S (4); + T (strcmp (a3, s)); /* { dg-warning "\\\[-Wstringop-overflow" } */ +} + + +void test_strncmp_cst (void) +{ + T (strncmp (S (0), a3, 1)); + T (strncmp (S (1), a3, 2)); + T (strncmp (S (2), a3, 3)); + T (strncmp (S (3), a3, 3)); + T (strncmp (S (3), a3, 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strncmp (S (9), a3, 3)); + T (strncmp (S (9), a3, 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strncmp (S (9), a3, 5)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strncmp (a3, S (0), 1)); + T (strncmp (a3, S (1), 2)); + T (strncmp (a3, S (2), 3)); + T (strncmp (a3, S (3), 3)); + T (strncmp (a3, S (3), 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strncmp (a3, S (9), 3)); + T (strncmp (a3, S (9), 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strncmp (a3, S (9), 5)); /* { dg-warning "\\\[-Wstringop-overflow" } */ +} + +void test_strncmp_range (const char *s) +{ + T (strncmp (a3, S (2), UR (0, 3))); + T (strncmp (a3, S (2), UR (1, 4))); + T (strncmp (a3, S (2), UR (2, 5))); + T (strncmp (a3, S (2), UR (3, 6))); + T (strncmp (a3, S (2), UR (4, 7))); + + T (strncmp (a3, S (5), UR (0, 3))); + T (strncmp (a3, S (5), UR (1, 4))); + T (strncmp (a3, S (5), UR (2, 5))); + T (strncmp (a3, S (5), UR (3, 6))); + T (strncmp (a3, S (5), UR (4, 7))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strncmp (a3, S (5), UR (7, 9))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + s = signed_value () < 0 ? S (0) : S (1); + T (strncmp (a3, s, UR (1, 3))); + T (strncmp (a3, s, UR (2, 5))); + + s = signed_value () < 0 ? S (2) : S (5); + T (strncmp (a3, s, UR (1, 3))); + + s = signed_value () < 0 ? S (2) : S (5); + T (strncmp (a3, s, UR (1, 4))); + T (strncmp (a3, s, UR (2, 5))); + T (strncmp (a3, s, UR (3, 6))); + T (strncmp (a3, s, UR (4, 7))); /* { dg-warning "\\\[-Wstringop-overflow" } */ +} + +void test_strncasecmp (void) +{ + T (strncasecmp (S (0), a3, 1)); + T (strncasecmp (S (1), a3, 2)); + T (strncasecmp (S (2), a3, 3)); + T (strncasecmp (S (3), a3, 3)); + T (strncasecmp (S (3), a3, 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strncasecmp (S (9), a3, 3)); + T (strncasecmp (S (9), a3, 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strncasecmp (S (9), a3, 5)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strncasecmp (a3, S (0), 1)); + T (strncasecmp (a3, S (1), 2)); + T (strncasecmp (a3, S (2), 3)); + T (strncasecmp (a3, S (3), 3)); + T (strncasecmp (a3, S (3), 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strncasecmp (a3, S (9), 3)); + T (strncasecmp (a3, S (9), 4)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strncasecmp (a3, S (9), 5)); /* { dg-warning "\\\[-Wstringop-overflow" } */ +} + +void test_strspn (void) +{ + /* strspn must traverse all characters in the second argument except + when the first string is empty. */ + T (strspn (S (0), a3)); + T (strspn (S (1), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strspn (S (2), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strspn (S (3), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strspn (S (9), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + /* Similarly, strspn must traverse all characters in the first argument + except when the second string is empty. */ + T (strspn (a3, S (0))); + T (strspn (a3, S (1))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strspn (a3, S (2))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strspn (a3, S (3))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strspn (a3, S (9))); /* { dg-warning "\\\[-Wstringop-overflow" } */ +} + +void test_strcspn (void) +{ + T (strcspn (S (0), a3)); + T (strcspn (S (1), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcspn (S (2), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcspn (S (3), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcspn (S (9), a3)); /* { dg-warning "\\\[-Wstringop-overflow" } */ + + T (strcspn (a3, S (0))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcspn (a3, S (1))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcspn (a3, S (2))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcspn (a3, S (3))); /* { dg-warning "\\\[-Wstringop-overflow" } */ + T (strcspn (a3, S (9))); /* { dg-warning "\\\[-Wstringop-overflow" } */ +}
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. Martin