Version 1.6.47
Cherry-pick: Synthesize lambda classes prior to each wave
CL: https://r8-review.googlesource.com/c/r8/+/43986
Cherry-pick: Treat instance-put as having side-effects if it may store an object with a non-default finalize()
CL: https://r8-review.googlesource.com/c/r8/+/44118
Cherry-pick: Fix may-have-finalize-method cache
CL: https://r8-review.googlesource.com/c/r8/+/43952
Cherry-pick: Reland "Treat static-puts that store objects with a non-default finalize() method as having side effects"
CL: https://r8-review.googlesource.com/c/r8/+/43902
Bug: 142203515
Bug: 137836288
Change-Id: I06ecc3c1c014619b4bc16545625576a15bda4760
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 091b934..e374f53 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.6.46";
+ public static final String LABEL = "1.6.47";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
index c4e704c..7d2cc6f 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithSubtyping.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.graph;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement;
import com.android.tools.r8.ir.desugar.LambdaDescriptor;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.SetUtils;
@@ -34,7 +35,8 @@
private static final Set<DexType> NO_DIRECT_SUBTYPE = ImmutableSet.of();
private static class TypeInfo {
- final DexType type;
+
+ private final DexType type;
int hierarchyLevel = UNKNOWN_LEVEL;
/**
@@ -87,7 +89,7 @@
setLevel(ROOT_LEVEL);
}
- void tagAsInteface() {
+ void tagAsInterface() {
setLevel(INTERFACE_LEVEL);
}
@@ -123,6 +125,11 @@
// Map from types to their subtyping information.
private final Map<DexType, TypeInfo> typeInfo;
+ // Caches which static types that may store an object that has a non-default finalize() method.
+ // E.g., `java.lang.Object -> TRUE` if there is a subtype of Object that overrides finalize().
+ private final Map<DexType, Boolean> mayHaveFinalizeMethodDirectlyOrIndirectlyCache =
+ new ConcurrentHashMap<>();
+
public AppInfoWithSubtyping(DexApplication application) {
super(application);
typeInfo = new ConcurrentHashMap<>();
@@ -226,7 +233,7 @@
getTypeInfo(inter).addInterfaceSubtype(holder);
}
if (holderClass.isInterface()) {
- getTypeInfo(holder).tagAsInteface();
+ getTypeInfo(holder).tagAsInterface();
}
} else {
if (baseClass.isProgramClass() || baseClass.isClasspathClass()) {
@@ -709,4 +716,56 @@
public boolean inDifferentHierarchy(DexType type1, DexType type2) {
return !isSubtype(type1, type2) && !isSubtype(type2, type1);
}
+
+ public boolean mayHaveFinalizeMethodDirectlyOrIndirectly(ClassTypeLatticeElement type) {
+ Set<DexType> interfaces = type.getInterfaces();
+ if (!interfaces.isEmpty()) {
+ for (DexType interfaceType : interfaces) {
+ if (computeMayHaveFinalizeMethodDirectlyOrIndirectlyIfAbsent(interfaceType, false)) {
+ return true;
+ }
+ }
+ return false;
+ }
+ return computeMayHaveFinalizeMethodDirectlyOrIndirectlyIfAbsent(type.getClassType(), true);
+ }
+
+ private boolean computeMayHaveFinalizeMethodDirectlyOrIndirectlyIfAbsent(
+ DexType type, boolean lookUpwards) {
+ assert type.isClassType();
+ Boolean cache = mayHaveFinalizeMethodDirectlyOrIndirectlyCache.get(type);
+ if (cache != null) {
+ return cache;
+ }
+ DexClass clazz = definitionFor(type);
+ if (clazz == null) {
+ // This is strictly not conservative but is needed to avoid that we treat Object as having
+ // a subtype that has a non-default finalize() implementation.
+ mayHaveFinalizeMethodDirectlyOrIndirectlyCache.put(type, false);
+ return false;
+ }
+ if (clazz.isProgramClass()) {
+ if (lookUpwards) {
+ DexEncodedMethod resolutionResult =
+ resolveMethod(type, dexItemFactory().objectMethods.finalize).asSingleTarget();
+ if (resolutionResult != null && resolutionResult.isProgramMethod(this)) {
+ mayHaveFinalizeMethodDirectlyOrIndirectlyCache.put(type, true);
+ return true;
+ }
+ } else {
+ if (clazz.lookupVirtualMethod(dexItemFactory().objectMethods.finalize) != null) {
+ mayHaveFinalizeMethodDirectlyOrIndirectlyCache.put(type, true);
+ return true;
+ }
+ }
+ }
+ for (DexType subtype : allImmediateSubtypes(type)) {
+ if (computeMayHaveFinalizeMethodDirectlyOrIndirectlyIfAbsent(subtype, false)) {
+ mayHaveFinalizeMethodDirectlyOrIndirectlyCache.put(type, true);
+ return true;
+ }
+ }
+ mayHaveFinalizeMethodDirectlyOrIndirectlyCache.put(type, false);
+ return false;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
index e041d5a..77e084d 100644
--- a/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
+++ b/src/main/java/com/android/tools/r8/ir/code/FieldInstruction.java
@@ -8,9 +8,12 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
+import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.AbstractError;
+import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import java.util.Collections;
import java.util.List;
@@ -131,4 +134,39 @@
// TODO(jsjeon): what if the target field is known to be non-null?
return true;
}
+
+ /**
+ * Returns {@code true} if this instruction may store an instance of a class that has a non-
+ * default finalize() method in a field. In that case, it is not safe to remove this instruction,
+ * since that could change the lifetime of the value.
+ */
+ boolean isStoringObjectWithFinalizer(AppInfoWithLiveness appInfo) {
+ assert isFieldPut();
+ TypeLatticeElement type = value().getTypeLattice();
+ TypeLatticeElement baseType =
+ type.isArrayType() ? type.asArrayTypeLatticeElement().getArrayBaseTypeLattice() : type;
+ if (baseType.isClassType()) {
+ Value root = value().getAliasedValue();
+ if (!root.isPhi() && root.definition.isNewInstance()) {
+ DexClass clazz = appInfo.definitionFor(root.definition.asNewInstance().clazz);
+ if (clazz == null) {
+ return true;
+ }
+ if (clazz.superType == null) {
+ return false;
+ }
+ DexItemFactory dexItemFactory = appInfo.dexItemFactory();
+ DexEncodedMethod resolutionResult =
+ appInfo
+ .resolveMethod(clazz.type, dexItemFactory.objectMethods.finalize)
+ .asSingleTarget();
+ return resolutionResult != null && resolutionResult.isProgramMethod(appInfo);
+ }
+
+ return appInfo.mayHaveFinalizeMethodDirectlyOrIndirectly(
+ baseType.asClassTypeLatticeElement());
+ }
+
+ return false;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
index 4dcb749..32484c3 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstancePut.java
@@ -110,7 +110,8 @@
DexEncodedField encodedField = appInfoWithLiveness.resolveField(getField());
assert encodedField != null : "NoSuchFieldError (resolution failure) should be caught.";
- return appInfoWithLiveness.isFieldRead(encodedField);
+ return appInfoWithLiveness.isFieldRead(encodedField)
+ || isStoringObjectWithFinalizer(appInfoWithLiveness);
}
// In D8, we always have to assume that the field can be read, and thus have side effects.
diff --git a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
index 6db965b..63e5c10 100644
--- a/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
+++ b/src/main/java/com/android/tools/r8/ir/code/StaticPut.java
@@ -120,7 +120,8 @@
return false;
}
- return appInfoWithLiveness.isFieldRead(encodedField);
+ return appInfoWithLiveness.isFieldRead(encodedField)
+ || isStoringObjectWithFinalizer(appInfoWithLiveness);
}
// In D8, we always have to assume that the field can be read, and thus have side effects.
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 9c4215e..7dffe9d 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
@@ -796,8 +796,12 @@
return builder.build();
}
- private void waveStart() {
+ private void waveStart(Collection<DexEncodedMethod> wave) {
onWaveDoneActions = Collections.synchronizedList(new ArrayList<>());
+
+ if (lambdaRewriter != null) {
+ wave.forEach(method -> lambdaRewriter.synthesizeLambdaClassesFor(method, lensCodeRewriter));
+ }
}
private void waveDone() {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index c093b1c..7cab0e3 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -118,24 +118,8 @@
if (current.isInvokeCustom()) {
InvokeCustom invokeCustom = current.asInvokeCustom();
DexCallSite callSite = invokeCustom.getCallSite();
- DexProto newMethodProto =
- factory.applyClassMappingToProto(
- callSite.methodProto, graphLense::lookupType, protoFixupCache);
- DexMethodHandle newBootstrapMethod = rewriteDexMethodHandle(
- callSite.bootstrapMethod, method, NOT_ARGUMENT_TO_LAMBDA_METAFACTORY);
- boolean isLambdaMetaFactory =
- factory.isLambdaMetafactoryMethod(callSite.bootstrapMethod.asMethod());
- MethodHandleUse methodHandleUse = isLambdaMetaFactory
- ? ARGUMENT_TO_LAMBDA_METAFACTORY
- : NOT_ARGUMENT_TO_LAMBDA_METAFACTORY;
- List<DexValue> newArgs =
- rewriteBootstrapArgs(callSite.bootstrapArgs, method, methodHandleUse);
- if (!newMethodProto.equals(callSite.methodProto)
- || newBootstrapMethod != callSite.bootstrapMethod
- || !newArgs.equals(callSite.bootstrapArgs)) {
- DexCallSite newCallSite =
- factory.createCallSite(
- callSite.methodName, newMethodProto, newBootstrapMethod, newArgs);
+ DexCallSite newCallSite = rewriteCallSite(callSite, method);
+ if (newCallSite != callSite) {
Value newOutValue = makeOutValue(invokeCustom, code);
InvokeCustom newInvokeCustom =
new InvokeCustom(newCallSite, newOutValue, invokeCustom.inValues());
@@ -422,6 +406,28 @@
assert code.hasNoVerticallyMergedClasses(appView);
}
+ public DexCallSite rewriteCallSite(DexCallSite callSite, DexEncodedMethod context) {
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ DexProto newMethodProto =
+ dexItemFactory.applyClassMappingToProto(
+ callSite.methodProto, appView.graphLense()::lookupType, protoFixupCache);
+ DexMethodHandle newBootstrapMethod =
+ rewriteDexMethodHandle(
+ callSite.bootstrapMethod, context, NOT_ARGUMENT_TO_LAMBDA_METAFACTORY);
+ boolean isLambdaMetaFactory =
+ dexItemFactory.isLambdaMetafactoryMethod(callSite.bootstrapMethod.asMethod());
+ MethodHandleUse methodHandleUse =
+ isLambdaMetaFactory ? ARGUMENT_TO_LAMBDA_METAFACTORY : NOT_ARGUMENT_TO_LAMBDA_METAFACTORY;
+ List<DexValue> newArgs = rewriteBootstrapArgs(callSite.bootstrapArgs, context, methodHandleUse);
+ if (!newMethodProto.equals(callSite.methodProto)
+ || newBootstrapMethod != callSite.bootstrapMethod
+ || !newArgs.equals(callSite.bootstrapArgs)) {
+ return dexItemFactory.createCallSite(
+ callSite.methodName, newMethodProto, newBootstrapMethod, newArgs);
+ }
+ return callSite;
+ }
+
// If the given invoke is on the form "invoke-direct A.<init>, v0, ..." and the definition of
// value v0 is "new-instance v0, B", where B is a subtype of A (see the Art800 and B116282409
// tests), then fail with a compilation error if A has previously been merged into B.
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java
index 195583d..9a55240 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/MethodProcessor.java
@@ -92,7 +92,7 @@
*/
public <E extends Exception> void forEachMethod(
ThrowingBiConsumer<DexEncodedMethod, Predicate<DexEncodedMethod>, E> consumer,
- Action waveStart,
+ Consumer<Collection<DexEncodedMethod>> waveStart,
Action waveDone,
ExecutorService executorService)
throws ExecutionException {
@@ -100,7 +100,7 @@
Collection<DexEncodedMethod> wave = waves.removeFirst();
assert wave.size() > 0;
List<Future<?>> futures = new ArrayList<>();
- waveStart.execute();
+ waveStart.accept(wave);
for (DexEncodedMethod method : wave) {
futures.add(
executorService.submit(
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
index 65eb486..30c22b4 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
@@ -7,6 +7,8 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.Code;
+import com.android.tools.r8.graph.DefaultUseRegistry;
import com.android.tools.r8.graph.DexApplication.Builder;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -30,6 +32,7 @@
import com.android.tools.r8.ir.code.StaticGet;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.conversion.IRConverter;
+import com.android.tools.r8.ir.conversion.LensCodeRewriter;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableSet;
@@ -101,6 +104,36 @@
this.createInstanceMethodName = factory.createString(LAMBDA_CREATE_INSTANCE_METHOD_NAME);
}
+ public void synthesizeLambdaClassesFor(
+ DexEncodedMethod method, LensCodeRewriter lensCodeRewriter) {
+ if (!method.hasCode() || method.isProcessed()) {
+ // Nothing to desugar.
+ return;
+ }
+
+ Code code = method.getCode();
+ if (!code.isCfCode()) {
+ // Nothing to desugar.
+ return;
+ }
+
+ // Introduce a lambda class in AppInfo for each call site such that we do not modify the
+ // application (and, in particular, the class hierarchy) during wave processing.
+ code.registerCodeReferences(
+ method,
+ new DefaultUseRegistry(appView.dexItemFactory()) {
+
+ @Override
+ public void registerCallSite(DexCallSite callSite) {
+ LambdaDescriptor descriptor =
+ inferLambdaDescriptor(lensCodeRewriter.rewriteCallSite(callSite, method));
+ if (descriptor != LambdaDescriptor.MATCH_FAILED) {
+ getOrCreateLambdaClass(descriptor, method.method.holder);
+ }
+ }
+ });
+ }
+
/**
* Detect and desugar lambdas and method references found in the code.
*
diff --git a/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/PutObjectWithFinalizeTest.java b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/PutObjectWithFinalizeTest.java
new file mode 100644
index 0000000..d133adf
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/analysis/sideeffect/PutObjectWithFinalizeTest.java
@@ -0,0 +1,151 @@
+// Copyright (c) 2019, 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.analysis.sideeffect;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.FieldSubject;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class PutObjectWithFinalizeTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ public PutObjectWithFinalizeTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(PutObjectWithFinalizeTest.class)
+ .addKeepMainRule(TestClass.class)
+ // The class staticizer does not consider the finalize() method.
+ .addOptionsModification(options -> options.enableClassStaticizer = false)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(TestClass.class);
+ assertThat(classSubject, isPresent());
+
+ MethodSubject mainSubject = classSubject.clinit();
+ assertThat(mainSubject, isPresent());
+
+ List<String> presentFields =
+ ImmutableList.of(
+ "directInstanceWithDirectFinalizer",
+ "directInstanceWithIndirectFinalizer",
+ "indirectInstanceWithDirectFinalizer",
+ "indirectInstanceWithIndirectFinalizer",
+ "otherIndirectInstanceWithoutFinalizer",
+ "arrayWithDirectFinalizer",
+ "arrayWithIndirectFinalizer",
+ "otherArrayInstanceWithoutFinalizer");
+ for (String name : presentFields) {
+ FieldSubject fieldSubject = classSubject.uniqueFieldWithName(name);
+ assertThat(fieldSubject, isPresent());
+ assertTrue(
+ mainSubject
+ .streamInstructions()
+ .filter(InstructionSubject::isStaticPut)
+ .map(InstructionSubject::getField)
+ .map(field -> field.name.toSourceString())
+ .anyMatch(fieldSubject.getFinalName()::equals));
+ }
+
+ List<String> absentFields =
+ ImmutableList.of(
+ "directInstanceWithoutFinalizer",
+ "otherDirectInstanceWithoutFinalizer",
+ "indirectInstanceWithoutFinalizer",
+ "arrayWithoutFinalizer");
+ for (String name : absentFields) {
+ assertThat(classSubject.uniqueFieldWithName(name), not(isPresent()));
+ }
+ })
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ }
+
+ static class TestClass {
+
+ static A directInstanceWithDirectFinalizer = new A();
+ static A directInstanceWithIndirectFinalizer = new B();
+ static Object directInstanceWithoutFinalizer = new C();
+ static Object otherDirectInstanceWithoutFinalizer = new Object();
+
+ static A indirectInstanceWithDirectFinalizer = createA();
+ static A indirectInstanceWithIndirectFinalizer = createB();
+ static Object indirectInstanceWithoutFinalizer = createC();
+ static Object otherIndirectInstanceWithoutFinalizer = createObject();
+
+ static A[] arrayWithDirectFinalizer = new A[42];
+ static A[] arrayWithIndirectFinalizer = new B[42];
+ static Object[] arrayWithoutFinalizer = new C[42];
+ static Object[] otherArrayInstanceWithoutFinalizer = new Object[42];
+
+ public static void main(String[] args) {
+ System.out.println("Hello world!");
+ }
+
+ @NeverInline
+ static A createA() {
+ return new A();
+ }
+
+ @NeverInline
+ static B createB() {
+ return new B();
+ }
+
+ @NeverInline
+ static C createC() {
+ return new C();
+ }
+
+ @NeverInline
+ static Object createObject() {
+ return new Object();
+ }
+ }
+
+ @NeverMerge
+ static class A {
+
+ @Override
+ public void finalize() {
+ System.out.println("Finalize!");
+ }
+ }
+
+ static class B extends A {}
+
+ static class C {}
+}
