Merge "Ignore monitor exit for field load elimination"
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 805bdf7..fd547cf 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -235,7 +235,7 @@
System.setOut(new PrintStream(ByteStreams.nullOutputStream()));
}
// TODO(b/65390962): Remove this warning once the CF backend is complete.
- if (options.isGeneratingClassFiles()) {
+ if (options.isGeneratingClassFiles() && !options.testing.suppressExperimentalCfBackendWarning) {
options.reporter.warning(new StringDiagnostic(
"R8 support for generating Java classfiles is incomplete and experimental. "
+ "Even if R8 appears to succeed, the generated output is likely incorrect."));
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 584b06d..f0ae9e0 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -618,6 +618,13 @@
internal.proguardCompatibilityRulesOutput = proguardCompatibilityRulesOutput;
internal.dataResourceConsumer = internal.programConsumer.getDataResourceConsumer();
+ // Default is to remove Java assertion code as Dalvik and Art does not reliable support
+ // Java assertions. When generation class file output always keep the Java assertions code.
+ assert internal.disableAssertions;
+ if (internal.isGeneratingClassFiles()) {
+ internal.disableAssertions = false;
+ }
+
// EXPERIMENTAL flags.
assert !internal.forceProguardCompatibility;
internal.forceProguardCompatibility = forceProguardCompatibility;
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 3586d6d..eb8574d 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -634,6 +634,7 @@
// class inliner, null value indicates that the method is not eligible.
private ClassInlinerEligibility classInlinerEligibility = null;
private TrivialInitializer trivialInitializerInfo = null;
+ private boolean initializerEnablingJavaAssertions = false;
private ParameterUsagesInfo parametersUsages = null;
private BitSet kotlinNotNullParamHints = null;
@@ -705,6 +706,14 @@
return this.trivialInitializerInfo;
}
+ private void setInitializerEnablingJavaAssertions() {
+ this.initializerEnablingJavaAssertions = true;
+ }
+
+ public boolean isInitializerEnablingJavaAssertions() {
+ return initializerEnablingJavaAssertions;
+ }
+
public long getReturnedConstant() {
assert returnsConstant();
return returnedConstant;
@@ -831,6 +840,10 @@
ensureMutableOI().setTrivialInitializer(info);
}
+ synchronized public void setInitializerEnablingJavaAssertions() {
+ ensureMutableOI().setInitializerEnablingJavaAssertions();
+ }
+
synchronized public void markForceInline() {
ensureMutableOI().markForceInline();
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 3ea97e4..9433887 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -108,6 +108,7 @@
private final boolean enableWholeProgramOptimizations;
private final OptimizationFeedback ignoreOptimizationFeedback = new OptimizationFeedbackIgnore();
+ private final OptimizationFeedback simpleOptimizationFeedback = new OptimizationFeedbackSimple();
private DexString highestSortingString;
private IRConverter(
@@ -357,19 +358,34 @@
ExecutorService executor) throws ExecutionException {
List<Future<?>> futures = new ArrayList<>();
for (DexProgramClass clazz : classes) {
- futures.add(executor.submit(() -> clazz.forEachMethod(this::convertMethodToDex)));
+ futures.add(executor.submit(() -> convertMethodsToDex(clazz)));
}
ThreadUtils.awaitFutures(futures);
}
- void convertMethodToDex(DexEncodedMethod method) {
+ private void convertMethodsToDex(DexProgramClass clazz) {
+ // When converting all methods on a class always convert <clinit> first.
+ for (DexEncodedMethod method : clazz.directMethods()) {
+ if (method.isClassInitializer()) {
+ convertMethodToDex(method);
+ break;
+ }
+ }
+ clazz.forEachMethod(method -> {
+ if (!method.isClassInitializer()) {
+ convertMethodToDex(method);
+ }
+ });
+ }
+
+ private void convertMethodToDex(DexEncodedMethod method) {
assert options.isGeneratingDex();
if (method.getCode() != null) {
boolean matchesMethodFilter = options.methodMatchesFilter(method);
if (matchesMethodFilter) {
if (!(options.passthroughDexCode && method.getCode().isDexCode())) {
// We do not process in call graph order, so anything could be a leaf.
- rewriteCode(method, ignoreOptimizationFeedback, x -> true, CallSiteInformation.empty(),
+ rewriteCode(method, simpleOptimizationFeedback, x -> true, CallSiteInformation.empty(),
Outliner::noProcessing);
}
updateHighestSortingStrings(method);
@@ -668,7 +684,7 @@
codeRewriter.removeSwitchMaps(code);
}
if (options.disableAssertions) {
- codeRewriter.disableAssertions(code);
+ codeRewriter.disableAssertions(appInfo, method, code, feedback);
}
if (options.enableNonNullTracking && nonNullTracker != null) {
nonNullTracker.addNonNull(code);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
index c26aecf..49bfac8 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedback.java
@@ -21,6 +21,7 @@
void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark);
void setClassInlinerEligibility(DexEncodedMethod method, ClassInlinerEligibility eligibility);
void setTrivialInitializer(DexEncodedMethod method, TrivialInitializer info);
+ void setInitializerEnablingJavaAssertions(DexEncodedMethod method);
void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo);
void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
index 2b1a6db..1d7a2d6 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackDirect.java
@@ -60,6 +60,11 @@
}
@Override
+ public void setInitializerEnablingJavaAssertions(DexEncodedMethod method) {
+ method.setInitializerEnablingJavaAssertions();
+ }
+
+ @Override
public void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo) {
method.setParameterUsages(parameterUsagesInfo);
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
index a547b36..2fb2969 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackIgnore.java
@@ -44,6 +44,10 @@
}
@Override
+ public void setInitializerEnablingJavaAssertions(DexEncodedMethod method) {
+ }
+
+ @Override
public void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo) {
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackSimple.java b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackSimple.java
new file mode 100644
index 0000000..b31f28a
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/conversion/OptimizationFeedbackSimple.java
@@ -0,0 +1,77 @@
+// Copyright (c) 2017, 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.ir.conversion;
+
+import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexEncodedMethod.ClassInlinerEligibility;
+import com.android.tools.r8.graph.DexEncodedMethod.TrivialInitializer;
+import com.android.tools.r8.graph.ParameterUsagesInfo;
+import com.android.tools.r8.ir.optimize.Inliner.Constraint;
+import java.util.BitSet;
+
+public class OptimizationFeedbackSimple implements OptimizationFeedback {
+
+ @Override
+ public void methodReturnsArgument(DexEncodedMethod method, int argument) {
+ // Ignored.
+ }
+
+ @Override
+ public void methodReturnsConstant(DexEncodedMethod method, long value) {
+ // Ignored.
+ }
+
+ @Override
+ public void methodNeverReturnsNull(DexEncodedMethod method) {
+ // Ignored.
+ }
+
+ @Override
+ public void methodNeverReturnsNormally(DexEncodedMethod method) {
+ // Ignored.
+ }
+
+ @Override
+ public void markProcessed(DexEncodedMethod method, Constraint state) {
+ // Just as processed, don't provide any inlining constraints.
+ method.markProcessed(Constraint.NEVER);
+ }
+
+ @Override
+ public void markCheckNullReceiverBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {
+ // Ignored.
+ }
+
+ @Override
+ public void markTriggerClassInitBeforeAnySideEffect(DexEncodedMethod method, boolean mark) {
+ // Ignored.
+ }
+
+ @Override
+ public void setClassInlinerEligibility(
+ DexEncodedMethod method, ClassInlinerEligibility eligibility) {
+ // Ignored.
+ }
+
+ @Override
+ public void setTrivialInitializer(DexEncodedMethod method, TrivialInitializer info) {
+ // Ignored.
+ }
+
+ @Override
+ public void setInitializerEnablingJavaAssertions(DexEncodedMethod method) {
+ method.setInitializerEnablingJavaAssertions();
+ }
+
+ @Override
+ public void setParameterUsages(DexEncodedMethod method, ParameterUsagesInfo parameterUsagesInfo) {
+ // Ignored.
+ }
+
+ @Override
+ public void setKotlinNotNullParamHints(DexEncodedMethod method, BitSet hints) {
+ // Ignored.
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 94e8147..d87a508 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -1265,7 +1265,29 @@
* }
* </pre>
*/
- public void disableAssertions(IRCode code) {
+ public void disableAssertions(
+ AppInfo appInfo, DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
+ if (method.isClassInitializer()) {
+ if (!hasJavacClinitAssertionCode(code)) {
+ return;
+ }
+ // Mark the clinit as having code to turn on assertions.
+ feedback.setInitializerEnablingJavaAssertions(method);
+ } else {
+ // If the clinit of this class did not have have code to turn on assertions don't try to
+ // remove assertion code from the method.
+ DexClass clazz = appInfo.definitionFor(method.method.holder);
+ if (clazz == null) {
+ return;
+ }
+ DexEncodedMethod clinit = clazz.getClassInitializer();
+ if (clinit == null
+ || !clinit.isProcessed()
+ || !clinit.getOptimizationInfo().isInitializerEnablingJavaAssertions()) {
+ return;
+ }
+ }
+
InstructionIterator iterator = code.instructionIterator();
while (iterator.hasNext()) {
Instruction current = iterator.next();
@@ -1288,6 +1310,78 @@
}
}
+ private boolean isClassDesiredAssertionStatusInvoke(Instruction instruction) {
+ return instruction.isInvokeMethod()
+ && instruction.asInvokeMethod().getInvokedMethod()
+ == dexItemFactory.classMethods.desiredAssertionStatus;
+ }
+
+ private boolean isAssertionsDisabledFieldPut(Instruction instruction) {
+ return instruction.isStaticPut()
+ && instruction.asStaticPut().getField().name == dexItemFactory.assertionsDisabled;
+ }
+
+ private boolean isNotDebugInstruction(Instruction instruction) {
+ return !instruction.isDebugInstruction();
+ }
+
+ private Value blockWithSingleConstNumberAndGoto(BasicBlock block) {
+ InstructionIterator iterator = block.iterator();
+ Instruction constNumber = iterator.nextUntil(this::isNotDebugInstruction);
+ if (constNumber == null || !constNumber.isConstNumber()) {
+ return null;
+ }
+ Instruction exit = iterator.nextUntil(this::isNotDebugInstruction);
+ return exit != null && exit.isGoto() ? constNumber.outValue() : null;
+ }
+
+ private Value blockWithAssertionsDisabledFieldPut(BasicBlock block) {
+ InstructionIterator iterator = block.iterator();
+ Instruction fieldPut = iterator.nextUntil(this::isNotDebugInstruction);
+ return fieldPut != null
+ && isAssertionsDisabledFieldPut(fieldPut) ? fieldPut.inValues().get(0) : null;
+ }
+
+ private boolean hasJavacClinitAssertionCode(IRCode code) {
+ InstructionIterator iterator = code.instructionIterator();
+ Instruction current = iterator.nextUntil(this::isClassDesiredAssertionStatusInvoke);
+ if (current == null) {
+ return false;
+ }
+
+ Value DesiredAssertionStatus = current.outValue();
+ assert iterator.hasNext();
+ current = iterator.next();
+ if (!current.isIf()
+ || !current.asIf().isZeroTest()
+ || current.asIf().inValues().get(0) != DesiredAssertionStatus) {
+ return false;
+ }
+
+ If theIf = current.asIf();
+ BasicBlock trueTarget = theIf.getTrueTarget();
+ BasicBlock falseTarget = theIf.fallthroughBlock();
+ if (trueTarget == falseTarget) {
+ return false;
+ }
+
+ Value trueValue = blockWithSingleConstNumberAndGoto(trueTarget);
+ Value falseValue = blockWithSingleConstNumberAndGoto(falseTarget);
+ if (trueValue == null
+ || falseValue == null
+ || (trueTarget.exit().asGoto().getTarget() != falseTarget.exit().asGoto().getTarget())) {
+ return false;
+ }
+
+ BasicBlock target = trueTarget.exit().asGoto().getTarget();
+ Value storeValue = blockWithAssertionsDisabledFieldPut(target);
+ return storeValue != null
+ && storeValue.isPhi()
+ && storeValue.asPhi().getOperands().size() == 2
+ && storeValue.asPhi().getOperands().contains(trueValue)
+ && storeValue.asPhi().getOperands().contains(falseValue);
+ }
+
// Check if the static put is a constant derived from the class holding the method.
// This checks for java.lang.Class.getName and java.lang.Class.getSimpleName.
private boolean isClassNameConstant(DexEncodedMethod method, StaticPut put) {
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index eb8ee0e..3909dfd 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -448,6 +448,7 @@
public boolean dontCreateMarkerInD8 = false;
public boolean forceJumboStringProcessing = false;
public Set<Inliner.Reason> validInliningReasons = null;
+ public boolean suppressExperimentalCfBackendWarning = false;
}
public boolean canUseInvokePolymorphicOnVarHandle() {
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 8adddfd..855265d 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -54,6 +54,11 @@
public class TestBase {
+ protected enum Backend {
+ CF,
+ DEX
+ };
+
// Actually running Proguard should only be during development.
private static final boolean RUN_PROGUARD = System.getProperty("run_proguard") != null;
// Actually running r8.jar in a forked process.
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java b/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java
index b9ff7ea..f3f1211 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierClassSignatureTest.java
@@ -7,7 +7,6 @@
import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
import static com.android.tools.r8.utils.DexInspectorMatchers.isRenamed;
import static org.hamcrest.CoreMatchers.not;
-
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
@@ -21,6 +20,7 @@
import static org.objectweb.asm.Opcodes.RETURN;
import static org.objectweb.asm.Opcodes.V1_8;
+import com.android.tools.r8.ClassFileConsumer;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DiagnosticsChecker;
import com.android.tools.r8.R8Command;
@@ -33,11 +33,17 @@
import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import java.util.Arrays;
+import java.util.Collection;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.MethodVisitor;
+@RunWith(Parameterized.class)
public class MinifierClassSignatureTest extends TestBase {
/*
@@ -62,6 +68,16 @@
String outerSignature = "<T:Ljava/lang/Object;>Ljava/lang/Object;";
String extendsInnerSignature = "LOuter<TT;>.Inner;";
String extendsInnerInnerSignature = "LOuter<TT;>.Inner.InnerInner;";
+ Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Collection<Backend> data() {
+ return Arrays.asList(Backend.values());
+ }
+
+ public MinifierClassSignatureTest(Backend backend) {
+ this.backend = backend;
+ }
private byte[] dumpSimple(String classSignature) throws Exception {
@@ -290,26 +306,36 @@
Consumer<DexInspector> inspect)
throws Exception {
DiagnosticsChecker checker = new DiagnosticsChecker();
- DexInspector inspector = new DexInspector(
- ToolHelper.runR8(R8Command.builder(checker)
- .addClassProgramData(dumpSimple(signatures.get("Simple")), Origin.unknown())
- .addClassProgramData(dumpBase(signatures.get("Base")), Origin.unknown())
- .addClassProgramData(dumpOuter(signatures.get("Outer")), Origin.unknown())
- .addClassProgramData(dumpInner(signatures.get("Outer$Inner")), Origin.unknown())
- .addClassProgramData(
- dumpExtendsInner(signatures.get("Outer$ExtendsInner")), Origin.unknown())
- .addClassProgramData(
- dumpInnerInner(signatures.get("Outer$Inner$InnerInner")), Origin.unknown())
- .addClassProgramData(
- dumpExtendsInnerInner(
- signatures.get("Outer$Inner$ExtendsInnerInner")), Origin.unknown())
- .addProguardConfiguration(ImmutableList.of(
- "-keepattributes InnerClasses,EnclosingMethod,Signature",
- "-keep,allowobfuscation class **"
- ), Origin.unknown())
- .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
- .setProguardMapConsumer(StringConsumer.emptyConsumer())
- .build()));
+ assert (backend == Backend.CF || backend == Backend.DEX);
+ DexInspector inspector =
+ new DexInspector(
+ ToolHelper.runR8(
+ R8Command.builder(checker)
+ .addClassProgramData(dumpSimple(signatures.get("Simple")), Origin.unknown())
+ .addClassProgramData(dumpBase(signatures.get("Base")), Origin.unknown())
+ .addClassProgramData(dumpOuter(signatures.get("Outer")), Origin.unknown())
+ .addClassProgramData(dumpInner(signatures.get("Outer$Inner")), Origin.unknown())
+ .addClassProgramData(
+ dumpExtendsInner(signatures.get("Outer$ExtendsInner")), Origin.unknown())
+ .addClassProgramData(
+ dumpInnerInner(signatures.get("Outer$Inner$InnerInner")), Origin.unknown())
+ .addClassProgramData(
+ dumpExtendsInnerInner(signatures.get("Outer$Inner$ExtendsInnerInner")),
+ Origin.unknown())
+ .addProguardConfiguration(
+ ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod,Signature",
+ "-keep,allowobfuscation class **"),
+ Origin.unknown())
+ .setProgramConsumer(
+ backend == Backend.DEX
+ ? DexIndexedConsumer.emptyConsumer()
+ : ClassFileConsumer.emptyConsumer())
+ .setProguardMapConsumer(StringConsumer.emptyConsumer())
+ .build(),
+ options -> {
+ options.testing.suppressExperimentalCfBackendWarning = true;
+ }));
// All classes are kept, and renamed.
assertThat(inspector.clazz("Simple"), isRenamed());
assertThat(inspector.clazz("Base"), isRenamed());
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierFieldSignatureTest.java b/src/test/java/com/android/tools/r8/naming/MinifierFieldSignatureTest.java
index e48ce56..a5351e2 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierFieldSignatureTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierFieldSignatureTest.java
@@ -21,6 +21,7 @@
import static org.objectweb.asm.Opcodes.RETURN;
import static org.objectweb.asm.Opcodes.V1_8;
+import com.android.tools.r8.ClassFileConsumer;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DiagnosticsChecker;
import com.android.tools.r8.R8Command;
@@ -34,13 +35,18 @@
import com.android.tools.r8.utils.DexInspector.FieldSubject;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.Map;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.MethodVisitor;
-
+@RunWith(Parameterized.class)
public class MinifierFieldSignatureTest extends TestBase {
/*
@@ -59,6 +65,16 @@
private String anArrayOfXSignature = "[TX;";
private String aFieldsOfXSignature = "LFields<TX;>;";
private String aFieldsOfXInnerSignature = "LFields<TX;>.Inner;";
+ private Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Collection<Backend> data() {
+ return Arrays.asList(Backend.values());
+ }
+
+ public MinifierFieldSignatureTest(Backend backend) {
+ this.backend = backend;
+ }
public byte[] dumpFields(Map<String, String> signatures) throws Exception {
@@ -164,17 +180,27 @@
Consumer<DexInspector> inspect)
throws Exception {
DiagnosticsChecker checker = new DiagnosticsChecker();
- DexInspector inspector = new DexInspector(
- ToolHelper.runR8(R8Command.builder(checker)
- .addClassProgramData(dumpFields(signatures), Origin.unknown())
- .addClassProgramData(dumpInner(), Origin.unknown())
- .addProguardConfiguration(ImmutableList.of(
- "-keepattributes InnerClasses,EnclosingMethod,Signature",
- "-keep,allowobfuscation class ** { *; }"
- ), Origin.unknown())
- .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
- .setProguardMapConsumer(StringConsumer.emptyConsumer())
- .build()));
+ assert (backend == Backend.CF || backend == Backend.DEX);
+ DexInspector inspector =
+ new DexInspector(
+ ToolHelper.runR8(
+ R8Command.builder(checker)
+ .addClassProgramData(dumpFields(signatures), Origin.unknown())
+ .addClassProgramData(dumpInner(), Origin.unknown())
+ .addProguardConfiguration(
+ ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod,Signature",
+ "-keep,allowobfuscation class ** { *; }"),
+ Origin.unknown())
+ .setProgramConsumer(
+ backend == Backend.DEX
+ ? DexIndexedConsumer.emptyConsumer()
+ : ClassFileConsumer.emptyConsumer())
+ .setProguardMapConsumer(StringConsumer.emptyConsumer())
+ .build(),
+ options -> {
+ options.testing.suppressExperimentalCfBackendWarning = true;
+ }));
// All classes are kept, and renamed.
ClassSubject clazz = inspector.clazz("Fields");
assertThat(clazz, isRenamed());
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierMethodSignatureTest.java b/src/test/java/com/android/tools/r8/naming/MinifierMethodSignatureTest.java
index 41750ae..baeedbb 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierMethodSignatureTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierMethodSignatureTest.java
@@ -24,6 +24,7 @@
import static org.objectweb.asm.Opcodes.RETURN;
import static org.objectweb.asm.Opcodes.V1_8;
+import com.android.tools.r8.ClassFileConsumer;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DiagnosticsChecker;
import com.android.tools.r8.R8Command;
@@ -37,12 +38,18 @@
import com.android.tools.r8.utils.DexInspector.MethodSubject;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.Map;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.MethodVisitor;
+@RunWith(Parameterized.class)
public class MinifierMethodSignatureTest extends TestBase {
/*
@@ -61,6 +68,16 @@
private String parameterizedReturnSignature = "()LMethods<TX;>.Inner;";
private String parameterizedArgumentsSignature = "(TX;LMethods<TX;>.Inner;)V";
private String parametrizedThrowsSignature = "()V^TX;";
+ Backend backend;
+
+ @Parameters(name = "Backend: {0}")
+ public static Collection<Backend> data() {
+ return Arrays.asList(Backend.values());
+ }
+
+ public MinifierMethodSignatureTest(Backend backend) {
+ this.backend = backend;
+ }
private byte[] dumpMethods(Map<String, String> signatures) throws Exception {
@@ -185,17 +202,27 @@
Consumer<DexInspector> inspect)
throws Exception {
DiagnosticsChecker checker = new DiagnosticsChecker();
- DexInspector inspector = new DexInspector(
- ToolHelper.runR8(R8Command.builder(checker)
- .addClassProgramData(dumpMethods(signatures), Origin.unknown())
- .addClassProgramData(dumpInner(), Origin.unknown())
- .addProguardConfiguration(ImmutableList.of(
- "-keepattributes InnerClasses,EnclosingMethod,Signature",
- "-keep,allowobfuscation class ** { *; }"
- ), Origin.unknown())
- .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
- .setProguardMapConsumer(StringConsumer.emptyConsumer())
- .build()));
+ assert (backend == Backend.CF || backend == Backend.DEX);
+ DexInspector inspector =
+ new DexInspector(
+ ToolHelper.runR8(
+ R8Command.builder(checker)
+ .addClassProgramData(dumpMethods(signatures), Origin.unknown())
+ .addClassProgramData(dumpInner(), Origin.unknown())
+ .addProguardConfiguration(
+ ImmutableList.of(
+ "-keepattributes InnerClasses,EnclosingMethod,Signature",
+ "-keep,allowobfuscation class ** { *; }"),
+ Origin.unknown())
+ .setProgramConsumer(
+ backend == Backend.DEX
+ ? DexIndexedConsumer.emptyConsumer()
+ : ClassFileConsumer.emptyConsumer())
+ .setProguardMapConsumer(StringConsumer.emptyConsumer())
+ .build(),
+ options -> {
+ options.testing.suppressExperimentalCfBackendWarning = true;
+ }));
// All classes are kept, and renamed.
ClassSubject clazz = inspector.clazz("Methods");
assertThat(clazz, isRenamed());
diff --git a/src/test/java/com/android/tools/r8/rewrite/assertions/ChromuimAssertionHookMock.java b/src/test/java/com/android/tools/r8/rewrite/assertions/ChromuimAssertionHookMock.java
new file mode 100644
index 0000000..772ef09
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/rewrite/assertions/ChromuimAssertionHookMock.java
@@ -0,0 +1,11 @@
+// Copyright (c) 2018, 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;
+
+public class ChromuimAssertionHookMock {
+ public static void assertFailureHandler(AssertionError assertion) {
+ System.out.println("Got AssertionError " + assertion);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/rewrite/assertions/ClassWithAssertions.java b/src/test/java/com/android/tools/r8/rewrite/assertions/ClassWithAssertions.java
index cc56790..49fe85e 100644
--- a/src/test/java/com/android/tools/r8/rewrite/assertions/ClassWithAssertions.java
+++ b/src/test/java/com/android/tools/r8/rewrite/assertions/ClassWithAssertions.java
@@ -16,7 +16,9 @@
}
int getX() {
+ System.out.println("1");
assert condition();
+ System.out.println("2");
return x;
}
diff --git a/src/test/java/com/android/tools/r8/rewrite/assertions/RemoveAssertionsTest.java b/src/test/java/com/android/tools/r8/rewrite/assertions/RemoveAssertionsTest.java
index 3c724c0..c118bad 100644
--- a/src/test/java/com/android/tools/r8/rewrite/assertions/RemoveAssertionsTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/assertions/RemoveAssertionsTest.java
@@ -4,17 +4,139 @@
package com.android.tools.r8.rewrite.assertions;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.DexIndexedConsumer;
+import com.android.tools.r8.OutputMode;
+import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import java.util.function.Consumer;
+import java.util.function.Function;
import org.junit.Test;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.Type;
+
+// This ASM class visitor has been adapted from
+// https://chromium.googlesource.com/chromium/src/+/164e81fcd0828b40f5496e9025349ea728cde7f5/build/android/bytecode/java/org/chromium/bytecode/AssertionEnablerClassAdapter.java
+// See b/110887293.
+
+/**
+ * An ClassVisitor for replacing Java ASSERT statements with a function by modifying Java bytecode.
+ *
+ * We do this in two steps, first step is to enable assert.
+ * Following bytecode is generated for each class with ASSERT statements:
+ * 0: ldc #8 // class CLASSNAME
+ * 2: invokevirtual #9 // Method java/lang/Class.desiredAssertionStatus:()Z
+ * 5: ifne 12
+ * 8: iconst_1
+ * 9: goto 13
+ * 12: iconst_0
+ * 13: putstatic #2 // Field $assertionsDisabled:Z
+ * Replaces line #13 to the following:
+ * 13: pop
+ * Consequently, $assertionsDisabled is assigned the default value FALSE.
+ * This is done in the first if statement in overridden visitFieldInsn. We do this per per-assert.
+ *
+ * Second step is to replace assert statement with a function:
+ * The followed instructions are generated by a java assert statement:
+ * getstatic #3 // Field $assertionsDisabled:Z
+ * ifne 118 // Jump to instruction as if assertion if not enabled
+ * ...
+ * ifne 19
+ * new #4 // class java/lang/AssertionError
+ * dup
+ * ldc #5 // String (don't have this line if no assert message given)
+ * invokespecial #6 // Method java/lang/AssertionError.
+ * athrow
+ * Replace athrow with:
+ * invokestatic #7 // Method org/chromium/base/JavaExceptionReporter.assertFailureHandler
+ * goto 118
+ * JavaExceptionReporter.assertFailureHandler is a function that handles the AssertionError,
+ * 118 is the instruction to execute as if assertion if not enabled.
+ */
+class AssertionEnablerClassAdapter extends ClassVisitor {
+ AssertionEnablerClassAdapter(ClassVisitor visitor) {
+ super(Opcodes.ASM6, visitor);
+ }
+
+ @Override
+ public MethodVisitor visitMethod(final int access, final String name, String desc,
+ String signature, String[] exceptions) {
+ return new RewriteAssertMethodVisitor(
+ Opcodes.ASM5, super.visitMethod(access, name, desc, signature, exceptions));
+ }
+
+ static class RewriteAssertMethodVisitor extends MethodVisitor {
+ static final String ASSERTION_DISABLED_NAME = "$assertionsDisabled";
+ static final String INSERT_INSTRUCTION_NAME = "assertFailureHandler";
+ static final String INSERT_INSTRUCTION_DESC =
+ Type.getMethodDescriptor(Type.VOID_TYPE, Type.getObjectType("java/lang/AssertionError"));
+ static final boolean INSERT_INSTRUCTION_ITF = false;
+
+ boolean mStartLoadingAssert;
+ Label mGotoLabel;
+
+ public RewriteAssertMethodVisitor(int api, MethodVisitor mv) {
+ super(api, mv);
+ }
+
+ @Override
+ public void visitFieldInsn(int opcode, String owner, String name, String desc) {
+ if (opcode == Opcodes.PUTSTATIC && name.equals(ASSERTION_DISABLED_NAME)) {
+ super.visitInsn(Opcodes.POP); // enable assert
+ } else if (opcode == Opcodes.GETSTATIC && name.equals(ASSERTION_DISABLED_NAME)) {
+ mStartLoadingAssert = true;
+ super.visitFieldInsn(opcode, owner, name, desc);
+ } else {
+ super.visitFieldInsn(opcode, owner, name, desc);
+ }
+ }
+
+ @Override
+ public void visitJumpInsn(int opcode, Label label) {
+ if (mStartLoadingAssert && opcode == Opcodes.IFNE && mGotoLabel == null) {
+ mGotoLabel = label;
+ }
+ super.visitJumpInsn(opcode, label);
+ }
+
+ @Override
+ public void visitInsn(int opcode) {
+ if (!mStartLoadingAssert || opcode != Opcodes.ATHROW) {
+ super.visitInsn(opcode);
+ } else {
+ super.visitMethodInsn(
+ Opcodes.INVOKESTATIC,
+ ChromuimAssertionHookMock.class.getCanonicalName().replace('.', '/'),
+ INSERT_INSTRUCTION_NAME,
+ INSERT_INSTRUCTION_DESC,
+ INSERT_INSTRUCTION_ITF);
+ super.visitJumpInsn(Opcodes.GOTO, mGotoLabel);
+ mStartLoadingAssert = false;
+ mGotoLabel = null;
+ }
+ }
+ }
+}
public class RemoveAssertionsTest extends TestBase {
@@ -38,4 +160,73 @@
clazz.method(new MethodSignature(Constants.CLASS_INITIALIZER_NAME, "void", new String[]{}));
assertTrue(!clinit.isPresent());
}
+
+ private Path buildTestToCf(Consumer<InternalOptions> consumer) throws Exception {
+ Path outputJar = temp.getRoot().toPath().resolve("output.jar");
+ R8Command command =
+ ToolHelper.prepareR8CommandBuilder(readClasses(ClassWithAssertions.class))
+ .setMode(CompilationMode.DEBUG)
+ .setOutput(outputJar, OutputMode.ClassFile)
+ .build();
+ ToolHelper.runR8(command, consumer);
+ return outputJar;
+ }
+
+ @Test
+ public void testCfOutput() throws Exception {
+ String main = ClassWithAssertions.class.getCanonicalName();
+ ProcessResult result;
+ // Assertion is hit.
+ result = ToolHelper.runJava(buildTestToCf(options -> {}), "-ea", main, "0");
+ assertEquals(1, result.exitCode);
+ assertEquals("1\n", result.stdout);
+ // Assertion is not hit.
+ result = ToolHelper.runJava(buildTestToCf(options -> {}), "-ea", main, "1");
+ assertEquals(0, result.exitCode);
+ assertEquals("1\n2\n", result.stdout);
+ // Assertion is hit, but removed.
+ result = ToolHelper.runJava(
+ buildTestToCf(
+ options -> options.disableAssertions = true), "-ea", main, "0");
+ assertEquals(0, result.exitCode);
+ assertEquals("1\n2\n", result.stdout);
+ }
+
+ private byte[] identity(byte[] classBytes) {
+ return classBytes;
+ }
+
+ private byte[] chromiumAssertionEnabler(byte[] classBytes) {
+ ClassWriter writer = new ClassWriter(0);
+ new ClassReader(classBytes).accept(new AssertionEnablerClassAdapter(writer), 0);
+ return writer.toByteArray();
+ }
+
+ private AndroidApp runRegress110887293(Function<byte[], byte[]> rewriter) throws Exception {
+ return ToolHelper.runR8(
+ R8Command.builder()
+ .addClassProgramData(
+ rewriter.apply(ToolHelper.getClassAsBytes(ClassWithAssertions.class)),
+ Origin.unknown())
+ .addClassProgramData(
+ ToolHelper.getClassAsBytes(ChromuimAssertionHookMock.class), Origin.unknown())
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .setMode(CompilationMode.DEBUG)
+ .build());
+ }
+
+ @Test
+ public void regress110887293() throws Exception {
+ AndroidApp app;
+ // Assertions removed for default assertion code.
+ app = runRegress110887293(this::identity);
+ assertEquals("1\n2\n", runOnArt(app, ClassWithAssertions.class.getCanonicalName(), "0"));
+ assertEquals("1\n2\n", runOnArt(app, ClassWithAssertions.class.getCanonicalName(), "1"));
+ // Assertions not removed when default assertion code is not present.
+ app = runRegress110887293(this::chromiumAssertionEnabler);
+ assertEquals(
+ "1\nGot AssertionError java.lang.AssertionError\n2\n",
+ runOnArt(app, ClassWithAssertions.class.getCanonicalName(), "0"));
+ assertEquals("1\n2\n", runOnArt(app, ClassWithAssertions.class.getCanonicalName(), "1"));
+ }
}
