From 065e2e8e76c5bf21bd797f3c8baebe909e85a6cf Mon Sep 17 00:00:00 2001 From: Charles Mita Date: Tue, 7 Sep 2021 06:38:32 -0700 Subject: [PATCH] Apply Jacoco's instruction filtering to Java branch coverage Jacoco applies a set of filters during its coverage analysis to remove various compiler constructs that could lead to surprising outputs in coverage results, leaving behind coverage data that closer fits the code given to the compiler. A simple java example is the function: ``` public int foo(String input) { switch (input) { case "A": return 1; case "B": return 2; case "C": return 3; default: return -1 } } ``` This would generate at least double the number of expected branches as the compiler generates a second set of branches to operate on hashCode. Jacoco would normally ignore the first set of branches, reducing the number to the expected 4. Because Bazel applies its own branch analysis step, Bazel's branch coverage output does not benefit from this filtering. This change adapts the custom analyzer to record the necessary information during the ASM Tree walk in the same manner as Jacoco's regular analyzer in order to apply the filters. The filters have three output operations: * ignore - ignore a range of instructions * merge - merges the coverage information of two instructions * replaceBranches - retarget the output branches of an instruction The first of these, ignore, is by far the simplest to implement and covers the vast majority of use cases in Java. By mapping the AbstractInsnNode objects recorded during the ASM Tree walk to the Instruction objects used during branch analysis, we can simply skip over Instruction objects that have had their associated AbstractInsnNode object ignored by a filter. The remaining operations, merge and replaceBranches, are left unimplemeted for now; these require more care to handle, are harder to test, and affect only a minority of cases, specifically: * merge is only used to handle duplication of finally blocks * replaceBranches is used by Kotlin and Eclipse compiler filters Part of #12696 Closes #13896. PiperOrigin-RevId: 395235432 --- .../coverage/BranchDetailAnalyzer.java | 2 +- .../testing/coverage/ClassProbesMapper.java | 87 +++++++++++++++++- .../testing/coverage/MethodProbesMapper.java | 66 +++++++++++++- src/test/shell/bazel/BUILD | 2 + .../shell/bazel/bazel_coverage_java_test.sh | 91 +++++++++++++++++++ 5 files changed, 243 insertions(+), 5 deletions(-) diff --git a/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchDetailAnalyzer.java b/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchDetailAnalyzer.java index f62de9889d515d..85efed5fd1f66d 100644 --- a/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchDetailAnalyzer.java +++ b/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchDetailAnalyzer.java @@ -114,7 +114,7 @@ private IOException analyzerError(final String location, final Exception cause) // Generate the line to probeExp map so that we can evaluate the coverage. private Map mapProbes(final ClassReader reader) { - final ClassProbesMapper mapper = new ClassProbesMapper(); + final ClassProbesMapper mapper = new ClassProbesMapper(reader.getClassName()); final ClassProbesAdapter adapter = new ClassProbesAdapter(mapper, false); reader.accept(adapter, 0); diff --git a/src/java_tools/junitrunner/java/com/google/testing/coverage/ClassProbesMapper.java b/src/java_tools/junitrunner/java/com/google/testing/coverage/ClassProbesMapper.java index f6827810bbcc01..0633e1acf23794 100644 --- a/src/java_tools/junitrunner/java/com/google/testing/coverage/ClassProbesMapper.java +++ b/src/java_tools/junitrunner/java/com/google/testing/coverage/ClassProbesMapper.java @@ -14,30 +14,81 @@ package com.google.testing.coverage; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.TreeMap; +import org.jacoco.core.internal.analysis.StringPool; +import org.jacoco.core.internal.analysis.filter.Filters; +import org.jacoco.core.internal.analysis.filter.IFilter; +import org.jacoco.core.internal.analysis.filter.IFilterContext; import org.jacoco.core.internal.flow.ClassProbesVisitor; import org.jacoco.core.internal.flow.MethodProbesVisitor; +import org.objectweb.asm.AnnotationVisitor; +import org.objectweb.asm.Attribute; import org.objectweb.asm.FieldVisitor; /** A visitor that maps each source code line to the probes corresponding to the lines. */ -public class ClassProbesMapper extends ClassProbesVisitor { +public class ClassProbesMapper extends ClassProbesVisitor implements IFilterContext { private Map classLineToBranchExp; + private IFilter allFilters = Filters.all(); + + private StringPool stringPool; + + // IFilterContext state updating during visitations + private String className; + private String superClassName; + private Set classAnnotations = new HashSet<>(); + private Set classAttributes = new HashSet<>(); + private String sourceFileName; + private String sourceDebugExtension; + public Map result() { return classLineToBranchExp; } /** Create a new probe mapper object. */ - public ClassProbesMapper() { + public ClassProbesMapper(String className) { classLineToBranchExp = new TreeMap(); + stringPool = new StringPool(); + className = stringPool.get(className); + } + + @Override + public AnnotationVisitor visitAnnotation(final String desc, final boolean visible) { + classAnnotations.add(desc); + return super.visitAnnotation(desc, visible); + } + + @Override + public void visitAttribute(final Attribute attribute) { + classAttributes.add(attribute.type); + } + + @Override + public void visitSource(final String source, final String debug) { + sourceFileName = stringPool.get(source); + sourceDebugExtension = debug; + } + + @Override + public void visit( + int version, + int access, + String name, + String signature, + String superName, + String[] interfaces) { + superClassName = stringPool.get(name); } /** Returns a visitor for mapping method code. */ @Override public MethodProbesVisitor visitMethod( int access, String name, String desc, String signature, String[] exceptions) { - return new MethodProbesMapper() { + return new MethodProbesMapper(this, allFilters) { + @Override public void visitEnd() { super.visitEnd(); @@ -56,4 +107,34 @@ public FieldVisitor visitField( public void visitTotalProbeCount(int count) { // Nothing to do. Maybe perform some checks here. } + + @Override + public String getClassName() { + return className; + } + + @Override + public String getSuperClassName() { + return superClassName; + } + + @Override + public String getSourceDebugExtension() { + return sourceDebugExtension; + } + + @Override + public String getSourceFileName() { + return sourceFileName; + } + + @Override + public Set getClassAnnotations() { + return classAnnotations; + } + + @Override + public Set getClassAttributes() { + return classAttributes; + } } diff --git a/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java b/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java index e1f00dadef78c8..530a6189e41578 100644 --- a/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java +++ b/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java @@ -16,22 +16,30 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; import org.jacoco.core.internal.analysis.Instruction; +import org.jacoco.core.internal.analysis.filter.IFilter; +import org.jacoco.core.internal.analysis.filter.IFilterContext; +import org.jacoco.core.internal.analysis.filter.IFilterOutput; import org.jacoco.core.internal.flow.IFrame; import org.jacoco.core.internal.flow.LabelInfo; import org.jacoco.core.internal.flow.MethodProbesVisitor; import org.objectweb.asm.Handle; import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.MethodNode; /** * The mapper is a probes visitor that will cache control flow information as well as keeping track * of the probes as the main driver generates the probe ids. Upon finishing the method it uses the * information collected to generate the mapping information between probes and the instructions. */ -public class MethodProbesMapper extends MethodProbesVisitor { +public class MethodProbesMapper extends MethodProbesVisitor implements IFilterOutput { /* * The implementation roughly follows the same pattern of the Analyzer class of Jacoco. * @@ -57,6 +65,13 @@ public class MethodProbesMapper extends MethodProbesVisitor { private Instruction lastInstruction = null; private int currentLine = -1; private List