Don't shorten live ranges of const strings when there are monitors
This can cause issues if the code is inlined and new catch handlers
are attached to instructions that we already deemed non throwing
Bug: 151964517
Change-Id: I91f1140a461a5be3ec2288fb3d858ccc32e51e92
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 7f439de..0521d25 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -1826,6 +1826,15 @@
if (!next.isConstInstruction()) {
continue;
}
+
+ // We don't want to push const string instructions down into code that has monitors since
+ // we may attach catch handlers that are not catch-all when inlining. This is symmetric in how
+ // we don't do const string canonicalization.
+ if ((next.isConstString() || next.isDexItemBasedConstString())
+ && code.metadata().mayHaveMonitorInstruction()) {
+ continue;
+ }
+
ConstInstruction instruction = next.asConstInstruction();
if (!selector.test(instruction) || instruction.outValue().hasLocalInfo()) {
continue;
diff --git a/src/test/java/com/android/tools/r8/regress/b151964517/ConstStringWithMonitorTest.java b/src/test/java/com/android/tools/r8/regress/b151964517/ConstStringWithMonitorTest.java
index 7c8aa34..c37953b 100644
--- a/src/test/java/com/android/tools/r8/regress/b151964517/ConstStringWithMonitorTest.java
+++ b/src/test/java/com/android/tools/r8/regress/b151964517/ConstStringWithMonitorTest.java
@@ -16,7 +16,7 @@
@Parameterized.Parameters(name = "{0}")
public static TestParametersCollection data() {
- return getTestParameters().withDexRuntimes().build();
+ return getTestParameters().withAllRuntimes().withAllApiLevels().build();
}
public ConstStringWithMonitorTest(TestParameters parameters) {
@@ -28,11 +28,12 @@
testForR8(parameters.getBackend())
.addInnerClasses(ConstStringWithMonitorTest.class)
.noMinification()
+ .setMinApi(parameters.getApiLevel())
.allowAccessModification()
.addKeepMainRule(TestClass.class)
.compile()
- .runDex2Oat(parameters.getRuntime());
- // TODO(b/151964517): Should pass verification with assertNoVerificationErrors()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("foobar");
}
public static class TestClass {
