Ignore JaCoCo instrumentation when analyzing assertions enabling code
Bug: 169866678
Bug: 170197605
Change-Id: I13f97aed2bb801c5d7ff104bdef4b187fb297462
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java
index d60a589..eb82383 100644
--- a/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java
+++ b/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java
@@ -4,12 +4,14 @@
package com.android.tools.r8.graph.analysis;
+import com.android.tools.r8.cf.code.CfArrayStore;
import com.android.tools.r8.cf.code.CfConstNumber;
import com.android.tools.r8.cf.code.CfFieldInstruction;
import com.android.tools.r8.cf.code.CfGoto;
import com.android.tools.r8.cf.code.CfIf;
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfInvoke;
+import com.android.tools.r8.cf.code.CfLoad;
import com.android.tools.r8.cf.code.CfLogicalBinop;
import com.android.tools.r8.graph.CfCode;
import com.android.tools.r8.graph.Code;
@@ -90,6 +92,15 @@
// 16: invokevirtual #43 // Method java/lang/Class.desiredAssertionStatus:()Z
// 19: putstatic #45 // Field ENABLED:Z
// 22: return
+ //
+ // When JaCoCo instrumentation is applied the following instruction sequence is injected into
+ // each branch to register code coverage. This includes <clinit> where the vsalue for field
+ // $assertionsDisabled is determined by a branch structure.
+ //
+ // +0: aload_X
+ // +1: iconst_Y / ldc
+ // +2: iconst_Z / ldc
+ // +3 bastore
private static List<Class<?>> javacInstructionSequence =
ImmutableList.of(
@@ -100,6 +111,8 @@
CfFieldInstruction.class);
private static List<Class<?>> r8InstructionSequence =
ImmutableList.of(CfConstNumber.class, CfLogicalBinop.class, CfFieldInstruction.class);
+ private static List<Class<?>> jacocoInstructionSequence =
+ ImmutableList.of(CfLoad.class, CfConstNumber.class, CfConstNumber.class, CfArrayStore.class);
private boolean hasJavacClinitAssertionCode(CfCode code) {
for (int i = 0; i < code.getInstructions().size(); i++) {
@@ -160,6 +173,17 @@
return false;
}
+ private boolean skipSequence(List<Class<?>> sequence, CfCode code, int fromIndex) {
+ int i = fromIndex;
+ for (; i < code.getInstructions().size() && i - fromIndex < sequence.size(); i++) {
+ CfInstruction instruction = code.getInstructions().get(i);
+ if (instruction.getClass() != sequence.get(i - fromIndex)) {
+ return false;
+ }
+ }
+ return i - fromIndex == sequence.size();
+ }
+
private CfFieldInstruction isJavacInstructionSequence(CfCode code, int fromIndex) {
List<Class<?>> sequence = javacInstructionSequence;
int nextExpectedInstructionIndex = 0;
@@ -168,12 +192,27 @@
i < code.getInstructions().size() && nextExpectedInstructionIndex < sequence.size();
i++) {
instruction = code.getInstructions().get(i);
- if (instruction.isLabel() || instruction.isFrame()) {
- // Just ignore labels and frames.
+ if (instruction.isLabel() || instruction.isFrame() || instruction.isPosition()) {
+ // Just ignore labels, frames and positions.
continue;
}
if (instruction.getClass() != sequence.get(nextExpectedInstructionIndex)) {
- break;
+ // Skip injected JaCoCo injected code.
+ if (instruction.getClass() == jacocoInstructionSequence.get(0)
+ && skipSequence(jacocoInstructionSequence, code, i)) {
+ i += jacocoInstructionSequence.size();
+ if (i >= code.getInstructions().size()) {
+ break;
+ }
+ instruction = code.getInstructions().get(i);
+ }
+ if (instruction.isLabel() || instruction.isFrame() || instruction.isPosition()) {
+ // Just ignore labels, frames and positions.
+ continue;
+ }
+ if (instruction.getClass() != sequence.get(nextExpectedInstructionIndex)) {
+ break;
+ }
}
nextExpectedInstructionIndex++;
}
diff --git a/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsCheckerUtils.java b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsCheckerUtils.java
new file mode 100644
index 0000000..c9236d4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsCheckerUtils.java
@@ -0,0 +1,33 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.rewrite.assertions;
+
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.Matchers;
+import org.hamcrest.MatcherAssert;
+import org.junit.Assert;
+
+public class AssertionsCheckerUtils {
+
+ static void checkAssertionCodeEnabled(ClassSubject subject, String methodName) {
+ MatcherAssert.assertThat(subject, Matchers.isPresent());
+ // <clinit> is removed by R8.
+ if (subject.uniqueMethodWithName("<clinit>").isPresent()) {
+ Assert.assertFalse(
+ subject
+ .uniqueMethodWithName("<clinit>")
+ .streamInstructions()
+ .anyMatch(InstructionSubject::isStaticPut));
+ }
+ Assert.assertTrue(
+ subject
+ .uniqueMethodWithName(methodName)
+ .streamInstructions()
+ .anyMatch(InstructionSubject::isThrow));
+ }
+
+ private AssertionsCheckerUtils() {}
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationJacocoTest.java b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationJacocoTest.java
index 159ee81..11829f6 100644
--- a/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationJacocoTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationJacocoTest.java
@@ -16,7 +16,7 @@
import com.android.tools.r8.references.Reference;
import com.android.tools.r8.rewrite.assertions.testclasses.A;
import com.android.tools.r8.utils.DescriptorUtils;
-import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
@@ -41,6 +41,10 @@
this.parameters = parameters;
}
+ private void checkAssertionCodeEnabled(CodeInspector inspector) {
+ AssertionsCheckerUtils.checkAssertionCodeEnabled(inspector.clazz(A.class), "m");
+ }
+
@Test
public void testD8() throws Exception {
Assume.assumeTrue(parameters.isDexRuntime());
@@ -49,10 +53,9 @@
.addProgramClassFileData(transformClassWithJacocoInstrumentation(A.class))
.setMinApi(parameters.getApiLevel())
.addAssertionsConfiguration(AssertionsConfiguration.Builder::enableAllAssertions)
- .compile()
.run(parameters.getRuntime(), TestClass.class)
- // TODO(b/169866678): Should be "AssertionError in A".
- .assertSuccessWithOutput(StringUtils.lines());
+ .inspect(this::checkAssertionCodeEnabled)
+ .assertSuccessWithOutputLines("AssertionError in A");
}
@Test
@@ -64,10 +67,9 @@
.addKeepClassAndMembersRules(A.class, MockJacocoInit.class)
.setMinApi(parameters.getApiLevel())
.addAssertionsConfiguration(AssertionsConfiguration.Builder::enableAllAssertions)
- .compile()
.run(parameters.getRuntime(), TestClass.class)
- // TODO(b/169866678): Should be "AssertionError in A".
- .assertSuccessWithOutput(StringUtils.lines());
+ .inspect(this::checkAssertionCodeEnabled)
+ .assertSuccessWithOutputLines("AssertionError in A");
}
private byte[] transformClassWithJacocoInstrumentation(Class<?> clazz) throws IOException {
diff --git a/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationTest.java b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationTest.java
index 01c28c1..064cfdb 100644
--- a/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/assertions/AssertionsConfigurationTest.java
@@ -143,25 +143,8 @@
checkAssertionCodeRemoved(inspector.clazz(clazz));
}
- private void checkAssertionCodeEnabled(ClassSubject subject) {
- assertThat(subject, isPresent());
- // <clinit> is removed by R8.
- if (subject.uniqueMethodWithName("<clinit>").isPresent()) {
- assertFalse(
- subject
- .uniqueMethodWithName("<clinit>")
- .streamInstructions()
- .anyMatch(InstructionSubject::isStaticPut));
- }
- assertTrue(
- subject
- .uniqueMethodWithName("m")
- .streamInstructions()
- .anyMatch(InstructionSubject::isThrow));
- }
-
private void checkAssertionCodeEnabled(CodeInspector inspector, Class<?> clazz) {
- checkAssertionCodeEnabled(inspector.clazz(clazz));
+ AssertionsCheckerUtils.checkAssertionCodeEnabled(inspector.clazz(clazz), "m");
}
private void checkAssertionCodeLeft(CodeInspector inspector, Class<?> clazz) {
@@ -421,8 +404,8 @@
.compile()
.inspect(
inspector -> {
- checkAssertionCodeEnabled(inspector.clazz("Class1"));
- checkAssertionCodeEnabled(inspector.clazz("Class2"));
+ AssertionsCheckerUtils.checkAssertionCodeEnabled(inspector.clazz("Class1"), "m");
+ AssertionsCheckerUtils.checkAssertionCodeEnabled(inspector.clazz("Class2"), "m");
checkAssertionCodeRemoved(inspector.clazz(class1));
checkAssertionCodeRemoved(inspector.clazz(class2));
})
@@ -472,8 +455,10 @@
.compile()
.inspect(
inspector -> {
- checkAssertionCodeEnabled(inspector.clazz(TestClassForInnerClass.class));
- checkAssertionCodeEnabled(inspector.clazz(TestClassForInnerClass.InnerClass.class));
+ AssertionsCheckerUtils.checkAssertionCodeEnabled(
+ inspector.clazz(TestClassForInnerClass.class), "m");
+ AssertionsCheckerUtils.checkAssertionCodeEnabled(
+ inspector.clazz(TestClassForInnerClass.InnerClass.class), "m");
})
.run(parameters.getRuntime(), TestClassForInnerClass.class)
.assertSuccessWithOutputLines(
