From patchwork Tue Mar 29 16:13:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 1610727 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=t7S1W2qv; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4KSZQX4x9Rz9sCq for ; Wed, 30 Mar 2022 03:14:43 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 18108385E447 for ; Tue, 29 Mar 2022 16:14:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 18108385E447 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1648570481; bh=2cr8q6Ok+tvET94fNvTPaesHq6sPjoiznjc2yvnoSuc=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=t7S1W2qvp+we6sXGALra1T6aW+7NtU7EA8iieJrmsAiAQcjrqiMSX4OUqpNLqOVYC 3Pl8TVUEK5RcFLsZ01WT0LQvMqs0zt+QlFrH+7cDiLQ13+pG4fZ2Lc3mk/YsO6MMfc i7BLY+fKYEXAL6OZqXfKL2wovqIUJ/iPYKqmJ9NE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0EDAE3858C50 for ; Tue, 29 Mar 2022 16:13:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0EDAE3858C50 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-173-NYUwQVqFOny4-9o4wjB4_Q-1; Tue, 29 Mar 2022 12:13:56 -0400 X-MC-Unique: NYUwQVqFOny4-9o4wjB4_Q-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B47273802AE2 for ; Tue, 29 Mar 2022 16:13:55 +0000 (UTC) Received: from pdp-11.redhat.com (unknown [10.22.10.220]) by smtp.corp.redhat.com (Postfix) with ESMTP id 92728401E26; Tue, 29 Mar 2022 16:13:55 +0000 (UTC) To: GCC Patches Subject: [PATCH] gimple: Wrong -Wimplicit-fallthrough with if(1) [PR103597] Date: Tue, 29 Mar 2022 12:13:51 -0400 Message-Id: <20220329161351.41059-1-polacek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Marek Polacek via Gcc-patches From: Marek Polacek Reply-To: Marek Polacek Cc: Jakub Jelinek Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" 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 Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? 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. --- gcc/gimplify.cc | 54 ++++++- .../c-c++-common/Wimplicit-fallthrough-39.c | 140 ++++++++++++++++++ gcc/tree.h | 6 + 3 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wimplicit-fallthrough-39.c base-commit: 1dca4ca1bf2f1b05537a1052e373d8b0ff11e53c diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc index f62f150fc08..2588824dce2 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 00000000000..da4aef3a318 --- /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 2fedcf08b3d..cea49a500f2 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.