Don't make assumptions about sideeffects of class initializer in D8
Bug: b/366412380
Change-Id: I8e2c0e31a1734da5574ae0bc98d863a9bbf6e58b
diff --git a/src/main/java/com/android/tools/r8/graph/DexType.java b/src/main/java/com/android/tools/r8/graph/DexType.java
index 1bdd42a..b4e9ea1 100644
--- a/src/main/java/com/android/tools/r8/graph/DexType.java
+++ b/src/main/java/com/android/tools/r8/graph/DexType.java
@@ -173,8 +173,16 @@
public boolean classInitializationMayHaveSideEffectsInContext(
AppView<?> appView, ProgramDefinition context) {
+ // To ensure that we assume that there are no class initialization side effects directly on
+ // the super class in D8 we always return true if this is a new instance of a direct super type.
+ if (context.getContextClass().getSuperType().isIdenticalTo(this)
+ || context.getContextType().isIdenticalTo(this)) {
+ return false;
+ }
DexClass clazz = appView.definitionFor(this);
- return clazz == null || clazz.classInitializationMayHaveSideEffectsInContext(appView, context);
+ return clazz == null
+ || !appView.enableWholeProgramOptimizations()
+ || clazz.classInitializationMayHaveSideEffectsInContext(appView, context);
}
final boolean internalClassOrInterfaceMayHaveInitializationSideEffects(
diff --git a/src/test/java/com/android/tools/r8/regress/Regress366412380.java b/src/test/java/com/android/tools/r8/regress/Regress366412380.java
new file mode 100644
index 0000000..96e7bf0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/Regress366412380.java
@@ -0,0 +1,83 @@
+// Copyright (c) 2024, 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.regress;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+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.InstructionSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class Regress366412380 extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDefaultRuntimes().withAllApiLevels().build();
+ }
+
+ public Regress366412380(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ testForD8(parameters.getBackend())
+ .addInnerClasses(Regress366412380.class)
+ .release()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ // Ensure that we don't redundant field load eliminate the instance get of foo.
+ // In D8 we are not allowed to make assumptions about side effects of new instance.
+ assertTrue(
+ codeInspector
+ .clazz(TestClassNoSuper.class)
+ .uniqueMethodWithOriginalName("useFoo")
+ .iterateInstructions(InstructionSubject::isInstanceGet)
+ .hasNext());
+ if (parameters.isDexRuntime()) {
+ assertFalse(
+ codeInspector
+ .clazz(TestClassExtendsBar.class)
+ .uniqueMethodWithOriginalName("useFoo")
+ .iterateInstructions(InstructionSubject::isInstanceGet)
+ .hasNext());
+ }
+ });
+ }
+
+ public static class Foo {}
+
+ public static class Bar {
+ public Bar() {}
+
+ public Bar(Foo foo) {}
+ }
+
+ public static class TestClassExtendsBar extends Bar {
+ protected Foo foo;
+
+ public void useFoo() {
+ foo = new Foo();
+ new Bar(foo);
+ }
+ }
+
+ public static class TestClassNoSuper {
+ protected Foo foo;
+
+ public void useFoo() {
+ foo = new Foo();
+ new Bar(foo);
+ }
+ }
+}
