[clang] 859bf4d - [Coverage] Emit a gap region to cover switch bodies
Vedant Kumar via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 3 12:36:02 PST 2019
Author: Vedant Kumar
Date: 2019-12-03T12:35:54-08:00
New Revision: 859bf4d2bea2404bd2eac92451f2db4371ec6eb4
URL: https://github.com/llvm/llvm-project/commit/859bf4d2bea2404bd2eac92451f2db4371ec6eb4
DIFF: https://github.com/llvm/llvm-project/commit/859bf4d2bea2404bd2eac92451f2db4371ec6eb4.diff
LOG: [Coverage] Emit a gap region to cover switch bodies
Emit a gap region beginning where the switch body begins. This sets line
execution counts in the areas between non-overlapping cases to 0.
This also removes some special handling of the first case in a switch:
these are now treated like any other case.
This does not resolve an outstanding issue with case statement regions
that do not end when a region is terminated. But it should address
llvm.org/PR44011.
Differential Revision: https://reviews.llvm.org/D70571
Added:
Modified:
clang/docs/SourceBasedCodeCoverage.rst
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/switch.cpp
clang/test/CoverageMapping/switchmacro.c
Removed:
################################################################################
diff --git a/clang/docs/SourceBasedCodeCoverage.rst b/clang/docs/SourceBasedCodeCoverage.rst
index 73197a57713f..7e711819be34 100644
--- a/clang/docs/SourceBasedCodeCoverage.rst
+++ b/clang/docs/SourceBasedCodeCoverage.rst
@@ -302,3 +302,37 @@ Drawbacks and limitations
If the call to ``may_throw()`` propagates an exception into ``f``, the code
coverage tool may mark the ``return`` statement as executed even though it is
not. A call to ``longjmp()`` can have similar effects.
+
+Clang implementation details
+============================
+
+This section may be of interest to those wishing to understand or improve
+the clang code coverage implementation.
+
+Gap regions
+-----------
+
+Gap regions are source regions with counts. A reporting tool cannot set a line
+execution count to the count from a gap region unless that region is the only
+one on a line.
+
+Gap regions are used to eliminate unnatural artifacts in coverage reports, such
+as red "unexecuted" highlights present at the end of an otherwise covered line,
+or blue "executed" highlights present at the start of a line that is otherwise
+not executed.
+
+Switch statements
+-----------------
+
+The region mapping for a switch body consists of a gap region that covers the
+entire body (starting from the '{' in 'switch (...) {', and terminating where the
+last case ends). This gap region has a zero count: this causes "gap" areas in
+between case statements, which contain no executable code, to appear uncovered.
+
+When a switch case is visited, the parent region is extended: if the parent
+region has no start location, its start location becomes the start of the case.
+This is used to support switch statements without a ``CompoundStmt`` body, in
+which the switch body and the single case share a count.
+
+For switches with ``CompoundStmt`` bodies, a new region is created at the start
+of each switch case.
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0a7a4fe33ac2..bdecff39c88f 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1114,8 +1114,8 @@ struct CounterCoverageMappingBuilder
// Make a region for the body of the switch. If the body starts with
// a case, that case will reuse this region; otherwise, this covers
// the unreachable code at the beginning of the switch body.
- size_t Index =
- pushRegion(Counter::getZero(), getStart(CS->body_front()));
+ size_t Index = pushRegion(Counter::getZero(), getStart(CS));
+ getRegion().setGap(true);
for (const auto *Child : CS->children())
Visit(Child);
diff --git a/clang/test/CoverageMapping/switch.cpp b/clang/test/CoverageMapping/switch.cpp
index 30c64922201f..25ea4053f4e2 100644
--- a/clang/test/CoverageMapping/switch.cpp
+++ b/clang/test/CoverageMapping/switch.cpp
@@ -2,11 +2,11 @@
// CHECK: foo
void foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0
- switch(i) {
+ switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+4]]:10 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2
return;
case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3
- break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1
+ break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1
}
int x = 0; // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1
}
@@ -29,7 +29,7 @@ void bar(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+20]]:2 = #0
nop();
switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4
- nop(); // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0
+ nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7
nop();
}
@@ -47,7 +47,7 @@ void baz() { // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+5]]:2 = #0
// CHECK-NEXT: main
int main() { // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0
int i = 0;
- switch(i) {
+ switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+8]]:10 = 0
case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2
i = 1;
break;
@@ -58,16 +58,16 @@ int main() { // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0
break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1
}
switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+23]]:2 = #1
- case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #6
- i = 1;
+ case 0: // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+6]]:10 = 0
+ i = 1; // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #6
break;
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7
i = 2;
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = (#7 + #8)
break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+3]]:3 = #5
}
-
- switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+13]]:2 = #5
+ // CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+14]]:2 = #5
+ switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+6]]:11 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:11 = #10
case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:11 = (#10 + #11)
i = 11;
@@ -82,10 +82,23 @@ int main() { // CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0
return 0;
}
+ // CHECK: pr44011
+int pr44011(int i) { // CHECK-NEXT: File 0, [[@LINE]]:20 -> {{.*}}:2 = #0
+ switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:13 = 0
+
+ case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #2
+ return 0;
+
+ default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #3
+ return 1;
+ }
+} // A region for counter #1 is missing due to the missing return.
+
+
// FIXME: End location for "case 1" shouldn't point at the end of the switch.
// CHECK: fallthrough
int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 = #0
- switch(i) {
+ switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+9]]:10 = 0
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+8]]:10 = #2
i = 23;
case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3)
@@ -101,7 +114,7 @@ int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 =
void abort(void) __attribute((noreturn));
// CHECK: noret
int noret(int x) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+9]]:2
- switch (x) {
+ switch (x) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:14 = 0
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:12
abort();
case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13
diff --git a/clang/test/CoverageMapping/switchmacro.c b/clang/test/CoverageMapping/switchmacro.c
index f4c14f798f0b..fc0392fb91e5 100644
--- a/clang/test/CoverageMapping/switchmacro.c
+++ b/clang/test/CoverageMapping/switchmacro.c
@@ -4,7 +4,7 @@
// CHECK: foo
int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0
- switch (i) {
+ switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> {{[0-9]+}}:11 = 0
default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> {{[0-9]+}}:11 = #2
if (i == 1) // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = #2
return 0; // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #3
More information about the cfe-commits
mailing list