Fix incorrect instance-get canonicalization
Fixes: b/315877832
Change-Id: I09db1a0e0cb1b873417a1fe55a3ca5d782e77455
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
index c7ddc68..75a3e26 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ConstantCanonicalizer.java
@@ -423,13 +423,14 @@
if (instanceGet.instructionMayHaveSideEffects(appView, context)) {
return false;
}
+ NewInstance newInstance = null;
if (instanceGet.object().isDefinedByInstructionSatisfying(Instruction::isNewInstance)) {
- NewInstance newInstance = instanceGet.object().getDefinition().asNewInstance();
+ newInstance = instanceGet.object().getDefinition().asNewInstance();
if (newInstance.getUniqueConstructorInvoke(appView.dexItemFactory()) == null) {
return false;
}
}
- if (!isReadOfEffectivelyFinalFieldOutsideInitializer(instanceGet)) {
+ if (!isReadOfEffectivelyFinalFieldOutsideInitializer(instanceGet, newInstance)) {
return false;
}
if (getOrComputeIneligibleInstanceGetInstructions().contains(instanceGet)) {
@@ -476,7 +477,12 @@
return true;
}
- private boolean isReadOfEffectivelyFinalFieldOutsideInitializer(FieldGet fieldGet) {
+ private boolean isReadOfEffectivelyFinalFieldOutsideInitializer(StaticGet staticGet) {
+ return isReadOfEffectivelyFinalFieldOutsideInitializer(staticGet, null);
+ }
+
+ private boolean isReadOfEffectivelyFinalFieldOutsideInitializer(
+ FieldGet fieldGet, NewInstance newInstance) {
if (getOrComputeIsAccessingVolatileField()) {
// A final field may be initialized concurrently. A requirement for this is that the field is
// volatile. However, the reading or writing of another volatile field also allows for
@@ -504,6 +510,14 @@
if (!resolvedField.isFinalOrEffectivelyFinal(appViewWithClassHierarchy)) {
return false;
}
+ if (!resolvedField.getAccessFlags().isFinal() && newInstance != null) {
+ // The effectively final property captured in the enqueuer may be invalidated by constructor
+ // inlining (in particular, fields that used only to be written in instance initializers from
+ // the enclosing class may now be written outside such constructors). If we see an
+ // instance-get on a newly created instance, we therefore bail-out since the field may in
+ // principle not be effectively final in this method.
+ return false;
+ }
if (appView.getKeepInfo(resolvedField).isPinned(appView.options())) {
// The final flag could be unset using reflection.
return false;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/EffectivelyFinalInstanceFieldCanonicalizationAfterConstructorInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/EffectivelyFinalInstanceFieldCanonicalizationAfterConstructorInliningTest.java
index 5d80951..d5865d7 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/EffectivelyFinalInstanceFieldCanonicalizationAfterConstructorInliningTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/canonicalization/EffectivelyFinalInstanceFieldCanonicalizationAfterConstructorInliningTest.java
@@ -14,6 +14,7 @@
import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;
+/** Regression test for b/315877832. */
@RunWith(Parameterized.class)
public class EffectivelyFinalInstanceFieldCanonicalizationAfterConstructorInliningTest
extends TestBase {
@@ -35,11 +36,7 @@
.enableNoRedundantFieldLoadEliminationAnnotations()
.setMinApi(parameters)
.run(parameters.getRuntime(), Main.class)
- // TODO(b/315877832): Fix canonicalization.
- .assertSuccessWithOutputLines(
- parameters.canInitNewInstanceUsingSuperclassConstructor()
- ? "null, world!"
- : "Hello, world!");
+ .assertSuccessWithOutputLines("Hello, world!");
}
static class Main {
