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