From patchwork Wed Apr 19 02:52:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xionghu Luo X-Patchwork-Id: 85141 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp55477vqo; Tue, 18 Apr 2023 19:53:47 -0700 (PDT) X-Google-Smtp-Source: AKy350ZwzfSKnBefGdd0fp2vxvkt18Vnhwbo7Zkqhi5zHQDX9HmYm3G0zrSxYGJYIR8xlmkdWWqR X-Received: by 2002:a17:906:1b55:b0:94b:769f:3ba3 with SMTP id p21-20020a1709061b5500b0094b769f3ba3mr697210ejg.8.1681872827479; Tue, 18 Apr 2023 19:53:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681872827; cv=none; d=google.com; s=arc-20160816; b=ginxzs3BMXmCPlVd7jJkVIXhe1mhuFww61OrJ6o0pwFt8oKaWbg+TyakKcva+WbMQs 9JvA+E5WhpBN5MCpWQ/Ups6bKlSi4EnLiflKVB/lE35lRbYNfON4+/YAOh+xWmwh4yFg yYS7EvQKwDik/VBxEs1d48F416K1ekI+Rnwv9aBi2u2ES4Z27tCNnzbKBORiBVt4sICo HfBfmQ1qTPF5xr4xpKxJkLyo6QTpQf1JHdZeY9FajsQ2P1VpTmbdGl3fYiWuh9uE5znd wHklYvQ86iGzvfMwRFJ2Dvif8M0jTNovcHAh3iB/mRxJmMpIcXnZyXCA8+Y53aOQg4tT lYKg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:cc :to:dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=LVSPta1uJ36Dzf5qd59H1X6rfAsPsNtApjyBhME8b74=; b=rwi9XLV2QdE9sdkYNJCZVMUkYCGgxPg+Gwalw0OrsBJZErVhU3bcYFCWUWCEy/xCMC ap6EBzIdCE5yP42VB5sLsh6rEb7EaUS6PSlaPS43XfqEjtPvhHgeBlsqfmmGRiDPxvAy Cyn5LS3uCPFZup+vuXwqNSVm9ZUz9w7hoJoXa1M4R03mHKoNDZi9HZ4dc7c41tmdWRd6 WCuCMN4GIk7Hfw3FFRbNQ/2U8p9pZ0Nvg+uiaHXFxIqjJqKzbDrZGHR0YDR4mFlzpEkR DX38x6ykITELEEtHi0rucV4jJ4X1cBZcJqwRzUCQydWjI9SkSOJlw/hEY2MkZ1Cpl4Tk SYyg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=Y9PCteHG; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id wi14-20020a170906fd4e00b0095342e3e76asi802329ejb.33.2023.04.18.19.53.47 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Apr 2023 19:53:47 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=Y9PCteHG; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 063393858425 for ; Wed, 19 Apr 2023 02:53:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 063393858425 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1681872826; bh=LVSPta1uJ36Dzf5qd59H1X6rfAsPsNtApjyBhME8b74=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=Y9PCteHG5m/1XU4+Hf1UtPsr9D3klQB/xr7ScGAHUm06cCpqcNHx2kBZ0z/e8OQ0i 2r11iIC3p4O0NyJFb8tqq/qEbcwTBE7kHaag2VIJOYBvY8naE5+ts8HpcQfy1/nbJh p8wYU+vpXBUpgF7BavAwHP8VTGe18PxSAKMMuQeI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from VM-122-127-centos.localdomain (unknown [43.132.141.4]) by sourceware.org (Postfix) with ESMTPS id 9D4D03858D1E; Wed, 19 Apr 2023 02:52:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9D4D03858D1E Received: by VM-122-127-centos.localdomain (Postfix, from userid 1009) id 29EE04138E; Wed, 19 Apr 2023 10:52:57 +0800 (CST) To: gcc-patches@gcc.gnu.org Cc: luoxhu@gcc.gnu.org, rguenther@suse.de, Xionghu Luo Subject: [PATCH v5] gcov: Fix "do-while" structure in case statement leads to incorrect code coverage [PR93680] Date: Wed, 19 Apr 2023 10:52:14 +0800 Message-Id: <20230419025214.149675-1-xionghuluo@tencent.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NO_DNS_FOR_FROM, RCVD_IN_PBL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Xionghu Luo via Gcc-patches From: Xionghu Luo Reply-To: Xionghu Luo Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1763571482159015059?= X-GMAIL-MSGID: =?utf-8?q?1763571482159015059?= v5: Refine patch and send this for gcc14 stage1. v4: Address comments. 4.1. Handle GIMPLE_GOTO and GIMPLE_ASM. 4.2. Fix failure of limit-caselabels.c (labels on same line), pointer_array_1.f90 (unused labels) etc. v3: Add compute_target_labels and call it in the front of make_blocks_1. v2: Check whether two locus are on same line. Start a new basic block if two labels have different location when test-coverage. Regression tested pass on x86_64-linux-gnu and aarch64-linux-gnu, OK for master? 2023-03-22 Xionghu Luo Richard Biener gcc/ChangeLog: PR gcov/93680 * tree-cfg.cc (stmt_starts_bb_p): Check whether the label is in target_labels. (compute_target_labels): New function. (make_blocks_1): Call compute_target_labels. (same_line_p): Return true if two locus are both UNKOWN_LOCATION. gcc/testsuite/ChangeLog: PR gcov/93680 * g++.dg/gcov/gcov-1.C: Correct counts. * gcc.misc-tests/gcov-4.c: Likewise. * gcc.misc-tests/gcov-pr85332.c: Likewise. * lib/gcov.exp: Also clean gcda if fail. * gcc.misc-tests/gcov-pr93680.c: New test. Signed-off-by: Xionghu Luo --- gcc/tree-cfg.cc | 245 +++++++++++++------- gcc/testsuite/g++.dg/gcov/gcov-1.C | 2 +- gcc/testsuite/gcc.misc-tests/gcov-4.c | 2 +- gcc/testsuite/gcc.misc-tests/gcov-pr85332.c | 2 +- gcc/testsuite/gcc.misc-tests/gcov-pr93680.c | 24 ++ gcc/testsuite/lib/gcov.exp | 4 +- 6 files changed, 189 insertions(+), 90 deletions(-) create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr93680.c diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc index a9fcc7fd050..9dca30af397 100644 --- a/gcc/tree-cfg.cc +++ b/gcc/tree-cfg.cc @@ -164,7 +164,7 @@ static edge gimple_redirect_edge_and_branch (edge, basic_block); static edge gimple_try_redirect_by_replacing_jump (edge, basic_block); /* Various helpers. */ -static inline bool stmt_starts_bb_p (gimple *, gimple *); +static inline bool stmt_starts_bb_p (gimple *, gimple *, hash_set *); static int gimple_verify_flow_info (void); static void gimple_make_forwarder_block (edge); static gimple *first_non_label_stmt (basic_block); @@ -521,6 +521,68 @@ gimple_call_initialize_ctrl_altering (gimple *stmt) gimple_call_set_ctrl_altering (stmt, false); } +/* Compute target labels to save useful labels. */ +static void +compute_target_labels (gimple_seq seq, hash_set *target_labels) +{ + gimple *stmt = NULL; + gimple_stmt_iterator j = gsi_start (seq); + + while (!gsi_end_p (j)) + { + stmt = gsi_stmt (j); + + switch (gimple_code (stmt)) + { + case GIMPLE_COND: + { + gcond *cstmt = as_a (stmt); + tree true_label = gimple_cond_true_label (cstmt); + tree false_label = gimple_cond_false_label (cstmt); + target_labels->add (true_label); + target_labels->add (false_label); + } + break; + case GIMPLE_SWITCH: + { + gswitch *gstmt = as_a (stmt); + size_t i, n = gimple_switch_num_labels (gstmt); + tree elt, label; + for (i = 0; i < n; i++) + { + elt = gimple_switch_label (gstmt, i); + label = CASE_LABEL (elt); + target_labels->add (label); + } + } + break; + case GIMPLE_GOTO: + if (!computed_goto_p (stmt)) + { + tree dest = gimple_goto_dest (stmt); + target_labels->add (dest); + } + break; + case GIMPLE_ASM: + { + gasm *asm_stmt = as_a (stmt); + int i, n = gimple_asm_nlabels (asm_stmt); + for (i = 0; i < n; ++i) + { + tree cons = gimple_asm_label_op (asm_stmt, i); + target_labels->add (cons); + } + } + break; + + default: + break; + } + + gsi_next (&j); + } +} + /* Insert SEQ after BB and build a flowgraph. */ @@ -532,6 +594,10 @@ make_blocks_1 (gimple_seq seq, basic_block bb) gimple *prev_stmt = NULL; bool start_new_block = true; bool first_stmt_of_seq = true; + hash_set target_labels; + + if (!optimize) + compute_target_labels (seq, &target_labels); while (!gsi_end_p (i)) { @@ -553,7 +619,7 @@ make_blocks_1 (gimple_seq seq, basic_block bb) /* If the statement starts a new basic block or if we have determined in a previous pass that we need to create a new block for STMT, do so now. */ - if (start_new_block || stmt_starts_bb_p (stmt, prev_stmt)) + if (start_new_block || stmt_starts_bb_p (stmt, prev_stmt, &target_labels)) { if (!first_stmt_of_seq) gsi_split_seq_before (&i, &seq); @@ -854,102 +920,102 @@ make_edges_bb (basic_block bb, struct omp_region **pcur_region, int *pomp_index) int ret = 0; if (!last) - return ret; - - switch (gimple_code (last)) + fallthru = true; + else + switch (gimple_code (last)) { - case GIMPLE_GOTO: - if (make_goto_expr_edges (bb)) - ret = 1; - fallthru = false; - break; - case GIMPLE_RETURN: - { - edge e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0); - e->goto_locus = gimple_location (last); + case GIMPLE_GOTO: + if (make_goto_expr_edges (bb)) + ret = 1; fallthru = false; - } - break; - case GIMPLE_COND: - make_cond_expr_edges (bb); - fallthru = false; - break; - case GIMPLE_SWITCH: - make_gimple_switch_edges (as_a (last), bb); - fallthru = false; - break; - case GIMPLE_RESX: - make_eh_edges (last); - fallthru = false; - break; - case GIMPLE_EH_DISPATCH: - fallthru = make_eh_dispatch_edges (as_a (last)); - break; + break; + case GIMPLE_RETURN: + { + edge e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0); + e->goto_locus = gimple_location (last); + fallthru = false; + } + break; + case GIMPLE_COND: + make_cond_expr_edges (bb); + fallthru = false; + break; + case GIMPLE_SWITCH: + make_gimple_switch_edges (as_a (last), bb); + fallthru = false; + break; + case GIMPLE_RESX: + make_eh_edges (last); + fallthru = false; + break; + case GIMPLE_EH_DISPATCH: + fallthru = make_eh_dispatch_edges (as_a (last)); + break; - case GIMPLE_CALL: - /* If this function receives a nonlocal goto, then we need to - make edges from this call site to all the nonlocal goto - handlers. */ - if (stmt_can_make_abnormal_goto (last)) - ret = 2; + case GIMPLE_CALL: + /* If this function receives a nonlocal goto, then we need to + make edges from this call site to all the nonlocal goto + handlers. */ + if (stmt_can_make_abnormal_goto (last)) + ret = 2; - /* If this statement has reachable exception handlers, then - create abnormal edges to them. */ - make_eh_edges (last); + /* If this statement has reachable exception handlers, then + create abnormal edges to them. */ + make_eh_edges (last); - /* BUILTIN_RETURN is really a return statement. */ - if (gimple_call_builtin_p (last, BUILT_IN_RETURN)) + /* BUILTIN_RETURN is really a return statement. */ + if (gimple_call_builtin_p (last, BUILT_IN_RETURN)) { make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0); fallthru = false; } - /* Some calls are known not to return. */ - else - fallthru = !gimple_call_noreturn_p (last); - break; + /* Some calls are known not to return. */ + else + fallthru = !gimple_call_noreturn_p (last); + break; - case GIMPLE_ASSIGN: - /* A GIMPLE_ASSIGN may throw internally and thus be considered - control-altering. */ - if (is_ctrl_altering_stmt (last)) - make_eh_edges (last); - fallthru = true; - break; + case GIMPLE_ASSIGN: + /* A GIMPLE_ASSIGN may throw internally and thus be considered + control-altering. */ + if (is_ctrl_altering_stmt (last)) + make_eh_edges (last); + fallthru = true; + break; - case GIMPLE_ASM: - make_gimple_asm_edges (bb); - fallthru = true; - break; + case GIMPLE_ASM: + make_gimple_asm_edges (bb); + fallthru = true; + break; - CASE_GIMPLE_OMP: - fallthru = omp_make_gimple_edges (bb, pcur_region, pomp_index); - break; + CASE_GIMPLE_OMP: + fallthru = omp_make_gimple_edges (bb, pcur_region, pomp_index); + break; - case GIMPLE_TRANSACTION: - { - gtransaction *txn = as_a (last); - tree label1 = gimple_transaction_label_norm (txn); - tree label2 = gimple_transaction_label_uninst (txn); + case GIMPLE_TRANSACTION: + { + gtransaction *txn = as_a (last); + tree label1 = gimple_transaction_label_norm (txn); + tree label2 = gimple_transaction_label_uninst (txn); - if (label1) - make_edge (bb, label_to_block (cfun, label1), EDGE_FALLTHRU); - if (label2) - make_edge (bb, label_to_block (cfun, label2), - EDGE_TM_UNINSTRUMENTED | (label1 ? 0 : EDGE_FALLTHRU)); + if (label1) + make_edge (bb, label_to_block (cfun, label1), EDGE_FALLTHRU); + if (label2) + make_edge (bb, label_to_block (cfun, label2), + EDGE_TM_UNINSTRUMENTED | (label1 ? 0 : EDGE_FALLTHRU)); - tree label3 = gimple_transaction_label_over (txn); - if (gimple_transaction_subcode (txn) - & (GTMA_HAVE_ABORT | GTMA_IS_OUTER)) - make_edge (bb, label_to_block (cfun, label3), EDGE_TM_ABORT); + tree label3 = gimple_transaction_label_over (txn); + if (gimple_transaction_subcode (txn) + & (GTMA_HAVE_ABORT | GTMA_IS_OUTER)) + make_edge (bb, label_to_block (cfun, label3), EDGE_TM_ABORT); - fallthru = false; - } - break; + fallthru = false; + } + break; - default: - gcc_assert (!stmt_ends_bb_p (last)); - fallthru = true; - break; + default: + gcc_assert (!stmt_ends_bb_p (last)); + fallthru = true; + break; } if (fallthru) @@ -1145,14 +1211,15 @@ next_discriminator_for_locus (int line) return (*slot)->discriminator; } -/* Return TRUE if LOCUS1 and LOCUS2 refer to the same source line. */ +/* Return TRUE if LOCUS1 and LOCUS2 refer to the same source line. Treat two + unknown locations as the same line. */ static bool same_line_p (location_t locus1, expanded_location *from, location_t locus2) { expanded_location to; - if (locus1 == locus2) + if (LOCATION_LOCUS (locus1) == LOCATION_LOCUS (locus2)) return true; to = expand_location (locus2); @@ -2832,7 +2899,8 @@ simple_goto_p (gimple *t) label. */ static inline bool -stmt_starts_bb_p (gimple *stmt, gimple *prev_stmt) +stmt_starts_bb_p (gimple *stmt, gimple *prev_stmt, + hash_set *target_labels) { if (stmt == NULL) return false; @@ -2860,6 +2928,15 @@ stmt_starts_bb_p (gimple *stmt, gimple *prev_stmt) || !DECL_ARTIFICIAL (gimple_label_label (plabel))) return true; + location_t prev_locus = gimple_location (plabel); + location_t locus = gimple_location (label_stmt); + expanded_location locus_e = expand_location (locus); + + if (!optimize + && target_labels->contains (gimple_label_label (label_stmt)) + && !same_line_p (locus, &locus_e, prev_locus)) + return true; + cfg_stats.num_merged_labels++; return false; } diff --git a/gcc/testsuite/g++.dg/gcov/gcov-1.C b/gcc/testsuite/g++.dg/gcov/gcov-1.C index ee383b480a8..01e7084fb03 100644 --- a/gcc/testsuite/g++.dg/gcov/gcov-1.C +++ b/gcc/testsuite/g++.dg/gcov/gcov-1.C @@ -263,7 +263,7 @@ test_switch (int i, int j) case 2: result = do_something (1024); break; - case 3: /* count(3) */ + case 3: /* count(2) */ case 4: /* branch(67) */ if (j == 2) /* count(3) */ diff --git a/gcc/testsuite/gcc.misc-tests/gcov-4.c b/gcc/testsuite/gcc.misc-tests/gcov-4.c index da7929ef7fc..792cda8cfce 100644 --- a/gcc/testsuite/gcc.misc-tests/gcov-4.c +++ b/gcc/testsuite/gcc.misc-tests/gcov-4.c @@ -122,7 +122,7 @@ top: } else { -else_: /* count(1) */ +else_: /* count(2) */ j = do_something (j); /* count(2) */ if (j) /* count(2) */ { diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr85332.c b/gcc/testsuite/gcc.misc-tests/gcov-pr85332.c index 73e50b19fc7..b37e760910c 100644 --- a/gcc/testsuite/gcc.misc-tests/gcov-pr85332.c +++ b/gcc/testsuite/gcc.misc-tests/gcov-pr85332.c @@ -7,7 +7,7 @@ int doit(int sel, int n, void *p) switch (sel) { - case 0: /* count(3) */ + case 0: /* count(1) */ do {*p0 += *p0;} while (--n); /* count(3) */ return *p0 == 0; /* count(1) */ diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr93680.c b/gcc/testsuite/gcc.misc-tests/gcov-pr93680.c new file mode 100644 index 00000000000..2fe340c4011 --- /dev/null +++ b/gcc/testsuite/gcc.misc-tests/gcov-pr93680.c @@ -0,0 +1,24 @@ +/* { dg-options "-fprofile-arcs -ftest-coverage" } */ +/* { dg-do run { target native } } */ + +int f(int s, int n) +{ + int p = 0; + + switch (s) + { + case 0: /* count(1) */ + do { p++; } while (--n); /* count(5) */ + return p; /* count(1) */ + + case 1: /* count(1) */ + do { p++; } while (--n); /* count(5) */ + return p; /* count(1) */ + } + + return 0; +} + +int main() { f(0, 5); f(1, 5); return 0; } + +/* { dg-final { run-gcov gcov-pr93680.c } } */ diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp index 80e74aeb220..07e1978d25d 100644 --- a/gcc/testsuite/lib/gcov.exp +++ b/gcc/testsuite/lib/gcov.exp @@ -424,9 +424,7 @@ proc run-gcov { args } { } if { $tfailed > 0 } { fail "$testname gcov: $lfailed failures in line counts, $bfailed in branch percentages, $cfailed in return percentages, $ifailed in intermediate format" - if { $xfailed } { - clean-gcov $testcase - } + clean-gcov $testcase } else { pass "$testname gcov" clean-gcov $testcase