Attached is an updated patch with a fix for a bad assumption
exposed by building the linux kernel.
On 02/19/2018 07:50 PM, Martin Sebor wrote:
PR 84468 points out a false positive in -Wstringop-truncation
in code like this:
struct A { char a[4]; };
void f (struct A *p, const struct A *q)
{
if (p->a)
strncpy (p->a, q->a, sizeof p->a - 1); // warning here
p->a[3] = '\0';
}
The warning is due to the code checking only the same basic block
as the one with the strncpy call for an assignment to the destination
to avoid it, but failing to check the successor basic block if there
is no subsequent statement in the current block. (Eliminating
the conditional is being tracked in PR 21474.)
The attached test case adds logic to avoid this false positive.
I don't know under what circumstances there could be more than
one successor block here so I don't handle that case.
Tested on x86_64-linux.
Martin
PR tree-optimization/84468 - bogus -Wstringop-truncation despite assignment after conditional strncpy
gcc/testsuite/ChangeLog:
PR tree-optimization/84468
* gcc.dg/Wstringop-truncation.c: New test.
gcc/ChangeLog:
PR tree-optimization/84468
* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Also consider successor
basic blocks.
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c (revision 257963)
+++ gcc/tree-ssa-strlen.c (working copy)
@@ -1856,8 +1856,20 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi
avoid the truncation warning. */
gsi_next_nondebug (&gsi);
gimple *next_stmt = gsi_stmt (gsi);
+ if (!next_stmt)
+ {
+ /* When there is no statement in the same basic block check
+ the immediate successor block. */
+ if (basic_block bb = gimple_bb (stmt))
+ {
+ basic_block nextbb
+ = EDGE_COUNT (bb->succs) ? EDGE_SUCC (bb, 0)->dest : NULL;
+ gimple_stmt_iterator it = gsi_start_bb (nextbb);
+ next_stmt = gsi_stmt (it);
+ }
+ }
- if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
+ if (next_stmt && is_gimple_assign (next_stmt))
{
tree lhs = gimple_assign_lhs (next_stmt);
tree_code code = TREE_CODE (lhs);
Index: gcc/testsuite/gcc.dg/Wstringop-truncation.c
===================================================================
--- gcc/testsuite/gcc.dg/Wstringop-truncation.c (nonexistent)
+++ gcc/testsuite/gcc.dg/Wstringop-truncation.c (working copy)
@@ -0,0 +1,132 @@
+/* PR tree-optimization/84468 - Inconsistent -Wstringop-truncation warnings
+ with -O2
+ { dg-do compile }
+ { dg-options "-O2 -Wstringop-truncation -ftrack-macro-expansion=0" } */
+
+#define strncpy __builtin_strncpy
+
+struct A
+{
+ char a[4];
+};
+
+void no_pred_succ_lit (struct A *p)
+{
+ /* The following is folded early on, before the strncpy statement
+ has a basic block. Verify that the case is handled gracefully
+ (i.e., there's no assumption that the statement does have
+ a basic block). */
+ strncpy (p->a, "1234", sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */
+}
+
+/* Verify a strncpy call in a basic block with no predecessor or
+ successor. */
+void no_pred_succ (struct A *p, const struct A *q)
+{
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */
+}
+
+
+/* Verify a strncpy call in a basic block with no successor. */
+void no_succ (struct A *p, const struct A *q)
+{
+ if (q->a)
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */
+}
+
+
+/* Verify a strncpy call in a basic block with nul assignment in
+ a successor block. */
+void succ (struct A *p, const struct A *q)
+{
+ /* Verify that the assignment suppresses the warning for the conditional
+ strcnpy call. The conditional should be folded to true since the
+ address of an array can never be null (see bug 84470). */
+ if (q->a)
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+ p->a[sizeof p->a - 1] = 0;
+}
+
+
+void succ_2 (struct A *p, const struct A *q, int i)
+{
+ /* Same as above but with a conditional that cannot be eliminated. */
+ if (i < 0)
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */
+
+ p->a[sizeof p->a - 1] = 0;
+}
+
+
+/* Verify a strncpy call in a basic block with nul assignment in
+ the next successor block. */
+int next_succ (struct A *p, const struct A *q, int i, int j)
+{
+ /* Same as above but with a nested conditionals with else clauses. */
+ if (i < 0)
+ {
+ if (j < 0)
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */
+ }
+ else
+ __builtin_strcpy (p->a, q->a);
+
+ p->a[sizeof p->a - 1] = 0;
+ return 0;
+}
+
+
+int next_succ_1 (struct A *p, const struct A *q, int i, int j)
+{
+ /* Same as above but with a nested conditionals with else clauses. */
+ if (i < 0)
+ {
+ if (j < 0)
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */
+ else
+ strncpy (p->a, q->a, sizeof p->a - 2); /* { dg-bogus "\\\[-Wstringop-truncation" } */
+ }
+
+ p->a[sizeof p->a - 2] = 0;
+ return 1;
+}
+
+
+int next_succ_2 (struct A *p, const struct A *q, int i, int j)
+{
+ /* Same as above but with a nested conditionals with else clauses. */
+ if (i < 0)
+ {
+ if (j < 0)
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-bogus "\\\[-Wstringop-truncation" } */
+ else
+ strncpy (p->a, q->a, sizeof p->a - 2); /* { dg-bogus "\\\[-Wstringop-truncation" } */
+ }
+ else
+ __builtin_strcpy (p->a, q->a);
+
+ p->a[sizeof p->a - 2] = 0;
+ return 2;
+}
+
+
+void cond_succ_warn (struct A *p, const struct A *q, int i)
+{
+ /* Verify that a conditional assignment doesn't suppress the warning. */
+ strncpy (p->a, q->a, sizeof p->a - 1); /* { dg-warning "\\\[-Wstringop-truncation" } */
+
+ if (i < 0)
+ p->a[sizeof p->a - 1] = 0;
+}
+
+void cond_succ_nowarn (struct A *p, const struct A *q)
+{
+ /* Verify that distinct but provably equivalent conditionals are
+ recognized and don't trigger the warning. */
+ if (p != q)
+ strncpy (p->a, q->a, sizeof p->a - 1);
+
+ if (p->a != q->a)
+ p->a[sizeof p->a - 1] = 0;
+}
