Do not optimize append indexing into char[] or CharSequence
Current analysis did not take into account that these methods might throw.
Fixes: b/369739224
Bug: b/369971265
Change-Id: Ied98075962ffd01ec1203bae4d934d00b4c43bf7
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 8f6be1b..d1b86f0 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -2475,7 +2475,7 @@
}
@SuppressWarnings("ReferenceEquality")
- public boolean isAppendSubArrayMethod(DexMethod method) {
+ public boolean isAppendSubArrayOrSubCharSequenceMethod(DexMethod method) {
return appendSubCharArray == method || appendSubCharSequence == method;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java
index 2c43b60..d8569d7 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/string/StringBuilderOracle.java
@@ -12,7 +12,6 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InvokeDirect;
-import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.Value;
import java.util.List;
@@ -121,7 +120,8 @@
return null;
}
if (isAppend(instruction)
- && !isAppendWithSubArray(instruction.asInvokeMethodWithReceiver())) {
+ && !isAppendWithSubArrayOrSubCharSequence(
+ instruction.asInvokeMethodWithReceiver().getInvokedMethod())) {
return getConstantStringForAppend(instruction.asInvokeVirtual());
} else if (isInit(instruction)) {
return getConstantStringForInit(instruction.asInvokeDirect());
@@ -178,14 +178,18 @@
return false;
}
DexMethod invokedMethod = instruction.asInvokeMethod().getInvokedMethod();
+ if (isAppendWithSubArrayOrSubCharSequence(invokedMethod)) {
+ // Do not optimize append indexing into char[] or CharSequence. These might throw without
+ // additional analysis on the arguments. See b/369739224 and b/369971265 for details.
+ return false;
+ }
return factory.stringBuilderMethods.isAppendMethod(invokedMethod)
|| factory.stringBufferMethods.isAppendMethod(invokedMethod);
}
- public boolean isAppendWithSubArray(InvokeMethodWithReceiver instruction) {
- DexMethod invokedMethod = instruction.asInvokeMethod().getInvokedMethod();
- return factory.stringBuilderMethods.isAppendSubArrayMethod(invokedMethod)
- || factory.stringBufferMethods.isAppendSubArrayMethod(invokedMethod);
+ public boolean isAppendWithSubArrayOrSubCharSequence(DexMethod invokedMethod) {
+ return factory.stringBuilderMethods.isAppendSubArrayOrSubCharSequenceMethod(invokedMethod)
+ || factory.stringBufferMethods.isAppendSubArrayOrSubCharSequenceMethod(invokedMethod);
}
@Override
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/string/B369739224Test.java b/src/test/java/com/android/tools/r8/ir/optimize/string/B369739224Test.java
index 615ba60..463e20b 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/string/B369739224Test.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/string/B369739224Test.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.StringUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -15,6 +16,9 @@
@RunWith(Parameterized.class)
public class B369739224Test extends TestBase {
+ private final String EXPECTED_OUTPUT =
+ StringUtils.lines("Caught!", "Caught!", "Caught!", "Caught!");
+
@Parameter(0)
public TestParameters parameters;
@@ -29,7 +33,7 @@
testForJvm(parameters)
.addInnerClasses(getClass())
.run(parameters.getRuntime(), TestClass.class)
- .assertFailureWithErrorThatThrows(IndexOutOfBoundsException.class);
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
}
@Test
@@ -37,9 +41,9 @@
parameters.assumeDexRuntime();
testForD8(parameters.getBackend())
.addInnerClasses(getClass())
- .setMinApi(parameters.getApiLevel())
+ .setMinApi(parameters)
.run(parameters.getRuntime(), TestClass.class)
- .assertFailureWithErrorThatThrows(IndexOutOfBoundsException.class);
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
}
@Test
@@ -49,17 +53,35 @@
.addKeepMainRule(TestClass.class)
.setMinApi(parameters)
.run(parameters.getRuntime(), TestClass.class)
- // TODO(b/369739224): Should throw IndexOutOfBoundsException.
- .assertSuccessWithOutputLines("46");
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
}
static class TestClass {
public static void main(String[] args) {
- String f = "";
+ String s = "";
+ char[] a = new char[0];
int c = '.';
- new StringBuilder().append(f, 0, c);
- System.out.println(c);
+ try {
+ new StringBuilder().append(s, 0, c);
+ } catch (IndexOutOfBoundsException e) {
+ System.out.println("Caught!");
+ }
+ try {
+ new StringBuffer().append(s, 0, c);
+ } catch (IndexOutOfBoundsException e) {
+ System.out.println("Caught!");
+ }
+ try {
+ new StringBuilder().append(a, 0, c);
+ } catch (IndexOutOfBoundsException e) {
+ System.out.println("Caught!");
+ }
+ try {
+ new StringBuffer().append(a, 0, c);
+ } catch (IndexOutOfBoundsException e) {
+ System.out.println("Caught!");
+ }
}
}
}