Fix illegal inlining of non-forwarding constructor call to constructor in same class
Bug: 176766039
Change-Id: I6383d317522e33393372707643850fd7c21708ff
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 4534c9b..79f36ad 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -439,7 +439,10 @@
@Override
public boolean canInlineInstanceInitializer(
- IRCode inlinee, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
+ IRCode code,
+ IRCode inlinee,
+ InvokeDirect invoke,
+ WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
// In the Java VM Specification section "4.10.2.4. Instance Initialization Methods and
// Newly Created Objects" it says:
//
@@ -451,9 +454,12 @@
// is expected to adhere to the VM specification.
DexType callerMethodHolder = method.getHolderType();
DexType calleeMethodHolder = inlinee.method().getHolderType();
- // Calling a constructor on the same class from a constructor can always be inlined.
+
+ // Forwarding constructor calls that target a constructor in the same class can always be
+ // inlined.
if (method.getDefinition().isInstanceInitializer()
- && callerMethodHolder == calleeMethodHolder) {
+ && callerMethodHolder == calleeMethodHolder
+ && invoke.getReceiver() == code.getThis()) {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
index eea6b7c..c25755c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ForcedInliningOracle.java
@@ -11,6 +11,7 @@
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.InlineeWithReason;
@@ -107,7 +108,10 @@
@Override
public boolean canInlineInstanceInitializer(
- IRCode code, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
+ IRCode code,
+ IRCode inlinee,
+ InvokeDirect invoke,
+ WhyAreYouNotInliningReporter whyAreYouNotInliningReporter) {
return true;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 96c9234..8f510fb 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -1049,7 +1049,7 @@
assert !singleTargetMethod.isClassInitializer();
if (singleTargetMethod.isInstanceInitializer()
&& !strategy.canInlineInstanceInitializer(
- inlinee.code, whyAreYouNotInliningReporter)) {
+ code, inlinee.code, invoke.asInvokeDirect(), whyAreYouNotInliningReporter)) {
assert whyAreYouNotInliningReporter.unsetReasonHasBeenReportedFlag();
continue;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
index cda0a6b..1dc076f 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningStrategy.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.IRCode;
+import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.optimize.Inliner.InlineAction;
import com.android.tools.r8.ir.optimize.Inliner.InlineeWithReason;
@@ -22,7 +23,10 @@
WhyAreYouNotInliningReporter whyAreYouNotInliningReporter);
boolean canInlineInstanceInitializer(
- IRCode code, WhyAreYouNotInliningReporter whyAreYouNotInliningReporter);
+ IRCode code,
+ IRCode inlinee,
+ InvokeDirect invoke,
+ WhyAreYouNotInliningReporter whyAreYouNotInliningReporter);
/** Return true if there is still budget for inlining into this method. */
boolean stillHasBudget(
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/IllegalInliningOfMergedConstructorTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/IllegalInliningOfMergedConstructorTest.java
new file mode 100644
index 0000000..aad40d0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/IllegalInliningOfMergedConstructorTest.java
@@ -0,0 +1,89 @@
+// Copyright (c) 2021, 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.classmerging.horizontal;
+
+import com.android.tools.r8.NeverClassInline;
+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;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class IllegalInliningOfMergedConstructorTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection params() {
+ return getTestParameters().withAllRuntimesAndApiLevels().withAllApiLevels().build();
+ }
+
+ public IllegalInliningOfMergedConstructorTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addEnumUnboxingInspector(inspector -> inspector.assertUnboxed(Reprocess.class))
+ .addHorizontallyMergedClassesInspector(
+ inspector -> inspector.assertMergedInto(B.class, A.class))
+ .addOptionsModification(options -> options.inliningInstructionLimit = 4)
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("Hello w0rld!");
+ }
+
+ static class Main {
+
+ public static void main(String[] args) {
+ new A((System.currentTimeMillis() > 0 ? Reprocess.A : Reprocess.B)).foo();
+ }
+ }
+
+ @NeverClassInline
+ static class A {
+
+ A(Reprocess r) {
+ new B().foo();
+
+ // This will force this constructor to be reprocessed, which will cause the inliner to attempt
+ // to inline B.<init>() into A.<init>(). Since B.<init>() has been moved to A as a result of
+ // horizontal class merging, the inliner will check if B.<init>() is eligible for inlining,
+ // which it is not in this case, due the assignment of $r8$classId.
+ System.out.print(r.ordinal());
+ }
+
+ @NeverInline
+ void foo() {
+ System.out.println("rld!");
+ }
+ }
+
+ @NeverClassInline
+ static class B {
+
+ public B() {}
+
+ @NeverInline
+ void foo() {
+ System.out.print("Hello w");
+ }
+ }
+
+ enum Reprocess {
+ A,
+ B
+ }
+}
