Treat instance-put as having side-effects if it may store an object with a non-default finalize()
Bug: 137836288
Change-Id: I939386a925a8eba94eeb27a90d180c1c89a9316f
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 460885c..e4f80a81 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;
@@ -714,8 +715,17 @@
return !isSubtype(type1, type2) && !isSubtype(type2, type1);
}
- public boolean mayHaveFinalizeMethodDirectlyOrIndirectly(DexType type) {
- return computeMayHaveFinalizeMethodDirectlyOrIndirectlyIfAbsent(type, true);
+ 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(
@@ -727,8 +737,10 @@
}
DexClass clazz = definitionFor(type);
if (clazz == null) {
- mayHaveFinalizeMethodDirectlyOrIndirectlyCache.put(type, true);
- return true;
+ // 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) {
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 b95376e..72e4c9d 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,13 +8,16 @@
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.fieldvalueanalysis.AbstractFieldSet;
import com.android.tools.r8.ir.analysis.fieldvalueanalysis.ConcreteMutableFieldSet;
import com.android.tools.r8.ir.analysis.fieldvalueanalysis.EmptyFieldSet;
import com.android.tools.r8.ir.analysis.fieldvalueanalysis.UnknownFieldSet;
+import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import java.util.Collections;
import java.util.List;
@@ -163,4 +166,39 @@
assert isFieldPut();
return EmptyFieldSet.getInstance();
}
+
+ /**
+ * 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 5d8fb67..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
@@ -15,16 +15,12 @@
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.errors.Unreachable;
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.ClassInitializationAnalysis;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.AnalysisAssumption;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.Query;
-import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.conversion.CfBuilder;
import com.android.tools.r8.ir.conversion.DexBuilder;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
@@ -132,40 +128,6 @@
return true;
}
- /**
- * Returns {@code true} if this instruction may store a value that has a non-default finalize()
- * method in a static field. In that case, it is not safe to remove this instruction, since that
- * could change the lifetime of the value.
- */
- private boolean isStoringObjectWithFinalizer(AppInfoWithLiveness appInfo) {
- 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().getClassType());
- }
-
- return false;
- }
-
@Override
public boolean canBeDeadCode(AppView<?> appView, IRCode code) {
// static-put can be dead as long as it cannot have any of the following:
