Don't inline into constructors when producing class files
This can lead to verification errors when the inlined code has control flow jumping over the super constructor call
Bug: 136250031
Change-Id: I67493805967237aa95a614d510f0f88df57b3bff
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 ff50f91..89adfc7 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
@@ -171,6 +171,14 @@
return false;
}
+ // We don't inline into constructors when producing class files since this can mess up
+ // the stackmap, see b/136250031
+ if (method.isInitializer()
+ && appView.options().isGeneratingClassFiles()
+ && reason != Reason.FORCE) {
+ return false;
+ }
+
if (method == candidate) {
// Cannot handle recursive inlining at this point.
// Force inlined method should never be recursive.
diff --git a/src/test/examples/inlining/CheckDiscardedConstructor.java b/src/test/examples/inlining/CheckDiscardedConstructor.java
new file mode 100644
index 0000000..d7be28c
--- /dev/null
+++ b/src/test/examples/inlining/CheckDiscardedConstructor.java
@@ -0,0 +1,8 @@
+// Copyright (c) 2019, 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 inlining;
+
+public @interface CheckDiscardedConstructor {
+
+}
diff --git a/src/test/examples/inlining/InlineConstructorFinalField.java b/src/test/examples/inlining/InlineConstructorFinalField.java
index 093a2ac..a23c3dc 100644
--- a/src/test/examples/inlining/InlineConstructorFinalField.java
+++ b/src/test/examples/inlining/InlineConstructorFinalField.java
@@ -7,7 +7,7 @@
public final int number;
- @CheckDiscarded
+ @CheckDiscardedConstructor
InlineConstructorFinalField(int value) {
number = value;
}
diff --git a/src/test/examples/inlining/InlineConstructorOfInner.java b/src/test/examples/inlining/InlineConstructorOfInner.java
index cec7d3f..6bc8abb 100644
--- a/src/test/examples/inlining/InlineConstructorOfInner.java
+++ b/src/test/examples/inlining/InlineConstructorOfInner.java
@@ -9,7 +9,7 @@
int a;
- @CheckDiscarded
+ @CheckDiscardedConstructor
Inner(int a) {
this.a = a;
}
diff --git a/src/test/examples/inlining/keep-rules-discard-constructor.txt b/src/test/examples/inlining/keep-rules-discard-constructor.txt
new file mode 100644
index 0000000..fab5f5d
--- /dev/null
+++ b/src/test/examples/inlining/keep-rules-discard-constructor.txt
@@ -0,0 +1,8 @@
+# Copyright (c) 2019, 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.
+
+# Check that constructors have been inlined
+-checkdiscard class * {
+ @inlining.CheckDiscardedConstructor *;
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/Regress136250031.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/Regress136250031.java
new file mode 100644
index 0000000..fbc405e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/Regress136250031.java
@@ -0,0 +1,52 @@
+// Copyright (c) 2019, 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.inliner;
+
+import com.android.tools.r8.TestBase;
+import org.junit.Test;
+
+public class Regress136250031 extends TestBase {
+
+ @Test
+ public void test() throws Exception {
+ testForR8(Backend.CF)
+ .addInnerClasses(Regress136250031.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepClassAndMembersRules(B.class)
+ .run(TestClass.class)
+ .assertSuccess();
+ }
+
+ static class TestClass {
+ public static void main(String[] args) {
+ new B(new C());
+ }
+ }
+
+ static class A {
+ A(String s) {
+ System.out.println(s);
+ }
+ }
+
+ static class B extends A {
+ B(C c) {
+ super(c.instance.toString());
+ }
+ }
+
+ static class C {
+ public C instance;
+
+ C() {
+ instance = System.currentTimeMillis() > 0 ? this : null;
+ }
+
+ @Override
+ public String toString() {
+ return "42";
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/regress/b69825683/innerconstructsouter/Outer.java b/src/test/java/com/android/tools/r8/regress/b69825683/innerconstructsouter/Outer.java
index 78d9d05..8be969a 100644
--- a/src/test/java/com/android/tools/r8/regress/b69825683/innerconstructsouter/Outer.java
+++ b/src/test/java/com/android/tools/r8/regress/b69825683/innerconstructsouter/Outer.java
@@ -23,7 +23,7 @@
Inner builder = new Inner();
builder.build();
for (java.lang.reflect.Constructor m : Outer.class.getDeclaredConstructors()) {
- System.out.println(m);
+ System.out.println(m.getName());
}
}
}
diff --git a/src/test/java/com/android/tools/r8/regress/b69825683/outerconstructsinner/Outer.java b/src/test/java/com/android/tools/r8/regress/b69825683/outerconstructsinner/Outer.java
index 3222964..dfaf6f9 100644
--- a/src/test/java/com/android/tools/r8/regress/b69825683/outerconstructsinner/Outer.java
+++ b/src/test/java/com/android/tools/r8/regress/b69825683/outerconstructsinner/Outer.java
@@ -22,7 +22,7 @@
public static void main(String args[]) {
new Outer();
for (java.lang.reflect.Constructor m : Outer.Inner.class.getDeclaredConstructors()) {
- System.out.println(m);
+ System.out.println(m.getName());
}
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingInliningTest.java b/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingInliningTest.java
index 2a53f1b..a6fb153 100644
--- a/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingInliningTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/examples/TreeShakingInliningTest.java
@@ -38,7 +38,12 @@
@Test
public void testKeeprulesdiscard() throws Exception {
+ // On the cf backend, we don't inline into constructors, see: b/136250031
+ List<String> keepRules = getBackend() == Backend.CF
+ ? ImmutableList.of("src/test/examples/inlining/keep-rules-discard.txt")
+ : ImmutableList.of("src/test/examples/inlining/keep-rules-discard.txt",
+ "src/test/examples/inlining/keep-rules-discard-constructor.txt");
runTest(
- null, null, null, ImmutableList.of("src/test/examples/inlining/keep-rules-discard.txt"));
+ null, null, null, keepRules);
}
}
