This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] avoid bogus -Wstringop-overflow for strncpy with _FORTIFY_SOURCE (PR 82646)
- From: Martin Sebor <msebor at gmail dot com>
- To: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 5 Dec 2017 16:47:26 -0700
- Subject: [PATCH] avoid bogus -Wstringop-overflow for strncpy with _FORTIFY_SOURCE (PR 82646)
- Authentication-results: sourceware.org; auth=none
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=]" } */
+}