Version 2.1.59
Cherry pick: Fix class inliner error when singleton instance flows to static invoke
CL: https://r8-review.googlesource.com/c/r8/+/52741/
Cherry pick: Reproduce class inliner bug when singleton instance follows to static invoke
CL: https://r8-review.googlesource.com/c/r8/+/52740/
Cherry pick: Reproduce class inliner bug that incorrectly removed global side effect
CL: https://r8-review.googlesource.com/c/r8/+/52671/
Cherry pick: Restrict class inliner to maintain visible side-effects.
CL: https://r8-review.googlesource.com/c/r8/+/52600
Bug: 160942326
Change-Id: Ie4262245fa2e2c0feb14dab5d3192497b375a734
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 9a30ef1..ea76867 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 = "2.1.58";
+ public static final String LABEL = "2.1.59";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerEligibilityInfo.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerEligibilityInfo.java
index 8d178a3..35eaaf5 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerEligibilityInfo.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerEligibilityInfo.java
@@ -22,13 +22,16 @@
final OptionalBool returnsReceiver;
final boolean hasMonitorOnReceiver;
+ final boolean modifiesInstanceFields;
public ClassInlinerEligibilityInfo(
List<Pair<Invoke.Type, DexMethod>> callsReceiver,
OptionalBool returnsReceiver,
- boolean hasMonitorOnReceiver) {
+ boolean hasMonitorOnReceiver,
+ boolean modifiesInstanceFields) {
this.callsReceiver = callsReceiver;
this.returnsReceiver = returnsReceiver;
this.hasMonitorOnReceiver = hasMonitorOnReceiver;
+ this.modifiesInstanceFields = modifiesInstanceFields;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index 74d07d2..aa207e8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -1048,6 +1048,12 @@
// We will not be able to remove the monitor instruction afterwards.
return false;
}
+ if (eligibility.modifiesInstanceFields) {
+ // The static instance could be accessed from elsewhere. Therefore, we cannot
+ // allow side-effects to be removed and therefore cannot class inline method
+ // calls that modifies the instance.
+ return false;
+ }
}
// If the method returns receiver and the return value is actually
@@ -1198,9 +1204,10 @@
}
if (root.isStaticGet()) {
- // If we are class inlining a singleton instance from a static-get, then we don't the value of
- // the fields.
- if (parameterUsage.hasFieldRead) {
+ // If we are class inlining a singleton instance from a static-get, then we don't know
+ // the value of the fields, and we also can't optimize away instance-field assignments, as
+ // they have global side effects.
+ if (parameterUsage.hasFieldAssignment || parameterUsage.hasFieldRead) {
return false;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
index 13f850e..ceb1c0b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/info/MethodOptimizationInfoCollector.java
@@ -191,6 +191,7 @@
List<Pair<Invoke.Type, DexMethod>> callsReceiver = new ArrayList<>();
boolean seenSuperInitCall = false;
boolean seenMonitor = false;
+ boolean modifiesInstanceFields = false;
AliasedValueConfiguration configuration =
AssumeAndCheckCastAliasedValueConfiguration.getInstance();
@@ -219,6 +220,7 @@
if (isReceiverAlias.test(instancePutInstruction.value())) {
return;
}
+ modifiesInstanceFields = true;
}
DexField field = insn.asFieldInstruction().getField();
if (appView.appInfo().resolveField(field).isFailedOrUnknownResolution()) {
@@ -292,7 +294,8 @@
new ClassInlinerEligibilityInfo(
callsReceiver,
new ClassInlinerReceiverAnalysis(appView, definition, code).computeReturnsReceiver(),
- seenMonitor || synchronizedVirtualMethod));
+ seenMonitor || synchronizedVirtualMethod,
+ modifiesInstanceFields));
}
private void identifyParameterUsages(
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/StatefulSingletonClassInliningTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/StatefulSingletonClassInliningTest.java
new file mode 100644
index 0000000..17ccc82
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/StatefulSingletonClassInliningTest.java
@@ -0,0 +1,71 @@
+// Copyright (c) 2020, 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.optimize.classinliner;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class StatefulSingletonClassInliningTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public StatefulSingletonClassInliningTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(StatefulSingletonClassInliningTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("true");
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ State.get().set();
+ State.get().print();
+ }
+ }
+
+ static class State {
+
+ static StatefulSingleton SINGLETON = new StatefulSingleton();
+
+ static StatefulSingleton get() {
+ return SINGLETON;
+ }
+ }
+
+ static class StatefulSingleton {
+
+ boolean field;
+
+ @NeverInline
+ void set() {
+ field = true;
+ }
+
+ void print() {
+ System.out.println(field);
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/StatefulSingletonClassInliningWithStaticEscapeTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/StatefulSingletonClassInliningWithStaticEscapeTest.java
new file mode 100644
index 0000000..4b35783
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/StatefulSingletonClassInliningWithStaticEscapeTest.java
@@ -0,0 +1,71 @@
+// Copyright (c) 2020, 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.optimize.classinliner;
+
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class StatefulSingletonClassInliningWithStaticEscapeTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public StatefulSingletonClassInliningWithStaticEscapeTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(StatefulSingletonClassInliningWithStaticEscapeTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("true");
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ State.set(State.get());
+ State.get().print();
+ }
+ }
+
+ static class State {
+
+ static StatefulSingleton SINGLETON = new StatefulSingleton();
+
+ static StatefulSingleton get() {
+ return SINGLETON;
+ }
+
+ @NeverInline
+ static void set(StatefulSingleton state) {
+ state.field = true;
+ }
+ }
+
+ static class StatefulSingleton {
+
+ boolean field;
+
+ void print() {
+ System.out.println(field);
+ }
+ }
+}
