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!");
+      }
     }
   }
 }