The warning suppression for -Wstringop-truncation looks for the next statement after a truncating strncpy to see if it adds a terminating nul. This only works when the next statement can be reached using the Gimple statement iterator which isn't until after gimplification. As a result, strncpy calls that truncate their constant argument that are being folded to memcpy this early get diagnosed even if they are followed by the nul assignment:
const char s[] = "12345"; char d[3]; void f (void) { strncpy (d, s, sizeof d - 1); // -Wstringop-truncation d[sizeof d - 1] = 0; } To avoid the warning I propose to defer folding strncpy to memcpy until the pointer to the basic block the strnpy call is in can be used to try to reach the next statement (this happens as early as ccp1). I'm aware of the preference to fold things early but in the case of strncpy (a relatively rarely used function that is often misused), getting the warning right while folding a bit later but still fairly early on seems like a reasonable compromise. I fear that otherwise, the false positives will drive users to adopt other unsafe solutions (like memcpy) where these kinds of bugs cannot be as readily detected. Tested on x86_64-linux. Martin PS There still are outstanding cases where the warning can be avoided. I xfailed them in the test for now but will still try to get them to work for GCC 9.
PR tree-optimization/87028 - false positive -Wstringop-truncation strncpy with global variable source string gcc/ChangeLog: PR tree-optimization/87028 * gimple-fold.c (gimple_fold_builtin_strncpy): Avoid folding when statement doesn't belong to a basic block. * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Handle MEM_REF on the left hand side of assignment. gcc/testsuite/ChangeLog: PR tree-optimization/87028 * c-c++-common/Wstringop-truncation.c: Remove xfails. * gcc.dg/Wstringop-truncation-5.c: New test. diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 07341eb..284c2fb 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -1702,6 +1702,11 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, if (tree_int_cst_lt (ssize, len)) return false; + /* Defer warning (and folding) until the next statement in the basic + block is reachable. */ + if (!gimple_bb (stmt)) + return false; + /* Diagnose truncation that leaves the copy unterminated. */ maybe_diag_stxncpy_trunc (*gsi, src, len); diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation.c b/gcc/testsuite/c-c++-common/Wstringop-truncation.c index e78e85e..4ddb9bd 100644 --- a/gcc/testsuite/c-c++-common/Wstringop-truncation.c +++ b/gcc/testsuite/c-c++-common/Wstringop-truncation.c @@ -329,9 +329,8 @@ void test_strncpy_array (Dest *pd, int i, const char* s) of the array to NUL is not diagnosed. */ { /* This might be better written using memcpy() but it's safe so - it probably shouldn't be diagnosed. It currently triggers - a warning because of bug 81704. */ - strncpy (dst7, "0123456", sizeof dst7); /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */ + it isn't diagnosed. See pr81704 and pr87028. */ + strncpy (dst7, "0123456", sizeof dst7); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ dst7[sizeof dst7 - 1] = '\0'; sink (dst7); } @@ -350,7 +349,7 @@ void test_strncpy_array (Dest *pd, int i, const char* s) } { - strncpy (pd->a5, "01234", sizeof pd->a5); /* { dg-bogus "\\\[-Wstringop-truncation]" "bug 81704" { xfail *-*-* } } */ + strncpy (pd->a5, "01234", sizeof pd->a5); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ pd->a5[sizeof pd->a5 - 1] = '\0'; sink (pd); } diff --git a/gcc/testsuite/gcc.dg/Wstringop-truncation-5.c b/gcc/testsuite/gcc.dg/Wstringop-truncation-5.c new file mode 100644 index 0000000..03bcba9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wstringop-truncation-5.c @@ -0,0 +1,111 @@ +/* PR tree-optimization/87028 - false positive -Wstringop-truncation + strncpy with global variable source string + { dg-do compile } + { dg-options "-O2 -Wstringop-truncation" } */ + +char *strncpy (char *, const char *, __SIZE_TYPE__); + +void sink (char*); + +#define STR "1234567890" + +struct S +{ + char a[5], b[5], *p; +}; + +const char arr[] = STR; +const char arr2[][10] = { "123", STR }; + +const char* const ptr = STR; + +char a[5]; + +void test_literal (char *d, struct S *s) +{ + strncpy (d, STR, 3); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + d[3] = '\0'; + + strncpy (a, STR, sizeof a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + a[sizeof a - 1] = '\0'; + + strncpy (s->a, STR, sizeof s->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + s->a[sizeof s->a - 1] = '\0'; + + strncpy (&s->b[0], STR, sizeof s->b - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + s->b[sizeof s->b - 1] = '\0'; + + strncpy (s->p, STR, 4); /* { dg-bogus "\\\[-Wstringop-truncation]" "pr?????" { xfail *-*-* } } */ + s->p[4] = '\0'; +} + +void test_global_arr (char *d, struct S *s) +{ + strncpy (d, arr, 4); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + d[4] = '\0'; + + strncpy (a, arr, sizeof a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + a[sizeof a - 1] = '\0'; + + strncpy (s->a, arr, sizeof s->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + s->a[sizeof s->a - 1] = '\0'; + + strncpy (s->p, arr, 5); /* { dg-bogus "\\\[-Wstringop-truncation]" "pr?????" { xfail *-*-* } } */ + s->p[5] = '\0'; +} + +void test_global_arr2 (char *d, struct S *s) +{ + strncpy (a, arr2[1], sizeof a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + a[sizeof a - 1] = '\0'; + + strncpy (s->a, arr2[1], sizeof s->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + s->a[sizeof s->a - 1] = '\0'; + + strncpy (s->b, arr2[0], sizeof s->a - 1); +} + +void test_global_ptr (struct S *s) +{ + strncpy (a, ptr, sizeof a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + a[sizeof a - 1] = '\0'; + + strncpy (s->a, ptr, sizeof s->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + s->a[sizeof s->a - 1] = '\0'; + + strncpy (s->p, ptr, 6); /* { dg-bogus "\\\[-Wstringop-truncation]" "pr?????" { xfail *-*-* } } */ + s->p[6] = '\0'; +} + +void test_local_arr (struct S *s) +{ + const char arr[] = STR; + + strncpy (a, arr, sizeof a - 1); + a[sizeof a - 1] = '\0'; + + strncpy (s->a, arr, sizeof s->a - 1); + s->a[sizeof s->a - 1] = '\0'; + + char d[3]; + strncpy (d, arr, sizeof d - 1); + d[sizeof d - 1] = '\0'; + sink (d); +} + +void test_local_ptr (struct S *s) +{ + const char* const ptr = STR; + + strncpy (a, ptr, sizeof a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + a[sizeof a - 1] = '\0'; + + strncpy (s->a, ptr, sizeof s->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */ + s->a[sizeof s->a - 1] = '\0'; +} + +void test_compound_literal (struct S *s) +{ + strncpy (s->a, (char[]){ STR }, sizeof s->a - 1); + s->a[sizeof s->a - 1] = '\0'; +} diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index d0792aa..f1988f6 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -1981,6 +1981,23 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) && known_eq (dstoff, lhsoff) && operand_equal_p (dstbase, lhsbase, 0)) return false; + + if (code == MEM_REF + && TREE_CODE (lhsbase) == SSA_NAME + && known_eq (dstoff, lhsoff)) + { + /* Extract the referenced variable from something like + MEM[(char *)d_3(D) + 3B] = 0; */ + gimple *def = SSA_NAME_DEF_STMT (lhsbase); + if (gimple_nop_p (def)) + { + lhsbase = SSA_NAME_VAR (lhsbase); + if (lhsbase + && dstbase + && operand_equal_p (dstbase, lhsbase, 0)) + return false; + } + } } int prec = TYPE_PRECISION (TREE_TYPE (cnt));