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 bogus -Wstringop-overflow for strncpy with _FORTIFY_SOURCE (PR 82646)


PR middle-end/82646 - bogus -Wstringop-overflow with
-D_FORTIFY_SOURCE=2 on strncpy with range to a member array,

The bug points out a false positive in a call to strncpy() when
_FORTIFY_SOURCE is defined that doesn't exist otherwise.

The problem is that __builtin_strncpy buffer overflow checking
is done along with the expansion of the intrinsic in one place
and __builtin___strncpy_chk is handled differently in another,
and the two are out of sync.

The attached patch corrects the choice of arguments used for
overflow detection in __builtin___strncpy_chk and aligns
the diagnostics between the two intrinsics.

Martin
PR tree-optimization/82646 - bogus -Wstringop-overflow with -D_FORTIFY_SOURCE=2 on strncpy with range to a member array

gcc/ChangeLog:

	PR tree-optimization/82646
	* builtins.c (maybe_emit_chk_warning): Use size as the bound for
	strncpy, not maxlen.

gcc/testsuite/ChangeLog:

	PR tree-optimization/82646
	* gcc.dg/builtin-stringop-chk-1.c: Adjust.
	* gcc.dg/builtin-stringop-chk-9.c: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 097e1b7..6b25253 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -9861,6 +9861,8 @@ maybe_emit_chk_warning (tree exp, enum built_in_function fcode)
      (such as __strncat_chk) or null if the operation isn't bounded
      (such as __strcat_chk).  */
   tree maxlen = NULL_TREE;
+  /* The exact size of the access (such as in __strncpy_chk).  */
+  tree size = NULL_TREE;
 
   switch (fcode)
     {
@@ -9888,7 +9890,7 @@ maybe_emit_chk_warning (tree exp, enum built_in_function fcode)
     case BUILT_IN_STRNCPY_CHK:
     case BUILT_IN_STPNCPY_CHK:
       srcstr = CALL_EXPR_ARG (exp, 1);
-      maxlen = CALL_EXPR_ARG (exp, 2);
+      size = CALL_EXPR_ARG (exp, 2);
       objsize = CALL_EXPR_ARG (exp, 3);
       break;
 
@@ -9911,7 +9913,7 @@ maybe_emit_chk_warning (tree exp, enum built_in_function fcode)
     }
 
   check_sizes (OPT_Wstringop_overflow_, exp,
-	       /*size=*/NULL_TREE, maxlen, srcstr, objsize);
+	       size, maxlen, srcstr, objsize);
 }
 
 /* Emit warning if a buffer overflow is detected at compile time
diff --git a/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c b/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
index 35cc6dc..10048f3 100644
--- a/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-stringop-chk-1.c
@@ -36,7 +36,7 @@ test (int arg, ...)
   vx = stpcpy (&buf2[18], "a");
   vx = stpcpy (&buf2[18], "ab"); /* { dg-warning "writing 3" "stpcpy" } */
   strncpy (&buf2[18], "a", 2);
-  strncpy (&buf2[18], "a", 3); /* { dg-warning "specified bound 3 exceeds destination size 2" "strncpy" } */
+  strncpy (&buf2[18], "a", 3); /* { dg-warning "writing 3 bytes into a region of size 2" "strncpy" } */
   strncpy (&buf2[18], "abc", 2);
   strncpy (&buf2[18], "abc", 3); /* { dg-warning "writing 3 " "strncpy" } */
   memset (buf2, '\0', sizeof (buf2));
@@ -93,7 +93,7 @@ void
 test2 (const H h)
 {
   char c;
-  strncpy (&c, str, 3); /* { dg-warning "specified bound 3 exceeds destination size 1" "strncpy" } */
+  strncpy (&c, str, 3); /* { dg-warning "writing 3 bytes into a region of size 1" "strncpy" } */
 
   struct { char b[4]; } x;
   sprintf (x.b, "%s", "ABCD"); /* { dg-warning "writing 5" "sprintf" } */
diff --git a/gcc/testsuite/gcc.dg/builtin-stringop-chk-9.c b/gcc/testsuite/gcc.dg/builtin-stringop-chk-9.c
new file mode 100644
index 0000000..b5464c2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtin-stringop-chk-9.c
@@ -0,0 +1,150 @@
+/* PR middle-end/82646 - bogus -Wstringop-overflow with -D_FORTIFY_SOURCE=2
+   on strncpy with range to a member array
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-overflow -ftrack-macro-expansion=0" } */
+
+#define bos(p)   __builtin_object_size (p, 1)
+
+struct S {
+  char a[5];
+  void (*pf)(void);
+};
+
+/* Verify that none of the string function calls below triggers a warning.  */
+
+char* test_stpncpy_const_nowarn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  return __builtin_stpncpy (p->a, "123456", n);
+}
+
+char* test_strncpy_const_nowarn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  return __builtin_strncpy (p->a, "1234567", n);
+}
+
+char* test_stpncpy_chk_const_nowarn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  return __builtin___stpncpy_chk (p->a, "12345678", n, bos (p->a));
+}
+
+char* test_strncpy_chk_const_nowarn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  return __builtin___strncpy_chk (p->a, "123456789", n, bos (p->a));
+}
+
+
+char* test_stpncpy_range_nowarn (struct S *p, int n)
+{
+  if (n < sizeof p->a)
+    n = sizeof p->a;
+
+  return __builtin_stpncpy (p->a, "123456", n);
+}
+
+char* test_strncpy_range_nowarn (struct S *p, int n)
+{
+  if (n < sizeof p->a)
+    n = sizeof p->a;
+
+  return __builtin_strncpy (p->a, "1234567", n);
+}
+
+char* test_stpncpy_chk_range_nowarn (struct S *p, int n)
+{
+  if (n < sizeof p->a)
+    n = sizeof p->a;
+
+  return __builtin___stpncpy_chk (p->a, "12345678", n, bos (p->a));   /* { dg-bogus "\\\[-Wstringop-overflow=]" } */
+}
+
+char* test_strncpy_chk_range_nowarn (struct S *p, int n)
+{
+  if (n < sizeof p->a)
+    n = sizeof p->a;
+
+  return __builtin___strncpy_chk (p->a, "123456789", n, bos (p->a));  /* { dg-bogus "\\\[-Wstringop-overflow=]" } */
+}
+
+
+/* Verify that all of the string function calls below trigger a warning.  */
+
+char* test_stpncpy_const_warn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  ++n;
+
+  return __builtin_stpncpy (p->a, "123456", n);                       /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}
+
+char* test_strncpy_const_warn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  /* A call to strncpy() with a known string and small bound is folded
+     into memcpy() which defeats the warning in this case since memcpy
+     uses Object Size Type 0, i.e., the largest object that p->a may
+     be a part of.  Use a larger bound to get around this here.  */
+  n += 11;
+
+  return __builtin_strncpy (p->a, "1234567", n);                      /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}
+
+char* test_stpncpy_chk_const_warn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  ++n;
+
+  return __builtin___stpncpy_chk (p->a, "12345678", n, bos (p->a));   /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}
+
+char* test_strncpy_chk_const_warn (struct S *p)
+{
+  int n = sizeof p->a;
+
+  ++n;
+
+  return __builtin___strncpy_chk (p->a, "123456789", n, bos (p->a));  /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}
+
+
+char* test_stpncpy_range_warn (struct S *p, int n)
+{
+  if (n < sizeof p->a + 1)
+    n = sizeof p->a + 1;
+
+  return __builtin_stpncpy (p->a, "123456", n);                       /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}
+
+char* test_strncpy_range_warn (struct S *p, int n)
+{
+  if (n < sizeof p->a + 1)
+    n = sizeof p->a + 1;
+
+  return __builtin_strncpy (p->a, "1234567", n);                      /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}
+
+char* test_stpncpy_chk_range_warn (struct S *p, int n)
+{
+  if (n < sizeof p->a + 1)
+    n = sizeof p->a + 1;
+
+  return __builtin___stpncpy_chk (p->a, "12345678", n, bos (p->a));   /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}
+
+char* test_strncpy_chk_range_warn (struct S *p, int n)
+{
+  if (n < sizeof p->a + 1)
+    n = sizeof p->a + 1;
+
+  return __builtin___strncpy_chk (p->a, "123456789", n, bos (p->a));  /* { dg-warning "\\\[-Wstringop-overflow=]" } */
+}

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