This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664)


The -Wstringop-truncation handling for strncpy/stpncpy neglects
to consider that character arrays tracked by the strlen pass
are not necessarily nul-terminated.  It unconditionally adds
one when computing the size of each sequence to account for
the nul.  This leads to false positive warnings when checking
the validity of indices/pointers computed by the built-ins.

The attached patch corrects this by adding one for the nul
only when the character array is known to be nul-terminated.

Since GCC 7 does not issue the warning this is a 8/9 regression
that I would like to fix in both releases.  Is the patch okay
for trunk/gcc-8-branch?

Tested on x86_64-linux.

Martin
PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic on strncpy

gcc/ChangeLog:

	PR tree-optimization/89644
	* tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated
	arrays in determining sequence sizes in strncpy and stpncpy.

gcc/testsuite/ChangeLog:

	PR tree-optimization/89644
	* gcc.dg/Wstringop-truncation-8.c: New test.

diff --git a/gcc/testsuite/gcc.dg/Wstringop-truncation-8.c b/gcc/testsuite/gcc.dg/Wstringop-truncation-8.c
new file mode 100644
index 00000000000..1745da50a37
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-truncation-8.c
@@ -0,0 +1,94 @@
+/* PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic
+   on strncpy
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" }  */
+
+#define NONSTR __attribute__ ((nonstring))
+
+typedef __SIZE_TYPE__ size_t;
+
+size_t strlen (const char*);
+extern char* stpncpy (char*, const char*, size_t);
+extern char* strncpy (char*, const char*, size_t);
+
+void sink (char*, ...);
+
+char f0 (char *s)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  if (*s)
+    strncpy (a, s, sizeof a);       /* { dg-bogus "\\\[-Warray-bounds" } */
+  return a[0];
+}
+
+void f1 (char *s)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  if (*s)
+    strncpy (a, s, sizeof a);       /* { dg-bogus "\\\[-Warray-bounds" } */
+  sink (a);
+}
+
+char f2 (void)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 };
+  strncpy (a, b + 1, 5);            /* { dg-bogus "\\\[-Warray-bounds" } */
+  return a[0];
+}
+
+void f3 (void)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 };
+  strncpy (a, b + 2, 4);            /* { dg-bogus "\\\[-Warray-bounds" } */
+  sink (a);
+}
+
+void f4 (NONSTR char *d)
+{
+  char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 };
+  strncpy (d, b + 3, 3);            /* { dg-bogus "\\\[-Warray-bounds" } */
+  sink (d);
+}
+
+
+char g0 (char *s)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  if (*s)
+    stpncpy (a, s, sizeof a);       /* { dg-bogus "\\\[-Warray-bounds" } */
+  return a[0];
+}
+
+void g1 (char *s)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  char *p = 0;
+  if (*s)
+    p = stpncpy (a, s, sizeof a);   /* { dg-bogus "\\\[-Warray-bounds" } */
+  sink (a, p);
+}
+
+char g2 (void)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 };
+  stpncpy (a, b + 1, 5);            /* { dg-bogus "\\\[-Warray-bounds" } */
+  return a[0];
+}
+
+void g3 (void)
+{
+  char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 };
+  char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 };
+  char *p = stpncpy (a, b + 2, 4);  /* { dg-bogus "\\\[-Warray-bounds" } */
+  sink (a, p);
+}
+
+void g4 (NONSTR char *d)
+{
+  char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 };
+  char *p = stpncpy (d, b + 3, 3);  /* { dg-bogus "\\\[-Warray-bounds" } */
+  sink (d, p);
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 721832e3f19..1969c728dc7 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -2191,13 +2191,21 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi)
   int didx = get_stridx (dst);
   if (strinfo *sidst = didx > 0 ? get_strinfo (didx) : NULL)
     {
-      /* Compute the size of the destination string including the NUL.  */
+      /* Compute the size of the destination string including the nul
+	 if it is known to be nul-terminated.  */
       if (sidst->nonzero_chars)
 	{
-	  tree type = TREE_TYPE (sidst->nonzero_chars);
-	  dstsize = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars,
-				 build_int_cst (type, 1));
+	  if (sidst->endptr)
+	    {
+	      /* String is known to be nul-terminated.  */
+	      tree type = TREE_TYPE (sidst->nonzero_chars);
+	      dstsize = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars,
+				     build_int_cst (type, 1));
+	    }
+	  else
+	    dstsize = sidst->nonzero_chars;
 	}
+
       dst = sidst->ptr;
     }
 
@@ -2209,12 +2217,18 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi)
 	 over the terminating nul so SISRC->DONT_INVALIDATE must be left
 	 clear.  */
 
-      /* Compute the size of the source string including the NUL.  */
+      /* Compute the size of the source string including the terminating
+	 nul if its known to be nul-terminated.  */
       if (sisrc->nonzero_chars)
 	{
-	  tree type = TREE_TYPE (sisrc->nonzero_chars);
-	  srcsize = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars,
-				 build_int_cst (type, 1));
+	  if (sisrc->endptr)
+	    {
+	      tree type = TREE_TYPE (sisrc->nonzero_chars);
+	      srcsize = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars,
+				     build_int_cst (type, 1));
+	    }
+	  else
+	    srcsize = sisrc->nonzero_chars;
 	}
 
 	src = sisrc->ptr;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]