diff options
author | Marek Polacek <polacek@redhat.com> | 2022-03-28 18:19:20 -0400 |
---|---|---|
committer | Marek Polacek <polacek@redhat.com> | 2022-03-29 14:10:37 -0400 |
commit | d886a5248e66ab911391af18bf955beb87ee8461 (patch) | |
tree | f47537490d8af1be79da3e2789de16eb383f891e /gcc/gimplify.cc | |
parent | 89976d082488b3a7dc7520b980f854ce83043d38 (diff) | |
download | gcc-d886a5248e66ab911391af18bf955beb87ee8461.zip gcc-d886a5248e66ab911391af18bf955beb87ee8461.tar.gz gcc-d886a5248e66ab911391af18bf955beb87ee8461.tar.bz2 |
gimple: Wrong -Wimplicit-fallthrough with if(1) [PR103597]
This patch fixes a wrong -Wimplicit-fallthrough warning for
case 0:
if (1) // wrong may fallthrough
return 0;
case 1:
which in .gimple looks like
<D.1981>: // case 0
if (1 != 0) goto <D.1985>; else goto <D.1986>;
<D.1985>:
D.1987 = 0;
// predicted unlikely by early return (on trees) predictor.
return D.1987;
<D.1986>: // dead
<D.1982>: // case 1
and the warning thinks that <D.1986>: falls through to <D.1982>:. It
does not know that <D.1986> is effectively a dead label, only reachable
through fallthrough from previous instructions, never jumped to. To
that effect, Jakub introduced UNUSED_LABEL_P, which is set on such dead
labels.
collect_fallthrough_labels has code to deal with cases like
case 2:
if (e != 10)
i++; // this may fallthru, warn
else
return 44;
case 3:
which collects labels that may fall through. Here it sees the "goto <D.1990>;"
at the end of the then branch and so when the warning reaches
...
<D.1990>: // from if-then
<D.1984>: // case 3
it knows it should warn about the possible fallthrough. But an UNUSED_LABEL_P
is not a label that can fallthrough like that, so it should ignore those.
However, we still want to warn about this:
case 0:
if (1)
n++; // falls through
case 1:
so collect_fallthrough_labels needs to return the "n = n + 1;" statement, rather
than the dead label.
Co-authored-by: Jakub Jelinek <jakub@redhat.com>
PR middle-end/103597
gcc/ChangeLog:
* gimplify.cc (collect_fallthrough_labels): Don't push UNUSED_LABEL_Ps
into labels. Maybe set prev to the statement preceding UNUSED_LABEL_P.
(gimplify_cond_expr): Set UNUSED_LABEL_P.
* tree.h (UNUSED_LABEL_P): New.
gcc/testsuite/ChangeLog:
* c-c++-common/Wimplicit-fallthrough-39.c: New test.
Diffstat (limited to 'gcc/gimplify.cc')
-rw-r--r-- | gcc/gimplify.cc | 54 |
1 files changed, 48 insertions, 6 deletions
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index f62f150..2588824 100644 --- a/gcc/gimplify.cc +++ b/gcc/gimplify.cc @@ -2250,9 +2250,9 @@ last_stmt_in_scope (gimple *stmt) } } -/* Collect interesting labels in LABELS and return the statement preceding - another case label, or a user-defined label. Store a location useful - to give warnings at *PREVLOC (usually the location of the returned +/* Collect labels that may fall through into LABELS and return the statement + preceding another case label, or a user-defined label. Store a location + useful to give warnings at *PREVLOC (usually the location of the returned statement or of its surrounding scope). */ static gimple * @@ -2331,8 +2331,12 @@ collect_fallthrough_labels (gimple_stmt_iterator *gsi_p, if (gsi_end_p (*gsi_p)) break; - struct label_entry l = { false_lab, if_loc }; - labels->safe_push (l); + /* A dead label can't fall through. */ + if (!UNUSED_LABEL_P (false_lab)) + { + struct label_entry l = { false_lab, if_loc }; + labels->safe_push (l); + } /* Go to the last statement of the then branch. */ gsi_prev (gsi_p); @@ -2359,6 +2363,17 @@ collect_fallthrough_labels (gimple_stmt_iterator *gsi_p, labels->safe_push (l); } } + /* This case is about + if (1 != 0) goto <D.2022>; else goto <D.2023>; + <D.2022>: + n = n + 1; // #1 + <D.2023>: // #2 + <D.1988>: // #3 + where #2 is UNUSED_LABEL_P and we want to warn about #1 falling + through to #3. So set PREV to #1. */ + else if (UNUSED_LABEL_P (false_lab)) + prev = gsi_stmt (*gsi_p); + /* And move back. */ gsi_next (gsi_p); } @@ -4461,9 +4476,19 @@ gimplify_cond_expr (tree *expr_p, gimple_seq *pre_p, fallback_t fallback) if (TREE_OPERAND (expr, 1) == NULL_TREE && !have_else_clause_p && TREE_OPERAND (expr, 2) != NULL_TREE) - label_cont = label_true; + { + /* For if (0) {} else { code; } tell -Wimplicit-fallthrough + handling that label_cont == label_true can be only reached + through fallthrough from { code; }. */ + if (integer_zerop (COND_EXPR_COND (expr))) + UNUSED_LABEL_P (label_true) = 1; + label_cont = label_true; + } else { + bool then_side_effects + = (TREE_OPERAND (expr, 1) + && TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 1))); gimplify_seq_add_stmt (&seq, gimple_build_label (label_true)); have_then_clause_p = gimplify_stmt (&TREE_OPERAND (expr, 1), &seq); /* For if (...) { code; } else {} or @@ -4477,6 +4502,16 @@ gimplify_cond_expr (tree *expr_p, gimple_seq *pre_p, fallback_t fallback) gimple *g; label_cont = create_artificial_label (UNKNOWN_LOCATION); + /* For if (0) { non-side-effect-code } else { code } + tell -Wimplicit-fallthrough handling that label_cont can + be only reached through fallthrough from { code }. */ + if (integer_zerop (COND_EXPR_COND (expr))) + { + UNUSED_LABEL_P (label_true) = 1; + if (!then_side_effects) + UNUSED_LABEL_P (label_cont) = 1; + } + g = gimple_build_goto (label_cont); /* GIMPLE_COND's are very low level; they have embedded @@ -4493,6 +4528,13 @@ gimplify_cond_expr (tree *expr_p, gimple_seq *pre_p, fallback_t fallback) } if (!have_else_clause_p) { + /* For if (1) { code } or if (1) { code } else { non-side-effect-code } + tell -Wimplicit-fallthrough handling that label_false can be only + reached through fallthrough from { code }. */ + if (integer_nonzerop (COND_EXPR_COND (expr)) + && (TREE_OPERAND (expr, 2) == NULL_TREE + || !TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 2)))) + UNUSED_LABEL_P (label_false) = 1; gimplify_seq_add_stmt (&seq, gimple_build_label (label_false)); have_else_clause_p = gimplify_stmt (&TREE_OPERAND (expr, 2), &seq); } |