From ec505cc7952707db805802af83dd82776a1d949f Mon Sep 17 00:00:00 2001 From: Xionghu Luo Date: Tue, 28 Feb 2023 17:46:18 +0800 Subject: [PATCH v4] gcov: Fix "do-while" structure in case statement leads to incorrect code coverage [PR93680] 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? 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 false 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 | 97 ++++++++++++++++++++- 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, 122 insertions(+), 9 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..7b185200aaf 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); @@ -566,6 +632,9 @@ make_blocks_1 (gimple_seq seq, basic_block bb) codes. */ gimple_set_bb (stmt, bb); + if (!optimize && gimple_code (stmt) == GIMPLE_LABEL) + target_labels.add (gimple_label_label (as_a (stmt))); + /* If STMT is a basic block terminator, set START_NEW_BLOCK for the next iteration. */ if (stmt_ends_bb_p (stmt)) @@ -853,6 +922,12 @@ make_edges_bb (basic_block bb, struct omp_region **pcur_region, int *pomp_index) bool fallthru = false; int ret = 0; + if (!optimize && !last) + { + make_edge (bb, bb->next_bb, EDGE_FALLTHRU); + return 0; + } + if (!last) return ret; @@ -1152,6 +1227,10 @@ same_line_p (location_t locus1, expanded_location *from, location_t locus2) { expanded_location to; + if (LOCATION_LOCUS (locus1) == UNKNOWN_LOCATION + && LOCATION_LOCUS (locus2) == UNKNOWN_LOCATION) + return false; + if (locus1 == locus2) return true; @@ -2832,7 +2911,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 +2940,17 @@ 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)) + && (LOCATION_LOCUS (locus) != UNKNOWN_LOCATION + || LOCATION_LOCUS (prev_locus) != UNKNOWN_LOCATION) + && !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 -- 2.27.0