[PATCH] detect nonstring arguments to string functions (PR 82945)

public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed

* [PATCH] detect nonstring arguments to string functions (PR 82945)
@ 2017-11-13  8:13 Martin Sebor
  2017-11-13  8:46 ` Bernhard Reutner-Fischer
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Martin Sebor @ 2017-11-13  8:13 UTC (permalink / raw)
  To: Jeff Law, Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

The recently introduced -Wstringop-truncation warning relies
on the new nonstring attribute to allow the historical use case
of calling strncpy to completely fill the destination with a copy
of a string without adding a terminating nul.  Glibc is currently
considering making use of the attribute to decorate some of its
data members.  To help find misuses of such data members in
arguments to string functions like strlen or strdup, the attached
patch adds checking for this new attribute in these contexts.
The checking is intentionally done late so  that uses such arrays
that can be proven to be safe (and thus folded) are not diagnosed.

While testing this simple enhancement I noticed that the handling
I added for the nul termination in cases like

   strncpy (array, s, sizeof array);
   array[sizeof array - 1] = 0;

to avoid the new warning wasn't quite as robust as it could and
arguably should be so I improved it a bit to silently accept
more forms of the idiom.  For instance, this is now correctly
handled (and not diagnosed):

   *stpcpy (array, s, sizeof array - 1) = 0;

Martin

PS A useful future enhancement would be to detect the one byte
overflow in:

   *stpcpy (array, s, sizeof array) = 0;

[-- Attachment #2: gcc-82945.diff --]
[-- Type: text/x-patch, Size: 31030 bytes --]

PR tree-optimization/82945 - add warning for passing non-strings to functions that expect string arguments

gcc/ChangeLog:

	PR tree-optimization/82945
	* calls.c (get_attr_nonstring_decl, maybe_warn_nonstring_arg): New
	functions.
	(initialize_argument_information): Call maybe_warn_nonstring_arg.
	* calls.h (get_attr_nonstring_decl): Declare new function.
	* doc/extend.texi (attribute nonstring): Update.
	* gimple-fold.c (gimple_fold_builtin_strncpy): Call
	get_attr_nonstring_decl and handle it.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.  Improve
	detection of nul-termination.

gcc/testsuite/ChangeLog:

	PR tree-optimization/82945
	* c-c++-common/Wstringop-truncation-2.c: New test.
	* c-c++-common/Wstringop-truncation.c: Adjust.
	* c-c++-common/attr-nonstring-2.c: Adjust.
	* c-c++-common/attr-nonstring-3.c: New test.

diff --git a/gcc/calls.c b/gcc/calls.c
index 3730f43..c555f9a 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1494,6 +1494,139 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2])
     }
 }
 
+/* If EXPR refers to a character array or pointer declared attribute
+   nonstring return a decl for that array or pointer and set *REF to
+   the referenced enclosing object or pointer.  Otherwise return
+   null.  */
+
+tree
+get_attr_nonstring_decl (tree expr, tree *ref)
+{
+  tree dcl = expr;
+  if (TREE_CODE (dcl) == SSA_NAME)
+    {
+      if (SSA_NAME_IS_DEFAULT_DEF (dcl))
+	dcl = SSA_NAME_VAR (dcl);
+      else
+	{
+	  gimple *def = SSA_NAME_DEF_STMT (dcl);
+
+	  if (is_gimple_assign (def))
+	    {
+	      tree_code code = gimple_assign_rhs_code (def);
+	      if (code == ADDR_EXPR
+		  || code == COMPONENT_REF
+		  || code == VAR_DECL)
+		dcl = gimple_assign_rhs1 (def);
+	    }
+	}
+    }
+
+  if (TREE_CODE (dcl) == ADDR_EXPR)
+    dcl = TREE_OPERAND (dcl, 0);
+
+  if (ref)
+    *ref = dcl;
+
+  if (TREE_CODE (dcl) == COMPONENT_REF)
+    dcl = TREE_OPERAND (dcl, 1);
+
+  if (DECL_P (dcl)
+      && lookup_attribute ("nonstring", DECL_ATTRIBUTES (dcl)))
+    return dcl;
+
+  return NULL_TREE;
+}
+
+/* Warn about passing a non-string array/pointer to a function that
+   expects a nul-terminated string argument.  */
+
+static void
+maybe_warn_nonstring_arg (tree fndecl, tree exp)
+{
+  if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
+    return;
+
+  /* -1 terminated array of zero-based string arguments.  */
+  unsigned argno[] = { -1, -1, -1 };
+
+  switch (DECL_FUNCTION_CODE (fndecl))
+    {
+    case BUILT_IN_STRCASECMP:
+    case BUILT_IN_STRCMP:
+    case BUILT_IN_STRCSPN:
+    case BUILT_IN_STRSPN:
+    case BUILT_IN_STRNCMP:
+    case BUILT_IN_STRNCASECMP:
+    case BUILT_IN_VSSCANF:
+      argno[0] = 0;
+      argno[1] = 1;
+      break;
+
+    case BUILT_IN_STPCPY:
+    case BUILT_IN_STPNCPY:
+    case BUILT_IN_STRCAT:
+    case BUILT_IN_STRCPY:
+    case BUILT_IN_STRNCAT:
+    case BUILT_IN_STRNCPY:
+      argno[0] = 1;
+      break;
+
+    case BUILT_IN_FPRINTF:
+    case BUILT_IN_FPUTS:
+    case BUILT_IN_SPRINTF:
+    case BUILT_IN_STPCPY_CHK:
+    case BUILT_IN_STPNCPY_CHK:
+    case BUILT_IN_STRCAT_CHK:
+    case BUILT_IN_STRCPY_CHK:
+    case BUILT_IN_STRNCAT_CHK:
+    case BUILT_IN_STRNCPY_CHK:
+    case BUILT_IN_VFPRINTF:
+    case BUILT_IN_VSPRINTF:
+    case BUILT_IN_VFSCANF:
+      argno[0] = 1;
+      break;
+
+    case BUILT_IN_SNPRINTF:
+    case BUILT_IN_VSNPRINTF:
+      argno[0] = 2;
+      break;
+
+    case BUILT_IN_PRINTF:
+    case BUILT_IN_PRINTF_UNLOCKED:
+    case BUILT_IN_PUTS:
+    case BUILT_IN_PUTS_UNLOCKED:
+    case BUILT_IN_STRCHR:
+    case BUILT_IN_STRDUP:
+    case BUILT_IN_STRLEN:
+      argno[0] = 0;
+      break;
+
+    default:
+      return;
+    }
+
+  for (unsigned i = 0; argno[i] != -1U; ++i)
+    {
+      tree arg = CALL_EXPR_ARG (exp, argno[i]);
+      if (TREE_CODE (arg) == ADDR_EXPR)
+	arg = TREE_OPERAND (arg, 0);
+
+      /* See if the destination is declared with attribute "nonstring"
+	 and if so, avoid the truncation warning.  */
+      tree dcl = get_attr_nonstring_decl (arg);
+      if (!dcl)
+	continue;
+
+      location_t loc = EXPR_LOCATION (exp);
+      if (warning_at (loc, OPT_Wstringop_overflow_,
+		      "%qD argument %i declared attribute %<nonstring%>",
+		      fndecl, argno[i] + 1))
+	inform (DECL_SOURCE_LOCATION (dcl),
+		"argument %qD declared here", dcl);
+    }
+}
+
 /* Issue an error if CALL_EXPR was flagged as requiring
    tall-call optimization.  */
 
@@ -1943,6 +2076,10 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 	 alloc_size.  */
       maybe_warn_alloc_args_overflow (fndecl, exp, alloc_args, alloc_idx);
     }
+
+  /* Detect passing non-string arguments to functions expecting
+     nul-terminated strings.  */
+  maybe_warn_nonstring_arg (fndecl, exp);
 }
 
 /* Update ARGS_SIZE to contain the total size for the argument block.
diff --git a/gcc/calls.h b/gcc/calls.h
index df5817f..4f069b4 100644
--- a/gcc/calls.h
+++ b/gcc/calls.h
@@ -39,5 +39,6 @@ extern bool reference_callee_copied (CUMULATIVE_ARGS *, machine_mode,
 				     tree, bool);
 extern void maybe_warn_alloc_args_overflow (tree, tree, tree[2], int[2]);
 extern bool get_size_range (tree, tree[2]);
+extern tree get_attr_nonstring_decl (tree, tree * = NULL);
 
 #endif // GCC_CALLS_H
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 63b58c0..ccd1696 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -5974,22 +5974,31 @@ types (@pxref{Common Function Attributes},
 The @code{nonstring} variable attribute specifies that an object or member
 declaration with type array of @code{char} or pointer to @code{char} is
 intended to store character arrays that do not necessarily contain
-a terminating @code{NUL} character.  This is useful to avoid warnings
-when such an array or pointer is used as an argument to a bounded string
-manipulation function such as @code{strncpy}.  For example, without the
-attribute, GCC will issue a warning for the call below because it may
-truncate the copy without appending the terminating NUL character.  Using
-the attribute makes it possible to suppress the warning.
+a terminating @code{NUL} character.  This is useful in detecting uses
+of such arrays or pointers with functions that expect NUL-terminated
+strings, and to avoid warnings when such an array or pointer is used
+as an argument to a bounded string manipulation function such as
+@code{strncpy}.  For example, without the attribute, GCC will issue
+a warning for the @code{strncpy} call below because it may truncate
+the copy without appending the terminating @code{NUL} character.  Using
+the attribute makes it possible to suppress the warning.  However, when
+he array is declared with the attribute the call to @code{strlen} is
+diagnosed because when the array doesn't contain a @code{NUL}-terminated
+string the call is undefined.  To copy, compare, of search non-string
+character arrays use the @code{memcpy}, @code{memcmp}, @code{memchr},
+and other functions that operate on arrays of bytes.
 
 @smallexample
 struct Data
 @{
   char name [32] __attribute__ ((nonstring));
 @};
-void f (struct Data *pd, const char *s)
+
+int f (struct Data *pd, const char *s)
 @{
   strncpy (pd->name, s, sizeof pd->name);
   @dots{}
+  return strlen (pd->name);   // unsafe, gets a warning
 @}
 @end smallexample
 
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index adb6f3b..8c6cdff 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -62,6 +62,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "diagnostic-core.h"
 #include "intl.h"
+#include "calls.h"
 
 /* Return true when DECL can be referenced from current unit.
    FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
@@ -1558,25 +1559,31 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
 {
   gimple *stmt = gsi_stmt (*gsi);
   location_t loc = gimple_location (stmt);
+  bool nonstring = get_attr_nonstring_decl (dest) != NULL_TREE;
 
   /* If the LEN parameter is zero, return DEST.  */
   if (integer_zerop (len))
     {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      /* Warn about the lack of nul termination: the result is not
-	 a (nul-terminated) string.  */
-      tree slen = get_maxval_strlen (src, 0);
-      if (slen && !integer_zerop (slen))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes "
-		    "from a string of length %E",
-		    call, fndecl, slen);
-      else
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes",
-		    call, fndecl);
+      /* Avoid warning if the destination refers to a an array/pointer
+	 decorate with attribute nonstring.  */
+      if (!nonstring)
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  /* Warn about the lack of nul termination: the result is not
+	     a (nul-terminated) string.  */
+	  tree slen = get_maxval_strlen (src, 0);
+	  if (slen && !integer_zerop (slen))
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes "
+			"from a string of length %E",
+			call, fndecl, slen);
+	  else
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes",
+			call, fndecl);
+	}
 
       replace_call_with_value (gsi, dest);
       return true;
@@ -1601,53 +1608,36 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
   if (tree_int_cst_lt (ssize, len))
     return false;
 
-  if (tree_int_cst_lt (len, slen))
-    {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      warning_at (loc, OPT_Wstringop_truncation,
-		  (tree_int_cst_equal (size_one_node, len)
-		   ? G_("%G%qD output truncated copying %E byte "
-			"from a string of length %E")
-		   : G_("%G%qD output truncated copying %E bytes "
-		      "from a string of length %E")),
-		  call, fndecl, len, slen);
-    }
-  else if (tree_int_cst_equal (len, slen))
+  if (!nonstring)
     {
-      tree decl = dest;
-      if (TREE_CODE (decl) == SSA_NAME)
+      if (tree_int_cst_lt (len, slen))
 	{
-	  gimple *def_stmt = SSA_NAME_DEF_STMT (decl);
-	  if (is_gimple_assign (def_stmt))
-	    {
-	      tree_code code = gimple_assign_rhs_code (def_stmt);
-	      if (code == ADDR_EXPR || code == VAR_DECL)
-		decl = gimple_assign_rhs1 (def_stmt);
-	    }
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated copying %E byte "
+			    "from a string of length %E")
+		       : G_("%G%qD output truncated copying %E bytes "
+			    "from a string of length %E")),
+		      call, fndecl, len, slen);
+	}
+      else if (tree_int_cst_equal (len, slen))
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated before terminating nul "
+			    "copying %E byte from a string of the same "
+			    "length")
+		       : G_("%G%qD output truncated before terminating nul "
+			    "copying %E bytes from a string of the same "
+			    "length")),
+		      call, fndecl, len);
 	}
-
-      if (TREE_CODE (decl) == ADDR_EXPR)
-	decl = TREE_OPERAND (decl, 0);
-
-      if (TREE_CODE (decl) == COMPONENT_REF)
-	decl = TREE_OPERAND (decl, 1);
-
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      if (!DECL_P (decl)
-	  || !lookup_attribute ("nonstring", DECL_ATTRIBUTES (decl)))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    (tree_int_cst_equal (size_one_node, len)
-		     ? G_("%G%qD output truncated before terminating nul "
-			  "copying %E byte from a string of the same "
-			  "length")
-		     : G_("%G%qD output truncated before terminating nul "
-			  "copying %E bytes from a string of the same "
-			  "length")),
-		    call, fndecl, len);
     }
 
   /* OK transform into builtin memcpy.  */
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
new file mode 100644
index 0000000..8256070
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
@@ -0,0 +1,56 @@
+/* Verify that 
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-truncation -Wno-stringop-overflow -ftrack-macro-expansion=0" } */
+
+
+typedef __SIZE_TYPE__ size_t;
+
+struct A { char str[3]; };
+
+struct B { struct A a[3]; int i; };
+struct C { struct B b[3]; int i; };
+
+void stpncpy_nowarn_1 (struct C *pc, const char *s)
+{
+  __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str)[-1] = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  *__builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1) = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_3 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  d[-1] = 0;
+}
+
+void stpncpy_nowarn_4 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  *d = 0;
+}
+
+void strncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  pc->b[0].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_1 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[1].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[0].a[1].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
index c536a13..409a0e3 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-truncation.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
@@ -311,9 +311,11 @@ void test_strncpy_array (Dest *pd, int i, const char* s)
   /* Exercise destination with attribute "nonstring".  */
   CPY (pd->c3ns, "", 3);
   CPY (pd->c3ns, "", 1);
-  /* Truncation is still diagnosed -- using strncpy in this case is
-     pointless and should be replaced with memcpy.  */
-  CPY (pd->c3ns, "12", 1);          /* { dg-warning "output truncated copying 1 byte from a string of length 2" } */
+  /* It could be argued that truncation in the literal case should be
+     diagnosed even for non-strings.  Using strncpy in this case is
+     pointless and should be replaced with memcpy.  But it would likely
+     be viewed as a false positive.  */
+  CPY (pd->c3ns, "12", 1);
   CPY (pd->c3ns, "12", 2);
   CPY (pd->c3ns, "12", 3);
   CPY (pd->c3ns, "123", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-2.c b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
index 6e273e7..67fce03 100644
--- a/gcc/testsuite/c-c++-common/attr-nonstring-2.c
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
@@ -89,7 +89,7 @@ void test_pointer (const char *s, unsigned n)
   strncpy (pns_1, "a", 1);
   strncpy (pns_2, "ab", 2);
   strncpy (pns_3, "abc", 3);
-  strncpy (pns_3, s7, 3);         /* { dg-warning "output truncated copying 3 bytes from a string of length 7" } */
+  strncpy (pns_3, s7, 3);
 
   strncpy (pns_1, s, 1);
   strncpy (pns_2, s, 1);
@@ -105,6 +105,7 @@ void test_member_array (struct MemArrays *p, const char *s, unsigned n)
 {
   const char s7[] = "1234567";
 
+  strncpy (p->ma3, "", 0);
   strncpy (p->ma3, "a", 1);
   strncpy (p->ma4, "ab", 2);
   strncpy (p->ma5, "abc", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-3.c b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
new file mode 100644
index 0000000..a4da53e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
@@ -0,0 +1,356 @@
+/* Test to exercise warnings when an array declared with attribute "nonstring"
+   is passed to a function that expects a nul-terminate string as an argument.
+   { dg-do compile }
+   { dg-options "-O2 -Wattributes -Wstringop-overflow -ftrack-macro-expansion=0" }  */
+
+typedef __SIZE_TYPE__       size_t;
+typedef __builtin_va_list   va_list;
+
+#if __cplusplus
+extern "C" {
+#endif
+
+void* memchr (const void*, int, size_t);
+int memcmp (const void*, const void*, size_t);
+void* memcpy (void*, const void*, size_t);
+void* memmove (void*, const void*, size_t);
+
+int printf (const char*, ...);
+int puts (const char*);
+int puts_unlocked (const char*);
+int sprintf (char*, const char*, ...);
+int snprintf (char*, size_t, const char*, ...);
+int vsprintf (char*, const char*, va_list);
+int vsnprintf (char*, size_t, const char*, va_list);
+
+int strcmp (const char*, const char*);
+int strncmp (const char*, const char*, size_t);
+
+char* stpcpy (char*, const char*);
+char* stpncpy (char*, const char*, size_t);
+
+char* strcat (char*, const char*);
+char* strncat (char*, const char*, size_t);
+
+char* strcpy (char*, const char*);
+char* strncpy (char*, const char*, size_t);
+
+char* strchr (const char*, int);
+char* strdup (const char*);
+size_t strlen (const char*);
+char* strndup (const char*, size_t);
+
+#if __cplusplus
+}   /* extern "C" */
+#endif
+
+#define NONSTRING __attribute__ ((nonstring))
+
+char str[4];
+char arr[4] NONSTRING;
+
+char *ptr;
+char *parr NONSTRING;
+
+struct MemArrays
+{
+  char str[4];
+  char arr[4] NONSTRING;
+  char *parr NONSTRING;
+};
+
+void sink (int, ...);
+
+
+#define T(call)  sink (0, (call))
+
+void test_printf (struct MemArrays *p)
+{
+  T (printf (str));
+  T (printf (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (ptr));
+  T (printf (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (p->str));
+  T (printf (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_puts (struct MemArrays *p)
+{
+  T (puts (str));
+  T (puts (arr));               /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (ptr));
+  T (puts (parr));              /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (p->str));
+  T (puts (p->arr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_snprintf (char *d, size_t n, struct MemArrays *p)
+{
+  T (snprintf (d, n, str));
+  T (snprintf (d, n, arr));       /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, ptr));
+  T (snprintf (d, n, parr));      /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, p->str));
+  T (snprintf (d, n, p->arr));    /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_sprintf (char *d, struct MemArrays *p)
+{
+  T (sprintf (d, str));
+  T (sprintf (d, arr));           /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, ptr));
+  T (sprintf (d, parr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, p->str));
+  T (sprintf (d, p->arr));        /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_vsnprintf (char *d, size_t n, struct MemArrays *p, va_list va)
+{
+  T (vsnprintf (d, n, str, va));
+  T (vsnprintf (d, n, arr, va));  /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, ptr, va));
+  T (vsnprintf (d, n, parr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, p->str, va));
+  T (vsnprintf (d, n, p->arr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_vsprintf (char *d, struct MemArrays *p, va_list va)
+{
+  T (vsprintf (d, str, va));
+  T (vsprintf (d, arr, va));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, ptr, va));
+  T (vsprintf (d, parr, va));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, p->str, va));
+  T (vsprintf (d, p->arr, va));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_strcmp (struct MemArrays *p)
+{
+  T (strcmp (str, str));
+  T (strcmp (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (arr, str));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (str, ptr));
+  T (strcmp (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (parr, str));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (p->arr, p->str));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strcmp (p->parr, p->str));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strncmp (struct MemArrays *p, size_t n)
+{
+  T (strncmp (str, str, n));
+  T (strncmp (str, arr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (arr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (str, ptr, n));
+  T (strncmp (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (parr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (p->str, p->arr, n));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (p->arr, p->str, n));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strncmp (p->parr, p->str, n));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stpcpy (struct MemArrays *p)
+{
+  T (stpcpy (str, str));
+  T (stpcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (arr, str));
+
+  T (stpcpy (str, ptr));
+  T (stpcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (parr, str));
+
+  T (stpcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (p->arr, p->str));
+  T (stpcpy (p->parr, p->str));
+}
+
+
+void test_stpncpy (struct MemArrays *p, unsigned n)
+{
+  T (stpncpy (str, str, n));
+  T (stpncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (arr, str, n));
+
+  T (stpncpy (str, ptr, n));
+  T (stpncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (parr, str, n));
+
+  T (stpncpy (p->str, p->arr, n));/* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (p->arr, p->str, n));
+  T (stpncpy (p->parr, p->str, n));
+}
+
+
+void test_strcat (struct MemArrays *p)
+{
+  T (strcat (str, str));
+  T (strcat (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (arr, str));
+
+  T (strcat (str, ptr));
+  T (strcat (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (parr, str));
+
+  T (strcat (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (p->arr, p->str));
+  T (strcat (p->parr, p->str));
+}
+
+
+void test_strncat (struct MemArrays *p, unsigned n)
+{
+  T (strncat (str, str, n));
+  T (strncat (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (arr, str, n));
+
+  T (strncat (str, ptr, n));
+  T (strncat (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (parr, str, n));
+
+  T (strncat (p->str, p->arr, n));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (p->arr, p->str, n));
+  T (strncat (p->parr, p->str, n));
+}
+
+
+void test_strcpy (struct MemArrays *p)
+{
+  T (strcpy (str, str));
+  T (strcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (arr, str));
+
+  T (strcpy (str, ptr));
+  T (strcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (parr, str));
+
+  T (strcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (p->arr, p->str));
+  T (strcpy (p->parr, p->str));
+}
+
+
+void test_strncpy (struct MemArrays *p, unsigned n)
+{
+  T (strncpy (str, str, n));
+  T (strncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (arr, str, n));
+
+  T (strncpy (str, ptr, n));
+  T (strncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (parr, str, n));
+
+  T (strncpy (p->str, p->arr, n));  /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (p->arr, p->str, n));
+  T (strncpy (p->parr, p->str, n));
+}
+
+
+void test_strchr (struct MemArrays *p, int c)
+{
+  T (strchr (str, c));
+  T (strchr (arr, c));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (ptr, c));
+  T (strchr (parr, c));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (p->str, c));
+  T (strchr (p->arr, c));       /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strdup (struct MemArrays *p)
+{
+  T (strdup (str));
+  T (strdup (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (ptr));
+  T (strdup (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (p->str));
+  T (strdup (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stnrdup (struct MemArrays *p, size_t n)
+{
+  T (strndup (str, n));
+  T (strndup (arr, n));
+
+  T (strndup (ptr, n));
+  T (strndup (parr, n));
+
+  T (strndup (p->str, n));
+  T (strndup (p->arr, n));
+}
+
+
+void test_strlen (struct MemArrays *p)
+{
+  T (strlen (str));
+  T (strlen (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (ptr));
+  T (strlen (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (p->str));
+  T (strlen (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+/* Verify no warnings are issued for raw mempory functions.  */
+
+void test_mem_functions (struct MemArrays *p, int c, size_t n)
+{
+  T (memchr (arr, c, n));
+  T (memchr (parr, c, n));
+  T (memchr (p->arr, c, n));
+  T (memchr (p->parr, c, n));
+
+  T (memcmp (str, arr, n));
+  T (memcmp (arr, str, n));
+  T (memcmp (str, parr, n));
+  T (memcmp (parr, str, n));
+  T (memcmp (p->str, p->arr, n));
+  T (memcmp (p->arr, p->str, n));
+  T (memcmp (p->parr, p->str, n));
+
+  T (memcpy (str, arr, n));
+  T (memcpy (arr, str, n));
+  T (memcpy (str, parr, n));
+  T (memcpy (parr, str, n));
+  T (memcpy (p->str, p->arr, n));
+  T (memcpy (p->arr, p->str, n));
+  T (memcpy (p->parr, p->str, n));
+
+  T (memmove (str, arr, n));
+  T (memmove (arr, str, n));
+  T (memmove (str, parr, n));
+  T (memmove (parr, str, n));
+  T (memmove (p->str, p->arr, n));
+  T (memmove (p->arr, p->str, n));
+  T (memmove (p->parr, p->str, n));
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 2efa182..8b89dbe 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "intl.h"
 #include "attribs.h"
+#include "calls.h"
 
 /* A vector indexed by SSA_NAME_VERSION.  0 means unknown, positive value
    is an index into strinfo vector, negative value stands for
@@ -1733,35 +1734,15 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
     return false;
 
   tree dst = gimple_call_arg (stmt, 0);
-
-  /* See if the destination is declared with attribute "nonstring"
-     and if so, avoid the truncation warning.  */
-  if (TREE_CODE (dst) == SSA_NAME)
-    {
-      if (SSA_NAME_IS_DEFAULT_DEF (dst))
-	dst = SSA_NAME_VAR (dst);
-      else
-	{
-	  gimple *def = SSA_NAME_DEF_STMT (dst);
-
-	  if (is_gimple_assign (def)
-	      && gimple_assign_rhs_code (def) == ADDR_EXPR)
-	    dst = gimple_assign_rhs1 (def);
-	}
-    }
-
   tree dstdecl = dst;
   if (TREE_CODE (dstdecl) == ADDR_EXPR)
     dstdecl = TREE_OPERAND (dstdecl, 0);
 
-  {
-    tree d = dstdecl;
-    if (TREE_CODE (d) == COMPONENT_REF)
-      d = TREE_OPERAND (d, 1);
-
-    if (DECL_P (d) && lookup_attribute ("nonstring", DECL_ATTRIBUTES (d)))
-      return false;
-  }
+  /* If the destination refers to a an array/pointer declared nonstring
+     return early.  */
+  tree ref = NULL_TREE;
+  if (get_attr_nonstring_decl (dstdecl, &ref))
+    return false;
 
   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
      avoid the truncation warning.  */
@@ -1770,12 +1751,32 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
 
   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
     {
-      HOST_WIDE_INT off;
-      dstdecl = get_addr_base_and_unit_offset (dstdecl, &off);
-
       tree lhs = gimple_assign_lhs (next_stmt);
-      tree lhsbase = get_addr_base_and_unit_offset (lhs, &off);
-      if (lhsbase && operand_equal_p (dstdecl, lhsbase, 0))
+      tree_code code = TREE_CODE (lhs);
+      if (code == ARRAY_REF || code == MEM_REF)
+	lhs = TREE_OPERAND (lhs, 0);
+
+      tree func = gimple_call_fndecl (stmt);
+      if (DECL_FUNCTION_CODE (func) == BUILT_IN_STPNCPY)
+	{
+	  tree ret = gimple_call_lhs (stmt);
+	  if (ret && operand_equal_p (ret, lhs, 0))
+	    return false;
+	}
+
+      /* Determine the base address and offset of the reference,
+	 ignoring the innermost array index.  */
+      if (TREE_CODE (ref) == ARRAY_REF)
+	ref = TREE_OPERAND (ref, 0);
+
+      HOST_WIDE_INT dstoff;
+      tree dstbase = get_addr_base_and_unit_offset (ref, &dstoff);
+
+      HOST_WIDE_INT lhsoff;
+      tree lhsbase = get_addr_base_and_unit_offset (lhs, &lhsoff);
+      if (lhsbase
+	  && dstoff == lhsoff
+	  && operand_equal_p (dstbase, lhsbase, 0))
 	return false;
     }
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-13  8:13 [PATCH] detect nonstring arguments to string functions (PR 82945) Martin Sebor
@ 2017-11-13  8:46 ` Bernhard Reutner-Fischer
  2017-11-13 20:04 ` Jakub Jelinek
  2017-11-14  6:35 ` Martin Sebor
  2 siblings, 0 replies; 13+ messages in thread
From: Bernhard Reutner-Fischer @ 2017-11-13  8:46 UTC (permalink / raw)
  To: gcc-patches, Martin Sebor, Jeff Law, Gcc Patch List

On 13 November 2017 01:52:41 CET, Martin Sebor <msebor@gmail.com> wrote:

s/^\(he array\)/t\1/

thanks, 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-13  8:13 [PATCH] detect nonstring arguments to string functions (PR 82945) Martin Sebor
  2017-11-13  8:46 ` Bernhard Reutner-Fischer
@ 2017-11-13 20:04 ` Jakub Jelinek
  2017-11-14  1:21   ` Martin Sebor
  2017-11-14  6:35 ` Martin Sebor
  2 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2017-11-13 20:04 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jeff Law, Gcc Patch List

On Sun, Nov 12, 2017 at 05:52:41PM -0700, Martin Sebor wrote:
> +   the referenced enclosing object or pointer.  Otherwise return
> +   null.  */
> +
> +tree
> +get_attr_nonstring_decl (tree expr, tree *ref)
> +{
> +  tree dcl = expr;

Usually we call vars decl, not dcl.
Or what does it stand for?  In multiple other spots.

> +  /* -1 terminated array of zero-based string arguments.  */
> +  unsigned argno[] = { -1, -1, -1 };
> +
> +  switch (DECL_FUNCTION_CODE (fndecl))
> +    {
> +    case BUILT_IN_STRCASECMP:
> +    case BUILT_IN_STRCMP:
> +    case BUILT_IN_STRCSPN:
> +    case BUILT_IN_STRSPN:
> +    case BUILT_IN_STRNCMP:
> +    case BUILT_IN_STRNCASECMP:
> +    case BUILT_IN_VSSCANF:
> +      argno[0] = 0;
> +      argno[1] = 1;
> +      break;
> +
> +    case BUILT_IN_STPCPY:
> +    case BUILT_IN_STPNCPY:
> +    case BUILT_IN_STRCAT:
> +    case BUILT_IN_STRCPY:
> +    case BUILT_IN_STRNCAT:
> +    case BUILT_IN_STRNCPY:
> +      argno[0] = 1;
> +      break;
> +
> +    case BUILT_IN_FPRINTF:
> +    case BUILT_IN_FPUTS:
> +    case BUILT_IN_SPRINTF:
> +    case BUILT_IN_STPCPY_CHK:
> +    case BUILT_IN_STPNCPY_CHK:
> +    case BUILT_IN_STRCAT_CHK:
> +    case BUILT_IN_STRCPY_CHK:
> +    case BUILT_IN_STRNCAT_CHK:
> +    case BUILT_IN_STRNCPY_CHK:
> +    case BUILT_IN_VFPRINTF:
> +    case BUILT_IN_VSPRINTF:
> +    case BUILT_IN_VFSCANF:
> +      argno[0] = 1;
> +      break;
> +
> +    case BUILT_IN_SNPRINTF:
> +    case BUILT_IN_VSNPRINTF:
> +      argno[0] = 2;
> +      break;
> +
> +    case BUILT_IN_PRINTF:
> +    case BUILT_IN_PRINTF_UNLOCKED:
> +    case BUILT_IN_PUTS:
> +    case BUILT_IN_PUTS_UNLOCKED:
> +    case BUILT_IN_STRCHR:
> +    case BUILT_IN_STRDUP:
> +    case BUILT_IN_STRLEN:

How was the above list of builtins chosen?
I don't see why some are included and others that behave similarly aren't.
Say, you have vsscanf and vfscanf in the list, but not vscanf, fscanf,
scanf and sscanf.  Or {f,s,sn,}printf and v{f,s,sn}printf,
but not vprintf, and have printf_unlocked, but not fprintf_unlocked.
And no *printf_chk.

	Jakub

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-13 20:04 ` Jakub Jelinek
@ 2017-11-14  1:21   ` Martin Sebor
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Sebor @ 2017-11-14  1:21 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jeff Law, Gcc Patch List

On 11/13/2017 12:20 PM, Jakub Jelinek wrote:
> On Sun, Nov 12, 2017 at 05:52:41PM -0700, Martin Sebor wrote:
>> +   the referenced enclosing object or pointer.  Otherwise return
>> +   null.  */
>> +
>> +tree
>> +get_attr_nonstring_decl (tree expr, tree *ref)
>> +{
>> +  tree dcl = expr;
>
> Usually we call vars decl, not dcl.
> Or what does it stand for?  In multiple other spots.
>
>> +  /* -1 terminated array of zero-based string arguments.  */
>> +  unsigned argno[] = { -1, -1, -1 };
>> +
>> +  switch (DECL_FUNCTION_CODE (fndecl))
>> +    {
>> +    case BUILT_IN_STRCASECMP:
>> +    case BUILT_IN_STRCMP:
>> +    case BUILT_IN_STRCSPN:
>> +    case BUILT_IN_STRSPN:
>> +    case BUILT_IN_STRNCMP:
>> +    case BUILT_IN_STRNCASECMP:
>> +    case BUILT_IN_VSSCANF:
>> +      argno[0] = 0;
>> +      argno[1] = 1;
>> +      break;
>> +
>> +    case BUILT_IN_STPCPY:
>> +    case BUILT_IN_STPNCPY:
>> +    case BUILT_IN_STRCAT:
>> +    case BUILT_IN_STRCPY:
>> +    case BUILT_IN_STRNCAT:
>> +    case BUILT_IN_STRNCPY:
>> +      argno[0] = 1;
>> +      break;
>> +
>> +    case BUILT_IN_FPRINTF:
>> +    case BUILT_IN_FPUTS:
>> +    case BUILT_IN_SPRINTF:
>> +    case BUILT_IN_STPCPY_CHK:
>> +    case BUILT_IN_STPNCPY_CHK:
>> +    case BUILT_IN_STRCAT_CHK:
>> +    case BUILT_IN_STRCPY_CHK:
>> +    case BUILT_IN_STRNCAT_CHK:
>> +    case BUILT_IN_STRNCPY_CHK:
>> +    case BUILT_IN_VFPRINTF:
>> +    case BUILT_IN_VSPRINTF:
>> +    case BUILT_IN_VFSCANF:
>> +      argno[0] = 1;
>> +      break;
>> +
>> +    case BUILT_IN_SNPRINTF:
>> +    case BUILT_IN_VSNPRINTF:
>> +      argno[0] = 2;
>> +      break;
>> +
>> +    case BUILT_IN_PRINTF:
>> +    case BUILT_IN_PRINTF_UNLOCKED:
>> +    case BUILT_IN_PUTS:
>> +    case BUILT_IN_PUTS_UNLOCKED:
>> +    case BUILT_IN_STRCHR:
>> +    case BUILT_IN_STRDUP:
>> +    case BUILT_IN_STRLEN:
>
> How was the above list of builtins chosen?
> I don't see why some are included and others that behave similarly aren't.
> Say, you have vsscanf and vfscanf in the list, but not vscanf, fscanf,
> scanf and sscanf.  Or {f,s,sn,}printf and v{f,s,sn}printf,
> but not vprintf, and have printf_unlocked, but not fprintf_unlocked.
> And no *printf_chk.

Right.  It occurred to me only after I submitted the patch that
there's a better way to do this than by hardcoding the functions.
Let me post an updated patch.

Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-13  8:13 [PATCH] detect nonstring arguments to string functions (PR 82945) Martin Sebor
  2017-11-13  8:46 ` Bernhard Reutner-Fischer
  2017-11-13 20:04 ` Jakub Jelinek
@ 2017-11-14  6:35 ` Martin Sebor
  2017-11-17  0:27   ` Jeff Law
  2 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2017-11-14  6:35 UTC (permalink / raw)
  To: Jeff Law, Gcc Patch List, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 1804 bytes --]

Attached is an improved version that rather than hardcoding
the built-in functions to check uses the constness of each
built-in's char* argument as a trigger to do the checking.

This avoids the problem of the list being incomplete either
by omission or due to getting out of sync, and also makes it
trivial to extend the same approach to user-defined functions
in the future.

I've excluded strndup and strnlen (but no other "bounded"
string functions like strncmp) from the checking.

Martin

On 11/12/2017 05:52 PM, Martin Sebor wrote:
> The recently introduced -Wstringop-truncation warning relies
> on the new nonstring attribute to allow the historical use case
> of calling strncpy to completely fill the destination with a copy
> of a string without adding a terminating nul.  Glibc is currently
> considering making use of the attribute to decorate some of its
> data members.  To help find misuses of such data members in
> arguments to string functions like strlen or strdup, the attached
> patch adds checking for this new attribute in these contexts.
> The checking is intentionally done late so  that uses such arrays
> that can be proven to be safe (and thus folded) are not diagnosed.
>
> While testing this simple enhancement I noticed that the handling
> I added for the nul termination in cases like
>
>   strncpy (array, s, sizeof array);
>   array[sizeof array - 1] = 0;
>
> to avoid the new warning wasn't quite as robust as it could and
> arguably should be so I improved it a bit to silently accept
> more forms of the idiom.  For instance, this is now correctly
> handled (and not diagnosed):
>
>   *stpcpy (array, s, sizeof array - 1) = 0;
>
> Martin
>
> PS A useful future enhancement would be to detect the one byte
> overflow in:
>
>   *stpcpy (array, s, sizeof array) = 0;


[-- Attachment #2: gcc-82945.diff --]
[-- Type: text/x-patch, Size: 33301 bytes --]

PR tree-optimization/82945 - add warning for passing non-strings to functions that expect string arguments

gcc/ChangeLog:

	PR tree-optimization/82945
	* calls.c (get_attr_nonstring_decl, maybe_warn_nonstring_arg): New
	functions.
	(initialize_argument_information): Call maybe_warn_nonstring_arg.
	* calls.h (get_attr_nonstring_decl): Declare new function.
	* doc/extend.texi (attribute nonstring): Update.
	* gimple-fold.c (gimple_fold_builtin_strncpy): Call
	get_attr_nonstring_decl and handle it.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.  Improve
	detection of nul-termination.

gcc/testsuite/ChangeLog:

	PR tree-optimization/82945
	* c-c++-common/Wstringop-truncation-2.c: New test.
	* c-c++-common/Wstringop-truncation.c: Adjust.
	* c-c++-common/attr-nonstring-2.c: Adjust.
	* c-c++-common/attr-nonstring-3.c: New test.

diff --git a/gcc/calls.c b/gcc/calls.c
index 3730f43..197d907 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1494,6 +1494,108 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2])
     }
 }
 
+/* If EXPR refers to a character array or pointer declared attribute
+   nonstring return a decl for that array or pointer and set *REF to
+   the referenced enclosing object or pointer.  Otherwise returns
+   null.  */
+
+tree
+get_attr_nonstring_decl (tree expr, tree *ref)
+{
+  tree dcl = expr;
+  if (TREE_CODE (dcl) == SSA_NAME)
+    {
+      if (SSA_NAME_IS_DEFAULT_DEF (dcl))
+	dcl = SSA_NAME_VAR (dcl);
+      else
+	{
+	  gimple *def = SSA_NAME_DEF_STMT (dcl);
+
+	  if (is_gimple_assign (def))
+	    {
+	      tree_code code = gimple_assign_rhs_code (def);
+	      if (code == ADDR_EXPR
+		  || code == COMPONENT_REF
+		  || code == VAR_DECL)
+		dcl = gimple_assign_rhs1 (def);
+	    }
+	}
+    }
+
+  if (TREE_CODE (dcl) == ADDR_EXPR)
+    dcl = TREE_OPERAND (dcl, 0);
+
+  if (ref)
+    *ref = dcl;
+
+  if (TREE_CODE (dcl) == COMPONENT_REF)
+    dcl = TREE_OPERAND (dcl, 1);
+
+  if (DECL_P (dcl)
+      && lookup_attribute ("nonstring", DECL_ATTRIBUTES (dcl)))
+    return dcl;
+
+  return NULL_TREE;
+}
+
+/* Warn about passing a non-string array/pointer to a function that
+   expects a nul-terminated string argument.  */
+
+static void
+maybe_warn_nonstring_arg (tree fndecl, tree exp)
+{
+  if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
+    return;
+
+  /* It's safe to call strndup and strnlen with a non-string argument
+     since the functions provide an explicit bound for this purpose.  */
+  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRNDUP)
+    return;
+
+  /* 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.  */
+  function_args_iterator it;
+  function_args_iter_init (&it, TREE_TYPE (fndecl));
+
+  for (unsigned argno = 0; ; ++argno, function_args_iter_next (&it))
+    {
+      tree argtype = function_args_iter_cond (&it);
+      if (!argtype)
+	break;
+
+      if (TREE_CODE (argtype) != POINTER_TYPE)
+	continue;
+
+      argtype = TREE_TYPE (argtype);
+
+      if (TREE_CODE (argtype) != INTEGER_TYPE
+	  || !TYPE_READONLY (argtype))
+	continue;
+
+      argtype = TYPE_MAIN_VARIANT (argtype);
+      if (argtype != char_type_node)
+	continue;
+
+      tree callarg = CALL_EXPR_ARG (exp, argno);
+      if (TREE_CODE (callarg) == ADDR_EXPR)
+	callarg = TREE_OPERAND (callarg, 0);
+
+      /* See if the destination is declared with attribute "nonstring"
+	 and if so, issue a warning.  */
+      tree dcl = get_attr_nonstring_decl (callarg);
+      if (!dcl)
+	continue;
+
+      location_t loc = EXPR_LOCATION (exp);
+      if (warning_at (loc, OPT_Wstringop_overflow_,
+		      "%qD argument %i declared attribute %<nonstring%>",
+		      fndecl, argno + 1))
+	inform (DECL_SOURCE_LOCATION (dcl),
+		"argument %qD declared here", dcl);
+    }
+}
+
 /* Issue an error if CALL_EXPR was flagged as requiring
    tall-call optimization.  */
 
@@ -1943,6 +2045,10 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 	 alloc_size.  */
       maybe_warn_alloc_args_overflow (fndecl, exp, alloc_args, alloc_idx);
     }
+
+  /* Detect passing non-string arguments to functions expecting
+     nul-terminated strings.  */
+  maybe_warn_nonstring_arg (fndecl, exp);
 }
 
 /* Update ARGS_SIZE to contain the total size for the argument block.
diff --git a/gcc/calls.h b/gcc/calls.h
index df5817f..4f069b4 100644
--- a/gcc/calls.h
+++ b/gcc/calls.h
@@ -39,5 +39,6 @@ extern bool reference_callee_copied (CUMULATIVE_ARGS *, machine_mode,
 				     tree, bool);
 extern void maybe_warn_alloc_args_overflow (tree, tree, tree[2], int[2]);
 extern bool get_size_range (tree, tree[2]);
+extern tree get_attr_nonstring_decl (tree, tree * = NULL);
 
 #endif // GCC_CALLS_H
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index d887378..4680dea 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -5974,22 +5974,33 @@ types (@pxref{Common Function Attributes},
 The @code{nonstring} variable attribute specifies that an object or member
 declaration with type array of @code{char} or pointer to @code{char} is
 intended to store character arrays that do not necessarily contain
-a terminating @code{NUL} character.  This is useful to avoid warnings
-when such an array or pointer is used as an argument to a bounded string
-manipulation function such as @code{strncpy}.  For example, without the
-attribute, GCC will issue a warning for the call below because it may
-truncate the copy without appending the terminating NUL character.  Using
-the attribute makes it possible to suppress the warning.
+a terminating @code{NUL} character.  This is useful in detecting uses
+of such arrays or pointers with functions that expect NUL-terminated
+strings, and to avoid warnings when such an array or pointer is used
+as an argument to a bounded string manipulation function such as
+@code{strncpy}.  For example, without the attribute, GCC will issue
+a warning for the @code{strncpy} call below because it may truncate
+the copy without appending the terminating @code{NUL} character.  Using
+the attribute makes it possible to suppress the warning.  However, when
+the array is declared with the attribute the call to @code{strlen} is
+diagnosed because when the array doesn't contain a @code{NUL}-terminated
+string the call is undefined.  To copy, compare, of search non-string
+character arrays use the @code{memcpy}, @code{memcmp}, @code{memchr},
+and other functions that operate on arrays of bytes.  In addition,
+calling @code{strnlen} and @code{strndup} with such arrays is safe
+provided a suitable bound is specified, and not diagnosed.
 
 @smallexample
 struct Data
 @{
   char name [32] __attribute__ ((nonstring));
 @};
-void f (struct Data *pd, const char *s)
+
+int f (struct Data *pd, const char *s)
 @{
   strncpy (pd->name, s, sizeof pd->name);
   @dots{}
+  return strlen (pd->name);   // unsafe, gets a warning
 @}
 @end smallexample
 
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index adb6f3b..8c6cdff 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -62,6 +62,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "diagnostic-core.h"
 #include "intl.h"
+#include "calls.h"
 
 /* Return true when DECL can be referenced from current unit.
    FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
@@ -1558,25 +1559,31 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
 {
   gimple *stmt = gsi_stmt (*gsi);
   location_t loc = gimple_location (stmt);
+  bool nonstring = get_attr_nonstring_decl (dest) != NULL_TREE;
 
   /* If the LEN parameter is zero, return DEST.  */
   if (integer_zerop (len))
     {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      /* Warn about the lack of nul termination: the result is not
-	 a (nul-terminated) string.  */
-      tree slen = get_maxval_strlen (src, 0);
-      if (slen && !integer_zerop (slen))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes "
-		    "from a string of length %E",
-		    call, fndecl, slen);
-      else
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes",
-		    call, fndecl);
+      /* Avoid warning if the destination refers to a an array/pointer
+	 decorate with attribute nonstring.  */
+      if (!nonstring)
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  /* Warn about the lack of nul termination: the result is not
+	     a (nul-terminated) string.  */
+	  tree slen = get_maxval_strlen (src, 0);
+	  if (slen && !integer_zerop (slen))
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes "
+			"from a string of length %E",
+			call, fndecl, slen);
+	  else
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes",
+			call, fndecl);
+	}
 
       replace_call_with_value (gsi, dest);
       return true;
@@ -1601,53 +1608,36 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
   if (tree_int_cst_lt (ssize, len))
     return false;
 
-  if (tree_int_cst_lt (len, slen))
-    {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      warning_at (loc, OPT_Wstringop_truncation,
-		  (tree_int_cst_equal (size_one_node, len)
-		   ? G_("%G%qD output truncated copying %E byte "
-			"from a string of length %E")
-		   : G_("%G%qD output truncated copying %E bytes "
-		      "from a string of length %E")),
-		  call, fndecl, len, slen);
-    }
-  else if (tree_int_cst_equal (len, slen))
+  if (!nonstring)
     {
-      tree decl = dest;
-      if (TREE_CODE (decl) == SSA_NAME)
+      if (tree_int_cst_lt (len, slen))
 	{
-	  gimple *def_stmt = SSA_NAME_DEF_STMT (decl);
-	  if (is_gimple_assign (def_stmt))
-	    {
-	      tree_code code = gimple_assign_rhs_code (def_stmt);
-	      if (code == ADDR_EXPR || code == VAR_DECL)
-		decl = gimple_assign_rhs1 (def_stmt);
-	    }
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated copying %E byte "
+			    "from a string of length %E")
+		       : G_("%G%qD output truncated copying %E bytes "
+			    "from a string of length %E")),
+		      call, fndecl, len, slen);
+	}
+      else if (tree_int_cst_equal (len, slen))
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated before terminating nul "
+			    "copying %E byte from a string of the same "
+			    "length")
+		       : G_("%G%qD output truncated before terminating nul "
+			    "copying %E bytes from a string of the same "
+			    "length")),
+		      call, fndecl, len);
 	}
-
-      if (TREE_CODE (decl) == ADDR_EXPR)
-	decl = TREE_OPERAND (decl, 0);
-
-      if (TREE_CODE (decl) == COMPONENT_REF)
-	decl = TREE_OPERAND (decl, 1);
-
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      if (!DECL_P (decl)
-	  || !lookup_attribute ("nonstring", DECL_ATTRIBUTES (decl)))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    (tree_int_cst_equal (size_one_node, len)
-		     ? G_("%G%qD output truncated before terminating nul "
-			  "copying %E byte from a string of the same "
-			  "length")
-		     : G_("%G%qD output truncated before terminating nul "
-			  "copying %E bytes from a string of the same "
-			  "length")),
-		    call, fndecl, len);
     }
 
   /* OK transform into builtin memcpy.  */
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
new file mode 100644
index 0000000..8256070
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
@@ -0,0 +1,56 @@
+/* Verify that 
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-truncation -Wno-stringop-overflow -ftrack-macro-expansion=0" } */
+
+
+typedef __SIZE_TYPE__ size_t;
+
+struct A { char str[3]; };
+
+struct B { struct A a[3]; int i; };
+struct C { struct B b[3]; int i; };
+
+void stpncpy_nowarn_1 (struct C *pc, const char *s)
+{
+  __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str)[-1] = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  *__builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1) = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_3 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  d[-1] = 0;
+}
+
+void stpncpy_nowarn_4 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  *d = 0;
+}
+
+void strncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  pc->b[0].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_1 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[1].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[0].a[1].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
index c536a13..a6d418e 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-truncation.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
@@ -192,35 +192,35 @@ void test_strncpy_ptr (char *d, const char* s, const char *t, int i)
   CPY (d, CHOOSE ("123", "12"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */
 
   {
-    signed char n = strlen (s);   /* { dg-message "length computed here" } */
+    signed char n = strlen (s);     /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    short n = strlen (s);         /* { dg-message "length computed here" } */
+    short n = strlen (s);           /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    int n = strlen (s);           /* { dg-message "length computed here" } */
+    int n = strlen (s);             /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    unsigned n = strlen (s);      /* { dg-message "length computed here" } */
+    unsigned n = strlen (s);        /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
     size_t n;
-    n = strlen (s);               /* { dg-message "length computed here" } */
+    n = strlen (s);                 /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
     size_t n;
     char *dp2 = d + 1;
-    n = strlen (s);               /* { dg-message "length computed here" } */
+    n = strlen (s);                 /* { dg-message "length computed here" } */
     CPY (dp2, s, n);                /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
@@ -311,9 +311,11 @@ void test_strncpy_array (Dest *pd, int i, const char* s)
   /* Exercise destination with attribute "nonstring".  */
   CPY (pd->c3ns, "", 3);
   CPY (pd->c3ns, "", 1);
-  /* Truncation is still diagnosed -- using strncpy in this case is
-     pointless and should be replaced with memcpy.  */
-  CPY (pd->c3ns, "12", 1);          /* { dg-warning "output truncated copying 1 byte from a string of length 2" } */
+  /* It could be argued that truncation in the literal case should be
+     diagnosed even for non-strings.  Using strncpy in this case is
+     pointless and should be replaced with memcpy.  But it would likely
+     be viewed as a false positive.  */
+  CPY (pd->c3ns, "12", 1);
   CPY (pd->c3ns, "12", 2);
   CPY (pd->c3ns, "12", 3);
   CPY (pd->c3ns, "123", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-2.c b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
index 6e273e7..67fce03 100644
--- a/gcc/testsuite/c-c++-common/attr-nonstring-2.c
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
@@ -89,7 +89,7 @@ void test_pointer (const char *s, unsigned n)
   strncpy (pns_1, "a", 1);
   strncpy (pns_2, "ab", 2);
   strncpy (pns_3, "abc", 3);
-  strncpy (pns_3, s7, 3);         /* { dg-warning "output truncated copying 3 bytes from a string of length 7" } */
+  strncpy (pns_3, s7, 3);
 
   strncpy (pns_1, s, 1);
   strncpy (pns_2, s, 1);
@@ -105,6 +105,7 @@ void test_member_array (struct MemArrays *p, const char *s, unsigned n)
 {
   const char s7[] = "1234567";
 
+  strncpy (p->ma3, "", 0);
   strncpy (p->ma3, "a", 1);
   strncpy (p->ma4, "ab", 2);
   strncpy (p->ma5, "abc", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-3.c b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
new file mode 100644
index 0000000..19d657a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
@@ -0,0 +1,371 @@
+/* Test to exercise warnings when an array declared with attribute "nonstring"
+   is passed to a function that expects a nul-terminate string as an argument.
+   { dg-do compile }
+   { dg-options "-O2 -Wattributes -Wstringop-overflow -ftrack-macro-expansion=0" }  */
+
+typedef __SIZE_TYPE__       size_t;
+typedef __builtin_va_list   va_list;
+
+#if __cplusplus
+extern "C" {
+#endif
+
+void* memchr (const void*, int, size_t);
+int memcmp (const void*, const void*, size_t);
+void* memcpy (void*, const void*, size_t);
+void* memmove (void*, const void*, size_t);
+
+int printf (const char*, ...);
+int puts (const char*);
+int puts_unlocked (const char*);
+int sprintf (char*, const char*, ...);
+int snprintf (char*, size_t, const char*, ...);
+int vsprintf (char*, const char*, va_list);
+int vsnprintf (char*, size_t, const char*, va_list);
+
+int strcmp (const char*, const char*);
+int strncmp (const char*, const char*, size_t);
+
+char* stpcpy (char*, const char*);
+char* stpncpy (char*, const char*, size_t);
+
+char* strcat (char*, const char*);
+char* strncat (char*, const char*, size_t);
+
+char* strcpy (char*, const char*);
+char* strncpy (char*, const char*, size_t);
+
+char* strchr (const char*, int);
+char* strdup (const char*);
+size_t strlen (const char*);
+size_t strnlen (const char*, size_t);
+char* strndup (const char*, size_t);
+
+#if __cplusplus
+}   /* extern "C" */
+#endif
+
+#define NONSTRING __attribute__ ((nonstring))
+
+char str[4];
+char arr[4] NONSTRING;
+
+char *ptr;
+char *parr NONSTRING;
+
+struct MemArrays
+{
+  char str[4];
+  char arr[4] NONSTRING;
+  char *parr NONSTRING;
+};
+
+void sink (int, ...);
+
+
+#define T(call)  sink (0, (call))
+
+void test_printf (struct MemArrays *p)
+{
+  T (printf (str));
+  T (printf (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (ptr));
+  T (printf (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (p->str));
+  T (printf (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_puts (struct MemArrays *p)
+{
+  T (puts (str));
+  T (puts (arr));               /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (ptr));
+  T (puts (parr));              /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (p->str));
+  T (puts (p->arr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_snprintf (char *d, size_t n, struct MemArrays *p)
+{
+  T (snprintf (d, n, str));
+  T (snprintf (d, n, arr));       /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, ptr));
+  T (snprintf (d, n, parr));      /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, p->str));
+  T (snprintf (d, n, p->arr));    /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_sprintf (char *d, struct MemArrays *p)
+{
+  T (sprintf (d, str));
+  T (sprintf (d, arr));           /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, ptr));
+  T (sprintf (d, parr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, p->str));
+  T (sprintf (d, p->arr));        /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_vsnprintf (char *d, size_t n, struct MemArrays *p, va_list va)
+{
+  T (vsnprintf (d, n, str, va));
+  T (vsnprintf (d, n, arr, va));  /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, ptr, va));
+  T (vsnprintf (d, n, parr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, p->str, va));
+  T (vsnprintf (d, n, p->arr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_vsprintf (char *d, struct MemArrays *p, va_list va)
+{
+  T (vsprintf (d, str, va));
+  T (vsprintf (d, arr, va));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, ptr, va));
+  T (vsprintf (d, parr, va));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, p->str, va));
+  T (vsprintf (d, p->arr, va));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_strcmp (struct MemArrays *p)
+{
+  T (strcmp (str, str));
+  T (strcmp (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (arr, str));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (str, ptr));
+  T (strcmp (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (parr, str));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (p->arr, p->str));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strcmp (p->parr, p->str));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strncmp (struct MemArrays *p, size_t n)
+{
+  T (strncmp (str, str, n));
+  T (strncmp (str, arr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (arr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (str, ptr, n));
+  T (strncmp (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (parr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (p->str, p->arr, n));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (p->arr, p->str, n));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strncmp (p->parr, p->str, n));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stpcpy (struct MemArrays *p)
+{
+  T (stpcpy (str, str));
+  T (stpcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (arr, str));
+
+  T (stpcpy (str, ptr));
+  T (stpcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (parr, str));
+
+  T (stpcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (p->arr, p->str));
+  T (stpcpy (p->parr, p->str));
+}
+
+
+void test_stpncpy (struct MemArrays *p, unsigned n)
+{
+  T (stpncpy (str, str, n));
+  T (stpncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (arr, str, n));
+
+  T (stpncpy (str, ptr, n));
+  T (stpncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (parr, str, n));
+
+  T (stpncpy (p->str, p->arr, n));/* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (p->arr, p->str, n));
+  T (stpncpy (p->parr, p->str, n));
+}
+
+
+void test_strcat (struct MemArrays *p)
+{
+  T (strcat (str, str));
+  T (strcat (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (arr, str));
+
+  T (strcat (str, ptr));
+  T (strcat (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (parr, str));
+
+  T (strcat (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (p->arr, p->str));
+  T (strcat (p->parr, p->str));
+}
+
+
+void test_strncat (struct MemArrays *p, unsigned n)
+{
+  T (strncat (str, str, n));
+  T (strncat (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (arr, str, n));
+
+  T (strncat (str, ptr, n));
+  T (strncat (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (parr, str, n));
+
+  T (strncat (p->str, p->arr, n));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (p->arr, p->str, n));
+  T (strncat (p->parr, p->str, n));
+}
+
+
+void test_strcpy (struct MemArrays *p)
+{
+  T (strcpy (str, str));
+  T (strcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (arr, str));
+
+  T (strcpy (str, ptr));
+  T (strcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (parr, str));
+
+  T (strcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (p->arr, p->str));
+  T (strcpy (p->parr, p->str));
+}
+
+
+void test_strncpy (struct MemArrays *p, unsigned n)
+{
+  T (strncpy (str, str, n));
+  T (strncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (arr, str, n));
+
+  T (strncpy (str, ptr, n));
+  T (strncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (parr, str, n));
+
+  T (strncpy (p->str, p->arr, n));  /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (p->arr, p->str, n));
+  T (strncpy (p->parr, p->str, n));
+}
+
+
+void test_strchr (struct MemArrays *p, int c)
+{
+  T (strchr (str, c));
+  T (strchr (arr, c));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (ptr, c));
+  T (strchr (parr, c));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (p->str, c));
+  T (strchr (p->arr, c));       /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strdup (struct MemArrays *p)
+{
+  T (strdup (str));
+  T (strdup (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (ptr));
+  T (strdup (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (p->str));
+  T (strdup (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stnrdup (struct MemArrays *p, size_t n)
+{
+  T (strndup (str, n));
+  T (strndup (arr, n));
+
+  T (strndup (ptr, n));
+  T (strndup (parr, n));
+
+  T (strndup (p->str, n));
+  T (strndup (p->arr, n));
+}
+
+
+void test_strlen (struct MemArrays *p)
+{
+  T (strlen (str));
+  T (strlen (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (ptr));
+  T (strlen (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (p->str));
+  T (strlen (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strnlen (struct MemArrays *p, size_t n)
+{
+  T (strnlen (str, n));
+  T (strnlen (arr, n));
+
+  T (strnlen (ptr, n));
+  T (strnlen (parr, n));
+
+  T (strnlen (p->str, n));
+  T (strnlen (p->arr, n));
+}
+
+
+/* Verify no warnings are issued for raw mempory functions.  */
+
+void test_mem_functions (struct MemArrays *p, int c, size_t n)
+{
+  T (memchr (arr, c, n));
+  T (memchr (parr, c, n));
+  T (memchr (p->arr, c, n));
+  T (memchr (p->parr, c, n));
+
+  T (memcmp (str, arr, n));
+  T (memcmp (arr, str, n));
+  T (memcmp (str, parr, n));
+  T (memcmp (parr, str, n));
+  T (memcmp (p->str, p->arr, n));
+  T (memcmp (p->arr, p->str, n));
+  T (memcmp (p->parr, p->str, n));
+
+  T (memcpy (str, arr, n));
+  T (memcpy (arr, str, n));
+  T (memcpy (str, parr, n));
+  T (memcpy (parr, str, n));
+  T (memcpy (p->str, p->arr, n));
+  T (memcpy (p->arr, p->str, n));
+  T (memcpy (p->parr, p->str, n));
+
+  T (memmove (str, arr, n));
+  T (memmove (arr, str, n));
+  T (memmove (str, parr, n));
+  T (memmove (parr, str, n));
+  T (memmove (p->str, p->arr, n));
+  T (memmove (p->arr, p->str, n));
+  T (memmove (p->parr, p->str, n));
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 2efa182..8b89dbe 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "intl.h"
 #include "attribs.h"
+#include "calls.h"
 
 /* A vector indexed by SSA_NAME_VERSION.  0 means unknown, positive value
    is an index into strinfo vector, negative value stands for
@@ -1733,35 +1734,15 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
     return false;
 
   tree dst = gimple_call_arg (stmt, 0);
-
-  /* See if the destination is declared with attribute "nonstring"
-     and if so, avoid the truncation warning.  */
-  if (TREE_CODE (dst) == SSA_NAME)
-    {
-      if (SSA_NAME_IS_DEFAULT_DEF (dst))
-	dst = SSA_NAME_VAR (dst);
-      else
-	{
-	  gimple *def = SSA_NAME_DEF_STMT (dst);
-
-	  if (is_gimple_assign (def)
-	      && gimple_assign_rhs_code (def) == ADDR_EXPR)
-	    dst = gimple_assign_rhs1 (def);
-	}
-    }
-
   tree dstdecl = dst;
   if (TREE_CODE (dstdecl) == ADDR_EXPR)
     dstdecl = TREE_OPERAND (dstdecl, 0);
 
-  {
-    tree d = dstdecl;
-    if (TREE_CODE (d) == COMPONENT_REF)
-      d = TREE_OPERAND (d, 1);
-
-    if (DECL_P (d) && lookup_attribute ("nonstring", DECL_ATTRIBUTES (d)))
-      return false;
-  }
+  /* If the destination refers to a an array/pointer declared nonstring
+     return early.  */
+  tree ref = NULL_TREE;
+  if (get_attr_nonstring_decl (dstdecl, &ref))
+    return false;
 
   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
      avoid the truncation warning.  */
@@ -1770,12 +1751,32 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
 
   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
     {
-      HOST_WIDE_INT off;
-      dstdecl = get_addr_base_and_unit_offset (dstdecl, &off);
-
       tree lhs = gimple_assign_lhs (next_stmt);
-      tree lhsbase = get_addr_base_and_unit_offset (lhs, &off);
-      if (lhsbase && operand_equal_p (dstdecl, lhsbase, 0))
+      tree_code code = TREE_CODE (lhs);
+      if (code == ARRAY_REF || code == MEM_REF)
+	lhs = TREE_OPERAND (lhs, 0);
+
+      tree func = gimple_call_fndecl (stmt);
+      if (DECL_FUNCTION_CODE (func) == BUILT_IN_STPNCPY)
+	{
+	  tree ret = gimple_call_lhs (stmt);
+	  if (ret && operand_equal_p (ret, lhs, 0))
+	    return false;
+	}
+
+      /* Determine the base address and offset of the reference,
+	 ignoring the innermost array index.  */
+      if (TREE_CODE (ref) == ARRAY_REF)
+	ref = TREE_OPERAND (ref, 0);
+
+      HOST_WIDE_INT dstoff;
+      tree dstbase = get_addr_base_and_unit_offset (ref, &dstoff);
+
+      HOST_WIDE_INT lhsoff;
+      tree lhsbase = get_addr_base_and_unit_offset (lhs, &lhsoff);
+      if (lhsbase
+	  && dstoff == lhsoff
+	  && operand_equal_p (dstbase, lhsbase, 0))
 	return false;
     }
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-14  6:35 ` Martin Sebor
@ 2017-11-17  0:27   ` Jeff Law
  2017-11-17  8:42     ` Jakub Jelinek
                       ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Jeff Law @ 2017-11-17  0:27 UTC (permalink / raw)
  To: Martin Sebor, Gcc Patch List, Jakub Jelinek

On 11/13/2017 06:21 PM, Martin Sebor wrote:
> AttachedÂ isÂ anÂ improvedÂ versionÂ thatÂ ratherÂ thanÂ hardcoding
> theÂ built-inÂ functionsÂ toÂ checkÂ usesÂ theÂ constnessÂ ofÂ each
> built-in'sÂ char*Â argumentÂ asÂ aÂ triggerÂ toÂ doÂ theÂ checking.
> 
> ThisÂ avoidsÂ theÂ problemÂ ofÂ theÂ listÂ beingÂ incompleteÂ either
> byÂ omissionÂ orÂ dueÂ toÂ gettingÂ outÂ ofÂ sync,Â andÂ alsoÂ makesÂ it
> trivialÂ toÂ extendÂ theÂ sameÂ approachÂ toÂ user-definedÂ functions
> inÂ theÂ future.
> 
> I'veÂ excludedÂ strndupÂ andÂ strnlenÂ (butÂ noÂ otherÂ "bounded"
> stringÂ functionsÂ likeÂ strncmp)Â fromÂ theÂ checking.
> 
> Martin
> 
> OnÂ 11/12/2017Â 05:52Â PM,Â MartinÂ SeborÂ wrote:
>> TheÂ recentlyÂ introducedÂ -Wstringop-truncationÂ warningÂ relies
>> onÂ theÂ newÂ nonstringÂ attributeÂ toÂ allowÂ theÂ historicalÂ useÂ case
>> ofÂ callingÂ strncpyÂ toÂ completelyÂ fillÂ theÂ destinationÂ withÂ aÂ copy
>> ofÂ aÂ stringÂ withoutÂ addingÂ aÂ terminatingÂ nul.Â Â GlibcÂ isÂ currently
>> consideringÂ makingÂ useÂ ofÂ theÂ attributeÂ toÂ decorateÂ someÂ ofÂ its
>> dataÂ members.Â Â ToÂ helpÂ findÂ misusesÂ ofÂ suchÂ dataÂ membersÂ in
>> argumentsÂ toÂ stringÂ functionsÂ likeÂ strlenÂ orÂ strdup,Â theÂ attached
>> patchÂ addsÂ checkingÂ forÂ thisÂ newÂ attributeÂ inÂ theseÂ contexts.
>> TheÂ checkingÂ isÂ intentionallyÂ doneÂ lateÂ soÂ Â thatÂ usesÂ suchÂ arrays
>> thatÂ canÂ beÂ provenÂ toÂ beÂ safeÂ (andÂ thusÂ folded)Â areÂ notÂ diagnosed.
>>
>> WhileÂ testingÂ thisÂ simpleÂ enhancementÂ IÂ noticedÂ thatÂ theÂ handling
>> IÂ addedÂ forÂ theÂ nulÂ terminationÂ inÂ casesÂ like
>>
>> Â Â strncpyÂ (array,Â s,Â sizeofÂ array);
>> Â Â array[sizeofÂ arrayÂ -Â 1]Â =Â 0;
>>
>> toÂ avoidÂ theÂ newÂ warningÂ wasn'tÂ quiteÂ asÂ robustÂ asÂ itÂ couldÂ and
>> arguablyÂ shouldÂ beÂ soÂ IÂ improvedÂ itÂ aÂ bitÂ toÂ silentlyÂ accept
>> moreÂ formsÂ ofÂ theÂ idiom.Â Â ForÂ instance,Â thisÂ isÂ nowÂ correctly
>> handledÂ (andÂ notÂ diagnosed):
>>
>> Â Â *stpcpyÂ (array,Â s,Â sizeofÂ arrayÂ -Â 1)Â =Â 0;
>>
>> Martin
>>
>> PSÂ AÂ usefulÂ futureÂ enhancementÂ wouldÂ beÂ toÂ detectÂ theÂ oneÂ byte
>> overflowÂ in:
>>
>> Â Â *stpcpyÂ (array,Â s,Â sizeofÂ array)Â =Â 0;
> 
> 
> gcc-82945.diff
> 
> 
> PR tree-optimization/82945 - add warning for passing non-strings to functions that expect string arguments
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/82945
> 	* calls.c (get_attr_nonstring_decl, maybe_warn_nonstring_arg): New
> 	functions.
> 	(initialize_argument_information): Call maybe_warn_nonstring_arg.
> 	* calls.h (get_attr_nonstring_decl): Declare new function.
> 	* doc/extend.texi (attribute nonstring): Update.
> 	* gimple-fold.c (gimple_fold_builtin_strncpy): Call
> 	get_attr_nonstring_decl and handle it.
> 	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.  Improve
> 	detection of nul-termination.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/82945
> 	* c-c++-common/Wstringop-truncation-2.c: New test.
> 	* c-c++-common/Wstringop-truncation.c: Adjust.
> 	* c-c++-common/attr-nonstring-2.c: Adjust.
> 	* c-c++-common/attr-nonstring-3.c: New test.

> 
> diff --git a/gcc/calls.c b/gcc/calls.c
> index 3730f43..197d907 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -1494,6 +1494,108 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2])
>      }
>  }
>  
> +/* If EXPR refers to a character array or pointer declared attribute
> +   nonstring return a decl for that array or pointer and set *REF to
> +   the referenced enclosing object or pointer.  Otherwise returns
> +   null.  */
Generally we'd write NULL rather than null for a generic pointer and in
this specific case NULL_TREE is even better.

> +
> +tree
> +get_attr_nonstring_decl (tree expr, tree *ref)
> +{
> +  tree dcl = expr;
I think Jakub may have pointed this out, but we generally prefer decl.
Truly a nit though.


> +  if (TREE_CODE (dcl) == SSA_NAME)
> +    {
> +      if (SSA_NAME_IS_DEFAULT_DEF (dcl))
> +	dcl = SSA_NAME_VAR (dcl);
> +      else
> +	{
> +	  gimple *def = SSA_NAME_DEF_STMT (dcl);
> +
> +	  if (is_gimple_assign (def))
> +	    {
> +	      tree_code code = gimple_assign_rhs_code (def);
> +	      if (code == ADDR_EXPR
> +		  || code == COMPONENT_REF
> +		  || code == VAR_DECL)
> +		dcl = gimple_assign_rhs1 (def);
Note that COMPONENT_REFs can have arguments that are themselves
COMPONENT_REFS.  So you typically want to use a loop to strip all away.




> +  /* It's safe to call strndup and strnlen with a non-string argument
> +     since the functions provide an explicit bound for this purpose.  */
> +  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRNDUP)
> +    return;
So I see you handle BUILT_IN_STRNDUP here, but not BUILT_IN_STRNLEN.  Is
there a reason for the omission?


Jeff

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-17  0:27   ` Jeff Law
@ 2017-11-17  8:42     ` Jakub Jelinek
  2017-11-19 18:29       ` Martin Sebor
  2017-11-19 17:21     ` Martin Sebor
  2017-11-19 17:26     ` Martin Sebor
  2 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2017-11-17  8:42 UTC (permalink / raw)
  To: Jeff Law; +Cc: Martin Sebor, Gcc Patch List

On Thu, Nov 16, 2017 at 05:15:20PM -0700, Jeff Law wrote:
> > +	      tree_code code = gimple_assign_rhs_code (def);
> > +	      if (code == ADDR_EXPR
> > +		  || code == COMPONENT_REF
> > +		  || code == VAR_DECL)
> > +		dcl = gimple_assign_rhs1 (def);
> Note that COMPONENT_REFs can have arguments that are themselves
> COMPONENT_REFS.  So you typically want to use a loop to strip all away.

Do you want to handle just COMPONENT_REF, or other handled components
(ARRAY_REFs, etc.)?
We have handled_component_p predicate and get_base_address to strip them
(or get_inner_reference if you want other details about it).

	Jakub

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-17  0:27   ` Jeff Law
  2017-11-17  8:42     ` Jakub Jelinek
@ 2017-11-19 17:21     ` Martin Sebor
  2017-11-19 22:26       ` Jeff Law
  2017-11-19 17:26     ` Martin Sebor
  2 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2017-11-19 17:21 UTC (permalink / raw)
  To: Jeff Law, Gcc Patch List, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 6692 bytes --]

On 11/16/2017 05:15 PM, Jeff Law wrote:
> On 11/13/2017 06:21 PM, Martin Sebor wrote:
>> Attached is an improved version that rather than hardcoding
>> the built-in functions to check uses the constness of each
>> built-in's char* argument as a trigger to do the checking.
>>
>> This avoids the problem of the list being incomplete either
>> by omission or due to getting out of sync, and also makes it
>> trivial to extend the same approach to user-defined functions
>> in the future.
>>
>> I've excluded strndup and strnlen (but no other "bounded"
>> string functions like strncmp) from the checking.
>>
>> Martin
>>
>> On 11/12/2017 05:52 PM, Martin Sebor wrote:
>>> The recently introduced -Wstringop-truncation warning relies
>>> on the new nonstring attribute to allow the historical use case
>>> of calling strncpy to completely fill the destination with a copy
>>> of a string without adding a terminating nul.  Glibc is currently
>>> considering making use of the attribute to decorate some of its
>>> data members.  To help find misuses of such data members in
>>> arguments to string functions like strlen or strdup, the attached
>>> patch adds checking for this new attribute in these contexts.
>>> The checking is intentionally done late so  that uses such arrays
>>> that can be proven to be safe (and thus folded) are not diagnosed.
>>>
>>> While testing this simple enhancement I noticed that the handling
>>> I added for the nul termination in cases like
>>>
>>>   strncpy (array, s, sizeof array);
>>>   array[sizeof array - 1] = 0;
>>>
>>> to avoid the new warning wasn't quite as robust as it could and
>>> arguably should be so I improved it a bit to silently accept
>>> more forms of the idiom.  For instance, this is now correctly
>>> handled (and not diagnosed):
>>>
>>>   *stpcpy (array, s, sizeof array - 1) = 0;
>>>
>>> Martin
>>>
>>> PS A useful future enhancement would be to detect the one byte
>>> overflow in:
>>>
>>>   *stpcpy (array, s, sizeof array) = 0;
>>
>>
>> gcc-82945.diff
>>
>>
>> PR tree-optimization/82945 - add warning for passing non-strings to functions that expect string arguments
>>
>> gcc/ChangeLog:
>>
>> 	PR tree-optimization/82945
>> 	* calls.c (get_attr_nonstring_decl, maybe_warn_nonstring_arg): New
>> 	functions.
>> 	(initialize_argument_information): Call maybe_warn_nonstring_arg.
>> 	* calls.h (get_attr_nonstring_decl): Declare new function.
>> 	* doc/extend.texi (attribute nonstring): Update.
>> 	* gimple-fold.c (gimple_fold_builtin_strncpy): Call
>> 	get_attr_nonstring_decl and handle it.
>> 	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.  Improve
>> 	detection of nul-termination.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR tree-optimization/82945
>> 	* c-c++-common/Wstringop-truncation-2.c: New test.
>> 	* c-c++-common/Wstringop-truncation.c: Adjust.
>> 	* c-c++-common/attr-nonstring-2.c: Adjust.
>> 	* c-c++-common/attr-nonstring-3.c: New test.
>
>>
>> diff --git a/gcc/calls.c b/gcc/calls.c
>> index 3730f43..197d907 100644
>> --- a/gcc/calls.c
>> +++ b/gcc/calls.c
>> @@ -1494,6 +1494,108 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2])
>>      }
>>  }
>>
>> +/* If EXPR refers to a character array or pointer declared attribute
>> +   nonstring return a decl for that array or pointer and set *REF to
>> +   the referenced enclosing object or pointer.  Otherwise returns
>> +   null.  */
> Generally we'd write NULL rather than null for a generic pointer and in
> this specific case NULL_TREE is even better.

I agree.  The C NULL macro, C++ nullptr, and GCC's NULL_TREE and
NULL_RTL are helpful both for type safety and readability of code.
But they all are implementation details that's ot not relevant to the 
specification
of an API or its users.

The established term that every C and C++ programmer is familiar
with is "null pointer."

  and exist
mainly for type safety.  They are not relevant to the specification
of an API or its users.  I didn't think too hard about it when
I wrote the comment (it just didn't seem important enough) but
now
that I have I believe "null" is better, clearer, and more future-
proof (in case the return type should ever change to some smart
pointer class).

(There also comments in this file and elsewhere in GCC that use
various forms of null.)

>> +tree
>> +get_attr_nonstring_decl (tree expr, tree *ref)
>> +{
>> +  tree dcl = expr;
> I think Jakub may have pointed this out, but we generally prefer decl.
> Truly a nit though.

Right.  I've changed it to DECL.

>> +  if (TREE_CODE (dcl) == SSA_NAME)
>> +    {
>> +      if (SSA_NAME_IS_DEFAULT_DEF (dcl))
>> +	dcl = SSA_NAME_VAR (dcl);
>> +      else
>> +	{
>> +	  gimple *def = SSA_NAME_DEF_STMT (dcl);
>> +
>> +	  if (is_gimple_assign (def))
>> +	    {
>> +	      tree_code code = gimple_assign_rhs_code (def);
>> +	      if (code == ADDR_EXPR
>> +		  || code == COMPONENT_REF
>> +		  || code == VAR_DECL)
>> +		dcl = gimple_assign_rhs1 (def);
> Note that COMPONENT_REFs can have arguments that are themselves
> COMPONENT_REFS.  So you typically want to use a loop to strip all away.

I have seen it in the early IL but this late I couldn't come
up with a test case where such a loop would be necessary.
The COMPONENT_REF always refers to the member array.  If you
can show me an example to test with I'll add the handling if
possible.  There are cases where it isn't, such as in

     struct A { char a[4] __attribute__ ((nonstring)); } *pa;

     strlen (pa->a + 1);

where pa->a is represented as a MEM_REF (char[4], pa, 1) with
the member information having been lost in translation.  (This
is the same problem that I had solved for the -Warray-bounds
warning for out-of bounds offsets by implementing it in
tree-ssa-forwprop.c: because that's where the member is folded
into a reference to the base object.)

>> +  /* It's safe to call strndup and strnlen with a non-string argument
>> +     since the functions provide an explicit bound for this purpose.  */
>> +  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRNDUP)
>> +    return;
> So I see you handle BUILT_IN_STRNDUP here, but not BUILT_IN_STRNLEN.  Is
> there a reason for the omission?

strnlen is not a GCC built-in so it's handled implicitly by
the test for DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL
just before the if statement above.  The test I added verifies
that strnlen is excluded.  (Bug 81384 tracks the missing
intrinsic support for strnlen.)

That said, more testing revealed that Glibc uses functions like
strncmp and strncpy to compare and copy non-strings, so I've
added those to the white list and improved the checking a bit.
I've also changed the hash_map to a pointer to make Jakub happy.



[-- Attachment #2: gcc-82945.diff --]
[-- Type: text/x-patch, Size: 35873 bytes --]

PR tree-optimization/82945 - add warning for passing non-strings to functions that expect string arguments

gcc/ChangeLog:

	PR tree-optimization/82945
	* calls.c (get_attr_nonstring_decl, maybe_warn_nonstring_arg): New
	functions.
	(initialize_argument_information): Call maybe_warn_nonstring_arg.
	* calls.h (get_attr_nonstring_decl): Declare new function.
	* doc/extend.texi (attribute nonstring): Update.
	* gimple-fold.c (gimple_fold_builtin_strncpy): Call
	get_attr_nonstring_decl and handle it.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.  Improve
	detection of nul-termination.
	(strlen_to_stridx): Change to a pointer.
	(handle_builtin_strlen, handle_builtin_stxncpy): Adjust.
	(pass_strlen::execute): Same.

gcc/testsuite/ChangeLog:

	PR tree-optimization/82945
	* c-c++-common/Wstringop-truncation-2.c: New test.
	* c-c++-common/Wstringop-truncation.c: Adjust.
	* c-c++-common/attr-nonstring-2.c: Adjust.
	* c-c++-common/attr-nonstring-3.c: New test.

diff --git a/gcc/calls.c b/gcc/calls.c
index 3730f43..197d907 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1494,6 +1494,108 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2])
     }
 }
 
+/* If EXPR refers to a character array or pointer declared attribute
+   nonstring return a decl for that array or pointer and set *REF to
+   the referenced enclosing object or pointer.  Otherwise returns
+   null.  */
+
+tree
+get_attr_nonstring_decl (tree expr, tree *ref)
+{
+  tree dcl = expr;
+  if (TREE_CODE (dcl) == SSA_NAME)
+    {
+      if (SSA_NAME_IS_DEFAULT_DEF (dcl))
+	dcl = SSA_NAME_VAR (dcl);
+      else
+	{
+	  gimple *def = SSA_NAME_DEF_STMT (dcl);
+
+	  if (is_gimple_assign (def))
+	    {
+	      tree_code code = gimple_assign_rhs_code (def);
+	      if (code == ADDR_EXPR
+		  || code == COMPONENT_REF
+		  || code == VAR_DECL)
+		dcl = gimple_assign_rhs1 (def);
+	    }
+	}
+    }
+
+  if (TREE_CODE (dcl) == ADDR_EXPR)
+    dcl = TREE_OPERAND (dcl, 0);
+
+  if (ref)
+    *ref = dcl;
+
+  if (TREE_CODE (dcl) == COMPONENT_REF)
+    dcl = TREE_OPERAND (dcl, 1);
+
+  if (DECL_P (dcl)
+      && lookup_attribute ("nonstring", DECL_ATTRIBUTES (dcl)))
+    return dcl;
+
+  return NULL_TREE;
+}
+
+/* Warn about passing a non-string array/pointer to a function that
+   expects a nul-terminated string argument.  */
+
+static void
+maybe_warn_nonstring_arg (tree fndecl, tree exp)
+{
+  if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
+    return;
+
+  /* It's safe to call strndup and strnlen with a non-string argument
+     since the functions provide an explicit bound for this purpose.  */
+  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRNDUP)
+    return;
+
+  /* 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.  */
+  function_args_iterator it;
+  function_args_iter_init (&it, TREE_TYPE (fndecl));
+
+  for (unsigned argno = 0; ; ++argno, function_args_iter_next (&it))
+    {
+      tree argtype = function_args_iter_cond (&it);
+      if (!argtype)
+	break;
+
+      if (TREE_CODE (argtype) != POINTER_TYPE)
+	continue;
+
+      argtype = TREE_TYPE (argtype);
+
+      if (TREE_CODE (argtype) != INTEGER_TYPE
+	  || !TYPE_READONLY (argtype))
+	continue;
+
+      argtype = TYPE_MAIN_VARIANT (argtype);
+      if (argtype != char_type_node)
+	continue;
+
+      tree callarg = CALL_EXPR_ARG (exp, argno);
+      if (TREE_CODE (callarg) == ADDR_EXPR)
+	callarg = TREE_OPERAND (callarg, 0);
+
+      /* See if the destination is declared with attribute "nonstring"
+	 and if so, issue a warning.  */
+      tree dcl = get_attr_nonstring_decl (callarg);
+      if (!dcl)
+	continue;
+
+      location_t loc = EXPR_LOCATION (exp);
+      if (warning_at (loc, OPT_Wstringop_overflow_,
+		      "%qD argument %i declared attribute %<nonstring%>",
+		      fndecl, argno + 1))
+	inform (DECL_SOURCE_LOCATION (dcl),
+		"argument %qD declared here", dcl);
+    }
+}
+
 /* Issue an error if CALL_EXPR was flagged as requiring
    tall-call optimization.  */
 
@@ -1943,6 +2045,10 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 	 alloc_size.  */
       maybe_warn_alloc_args_overflow (fndecl, exp, alloc_args, alloc_idx);
     }
+
+  /* Detect passing non-string arguments to functions expecting
+     nul-terminated strings.  */
+  maybe_warn_nonstring_arg (fndecl, exp);
 }
 
 /* Update ARGS_SIZE to contain the total size for the argument block.
diff --git a/gcc/calls.h b/gcc/calls.h
index df5817f..4f069b4 100644
--- a/gcc/calls.h
+++ b/gcc/calls.h
@@ -39,5 +39,6 @@ extern bool reference_callee_copied (CUMULATIVE_ARGS *, machine_mode,
 				     tree, bool);
 extern void maybe_warn_alloc_args_overflow (tree, tree, tree[2], int[2]);
 extern bool get_size_range (tree, tree[2]);
+extern tree get_attr_nonstring_decl (tree, tree * = NULL);
 
 #endif // GCC_CALLS_H
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index d887378..4680dea 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -5974,22 +5974,33 @@ types (@pxref{Common Function Attributes},
 The @code{nonstring} variable attribute specifies that an object or member
 declaration with type array of @code{char} or pointer to @code{char} is
 intended to store character arrays that do not necessarily contain
-a terminating @code{NUL} character.  This is useful to avoid warnings
-when such an array or pointer is used as an argument to a bounded string
-manipulation function such as @code{strncpy}.  For example, without the
-attribute, GCC will issue a warning for the call below because it may
-truncate the copy without appending the terminating NUL character.  Using
-the attribute makes it possible to suppress the warning.
+a terminating @code{NUL} character.  This is useful in detecting uses
+of such arrays or pointers with functions that expect NUL-terminated
+strings, and to avoid warnings when such an array or pointer is used
+as an argument to a bounded string manipulation function such as
+@code{strncpy}.  For example, without the attribute, GCC will issue
+a warning for the @code{strncpy} call below because it may truncate
+the copy without appending the terminating @code{NUL} character.  Using
+the attribute makes it possible to suppress the warning.  However, when
+the array is declared with the attribute the call to @code{strlen} is
+diagnosed because when the array doesn't contain a @code{NUL}-terminated
+string the call is undefined.  To copy, compare, of search non-string
+character arrays use the @code{memcpy}, @code{memcmp}, @code{memchr},
+and other functions that operate on arrays of bytes.  In addition,
+calling @code{strnlen} and @code{strndup} with such arrays is safe
+provided a suitable bound is specified, and not diagnosed.
 
 @smallexample
 struct Data
 @{
   char name [32] __attribute__ ((nonstring));
 @};
-void f (struct Data *pd, const char *s)
+
+int f (struct Data *pd, const char *s)
 @{
   strncpy (pd->name, s, sizeof pd->name);
   @dots{}
+  return strlen (pd->name);   // unsafe, gets a warning
 @}
 @end smallexample
 
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index adb6f3b..8c6cdff 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -62,6 +62,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "diagnostic-core.h"
 #include "intl.h"
+#include "calls.h"
 
 /* Return true when DECL can be referenced from current unit.
    FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
@@ -1558,25 +1559,31 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
 {
   gimple *stmt = gsi_stmt (*gsi);
   location_t loc = gimple_location (stmt);
+  bool nonstring = get_attr_nonstring_decl (dest) != NULL_TREE;
 
   /* If the LEN parameter is zero, return DEST.  */
   if (integer_zerop (len))
     {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      /* Warn about the lack of nul termination: the result is not
-	 a (nul-terminated) string.  */
-      tree slen = get_maxval_strlen (src, 0);
-      if (slen && !integer_zerop (slen))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes "
-		    "from a string of length %E",
-		    call, fndecl, slen);
-      else
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes",
-		    call, fndecl);
+      /* Avoid warning if the destination refers to a an array/pointer
+	 decorate with attribute nonstring.  */
+      if (!nonstring)
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  /* Warn about the lack of nul termination: the result is not
+	     a (nul-terminated) string.  */
+	  tree slen = get_maxval_strlen (src, 0);
+	  if (slen && !integer_zerop (slen))
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes "
+			"from a string of length %E",
+			call, fndecl, slen);
+	  else
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes",
+			call, fndecl);
+	}
 
       replace_call_with_value (gsi, dest);
       return true;
@@ -1601,53 +1608,36 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
   if (tree_int_cst_lt (ssize, len))
     return false;
 
-  if (tree_int_cst_lt (len, slen))
-    {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      warning_at (loc, OPT_Wstringop_truncation,
-		  (tree_int_cst_equal (size_one_node, len)
-		   ? G_("%G%qD output truncated copying %E byte "
-			"from a string of length %E")
-		   : G_("%G%qD output truncated copying %E bytes "
-		      "from a string of length %E")),
-		  call, fndecl, len, slen);
-    }
-  else if (tree_int_cst_equal (len, slen))
+  if (!nonstring)
     {
-      tree decl = dest;
-      if (TREE_CODE (decl) == SSA_NAME)
+      if (tree_int_cst_lt (len, slen))
 	{
-	  gimple *def_stmt = SSA_NAME_DEF_STMT (decl);
-	  if (is_gimple_assign (def_stmt))
-	    {
-	      tree_code code = gimple_assign_rhs_code (def_stmt);
-	      if (code == ADDR_EXPR || code == VAR_DECL)
-		decl = gimple_assign_rhs1 (def_stmt);
-	    }
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated copying %E byte "
+			    "from a string of length %E")
+		       : G_("%G%qD output truncated copying %E bytes "
+			    "from a string of length %E")),
+		      call, fndecl, len, slen);
+	}
+      else if (tree_int_cst_equal (len, slen))
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated before terminating nul "
+			    "copying %E byte from a string of the same "
+			    "length")
+		       : G_("%G%qD output truncated before terminating nul "
+			    "copying %E bytes from a string of the same "
+			    "length")),
+		      call, fndecl, len);
 	}
-
-      if (TREE_CODE (decl) == ADDR_EXPR)
-	decl = TREE_OPERAND (decl, 0);
-
-      if (TREE_CODE (decl) == COMPONENT_REF)
-	decl = TREE_OPERAND (decl, 1);
-
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      if (!DECL_P (decl)
-	  || !lookup_attribute ("nonstring", DECL_ATTRIBUTES (decl)))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    (tree_int_cst_equal (size_one_node, len)
-		     ? G_("%G%qD output truncated before terminating nul "
-			  "copying %E byte from a string of the same "
-			  "length")
-		     : G_("%G%qD output truncated before terminating nul "
-			  "copying %E bytes from a string of the same "
-			  "length")),
-		    call, fndecl, len);
     }
 
   /* OK transform into builtin memcpy.  */
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
new file mode 100644
index 0000000..8256070
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
@@ -0,0 +1,56 @@
+/* Verify that 
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-truncation -Wno-stringop-overflow -ftrack-macro-expansion=0" } */
+
+
+typedef __SIZE_TYPE__ size_t;
+
+struct A { char str[3]; };
+
+struct B { struct A a[3]; int i; };
+struct C { struct B b[3]; int i; };
+
+void stpncpy_nowarn_1 (struct C *pc, const char *s)
+{
+  __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str)[-1] = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  *__builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1) = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_3 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  d[-1] = 0;
+}
+
+void stpncpy_nowarn_4 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  *d = 0;
+}
+
+void strncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  pc->b[0].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_1 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[1].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[0].a[1].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
index c536a13..a6d418e 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-truncation.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
@@ -192,35 +192,35 @@ void test_strncpy_ptr (char *d, const char* s, const char *t, int i)
   CPY (d, CHOOSE ("123", "12"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */
 
   {
-    signed char n = strlen (s);   /* { dg-message "length computed here" } */
+    signed char n = strlen (s);     /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    short n = strlen (s);         /* { dg-message "length computed here" } */
+    short n = strlen (s);           /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    int n = strlen (s);           /* { dg-message "length computed here" } */
+    int n = strlen (s);             /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    unsigned n = strlen (s);      /* { dg-message "length computed here" } */
+    unsigned n = strlen (s);        /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
     size_t n;
-    n = strlen (s);               /* { dg-message "length computed here" } */
+    n = strlen (s);                 /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
     size_t n;
     char *dp2 = d + 1;
-    n = strlen (s);               /* { dg-message "length computed here" } */
+    n = strlen (s);                 /* { dg-message "length computed here" } */
     CPY (dp2, s, n);                /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
@@ -311,9 +311,11 @@ void test_strncpy_array (Dest *pd, int i, const char* s)
   /* Exercise destination with attribute "nonstring".  */
   CPY (pd->c3ns, "", 3);
   CPY (pd->c3ns, "", 1);
-  /* Truncation is still diagnosed -- using strncpy in this case is
-     pointless and should be replaced with memcpy.  */
-  CPY (pd->c3ns, "12", 1);          /* { dg-warning "output truncated copying 1 byte from a string of length 2" } */
+  /* It could be argued that truncation in the literal case should be
+     diagnosed even for non-strings.  Using strncpy in this case is
+     pointless and should be replaced with memcpy.  But it would likely
+     be viewed as a false positive.  */
+  CPY (pd->c3ns, "12", 1);
   CPY (pd->c3ns, "12", 2);
   CPY (pd->c3ns, "12", 3);
   CPY (pd->c3ns, "123", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-2.c b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
index 6e273e7..67fce03 100644
--- a/gcc/testsuite/c-c++-common/attr-nonstring-2.c
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
@@ -89,7 +89,7 @@ void test_pointer (const char *s, unsigned n)
   strncpy (pns_1, "a", 1);
   strncpy (pns_2, "ab", 2);
   strncpy (pns_3, "abc", 3);
-  strncpy (pns_3, s7, 3);         /* { dg-warning "output truncated copying 3 bytes from a string of length 7" } */
+  strncpy (pns_3, s7, 3);
 
   strncpy (pns_1, s, 1);
   strncpy (pns_2, s, 1);
@@ -105,6 +105,7 @@ void test_member_array (struct MemArrays *p, const char *s, unsigned n)
 {
   const char s7[] = "1234567";
 
+  strncpy (p->ma3, "", 0);
   strncpy (p->ma3, "a", 1);
   strncpy (p->ma4, "ab", 2);
   strncpy (p->ma5, "abc", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-3.c b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
new file mode 100644
index 0000000..19d657a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
@@ -0,0 +1,371 @@
+/* Test to exercise warnings when an array declared with attribute "nonstring"
+   is passed to a function that expects a nul-terminate string as an argument.
+   { dg-do compile }
+   { dg-options "-O2 -Wattributes -Wstringop-overflow -ftrack-macro-expansion=0" }  */
+
+typedef __SIZE_TYPE__       size_t;
+typedef __builtin_va_list   va_list;
+
+#if __cplusplus
+extern "C" {
+#endif
+
+void* memchr (const void*, int, size_t);
+int memcmp (const void*, const void*, size_t);
+void* memcpy (void*, const void*, size_t);
+void* memmove (void*, const void*, size_t);
+
+int printf (const char*, ...);
+int puts (const char*);
+int puts_unlocked (const char*);
+int sprintf (char*, const char*, ...);
+int snprintf (char*, size_t, const char*, ...);
+int vsprintf (char*, const char*, va_list);
+int vsnprintf (char*, size_t, const char*, va_list);
+
+int strcmp (const char*, const char*);
+int strncmp (const char*, const char*, size_t);
+
+char* stpcpy (char*, const char*);
+char* stpncpy (char*, const char*, size_t);
+
+char* strcat (char*, const char*);
+char* strncat (char*, const char*, size_t);
+
+char* strcpy (char*, const char*);
+char* strncpy (char*, const char*, size_t);
+
+char* strchr (const char*, int);
+char* strdup (const char*);
+size_t strlen (const char*);
+size_t strnlen (const char*, size_t);
+char* strndup (const char*, size_t);
+
+#if __cplusplus
+}   /* extern "C" */
+#endif
+
+#define NONSTRING __attribute__ ((nonstring))
+
+char str[4];
+char arr[4] NONSTRING;
+
+char *ptr;
+char *parr NONSTRING;
+
+struct MemArrays
+{
+  char str[4];
+  char arr[4] NONSTRING;
+  char *parr NONSTRING;
+};
+
+void sink (int, ...);
+
+
+#define T(call)  sink (0, (call))
+
+void test_printf (struct MemArrays *p)
+{
+  T (printf (str));
+  T (printf (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (ptr));
+  T (printf (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (p->str));
+  T (printf (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_puts (struct MemArrays *p)
+{
+  T (puts (str));
+  T (puts (arr));               /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (ptr));
+  T (puts (parr));              /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (p->str));
+  T (puts (p->arr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_snprintf (char *d, size_t n, struct MemArrays *p)
+{
+  T (snprintf (d, n, str));
+  T (snprintf (d, n, arr));       /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, ptr));
+  T (snprintf (d, n, parr));      /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, p->str));
+  T (snprintf (d, n, p->arr));    /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_sprintf (char *d, struct MemArrays *p)
+{
+  T (sprintf (d, str));
+  T (sprintf (d, arr));           /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, ptr));
+  T (sprintf (d, parr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, p->str));
+  T (sprintf (d, p->arr));        /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_vsnprintf (char *d, size_t n, struct MemArrays *p, va_list va)
+{
+  T (vsnprintf (d, n, str, va));
+  T (vsnprintf (d, n, arr, va));  /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, ptr, va));
+  T (vsnprintf (d, n, parr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, p->str, va));
+  T (vsnprintf (d, n, p->arr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_vsprintf (char *d, struct MemArrays *p, va_list va)
+{
+  T (vsprintf (d, str, va));
+  T (vsprintf (d, arr, va));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, ptr, va));
+  T (vsprintf (d, parr, va));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, p->str, va));
+  T (vsprintf (d, p->arr, va));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_strcmp (struct MemArrays *p)
+{
+  T (strcmp (str, str));
+  T (strcmp (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (arr, str));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (str, ptr));
+  T (strcmp (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (parr, str));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (p->arr, p->str));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strcmp (p->parr, p->str));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strncmp (struct MemArrays *p, size_t n)
+{
+  T (strncmp (str, str, n));
+  T (strncmp (str, arr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (arr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (str, ptr, n));
+  T (strncmp (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (parr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (p->str, p->arr, n));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (p->arr, p->str, n));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strncmp (p->parr, p->str, n));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stpcpy (struct MemArrays *p)
+{
+  T (stpcpy (str, str));
+  T (stpcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (arr, str));
+
+  T (stpcpy (str, ptr));
+  T (stpcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (parr, str));
+
+  T (stpcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (p->arr, p->str));
+  T (stpcpy (p->parr, p->str));
+}
+
+
+void test_stpncpy (struct MemArrays *p, unsigned n)
+{
+  T (stpncpy (str, str, n));
+  T (stpncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (arr, str, n));
+
+  T (stpncpy (str, ptr, n));
+  T (stpncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (parr, str, n));
+
+  T (stpncpy (p->str, p->arr, n));/* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (p->arr, p->str, n));
+  T (stpncpy (p->parr, p->str, n));
+}
+
+
+void test_strcat (struct MemArrays *p)
+{
+  T (strcat (str, str));
+  T (strcat (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (arr, str));
+
+  T (strcat (str, ptr));
+  T (strcat (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (parr, str));
+
+  T (strcat (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (p->arr, p->str));
+  T (strcat (p->parr, p->str));
+}
+
+
+void test_strncat (struct MemArrays *p, unsigned n)
+{
+  T (strncat (str, str, n));
+  T (strncat (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (arr, str, n));
+
+  T (strncat (str, ptr, n));
+  T (strncat (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (parr, str, n));
+
+  T (strncat (p->str, p->arr, n));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (p->arr, p->str, n));
+  T (strncat (p->parr, p->str, n));
+}
+
+
+void test_strcpy (struct MemArrays *p)
+{
+  T (strcpy (str, str));
+  T (strcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (arr, str));
+
+  T (strcpy (str, ptr));
+  T (strcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (parr, str));
+
+  T (strcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (p->arr, p->str));
+  T (strcpy (p->parr, p->str));
+}
+
+
+void test_strncpy (struct MemArrays *p, unsigned n)
+{
+  T (strncpy (str, str, n));
+  T (strncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (arr, str, n));
+
+  T (strncpy (str, ptr, n));
+  T (strncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (parr, str, n));
+
+  T (strncpy (p->str, p->arr, n));  /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (p->arr, p->str, n));
+  T (strncpy (p->parr, p->str, n));
+}
+
+
+void test_strchr (struct MemArrays *p, int c)
+{
+  T (strchr (str, c));
+  T (strchr (arr, c));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (ptr, c));
+  T (strchr (parr, c));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (p->str, c));
+  T (strchr (p->arr, c));       /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strdup (struct MemArrays *p)
+{
+  T (strdup (str));
+  T (strdup (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (ptr));
+  T (strdup (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (p->str));
+  T (strdup (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stnrdup (struct MemArrays *p, size_t n)
+{
+  T (strndup (str, n));
+  T (strndup (arr, n));
+
+  T (strndup (ptr, n));
+  T (strndup (parr, n));
+
+  T (strndup (p->str, n));
+  T (strndup (p->arr, n));
+}
+
+
+void test_strlen (struct MemArrays *p)
+{
+  T (strlen (str));
+  T (strlen (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (ptr));
+  T (strlen (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (p->str));
+  T (strlen (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strnlen (struct MemArrays *p, size_t n)
+{
+  T (strnlen (str, n));
+  T (strnlen (arr, n));
+
+  T (strnlen (ptr, n));
+  T (strnlen (parr, n));
+
+  T (strnlen (p->str, n));
+  T (strnlen (p->arr, n));
+}
+
+
+/* Verify no warnings are issued for raw mempory functions.  */
+
+void test_mem_functions (struct MemArrays *p, int c, size_t n)
+{
+  T (memchr (arr, c, n));
+  T (memchr (parr, c, n));
+  T (memchr (p->arr, c, n));
+  T (memchr (p->parr, c, n));
+
+  T (memcmp (str, arr, n));
+  T (memcmp (arr, str, n));
+  T (memcmp (str, parr, n));
+  T (memcmp (parr, str, n));
+  T (memcmp (p->str, p->arr, n));
+  T (memcmp (p->arr, p->str, n));
+  T (memcmp (p->parr, p->str, n));
+
+  T (memcpy (str, arr, n));
+  T (memcpy (arr, str, n));
+  T (memcpy (str, parr, n));
+  T (memcpy (parr, str, n));
+  T (memcpy (p->str, p->arr, n));
+  T (memcpy (p->arr, p->str, n));
+  T (memcpy (p->parr, p->str, n));
+
+  T (memmove (str, arr, n));
+  T (memmove (arr, str, n));
+  T (memmove (str, parr, n));
+  T (memmove (parr, str, n));
+  T (memmove (p->str, p->arr, n));
+  T (memmove (p->arr, p->str, n));
+  T (memmove (p->parr, p->str, n));
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 2efa182..6fa5beb 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "intl.h"
 #include "attribs.h"
+#include "calls.h"
 
 /* A vector indexed by SSA_NAME_VERSION.  0 means unknown, positive value
    is an index into strinfo vector, negative value stands for
@@ -153,7 +154,7 @@ struct decl_stridxlist_map
 static hash_map<tree_decl_hash, stridxlist> *decl_to_stridxlist_htab;
 
 typedef std::pair<int, location_t> stridx_strlenloc;
-static hash_map<tree, stridx_strlenloc> strlen_to_stridx;
+static hash_map<tree, stridx_strlenloc> *strlen_to_stridx;
 
 /* Obstack for struct stridxlist and struct decl_stridxlist_map.  */
 static struct obstack stridx_obstack;
@@ -1208,7 +1209,7 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
 	    }
 
 	  location_t loc = gimple_location (stmt);
-	  strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc));
+	  strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
 	  return;
 	}
     }
@@ -1254,7 +1255,7 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
       find_equal_ptrs (src, idx);
 
       location_t loc = gimple_location (stmt);
-      strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc));
+      strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
     }
 }
 
@@ -1733,35 +1734,15 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
     return false;
 
   tree dst = gimple_call_arg (stmt, 0);
-
-  /* See if the destination is declared with attribute "nonstring"
-     and if so, avoid the truncation warning.  */
-  if (TREE_CODE (dst) == SSA_NAME)
-    {
-      if (SSA_NAME_IS_DEFAULT_DEF (dst))
-	dst = SSA_NAME_VAR (dst);
-      else
-	{
-	  gimple *def = SSA_NAME_DEF_STMT (dst);
-
-	  if (is_gimple_assign (def)
-	      && gimple_assign_rhs_code (def) == ADDR_EXPR)
-	    dst = gimple_assign_rhs1 (def);
-	}
-    }
-
   tree dstdecl = dst;
   if (TREE_CODE (dstdecl) == ADDR_EXPR)
     dstdecl = TREE_OPERAND (dstdecl, 0);
 
-  {
-    tree d = dstdecl;
-    if (TREE_CODE (d) == COMPONENT_REF)
-      d = TREE_OPERAND (d, 1);
-
-    if (DECL_P (d) && lookup_attribute ("nonstring", DECL_ATTRIBUTES (d)))
-      return false;
-  }
+  /* If the destination refers to a an array/pointer declared nonstring
+     return early.  */
+  tree ref = NULL_TREE;
+  if (get_attr_nonstring_decl (dstdecl, &ref))
+    return false;
 
   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
      avoid the truncation warning.  */
@@ -1770,12 +1751,32 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
 
   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
     {
-      HOST_WIDE_INT off;
-      dstdecl = get_addr_base_and_unit_offset (dstdecl, &off);
-
       tree lhs = gimple_assign_lhs (next_stmt);
-      tree lhsbase = get_addr_base_and_unit_offset (lhs, &off);
-      if (lhsbase && operand_equal_p (dstdecl, lhsbase, 0))
+      tree_code code = TREE_CODE (lhs);
+      if (code == ARRAY_REF || code == MEM_REF)
+	lhs = TREE_OPERAND (lhs, 0);
+
+      tree func = gimple_call_fndecl (stmt);
+      if (DECL_FUNCTION_CODE (func) == BUILT_IN_STPNCPY)
+	{
+	  tree ret = gimple_call_lhs (stmt);
+	  if (ret && operand_equal_p (ret, lhs, 0))
+	    return false;
+	}
+
+      /* Determine the base address and offset of the reference,
+	 ignoring the innermost array index.  */
+      if (TREE_CODE (ref) == ARRAY_REF)
+	ref = TREE_OPERAND (ref, 0);
+
+      HOST_WIDE_INT dstoff;
+      tree dstbase = get_addr_base_and_unit_offset (ref, &dstoff);
+
+      HOST_WIDE_INT lhsoff;
+      tree lhsbase = get_addr_base_and_unit_offset (lhs, &lhsoff);
+      if (lhsbase
+	  && dstoff == lhsoff
+	  && operand_equal_p (dstbase, lhsbase, 0))
 	return false;
     }
 
@@ -1919,7 +1920,7 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi)
   /* If the length argument was computed from strlen(S) for some string
      S retrieve the strinfo index for the string (PSS->FIRST) alonng with
      the location of the strlen() call (PSS->SECOND).  */
-  stridx_strlenloc *pss = strlen_to_stridx.get (len);
+  stridx_strlenloc *pss = strlen_to_stridx->get (len);
   if (!pss || pss->first <= 0)
     {
       if (maybe_diag_stxncpy_trunc (*gsi, src, len))
@@ -2967,8 +2968,8 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)
 				  gimple_assign_rhs2 (stmt), stmt);
 
 	tree rhs1 = gimple_assign_rhs1 (stmt);
-	if (stridx_strlenloc *ps = strlen_to_stridx.get (rhs1))
-	  strlen_to_stridx.put (lhs, *ps);
+	if (stridx_strlenloc *ps = strlen_to_stridx->get (rhs1))
+	  strlen_to_stridx->put (lhs, stridx_strlenloc (*ps));
       }
     else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
 	{
@@ -3199,6 +3200,9 @@ public:
 unsigned int
 pass_strlen::execute (function *fun)
 {
+  gcc_assert (!strlen_to_stridx);
+  strlen_to_stridx = new hash_map<tree, stridx_strlenloc> ();
+
   ssa_ver_to_stridx.safe_grow_cleared (num_ssa_names);
   max_stridx = 1;
 
@@ -3220,7 +3224,9 @@ pass_strlen::execute (function *fun)
   laststmt.len = NULL_TREE;
   laststmt.stridx = 0;
 
-  strlen_to_stridx.empty ();
+  strlen_to_stridx->empty ();
+  delete strlen_to_stridx;
+  strlen_to_stridx = NULL;
 
   return 0;
 }



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-17  0:27   ` Jeff Law
  2017-11-17  8:42     ` Jakub Jelinek
  2017-11-19 17:21     ` Martin Sebor
@ 2017-11-19 17:26     ` Martin Sebor
  2 siblings, 0 replies; 13+ messages in thread
From: Martin Sebor @ 2017-11-19 17:26 UTC (permalink / raw)
  To: Jeff Law, Gcc Patch List, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 6620 bytes --]

On 11/16/2017 05:15 PM, Jeff Law wrote:
> On 11/13/2017 06:21 PM, Martin Sebor wrote:
>> Attached is an improved version that rather than hardcoding
>> the built-in functions to check uses the constness of each
>> built-in's char* argument as a trigger to do the checking.
>>
>> This avoids the problem of the list being incomplete either
>> by omission or due to getting out of sync, and also makes it
>> trivial to extend the same approach to user-defined functions
>> in the future.
>>
>> I've excluded strndup and strnlen (but no other "bounded"
>> string functions like strncmp) from the checking.
>>
>> Martin
>>
>> On 11/12/2017 05:52 PM, Martin Sebor wrote:
>>> The recently introduced -Wstringop-truncation warning relies
>>> on the new nonstring attribute to allow the historical use case
>>> of calling strncpy to completely fill the destination with a copy
>>> of a string without adding a terminating nul.  Glibc is currently
>>> considering making use of the attribute to decorate some of its
>>> data members.  To help find misuses of such data members in
>>> arguments to string functions like strlen or strdup, the attached
>>> patch adds checking for this new attribute in these contexts.
>>> The checking is intentionally done late so  that uses such arrays
>>> that can be proven to be safe (and thus folded) are not diagnosed.
>>>
>>> While testing this simple enhancement I noticed that the handling
>>> I added for the nul termination in cases like
>>>
>>>   strncpy (array, s, sizeof array);
>>>   array[sizeof array - 1] = 0;
>>>
>>> to avoid the new warning wasn't quite as robust as it could and
>>> arguably should be so I improved it a bit to silently accept
>>> more forms of the idiom.  For instance, this is now correctly
>>> handled (and not diagnosed):
>>>
>>>   *stpcpy (array, s, sizeof array - 1) = 0;
>>>
>>> Martin
>>>
>>> PS A useful future enhancement would be to detect the one byte
>>> overflow in:
>>>
>>>   *stpcpy (array, s, sizeof array) = 0;
>>
>>
>> gcc-82945.diff
>>
>>
>> PR tree-optimization/82945 - add warning for passing non-strings to functions that expect string arguments
>>
>> gcc/ChangeLog:
>>
>> 	PR tree-optimization/82945
>> 	* calls.c (get_attr_nonstring_decl, maybe_warn_nonstring_arg): New
>> 	functions.
>> 	(initialize_argument_information): Call maybe_warn_nonstring_arg.
>> 	* calls.h (get_attr_nonstring_decl): Declare new function.
>> 	* doc/extend.texi (attribute nonstring): Update.
>> 	* gimple-fold.c (gimple_fold_builtin_strncpy): Call
>> 	get_attr_nonstring_decl and handle it.
>> 	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.  Improve
>> 	detection of nul-termination.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR tree-optimization/82945
>> 	* c-c++-common/Wstringop-truncation-2.c: New test.
>> 	* c-c++-common/Wstringop-truncation.c: Adjust.
>> 	* c-c++-common/attr-nonstring-2.c: Adjust.
>> 	* c-c++-common/attr-nonstring-3.c: New test.
>
>>
>> diff --git a/gcc/calls.c b/gcc/calls.c
>> index 3730f43..197d907 100644
>> --- a/gcc/calls.c
>> +++ b/gcc/calls.c
>> @@ -1494,6 +1494,108 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2])
>>      }
>>  }
>>
>> +/* If EXPR refers to a character array or pointer declared attribute
>> +   nonstring return a decl for that array or pointer and set *REF to
>> +   the referenced enclosing object or pointer.  Otherwise returns
>> +   null.  */
> Generally we'd write NULL rather than null for a generic pointer and in
> this specific case NULL_TREE is even better.

I agree.  The C NULL macro, C++ nullptr, and GCC's NULL_TREE and
NULL_RTL are helpful both for type safety and readability of code.
But they all are an implementation detail that's not relevant to
the specification of an API or its users. The established term
that every C and C++ programmer is familiar with is "null pointer."
I didn't think too hard about it when I wrote the comment (it just
didn't seem important enough) but now that I have I believe "null"
is better, clearer, and more future- proof (in case the return
type should ever change to some smart pointer class).

(There also comments in this file and elsewhere in GCC that use
various forms of null.)

>> +tree
>> +get_attr_nonstring_decl (tree expr, tree *ref)
>> +{
>> +  tree dcl = expr;
> I think Jakub may have pointed this out, but we generally prefer decl.
> Truly a nit though.

Right.  I've changed it to DECL.

>> +  if (TREE_CODE (dcl) == SSA_NAME)
>> +    {
>> +      if (SSA_NAME_IS_DEFAULT_DEF (dcl))
>> +	dcl = SSA_NAME_VAR (dcl);
>> +      else
>> +	{
>> +	  gimple *def = SSA_NAME_DEF_STMT (dcl);
>> +
>> +	  if (is_gimple_assign (def))
>> +	    {
>> +	      tree_code code = gimple_assign_rhs_code (def);
>> +	      if (code == ADDR_EXPR
>> +		  || code == COMPONENT_REF
>> +		  || code == VAR_DECL)
>> +		dcl = gimple_assign_rhs1 (def);
> Note that COMPONENT_REFs can have arguments that are themselves
> COMPONENT_REFS.  So you typically want to use a loop to strip all away.

I have seen it in the early IL but this late I couldn't come
up with a test case where such a loop would be necessary.
The COMPONENT_REF always refers to the member array.  If you
can show me an example to test with I'll add the handling if
possible.  There are cases where it isn't, such as in

     struct A { char a[4] __attribute__ ((nonstring)); } *pa;

     strlen (pa->a + 1);

where pa->a is represented as a MEM_REF (char[4], pa, 1) with
the member information having been lost in translation.  (This
is the same problem that I had solved for the -Warray-bounds
warning for out-of bounds offsets by implementing it in
tree-ssa-forwprop.c: because that's where the member is folded
into a reference to the base object.)

>> +  /* It's safe to call strndup and strnlen with a non-string argument
>> +     since the functions provide an explicit bound for this purpose.  */
>> +  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRNDUP)
>> +    return;
> So I see you handle BUILT_IN_STRNDUP here, but not BUILT_IN_STRNLEN.  Is
> there a reason for the omission?

strnlen is not a GCC built-in so it's handled implicitly by
the test for DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL
just before the if statement above.  The test I added verifies
that strnlen is excluded.  (Bug 81384 tracks the missing
intrinsic support for strnlen.)

That said, more testing revealed that Glibc uses functions like
strncmp and strncpy to compare and copy non-strings, so I've
added those to the white list and improved the checking a bit.
I've also changed the hash_map to a pointer to make Jakub happy.

Attached is an updated patch.

Martin

[-- Attachment #2: gcc-82945.diff --]
[-- Type: text/x-patch, Size: 35873 bytes --]

PR tree-optimization/82945 - add warning for passing non-strings to functions that expect string arguments

gcc/ChangeLog:

	PR tree-optimization/82945
	* calls.c (get_attr_nonstring_decl, maybe_warn_nonstring_arg): New
	functions.
	(initialize_argument_information): Call maybe_warn_nonstring_arg.
	* calls.h (get_attr_nonstring_decl): Declare new function.
	* doc/extend.texi (attribute nonstring): Update.
	* gimple-fold.c (gimple_fold_builtin_strncpy): Call
	get_attr_nonstring_decl and handle it.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.  Improve
	detection of nul-termination.
	(strlen_to_stridx): Change to a pointer.
	(handle_builtin_strlen, handle_builtin_stxncpy): Adjust.
	(pass_strlen::execute): Same.

gcc/testsuite/ChangeLog:

	PR tree-optimization/82945
	* c-c++-common/Wstringop-truncation-2.c: New test.
	* c-c++-common/Wstringop-truncation.c: Adjust.
	* c-c++-common/attr-nonstring-2.c: Adjust.
	* c-c++-common/attr-nonstring-3.c: New test.

diff --git a/gcc/calls.c b/gcc/calls.c
index 3730f43..197d907 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1494,6 +1494,108 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2])
     }
 }
 
+/* If EXPR refers to a character array or pointer declared attribute
+   nonstring return a decl for that array or pointer and set *REF to
+   the referenced enclosing object or pointer.  Otherwise returns
+   null.  */
+
+tree
+get_attr_nonstring_decl (tree expr, tree *ref)
+{
+  tree dcl = expr;
+  if (TREE_CODE (dcl) == SSA_NAME)
+    {
+      if (SSA_NAME_IS_DEFAULT_DEF (dcl))
+	dcl = SSA_NAME_VAR (dcl);
+      else
+	{
+	  gimple *def = SSA_NAME_DEF_STMT (dcl);
+
+	  if (is_gimple_assign (def))
+	    {
+	      tree_code code = gimple_assign_rhs_code (def);
+	      if (code == ADDR_EXPR
+		  || code == COMPONENT_REF
+		  || code == VAR_DECL)
+		dcl = gimple_assign_rhs1 (def);
+	    }
+	}
+    }
+
+  if (TREE_CODE (dcl) == ADDR_EXPR)
+    dcl = TREE_OPERAND (dcl, 0);
+
+  if (ref)
+    *ref = dcl;
+
+  if (TREE_CODE (dcl) == COMPONENT_REF)
+    dcl = TREE_OPERAND (dcl, 1);
+
+  if (DECL_P (dcl)
+      && lookup_attribute ("nonstring", DECL_ATTRIBUTES (dcl)))
+    return dcl;
+
+  return NULL_TREE;
+}
+
+/* Warn about passing a non-string array/pointer to a function that
+   expects a nul-terminated string argument.  */
+
+static void
+maybe_warn_nonstring_arg (tree fndecl, tree exp)
+{
+  if (!fndecl || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
+    return;
+
+  /* It's safe to call strndup and strnlen with a non-string argument
+     since the functions provide an explicit bound for this purpose.  */
+  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRNDUP)
+    return;
+
+  /* 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.  */
+  function_args_iterator it;
+  function_args_iter_init (&it, TREE_TYPE (fndecl));
+
+  for (unsigned argno = 0; ; ++argno, function_args_iter_next (&it))
+    {
+      tree argtype = function_args_iter_cond (&it);
+      if (!argtype)
+	break;
+
+      if (TREE_CODE (argtype) != POINTER_TYPE)
+	continue;
+
+      argtype = TREE_TYPE (argtype);
+
+      if (TREE_CODE (argtype) != INTEGER_TYPE
+	  || !TYPE_READONLY (argtype))
+	continue;
+
+      argtype = TYPE_MAIN_VARIANT (argtype);
+      if (argtype != char_type_node)
+	continue;
+
+      tree callarg = CALL_EXPR_ARG (exp, argno);
+      if (TREE_CODE (callarg) == ADDR_EXPR)
+	callarg = TREE_OPERAND (callarg, 0);
+
+      /* See if the destination is declared with attribute "nonstring"
+	 and if so, issue a warning.  */
+      tree dcl = get_attr_nonstring_decl (callarg);
+      if (!dcl)
+	continue;
+
+      location_t loc = EXPR_LOCATION (exp);
+      if (warning_at (loc, OPT_Wstringop_overflow_,
+		      "%qD argument %i declared attribute %<nonstring%>",
+		      fndecl, argno + 1))
+	inform (DECL_SOURCE_LOCATION (dcl),
+		"argument %qD declared here", dcl);
+    }
+}
+
 /* Issue an error if CALL_EXPR was flagged as requiring
    tall-call optimization.  */
 
@@ -1943,6 +2045,10 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 	 alloc_size.  */
       maybe_warn_alloc_args_overflow (fndecl, exp, alloc_args, alloc_idx);
     }
+
+  /* Detect passing non-string arguments to functions expecting
+     nul-terminated strings.  */
+  maybe_warn_nonstring_arg (fndecl, exp);
 }
 
 /* Update ARGS_SIZE to contain the total size for the argument block.
diff --git a/gcc/calls.h b/gcc/calls.h
index df5817f..4f069b4 100644
--- a/gcc/calls.h
+++ b/gcc/calls.h
@@ -39,5 +39,6 @@ extern bool reference_callee_copied (CUMULATIVE_ARGS *, machine_mode,
 				     tree, bool);
 extern void maybe_warn_alloc_args_overflow (tree, tree, tree[2], int[2]);
 extern bool get_size_range (tree, tree[2]);
+extern tree get_attr_nonstring_decl (tree, tree * = NULL);
 
 #endif // GCC_CALLS_H
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index d887378..4680dea 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -5974,22 +5974,33 @@ types (@pxref{Common Function Attributes},
 The @code{nonstring} variable attribute specifies that an object or member
 declaration with type array of @code{char} or pointer to @code{char} is
 intended to store character arrays that do not necessarily contain
-a terminating @code{NUL} character.  This is useful to avoid warnings
-when such an array or pointer is used as an argument to a bounded string
-manipulation function such as @code{strncpy}.  For example, without the
-attribute, GCC will issue a warning for the call below because it may
-truncate the copy without appending the terminating NUL character.  Using
-the attribute makes it possible to suppress the warning.
+a terminating @code{NUL} character.  This is useful in detecting uses
+of such arrays or pointers with functions that expect NUL-terminated
+strings, and to avoid warnings when such an array or pointer is used
+as an argument to a bounded string manipulation function such as
+@code{strncpy}.  For example, without the attribute, GCC will issue
+a warning for the @code{strncpy} call below because it may truncate
+the copy without appending the terminating @code{NUL} character.  Using
+the attribute makes it possible to suppress the warning.  However, when
+the array is declared with the attribute the call to @code{strlen} is
+diagnosed because when the array doesn't contain a @code{NUL}-terminated
+string the call is undefined.  To copy, compare, of search non-string
+character arrays use the @code{memcpy}, @code{memcmp}, @code{memchr},
+and other functions that operate on arrays of bytes.  In addition,
+calling @code{strnlen} and @code{strndup} with such arrays is safe
+provided a suitable bound is specified, and not diagnosed.
 
 @smallexample
 struct Data
 @{
   char name [32] __attribute__ ((nonstring));
 @};
-void f (struct Data *pd, const char *s)
+
+int f (struct Data *pd, const char *s)
 @{
   strncpy (pd->name, s, sizeof pd->name);
   @dots{}
+  return strlen (pd->name);   // unsafe, gets a warning
 @}
 @end smallexample
 
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index adb6f3b..8c6cdff 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -62,6 +62,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "diagnostic-core.h"
 #include "intl.h"
+#include "calls.h"
 
 /* Return true when DECL can be referenced from current unit.
    FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
@@ -1558,25 +1559,31 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
 {
   gimple *stmt = gsi_stmt (*gsi);
   location_t loc = gimple_location (stmt);
+  bool nonstring = get_attr_nonstring_decl (dest) != NULL_TREE;
 
   /* If the LEN parameter is zero, return DEST.  */
   if (integer_zerop (len))
     {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      /* Warn about the lack of nul termination: the result is not
-	 a (nul-terminated) string.  */
-      tree slen = get_maxval_strlen (src, 0);
-      if (slen && !integer_zerop (slen))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes "
-		    "from a string of length %E",
-		    call, fndecl, slen);
-      else
-	warning_at (loc, OPT_Wstringop_truncation,
-		    "%G%qD destination unchanged after copying no bytes",
-		    call, fndecl);
+      /* Avoid warning if the destination refers to a an array/pointer
+	 decorate with attribute nonstring.  */
+      if (!nonstring)
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  /* Warn about the lack of nul termination: the result is not
+	     a (nul-terminated) string.  */
+	  tree slen = get_maxval_strlen (src, 0);
+	  if (slen && !integer_zerop (slen))
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes "
+			"from a string of length %E",
+			call, fndecl, slen);
+	  else
+	    warning_at (loc, OPT_Wstringop_truncation,
+			"%G%qD destination unchanged after copying no bytes",
+			call, fndecl);
+	}
 
       replace_call_with_value (gsi, dest);
       return true;
@@ -1601,53 +1608,36 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
   if (tree_int_cst_lt (ssize, len))
     return false;
 
-  if (tree_int_cst_lt (len, slen))
-    {
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      warning_at (loc, OPT_Wstringop_truncation,
-		  (tree_int_cst_equal (size_one_node, len)
-		   ? G_("%G%qD output truncated copying %E byte "
-			"from a string of length %E")
-		   : G_("%G%qD output truncated copying %E bytes "
-		      "from a string of length %E")),
-		  call, fndecl, len, slen);
-    }
-  else if (tree_int_cst_equal (len, slen))
+  if (!nonstring)
     {
-      tree decl = dest;
-      if (TREE_CODE (decl) == SSA_NAME)
+      if (tree_int_cst_lt (len, slen))
 	{
-	  gimple *def_stmt = SSA_NAME_DEF_STMT (decl);
-	  if (is_gimple_assign (def_stmt))
-	    {
-	      tree_code code = gimple_assign_rhs_code (def_stmt);
-	      if (code == ADDR_EXPR || code == VAR_DECL)
-		decl = gimple_assign_rhs1 (def_stmt);
-	    }
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated copying %E byte "
+			    "from a string of length %E")
+		       : G_("%G%qD output truncated copying %E bytes "
+			    "from a string of length %E")),
+		      call, fndecl, len, slen);
+	}
+      else if (tree_int_cst_equal (len, slen))
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
+	  gcall *call = as_a <gcall *> (stmt);
+
+	  warning_at (loc, OPT_Wstringop_truncation,
+		      (tree_int_cst_equal (size_one_node, len)
+		       ? G_("%G%qD output truncated before terminating nul "
+			    "copying %E byte from a string of the same "
+			    "length")
+		       : G_("%G%qD output truncated before terminating nul "
+			    "copying %E bytes from a string of the same "
+			    "length")),
+		      call, fndecl, len);
 	}
-
-      if (TREE_CODE (decl) == ADDR_EXPR)
-	decl = TREE_OPERAND (decl, 0);
-
-      if (TREE_CODE (decl) == COMPONENT_REF)
-	decl = TREE_OPERAND (decl, 1);
-
-      tree fndecl = gimple_call_fndecl (stmt);
-      gcall *call = as_a <gcall *> (stmt);
-
-      if (!DECL_P (decl)
-	  || !lookup_attribute ("nonstring", DECL_ATTRIBUTES (decl)))
-	warning_at (loc, OPT_Wstringop_truncation,
-		    (tree_int_cst_equal (size_one_node, len)
-		     ? G_("%G%qD output truncated before terminating nul "
-			  "copying %E byte from a string of the same "
-			  "length")
-		     : G_("%G%qD output truncated before terminating nul "
-			  "copying %E bytes from a string of the same "
-			  "length")),
-		    call, fndecl, len);
     }
 
   /* OK transform into builtin memcpy.  */
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
new file mode 100644
index 0000000..8256070
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation-2.c
@@ -0,0 +1,56 @@
+/* Verify that 
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-truncation -Wno-stringop-overflow -ftrack-macro-expansion=0" } */
+
+
+typedef __SIZE_TYPE__ size_t;
+
+struct A { char str[3]; };
+
+struct B { struct A a[3]; int i; };
+struct C { struct B b[3]; int i; };
+
+void stpncpy_nowarn_1 (struct C *pc, const char *s)
+{
+  __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str)[-1] = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  *__builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1) = 0;   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+}
+
+void stpncpy_nowarn_3 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  d[-1] = 0;
+}
+
+void stpncpy_nowarn_4 (struct C *pc, const char *s)
+{
+  char *d = __builtin_stpncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str - 1);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  *d = 0;
+}
+
+void strncpy_nowarn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+  pc->b[0].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_1 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[1].a[0].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
+
+void strncpy_warn_2 (struct C *pc, const char *s)
+{
+  __builtin_strncpy (pc->b[0].a[0].str, s, sizeof pc->b[0].a[0].str);   /* { dg-warning "specified bound 3 equals destination size" } */
+
+  pc->b[0].a[1].str[sizeof pc->b[0].a[0].str - 1] = 0;
+}
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
index c536a13..a6d418e 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-truncation.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c
@@ -192,35 +192,35 @@ void test_strncpy_ptr (char *d, const char* s, const char *t, int i)
   CPY (d, CHOOSE ("123", "12"), 1); /* { dg-warning ".strncpy\[^\n\r\]* output truncated copying 1 byte from a string of length 2" } */
 
   {
-    signed char n = strlen (s);   /* { dg-message "length computed here" } */
+    signed char n = strlen (s);     /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    short n = strlen (s);         /* { dg-message "length computed here" } */
+    short n = strlen (s);           /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    int n = strlen (s);           /* { dg-message "length computed here" } */
+    int n = strlen (s);             /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
-    unsigned n = strlen (s);      /* { dg-message "length computed here" } */
+    unsigned n = strlen (s);        /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
     size_t n;
-    n = strlen (s);               /* { dg-message "length computed here" } */
+    n = strlen (s);                 /* { dg-message "length computed here" } */
     CPY (d, s, n);                  /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
   {
     size_t n;
     char *dp2 = d + 1;
-    n = strlen (s);               /* { dg-message "length computed here" } */
+    n = strlen (s);                 /* { dg-message "length computed here" } */
     CPY (dp2, s, n);                /* { dg-warning ".strncpy\[^\n\r\]* output truncated before terminating nul copying as many bytes from a string as its length" } */
   }
 
@@ -311,9 +311,11 @@ void test_strncpy_array (Dest *pd, int i, const char* s)
   /* Exercise destination with attribute "nonstring".  */
   CPY (pd->c3ns, "", 3);
   CPY (pd->c3ns, "", 1);
-  /* Truncation is still diagnosed -- using strncpy in this case is
-     pointless and should be replaced with memcpy.  */
-  CPY (pd->c3ns, "12", 1);          /* { dg-warning "output truncated copying 1 byte from a string of length 2" } */
+  /* It could be argued that truncation in the literal case should be
+     diagnosed even for non-strings.  Using strncpy in this case is
+     pointless and should be replaced with memcpy.  But it would likely
+     be viewed as a false positive.  */
+  CPY (pd->c3ns, "12", 1);
   CPY (pd->c3ns, "12", 2);
   CPY (pd->c3ns, "12", 3);
   CPY (pd->c3ns, "123", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-2.c b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
index 6e273e7..67fce03 100644
--- a/gcc/testsuite/c-c++-common/attr-nonstring-2.c
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-2.c
@@ -89,7 +89,7 @@ void test_pointer (const char *s, unsigned n)
   strncpy (pns_1, "a", 1);
   strncpy (pns_2, "ab", 2);
   strncpy (pns_3, "abc", 3);
-  strncpy (pns_3, s7, 3);         /* { dg-warning "output truncated copying 3 bytes from a string of length 7" } */
+  strncpy (pns_3, s7, 3);
 
   strncpy (pns_1, s, 1);
   strncpy (pns_2, s, 1);
@@ -105,6 +105,7 @@ void test_member_array (struct MemArrays *p, const char *s, unsigned n)
 {
   const char s7[] = "1234567";
 
+  strncpy (p->ma3, "", 0);
   strncpy (p->ma3, "a", 1);
   strncpy (p->ma4, "ab", 2);
   strncpy (p->ma5, "abc", 3);
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-3.c b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
new file mode 100644
index 0000000..19d657a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-3.c
@@ -0,0 +1,371 @@
+/* Test to exercise warnings when an array declared with attribute "nonstring"
+   is passed to a function that expects a nul-terminate string as an argument.
+   { dg-do compile }
+   { dg-options "-O2 -Wattributes -Wstringop-overflow -ftrack-macro-expansion=0" }  */
+
+typedef __SIZE_TYPE__       size_t;
+typedef __builtin_va_list   va_list;
+
+#if __cplusplus
+extern "C" {
+#endif
+
+void* memchr (const void*, int, size_t);
+int memcmp (const void*, const void*, size_t);
+void* memcpy (void*, const void*, size_t);
+void* memmove (void*, const void*, size_t);
+
+int printf (const char*, ...);
+int puts (const char*);
+int puts_unlocked (const char*);
+int sprintf (char*, const char*, ...);
+int snprintf (char*, size_t, const char*, ...);
+int vsprintf (char*, const char*, va_list);
+int vsnprintf (char*, size_t, const char*, va_list);
+
+int strcmp (const char*, const char*);
+int strncmp (const char*, const char*, size_t);
+
+char* stpcpy (char*, const char*);
+char* stpncpy (char*, const char*, size_t);
+
+char* strcat (char*, const char*);
+char* strncat (char*, const char*, size_t);
+
+char* strcpy (char*, const char*);
+char* strncpy (char*, const char*, size_t);
+
+char* strchr (const char*, int);
+char* strdup (const char*);
+size_t strlen (const char*);
+size_t strnlen (const char*, size_t);
+char* strndup (const char*, size_t);
+
+#if __cplusplus
+}   /* extern "C" */
+#endif
+
+#define NONSTRING __attribute__ ((nonstring))
+
+char str[4];
+char arr[4] NONSTRING;
+
+char *ptr;
+char *parr NONSTRING;
+
+struct MemArrays
+{
+  char str[4];
+  char arr[4] NONSTRING;
+  char *parr NONSTRING;
+};
+
+void sink (int, ...);
+
+
+#define T(call)  sink (0, (call))
+
+void test_printf (struct MemArrays *p)
+{
+  T (printf (str));
+  T (printf (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (ptr));
+  T (printf (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (printf (p->str));
+  T (printf (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_puts (struct MemArrays *p)
+{
+  T (puts (str));
+  T (puts (arr));               /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (ptr));
+  T (puts (parr));              /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (puts (p->str));
+  T (puts (p->arr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_snprintf (char *d, size_t n, struct MemArrays *p)
+{
+  T (snprintf (d, n, str));
+  T (snprintf (d, n, arr));       /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, ptr));
+  T (snprintf (d, n, parr));      /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (snprintf (d, n, p->str));
+  T (snprintf (d, n, p->arr));    /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_sprintf (char *d, struct MemArrays *p)
+{
+  T (sprintf (d, str));
+  T (sprintf (d, arr));           /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, ptr));
+  T (sprintf (d, parr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (sprintf (d, p->str));
+  T (sprintf (d, p->arr));        /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_vsnprintf (char *d, size_t n, struct MemArrays *p, va_list va)
+{
+  T (vsnprintf (d, n, str, va));
+  T (vsnprintf (d, n, arr, va));  /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, ptr, va));
+  T (vsnprintf (d, n, parr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+
+  T (vsnprintf (d, n, p->str, va));
+  T (vsnprintf (d, n, p->arr, va)); /* { dg-warning "argument 3 declared attribute .nonstring." } */
+}
+
+
+void test_vsprintf (char *d, struct MemArrays *p, va_list va)
+{
+  T (vsprintf (d, str, va));
+  T (vsprintf (d, arr, va));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, ptr, va));
+  T (vsprintf (d, parr, va));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+  T (vsprintf (d, p->str, va));
+  T (vsprintf (d, p->arr, va));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+
+void test_strcmp (struct MemArrays *p)
+{
+  T (strcmp (str, str));
+  T (strcmp (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (arr, str));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (str, ptr));
+  T (strcmp (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (parr, str));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strcmp (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcmp (p->arr, p->str));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strcmp (p->parr, p->str));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strncmp (struct MemArrays *p, size_t n)
+{
+  T (strncmp (str, str, n));
+  T (strncmp (str, arr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (arr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (str, ptr, n));
+  T (strncmp (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (parr, str, n));     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strncmp (p->str, p->arr, n));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncmp (p->arr, p->str, n));    /* { dg-warning "argument 1 declared attribute .nonstring." } */
+  T (strncmp (p->parr, p->str, n));   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stpcpy (struct MemArrays *p)
+{
+  T (stpcpy (str, str));
+  T (stpcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (arr, str));
+
+  T (stpcpy (str, ptr));
+  T (stpcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (parr, str));
+
+  T (stpcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpcpy (p->arr, p->str));
+  T (stpcpy (p->parr, p->str));
+}
+
+
+void test_stpncpy (struct MemArrays *p, unsigned n)
+{
+  T (stpncpy (str, str, n));
+  T (stpncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (arr, str, n));
+
+  T (stpncpy (str, ptr, n));
+  T (stpncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (parr, str, n));
+
+  T (stpncpy (p->str, p->arr, n));/* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (stpncpy (p->arr, p->str, n));
+  T (stpncpy (p->parr, p->str, n));
+}
+
+
+void test_strcat (struct MemArrays *p)
+{
+  T (strcat (str, str));
+  T (strcat (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (arr, str));
+
+  T (strcat (str, ptr));
+  T (strcat (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (parr, str));
+
+  T (strcat (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcat (p->arr, p->str));
+  T (strcat (p->parr, p->str));
+}
+
+
+void test_strncat (struct MemArrays *p, unsigned n)
+{
+  T (strncat (str, str, n));
+  T (strncat (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (arr, str, n));
+
+  T (strncat (str, ptr, n));
+  T (strncat (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (parr, str, n));
+
+  T (strncat (p->str, p->arr, n));   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncat (p->arr, p->str, n));
+  T (strncat (p->parr, p->str, n));
+}
+
+
+void test_strcpy (struct MemArrays *p)
+{
+  T (strcpy (str, str));
+  T (strcpy (str, arr));          /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (arr, str));
+
+  T (strcpy (str, ptr));
+  T (strcpy (str, parr));         /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (parr, str));
+
+  T (strcpy (p->str, p->arr));    /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strcpy (p->arr, p->str));
+  T (strcpy (p->parr, p->str));
+}
+
+
+void test_strncpy (struct MemArrays *p, unsigned n)
+{
+  T (strncpy (str, str, n));
+  T (strncpy (str, arr, n));      /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (arr, str, n));
+
+  T (strncpy (str, ptr, n));
+  T (strncpy (str, parr, n));     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (parr, str, n));
+
+  T (strncpy (p->str, p->arr, n));  /* { dg-warning "argument 2 declared attribute .nonstring." } */
+  T (strncpy (p->arr, p->str, n));
+  T (strncpy (p->parr, p->str, n));
+}
+
+
+void test_strchr (struct MemArrays *p, int c)
+{
+  T (strchr (str, c));
+  T (strchr (arr, c));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (ptr, c));
+  T (strchr (parr, c));         /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strchr (p->str, c));
+  T (strchr (p->arr, c));       /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strdup (struct MemArrays *p)
+{
+  T (strdup (str));
+  T (strdup (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (ptr));
+  T (strdup (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strdup (p->str));
+  T (strdup (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_stnrdup (struct MemArrays *p, size_t n)
+{
+  T (strndup (str, n));
+  T (strndup (arr, n));
+
+  T (strndup (ptr, n));
+  T (strndup (parr, n));
+
+  T (strndup (p->str, n));
+  T (strndup (p->arr, n));
+}
+
+
+void test_strlen (struct MemArrays *p)
+{
+  T (strlen (str));
+  T (strlen (arr));             /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (ptr));
+  T (strlen (parr));            /* { dg-warning "argument 1 declared attribute .nonstring." } */
+
+  T (strlen (p->str));
+  T (strlen (p->arr));          /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+void test_strnlen (struct MemArrays *p, size_t n)
+{
+  T (strnlen (str, n));
+  T (strnlen (arr, n));
+
+  T (strnlen (ptr, n));
+  T (strnlen (parr, n));
+
+  T (strnlen (p->str, n));
+  T (strnlen (p->arr, n));
+}
+
+
+/* Verify no warnings are issued for raw mempory functions.  */
+
+void test_mem_functions (struct MemArrays *p, int c, size_t n)
+{
+  T (memchr (arr, c, n));
+  T (memchr (parr, c, n));
+  T (memchr (p->arr, c, n));
+  T (memchr (p->parr, c, n));
+
+  T (memcmp (str, arr, n));
+  T (memcmp (arr, str, n));
+  T (memcmp (str, parr, n));
+  T (memcmp (parr, str, n));
+  T (memcmp (p->str, p->arr, n));
+  T (memcmp (p->arr, p->str, n));
+  T (memcmp (p->parr, p->str, n));
+
+  T (memcpy (str, arr, n));
+  T (memcpy (arr, str, n));
+  T (memcpy (str, parr, n));
+  T (memcpy (parr, str, n));
+  T (memcpy (p->str, p->arr, n));
+  T (memcpy (p->arr, p->str, n));
+  T (memcpy (p->parr, p->str, n));
+
+  T (memmove (str, arr, n));
+  T (memmove (arr, str, n));
+  T (memmove (str, parr, n));
+  T (memmove (parr, str, n));
+  T (memmove (p->str, p->arr, n));
+  T (memmove (p->arr, p->str, n));
+  T (memmove (p->parr, p->str, n));
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 2efa182..6fa5beb 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "intl.h"
 #include "attribs.h"
+#include "calls.h"
 
 /* A vector indexed by SSA_NAME_VERSION.  0 means unknown, positive value
    is an index into strinfo vector, negative value stands for
@@ -153,7 +154,7 @@ struct decl_stridxlist_map
 static hash_map<tree_decl_hash, stridxlist> *decl_to_stridxlist_htab;
 
 typedef std::pair<int, location_t> stridx_strlenloc;
-static hash_map<tree, stridx_strlenloc> strlen_to_stridx;
+static hash_map<tree, stridx_strlenloc> *strlen_to_stridx;
 
 /* Obstack for struct stridxlist and struct decl_stridxlist_map.  */
 static struct obstack stridx_obstack;
@@ -1208,7 +1209,7 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
 	    }
 
 	  location_t loc = gimple_location (stmt);
-	  strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc));
+	  strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
 	  return;
 	}
     }
@@ -1254,7 +1255,7 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
       find_equal_ptrs (src, idx);
 
       location_t loc = gimple_location (stmt);
-      strlen_to_stridx.put (lhs, stridx_strlenloc (idx, loc));
+      strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
     }
 }
 
@@ -1733,35 +1734,15 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
     return false;
 
   tree dst = gimple_call_arg (stmt, 0);
-
-  /* See if the destination is declared with attribute "nonstring"
-     and if so, avoid the truncation warning.  */
-  if (TREE_CODE (dst) == SSA_NAME)
-    {
-      if (SSA_NAME_IS_DEFAULT_DEF (dst))
-	dst = SSA_NAME_VAR (dst);
-      else
-	{
-	  gimple *def = SSA_NAME_DEF_STMT (dst);
-
-	  if (is_gimple_assign (def)
-	      && gimple_assign_rhs_code (def) == ADDR_EXPR)
-	    dst = gimple_assign_rhs1 (def);
-	}
-    }
-
   tree dstdecl = dst;
   if (TREE_CODE (dstdecl) == ADDR_EXPR)
     dstdecl = TREE_OPERAND (dstdecl, 0);
 
-  {
-    tree d = dstdecl;
-    if (TREE_CODE (d) == COMPONENT_REF)
-      d = TREE_OPERAND (d, 1);
-
-    if (DECL_P (d) && lookup_attribute ("nonstring", DECL_ATTRIBUTES (d)))
-      return false;
-  }
+  /* If the destination refers to a an array/pointer declared nonstring
+     return early.  */
+  tree ref = NULL_TREE;
+  if (get_attr_nonstring_decl (dstdecl, &ref))
+    return false;
 
   /* Look for dst[i] = '\0'; after the stxncpy() call and if found
      avoid the truncation warning.  */
@@ -1770,12 +1751,32 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
 
   if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
     {
-      HOST_WIDE_INT off;
-      dstdecl = get_addr_base_and_unit_offset (dstdecl, &off);
-
       tree lhs = gimple_assign_lhs (next_stmt);
-      tree lhsbase = get_addr_base_and_unit_offset (lhs, &off);
-      if (lhsbase && operand_equal_p (dstdecl, lhsbase, 0))
+      tree_code code = TREE_CODE (lhs);
+      if (code == ARRAY_REF || code == MEM_REF)
+	lhs = TREE_OPERAND (lhs, 0);
+
+      tree func = gimple_call_fndecl (stmt);
+      if (DECL_FUNCTION_CODE (func) == BUILT_IN_STPNCPY)
+	{
+	  tree ret = gimple_call_lhs (stmt);
+	  if (ret && operand_equal_p (ret, lhs, 0))
+	    return false;
+	}
+
+      /* Determine the base address and offset of the reference,
+	 ignoring the innermost array index.  */
+      if (TREE_CODE (ref) == ARRAY_REF)
+	ref = TREE_OPERAND (ref, 0);
+
+      HOST_WIDE_INT dstoff;
+      tree dstbase = get_addr_base_and_unit_offset (ref, &dstoff);
+
+      HOST_WIDE_INT lhsoff;
+      tree lhsbase = get_addr_base_and_unit_offset (lhs, &lhsoff);
+      if (lhsbase
+	  && dstoff == lhsoff
+	  && operand_equal_p (dstbase, lhsbase, 0))
 	return false;
     }
 
@@ -1919,7 +1920,7 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi)
   /* If the length argument was computed from strlen(S) for some string
      S retrieve the strinfo index for the string (PSS->FIRST) alonng with
      the location of the strlen() call (PSS->SECOND).  */
-  stridx_strlenloc *pss = strlen_to_stridx.get (len);
+  stridx_strlenloc *pss = strlen_to_stridx->get (len);
   if (!pss || pss->first <= 0)
     {
       if (maybe_diag_stxncpy_trunc (*gsi, src, len))
@@ -2967,8 +2968,8 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)
 				  gimple_assign_rhs2 (stmt), stmt);
 
 	tree rhs1 = gimple_assign_rhs1 (stmt);
-	if (stridx_strlenloc *ps = strlen_to_stridx.get (rhs1))
-	  strlen_to_stridx.put (lhs, *ps);
+	if (stridx_strlenloc *ps = strlen_to_stridx->get (rhs1))
+	  strlen_to_stridx->put (lhs, stridx_strlenloc (*ps));
       }
     else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
 	{
@@ -3199,6 +3200,9 @@ public:
 unsigned int
 pass_strlen::execute (function *fun)
 {
+  gcc_assert (!strlen_to_stridx);
+  strlen_to_stridx = new hash_map<tree, stridx_strlenloc> ();
+
   ssa_ver_to_stridx.safe_grow_cleared (num_ssa_names);
   max_stridx = 1;
 
@@ -3220,7 +3224,9 @@ pass_strlen::execute (function *fun)
   laststmt.len = NULL_TREE;
   laststmt.stridx = 0;
 
-  strlen_to_stridx.empty ();
+  strlen_to_stridx->empty ();
+  delete strlen_to_stridx;
+  strlen_to_stridx = NULL;
 
   return 0;
 }



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-17  8:42     ` Jakub Jelinek
@ 2017-11-19 18:29       ` Martin Sebor
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Sebor @ 2017-11-19 18:29 UTC (permalink / raw)
  To: Jakub Jelinek, Jeff Law; +Cc: Gcc Patch List

On 11/17/2017 12:47 AM, Jakub Jelinek wrote:
> On Thu, Nov 16, 2017 at 05:15:20PM -0700, Jeff Law wrote:
>>> +	      tree_code code = gimple_assign_rhs_code (def);
>>> +	      if (code == ADDR_EXPR
>>> +		  || code == COMPONENT_REF
>>> +		  || code == VAR_DECL)
>>> +		dcl = gimple_assign_rhs1 (def);
>> Note that COMPONENT_REFs can have arguments that are themselves
>> COMPONENT_REFS.  So you typically want to use a loop to strip all away.
>
> Do you want to handle just COMPONENT_REF, or other handled components
> (ARRAY_REFs, etc.)?
> We have handled_component_p predicate and get_base_address to strip them
> (or get_inner_reference if you want other details about it).

Thanks.  I'm aware of the function and I considered using it here
in a loop as Jeff suggested and as is done elsewhere but couldn't
come up with a test case to exercise it (I tried a few variations
on the one below).  I don't know if that's because of my lack of
imagination or because these more complex structures end up
simplified by the time they reach the expander.  If you can show
me one where the loop is needed I'll adjust the code.

   struct A { char a[4], b[4] __attribute__ ((nonstring)); };
   struct B { struct A a; };
   struct C { struct B b; };

   void f (struct C *p)
   {
     __builtin_strcpy (p->b.a.a, p->b.a.b);
   }

Thanks
Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-19 17:21     ` Martin Sebor
@ 2017-11-19 22:26       ` Jeff Law
  2017-11-19 22:45         ` Martin Sebor
  0 siblings, 1 reply; 13+ messages in thread
From: Jeff Law @ 2017-11-19 22:26 UTC (permalink / raw)
  To: Martin Sebor, Gcc Patch List, Jakub Jelinek

On 11/19/2017 10:06 AM, Martin Sebor wrote:

>>> +/* If EXPR refers to a character array or pointer declared attribute
>>> +Â Â  nonstring return a decl for that array or pointer and set *REF to
>>> +Â Â  the referenced enclosing object or pointer.Â  Otherwise returns
>>> +Â Â  null.Â  */
>> Generally we'd write NULL rather than null for a generic pointer and in
>> this specific case NULL_TREE is even better.
> 
> I agree.Â  The C NULL macro, C++ nullptr, and GCC's NULL_TREE and
> NULL_RTL are helpful both for type safety and readability of code.
> But they all are implementation details that's ot not relevant to the
> specification
> of an API or its users.
> 
> The established term that every C and C++ programmer is familiar
> with is "null pointer."
> 
> Â and exist
> mainly for type safety.Â  They are not relevant to the specification
> of an API or its users.Â  I didn't think too hard about it when
> I wrote the comment (it just didn't seem important enough) but
> now
> that I have I believe "null" is better, clearer, and more future-
> proof (in case the return type should ever change to some smart
> pointer class).
> 
> (There also comments in this file and elsewhere in GCC that use
> various forms of null.)
Perhaps so, but please capitalize it -- while there may be "null"
references, I believe in all-caps is by far more common within comments
within the GCC sources.

WRT NULL vs NULL_TREE, I'd tend to disagree.  The function isn't going
to return NULL, it's going to return NULL_TREE.  That's actually already
exposed in the API/ABI by the return value's type.  The fact that NULL
and NULL_TREE are interchangeable is an implementation detail and we
shouldn't encourage folks to rely on that by way of the function comment.





> 
>>> +Â  if (TREE_CODE (dcl) == SSA_NAME)
>>> +Â Â Â  {
>>> +Â Â Â Â Â  if (SSA_NAME_IS_DEFAULT_DEF (dcl))
>>> +Â Â Â  dcl = SSA_NAME_VAR (dcl);
>>> +Â Â Â Â Â  else
>>> +Â Â Â  {
>>> +Â Â Â Â Â  gimple *def = SSA_NAME_DEF_STMT (dcl);
>>> +
>>> +Â Â Â Â Â  if (is_gimple_assign (def))
>>> +Â Â Â Â Â Â Â  {
>>> +Â Â Â Â Â Â Â Â Â  tree_code code = gimple_assign_rhs_code (def);
>>> +Â Â Â Â Â Â Â Â Â  if (code == ADDR_EXPR
>>> +Â Â Â Â Â Â Â Â Â  || code == COMPONENT_REF
>>> +Â Â Â Â Â Â Â Â Â  || code == VAR_DECL)
>>> +Â Â Â Â Â Â Â  dcl = gimple_assign_rhs1 (def);
>> Note that COMPONENT_REFs can have arguments that are themselves
>> COMPONENT_REFS.Â  So you typically want to use a loop to strip all away.
> 
> I have seen it in the early IL but this late I couldn't come
> up with a test case where such a loop would be necessary.
> The COMPONENT_REF always refers to the member array.Â  If you
> can show me an example to test with I'll add the handling if
> possible.Â  There are cases where it isn't, such as in
> 
> Â Â Â  struct A { char a[4] __attribute__ ((nonstring)); } *pa;
> 
> Â Â Â  strlen (pa->a + 1);
> 
> where pa->a is represented as a MEM_REF (char[4], pa, 1) with
> the member information having been lost in translation.Â  (This
> is the same problem that I had solved for the -Warray-bounds
> warning for out-of bounds offsets by implementing it in
> tree-ssa-forwprop.c: because that's where the member is folded
> into a reference to the base object.)
Set up a nested structure then reference it like a.b.c.d.


> 
>>> +Â  /* It's safe to call strndup and strnlen with a non-string argument
>>> +Â Â Â Â  since the functions provide an explicit bound for this
>>> purpose.Â  */
>>> +Â  if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_STRNDUP)
>>> +Â Â Â  return;
>> So I see you handle BUILT_IN_STRNDUP here, but not BUILT_IN_STRNLEN.Â  Is
>> there a reason for the omission?
> 
> strnlen is not a GCC built-in so it's handled implicitly by
> the test for DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL
> just before the if statement above.Â  The test I added verifies
> that strnlen is excluded.Â  (Bug 81384 tracks the missing
> intrinsic support for strnlen.)
> 
> That said, more testing revealed that Glibc uses functions like
> strncmp and strncpy to compare and copy non-strings, so I've
> added those to the white list and improved the checking a bit.
> I've also changed the hash_map to a pointer to make Jakub happy.
OK.  Thanks.


> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-19 22:26       ` Jeff Law
@ 2017-11-19 22:45         ` Martin Sebor
  2017-11-21  2:46           ` Jeff Law
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2017-11-19 22:45 UTC (permalink / raw)
  To: Jeff Law, Gcc Patch List, Jakub Jelinek

>> I have seen it in the early IL but this late I couldn't come
>> up with a test case where such a loop would be necessary.
>> The COMPONENT_REF always refers to the member array.  If you
>> can show me an example to test with I'll add the handling if
>> possible.  There are cases where it isn't, such as in
>>
>>     struct A { char a[4] __attribute__ ((nonstring)); } *pa;
>>
>>     strlen (pa->a + 1);
>>
>> where pa->a is represented as a MEM_REF (char[4], pa, 1) with
>> the member information having been lost in translation.  (This
>> is the same problem that I had solved for the -Warray-bounds
>> warning for out-of bounds offsets by implementing it in
>> tree-ssa-forwprop.c: because that's where the member is folded
>> into a reference to the base object.)
> Set up a nested structure then reference it like a.b.c.d.

I did that.  From my reply to Jakub:

   https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01671.html

with

   struct A { char a[4], b[4] __attribute__ ((nonstring)); };
   struct B { struct A a; };
   struct C { struct B b; };

   void f (struct C *p)
   {
     __builtin_strcpy (p->b.a.a, p->b.a.b);
   }

in get_attr_nonstring_decl() where EXPR is p->a.b.b it is
a COMPONENT_REF (COMPONENT_REF (..,), FIELD_DECL (b)).  I.e.,
the outermost FIELD_DECL is the one of interest.

The code extracts TREE_OPERAND (decl, 1) from this outermost
COMPONENT_REF, which is FIELD_DECL (b).

What test case gives a structure where it's necessary to loop
over the components to get at the field?  I can't think of one.

Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] detect nonstring arguments to string functions (PR 82945)
  2017-11-19 22:45         ` Martin Sebor
@ 2017-11-21  2:46           ` Jeff Law
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Law @ 2017-11-21  2:46 UTC (permalink / raw)
  To: Martin Sebor, Gcc Patch List, Jakub Jelinek

On 11/19/2017 03:26 PM, Martin Sebor wrote:
>>> I have seen it in the early IL but this late I couldn't come
>>> up with a test case where such a loop would be necessary.
>>> The COMPONENT_REF always refers to the member array.Â  If you
>>> can show me an example to test with I'll add the handling if
>>> possible.Â  There are cases where it isn't, such as in
>>>
>>> Â Â Â  struct A { char a[4] __attribute__ ((nonstring)); } *pa;
>>>
>>> Â Â Â  strlen (pa->a + 1);
>>>
>>> where pa->a is represented as a MEM_REF (char[4], pa, 1) with
>>> the member information having been lost in translation.Â  (This
>>> is the same problem that I had solved for the -Warray-bounds
>>> warning for out-of bounds offsets by implementing it in
>>> tree-ssa-forwprop.c: because that's where the member is folded
>>> into a reference to the base object.)
>> Set up a nested structure then reference it like a.b.c.d.
> 
> I did that.Â  From my reply to Jakub:
> 
> Â  https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01671.html
> 
> with
> 
> Â  struct A { char a[4], b[4] __attribute__ ((nonstring)); };
> Â  struct B { struct A a; };
> Â  struct C { struct B b; };
> 
> Â  void f (struct C *p)
> Â  {
> Â Â Â  __builtin_strcpy (p->b.a.a, p->b.a.b);
> Â  }
> 
> in get_attr_nonstring_decl() where EXPR is p->a.b.b it is
> a COMPONENT_REF (COMPONENT_REF (..,), FIELD_DECL (b)).Â  I.e.,
> the outermost FIELD_DECL is the one of interest.
> 
> The code extracts TREE_OPERAND (decl, 1) from this outermost
> COMPONENT_REF, which is FIELD_DECL (b).
> 
> What test case gives a structure where it's necessary to loop
> over the components to get at the field?Â  I can't think of one.
Bah.  It's so damn easy to forget that the outermost COMPONENT_REF
refers to the last component in the expression.

Can you incorporate the lazy initialization from Jakub and commit?

Jeff

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-11-21  0:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-13  8:13 [PATCH] detect nonstring arguments to string functions (PR 82945) Martin Sebor
2017-11-13  8:46 ` Bernhard Reutner-Fischer
2017-11-13 20:04 ` Jakub Jelinek
2017-11-14  1:21   ` Martin Sebor
2017-11-14  6:35 ` Martin Sebor
2017-11-17  0:27   ` Jeff Law
2017-11-17  8:42     ` Jakub Jelinek
2017-11-19 18:29       ` Martin Sebor
2017-11-19 17:21     ` Martin Sebor
2017-11-19 22:26       ` Jeff Law
2017-11-19 22:45         ` Martin Sebor
2017-11-21  2:46           ` Jeff Law
2017-11-19 17:26     ` Martin Sebor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).