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.