Fix incorrect field value propagation
Bug: b/307987907
Change-Id: I84e8b52e4e63b26da9220dd91b2ea983146b3ff3
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java b/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java
index 0f47d7e..559090b 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/fieldaccess/FieldAssignmentTracker.java
@@ -54,6 +54,7 @@
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
@@ -80,6 +81,9 @@
private final Map<DexProgramClass, ProgramFieldMap<AbstractValue>>
abstractFinalInstanceFieldValues = new ConcurrentHashMap<>();
+ private final Set<DexProgramClass> classesWithPrunedInstanceInitializers =
+ ConcurrentHashMap.newKeySet();
+
FieldAssignmentTracker(AppView<AppInfoWithLiveness> appView) {
this.abstractValueFactory = appView.abstractValueFactory();
this.appView = appView;
@@ -356,41 +360,49 @@
private void recordAllInstanceFieldPutsProcessed(
ProgramField field, OptimizationFeedbackDelayed feedback) {
- if (appView.appInfo().isInstanceFieldWrittenOnlyInInstanceInitializers(field)) {
- AbstractValue abstractValue = BottomValue.getInstance();
- DexProgramClass clazz = field.getHolder();
- for (DexEncodedMethod method : clazz.directMethods(DexEncodedMethod::isInstanceInitializer)) {
- InstanceFieldInitializationInfo fieldInitializationInfo =
- method
- .getOptimizationInfo()
- .getContextInsensitiveInstanceInitializerInfo()
- .fieldInitializationInfos()
- .get(field);
- if (fieldInitializationInfo.isSingleValue()) {
- abstractValue =
- appView
- .getAbstractValueFieldJoiner()
- .join(abstractValue, fieldInitializationInfo.asSingleValue(), field);
- if (abstractValue.isUnknown()) {
- break;
- }
- } else if (fieldInitializationInfo.isTypeInitializationInfo()) {
- // TODO(b/149732532): Not handled, for now.
- abstractValue = UnknownValue.getInstance();
- break;
- } else {
- assert fieldInitializationInfo.isArgumentInitializationInfo()
- || fieldInitializationInfo.isUnknown();
- abstractValue = UnknownValue.getInstance();
+ if (!appView.appInfo().isInstanceFieldWrittenOnlyInInstanceInitializers(field)) {
+ return;
+ }
+ DexProgramClass clazz = field.getHolder();
+ if (classesWithPrunedInstanceInitializers.contains(clazz)) {
+ // The current method is analyzing the instance-puts to the field in the instance initializers
+ // of the holder class. Due to single caller inlining of instance initializers some of the
+ // methods needed for the analysis may have been pruned from the app, in which case the
+ // analysis result will not be conservative.
+ return;
+ }
+ AbstractValue abstractValue = BottomValue.getInstance();
+ for (DexEncodedMethod method : clazz.directMethods(DexEncodedMethod::isInstanceInitializer)) {
+ InstanceFieldInitializationInfo fieldInitializationInfo =
+ method
+ .getOptimizationInfo()
+ .getContextInsensitiveInstanceInitializerInfo()
+ .fieldInitializationInfos()
+ .get(field);
+ if (fieldInitializationInfo.isSingleValue()) {
+ abstractValue =
+ appView
+ .getAbstractValueFieldJoiner()
+ .join(abstractValue, fieldInitializationInfo.asSingleValue(), field);
+ if (abstractValue.isUnknown()) {
break;
}
+ } else if (fieldInitializationInfo.isTypeInitializationInfo()) {
+ // TODO(b/149732532): Not handled, for now.
+ abstractValue = UnknownValue.getInstance();
+ break;
+ } else {
+ assert fieldInitializationInfo.isArgumentInitializationInfo()
+ || fieldInitializationInfo.isUnknown();
+ abstractValue = UnknownValue.getInstance();
+ break;
}
+ }
- assert !abstractValue.isBottom();
+ assert !abstractValue.isBottom();
- if (!abstractValue.isUnknown()) {
- feedback.recordFieldHasAbstractValue(field.getDefinition(), appView, abstractValue);
- }
+ if (!abstractValue.isUnknown()) {
+ feedback.recordFieldHasAbstractValue(field.getDefinition(), appView, abstractValue);
}
}
@@ -417,6 +429,16 @@
});
}
+ public void onMethodPruned(ProgramMethod method) {
+ onMethodCodePruned(method);
+ }
+
+ public void onMethodCodePruned(ProgramMethod method) {
+ if (method.getDefinition().isInstanceInitializer()) {
+ classesWithPrunedInstanceInitializers.add(method.getHolder());
+ }
+ }
+
public void waveDone(ProgramMethodSet wave, OptimizationFeedbackDelayed feedback) {
// This relies on the instance initializer info in the method optimization feedback. It is
// therefore important that the optimization info has been flushed in advance.
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 a7cd093..29fdae5 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
@@ -1169,6 +1169,7 @@
assert method.getHolder().lookupMethod(method.getReference()) == null;
appView.withArgumentPropagator(argumentPropagator -> argumentPropagator.onMethodPruned(method));
enumUnboxer.onMethodPruned(method);
+ fieldAccessAnalysis.fieldAssignmentTracker().onMethodPruned(method);
outliner.onMethodPruned(method);
if (inliner != null) {
inliner.onMethodPruned(method);
@@ -1186,6 +1187,7 @@
appView.withArgumentPropagator(
argumentPropagator -> argumentPropagator.onMethodCodePruned(method));
enumUnboxer.onMethodCodePruned(method);
+ fieldAccessAnalysis.fieldAssignmentTracker().onMethodCodePruned(method);
outliner.onMethodCodePruned(method);
if (inliner != null) {
inliner.onMethodCodePruned(method);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldsOnlyWrittenInInitializersWithSingleCallerPruningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldsOnlyWrittenInInitializersWithSingleCallerPruningTest.java
index 82aaa87..250dd0b 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldsOnlyWrittenInInitializersWithSingleCallerPruningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/membervaluepropagation/InstanceFieldsOnlyWrittenInInitializersWithSingleCallerPruningTest.java
@@ -62,8 +62,7 @@
.enableInliningAnnotations()
.setMinApi(parameters)
.run(parameters.getRuntime(), Main.class)
- // TODO(b/307987907): Should succeed with "Hello, world!".
- .assertSuccessWithOutputLines("HelloHello");
+ .assertSuccessWithOutputLines("Hello, world!");
}
static class Main {
