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.

From-SVN: r260541

gcc/ChangeLog

@@ -42,6 +42,13 @@
 
 2018-05-22  Martin Sebor  <msebor@redhat.com>
 
+	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.
+
+2018-05-22  Martin Sebor  <msebor@redhat.com>
+
 	PR tree-optimization/85826
 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Avoid
 	assuming that a DECL necesarily has a constant size.

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)
@@ -1616,15 +1617,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:
       {
@@ -1651,6 +1673,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
@@ -1693,18 +1742,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);
 

gcc/testsuite/ChangeLog

@@ -6,6 +6,13 @@
 
 2018-05-22  Martin Sebor  <msebor@redhat.com>
 
+	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.
+
+2018-05-22  Martin Sebor  <msebor@redhat.com>
+
 	PR tree-optimization/85826
 	* gcc.dg/Wrestrict-17.c: New test.
 

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;
 };

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);
 }
 
 

gcc/testsuite/c-c++-common/attr-nonstring-6.c

+/* 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" } */
+}