From: Marek Polacek Date: Mon, 28 Mar 2022 22:19:20 +0000 (-0400) Subject: gimple: Wrong -Wimplicit-fallthrough with if(1) [PR103597] X-Git-Tag: upstream/12.2.0~798 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d886a5248e66ab911391af18bf955beb87ee8461;p=platform%2Fupstream%2Fgcc.git 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 : // case 0 if (1 != 0) goto ; else goto ; : D.1987 = 0; // predicted unlikely by early return (on trees) predictor. return D.1987; : // dead : // case 1 and the warning thinks that : falls through to :. It does not know that 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 ;" at the end of the then branch and so when the warning reaches ... : // from if-then : // 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 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. --- 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 ; else goto ; + : + n = n + 1; // #1 + : // #2 + : // #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); } diff --git a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-39.c b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-39.c new file mode 100644 index 0000000..da4aef3 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-39.c @@ -0,0 +1,140 @@ +/* PR middle-end/103597 */ +/* { dg-do compile } */ +/* { dg-options "-Wimplicit-fallthrough" } */ + +#define E(c, e) if (c) e + +int +fn0 (int n) +{ + switch (n) + { + case 0: + E (1, return 0); + case 1: + return -1; + } + return 0; +} + +int +fn1 (int n) +{ + switch (n) + { + case 0: + E (1, goto out); + case 1: + return -1; + } +out: + return 0; +} + +int +fn2 (int n) +{ + switch (n) + { + case 0: + if (1) /* { dg-warning "statement may fall through" "" { target c++ } } */ + n++; /* { dg-warning "statement may fall through" "" { target c } } */ + case 1: /* { dg-message "here" } */ + return -1; + } + return 0; +} + +int +fn3 (int n) +{ + switch (n) + { + case 0: + if (0) /* { dg-warning "statement may fall through" } */ + return 0; + case 1: /* { dg-message "here" } */ + return -1; + } + return 0; +} + +int +fn4 (int n) +{ + switch (n) + { + case 0: + E (0, n++); + --n; /* { dg-warning "statement may fall through" } */ + case 1: /* { dg-message "here" } */ + return -1; + } + return 0; +} + +int +fn5 (int n) +{ + switch (n) + { + case 0: + if (1) + return 0; + else + return -1; + case 1: + return -1; + } + return 0; +} + +int +fn6 (int n) +{ + switch (n) + { + case 0: + if (1) + return 0; + else + { +meow: + n--; /* { dg-warning "statement may fall through" } */ + } + case 1: /* { dg-message "here" } */ + return -1; + case 2: + goto meow; + } + return 0; +} + +int +fn7 (int n) +{ + switch (n) + { + case 0: + if (1) + return 0; +woof: + case 1: + return -1; + } + return 0; +} + +int +fn8 (int n) +{ + switch (n) + { + case 0: + if (1) n++; /* { dg-warning "statement may fall through" } */ +woof: /* { dg-message "here" } */ + case 1: + return -1; + } + return 0; +} diff --git a/gcc/tree.h b/gcc/tree.h index 2fedcf0..cea49a5 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -787,6 +787,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int, #define SWITCH_BREAK_LABEL_P(NODE) \ (LABEL_DECL_CHECK (NODE)->base.protected_flag) +/* Set on label that is known not to be jumped to, it can be only + reached by falling through from previous statements. + This is used to implement -Wimplicit-fallthrough. */ +#define UNUSED_LABEL_P(NODE) \ + (LABEL_DECL_CHECK (NODE)->base.default_def_flag) + /* Nonzero means this expression is volatile in the C sense: its address should be of type `volatile WHATEVER *'. In other words, the declared item is volatile qualified.