Version 1.4.25

Cherry pick: Correctly mark and treat FillArrayData as a throwing instruction.
CL: https://r8-review.googlesource.com/c/r8/+/33037
Bug: 122887884

Cherry pick: Allow referencing graph nodes that are removed during tree shaking.
CL: https://r8-review.googlesource.com/c/r8/+/32980
Bug: 120959039

Change-Id: Ic06dcf651f49f2c3fa7860155e1948ca43e7c95f
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 4336ca5..6c27fb1 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
 
   // This field is accessed from release scripts using simple pattern matching.
   // Therefore, changing this field could break our release scripts.
-  public static final String LABEL = "1.4.24";
+  public static final String LABEL = "1.4.25";
 
   private Version() {
   }
diff --git a/src/main/java/com/android/tools/r8/code/FillArrayData.java b/src/main/java/com/android/tools/r8/code/FillArrayData.java
index 4569897..a49a31d 100644
--- a/src/main/java/com/android/tools/r8/code/FillArrayData.java
+++ b/src/main/java/com/android/tools/r8/code/FillArrayData.java
@@ -44,4 +44,9 @@
   public String toSmaliString(ClassNameMapper naming) {
     return formatSmaliString("v" + AA + ", :label_" + (getOffset() + BBBBBBBB));
   }
+
+  @Override
+  public boolean canThrow() {
+    return true;
+  }
 }
diff --git a/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java b/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
index de0f48c..ed51913 100644
--- a/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
+++ b/src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
@@ -8,7 +8,6 @@
 import com.android.tools.r8.code.FillArrayDataPayload;
 import com.android.tools.r8.dex.Constants;
 import com.android.tools.r8.errors.Unreachable;
-import com.android.tools.r8.graph.AppInfo;
 import com.android.tools.r8.graph.DexType;
 import com.android.tools.r8.ir.conversion.CfBuilder;
 import com.android.tools.r8.ir.conversion.DexBuilder;
@@ -74,9 +73,8 @@
   }
 
   @Override
-  public boolean canBeDeadCode(AppInfo appInfo, IRCode code) {
-    // Side-effects its input values.
-    return false;
+  public boolean instructionTypeCanThrow() {
+    return true;
   }
 
   @Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
index 24487aa..6acd9b1 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
@@ -355,6 +355,10 @@
       return index;
     }
     if (dex.canThrow()) {
+      // TODO(zerny): Remove this from block computation.
+      if (dex.hasPayload()) {
+        arrayFilledDataPayloadResolver.addPayloadUser((FillArrayData) dex);
+      }
       // If the instruction can throw and is in a try block, add edges to its catch successors.
       Try tryRange = getTryForOffset(offset);
       if (tryRange != null) {
@@ -397,10 +401,6 @@
       builder.ensureNormalSuccessorBlock(offset, offset + dex.getSize());
       return index;
     }
-    // TODO(zerny): Remove this from block computation.
-    if (dex.hasPayload()) {
-      arrayFilledDataPayloadResolver.addPayloadUser((FillArrayData) dex);
-    }
     // This instruction does not close the block.
     return -1;
   }
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 835399c..4c9b15e 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -1646,9 +1646,11 @@
   }
 
   public void addNewArrayFilledData(int arrayRef, int elementWidth, long size, short[] data) {
-    add(
+    NewArrayFilledData instruction =
         new NewArrayFilledData(
-            readRegister(arrayRef, ValueTypeConstraint.OBJECT), elementWidth, size, data));
+            readRegister(arrayRef, ValueTypeConstraint.OBJECT), elementWidth, size, data);
+    assert instruction.instructionTypeCanThrow();
+    addInstruction(instruction);
   }
 
   public void addNewInstance(int dest, DexType type) {
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 b1598bc..6a7e6ba 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
@@ -2697,6 +2697,9 @@
           if (contents == null) {
             continue;
           }
+          if (block.hasCatchHandlers()) {
+            continue;
+          }
           int arraySize = newArray.size().getConstInstruction().asConstNumber().getIntValue();
           NewArrayFilledData fillArray =
               new NewArrayFilledData(newArray.outValue(), elementSize, arraySize, contents);
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index f10d000..3376854 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2838,23 +2838,27 @@
   }
 
   ClassGraphNode getClassGraphNode(DexType type) {
-    return classNodes.computeIfAbsent(type,
-        t -> new ClassGraphNode(
-            appInfo.definitionFor(type).isLibraryClass(),
-            Reference.classFromDescriptor(t.toDescriptorString())));
+    return classNodes.computeIfAbsent(
+        type,
+        t -> {
+          DexClass definition = appInfo.definitionFor(t);
+          return new ClassGraphNode(
+              definition != null && definition.isLibraryClass(),
+              Reference.classFromDescriptor(t.toDescriptorString()));
+        });
   }
 
   MethodGraphNode getMethodGraphNode(DexMethod context) {
     return methodNodes.computeIfAbsent(
         context,
         m -> {
-          boolean isLibraryNode = appInfo.definitionFor(context.holder).isLibraryClass();
+          DexClass holderDefinition = appInfo.definitionFor(context.holder);
           Builder<TypeReference> builder = ImmutableList.builder();
           for (DexType param : m.proto.parameters.values) {
             builder.add(Reference.typeFromDescriptor(param.toDescriptorString()));
           }
           return new MethodGraphNode(
-              isLibraryNode,
+              holderDefinition != null && holderDefinition.isLibraryClass(),
               Reference.method(
                   Reference.classFromDescriptor(m.holder.toDescriptorString()),
                   m.name.toString(),
@@ -2868,13 +2872,15 @@
   FieldGraphNode getFieldGraphNode(DexField context) {
     return fieldNodes.computeIfAbsent(
         context,
-        f ->
-            new FieldGraphNode(
-                appInfo.definitionFor(context.getHolder()).isLibraryClass(),
-                Reference.field(
-                    Reference.classFromDescriptor(f.getHolder().toDescriptorString()),
-                    f.name.toString(),
-                    Reference.typeFromDescriptor(f.type.toDescriptorString()))));
+        f -> {
+          DexClass holderDefinition = appInfo.definitionFor(context.getHolder());
+          return new FieldGraphNode(
+              holderDefinition != null && holderDefinition.isLibraryClass(),
+              Reference.field(
+                  Reference.classFromDescriptor(f.getHolder().toDescriptorString()),
+                  f.name.toString(),
+                  Reference.typeFromDescriptor(f.type.toDescriptorString())));
+        });
   }
 
   KeepRuleGraphNode getKeepRuleGraphNode(ProguardKeepRule rule) {
diff --git a/src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884.java b/src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884.java
new file mode 100644
index 0000000..23650c9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884.java
@@ -0,0 +1,28 @@
+// 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.regress.b122887884;
+
+public class Regress122887884 {
+
+  public static int[] foo(String[] args) {
+    if (args.length == 0) {
+      return new int[] {0, 0};
+    }
+    String first = args[0];
+    try {
+      String[] split = first.split("");
+      if (split.length != 2) {
+        // This results in a new-array v2 v2 int[] at which point the exception handler must split.
+        return new int[] {0, 0};
+      }
+      return new int[] {1, 1};
+    } catch (Throwable t) {
+      return new int[] {0, 0};
+    }
+  }
+
+  public static void main(String[] args) {
+    System.out.println(foo(args)[0]);
+  }
+}
diff --git a/src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884Runner.java b/src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884Runner.java
new file mode 100644
index 0000000..54f32fd
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884Runner.java
@@ -0,0 +1,19 @@
+// 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.regress.b122887884;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+
+public class Regress122887884Runner extends TestBase {
+
+  private final Class<?> CLASS = Regress122887884.class;
+  private final String EXPECTED = StringUtils.lines("0");
+
+  @Test
+  public void test() throws Exception {
+    testForD8().addProgramClasses(CLASS).run(CLASS).assertSuccessWithOutput(EXPECTED);
+  }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTest.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTest.java
new file mode 100644
index 0000000..5992eff
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTest.java
@@ -0,0 +1,25 @@
+// 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.shaking.keptgraph;
+
+import com.android.tools.r8.NeverInline;
+
+public class RemovedClassTest {
+
+  public static class RemovedInnerClass {}
+
+  public static void main(String[] args) {
+    bar();
+  }
+
+  @NeverInline
+  public static void bar() {
+    System.out.println("called bar");
+  }
+
+  @NeverInline
+  public static void baz() {
+    System.out.println("called baz, created " + new RemovedClassTest().getClass().getSimpleName());
+  }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTestRunner.java b/src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTestRunner.java
new file mode 100644
index 0000000..c555b7e
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTestRunner.java
@@ -0,0 +1,84 @@
+// 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.shaking.keptgraph;
+
+import static com.android.tools.r8.references.Reference.classFromClass;
+import static com.android.tools.r8.references.Reference.methodFromMethod;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.graphinspector.GraphInspector;
+import com.android.tools.r8.utils.graphinspector.GraphInspector.QueryNode;
+import com.google.common.collect.ImmutableList;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+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 RemovedClassTestRunner extends TestBase {
+
+  private static final Class<?> CLASS = RemovedClassTest.class;
+  private static final Class<?> REMOVED_CLASS = RemovedClassTest.RemovedInnerClass.class;
+  private static final List<Class<?>> CLASSES = ImmutableList.of(CLASS, REMOVED_CLASS);
+
+  private static final String EXPECTED = StringUtils.lines("called bar");
+
+  private final Backend backend;
+
+  @Parameters(name = "{0}")
+  public static Backend[] data() {
+    return Backend.values();
+  }
+
+  public RemovedClassTestRunner(Backend backend) {
+    this.backend = backend;
+  }
+
+  @Test
+  public void testRemovedClass() throws Exception {
+    MethodReference mainMethod = methodFromMethod(CLASS.getDeclaredMethod("main", String[].class));
+    MethodReference barMethod = methodFromMethod(CLASS.getDeclaredMethod("bar"));
+    MethodReference bazMethod = methodFromMethod(CLASS.getDeclaredMethod("baz"));
+
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    R8TestCompileResult compileResult =
+        testForR8(backend)
+            .enableGraphInspector()
+            .enableInliningAnnotations()
+            .addProgramClasses(CLASSES)
+            .addKeepMethodRules(mainMethod)
+            .addKeepRules("-whyareyoukeeping class " + REMOVED_CLASS.getTypeName())
+            .redirectStdOut(new PrintStream(baos))
+            .compile();
+    String expectedOutput = StringUtils.lines("Nothing is keeping " + REMOVED_CLASS.getTypeName());
+    String compileOutput = new String(baos.toByteArray(), StandardCharsets.UTF_8);
+    assertEquals(expectedOutput, compileOutput);
+
+    GraphInspector inspector =
+        compileResult.run(CLASS).assertSuccessWithOutput(EXPECTED).graphInspector();
+
+    // The only root should be the keep main-method rule.
+    assertEquals(1, inspector.getRoots().size());
+    QueryNode root = inspector.rule(Origin.unknown(), 1, 1).assertRoot();
+
+    // Check that the call chain goes from root -> main(unchanged) -> bar(renamed).
+    inspector.method(barMethod).assertRenamed().assertInvokedFrom(mainMethod);
+    inspector.method(mainMethod).assertNotRenamed().assertKeptBy(root);
+
+    // Check baz is removed.
+    inspector.method(bazMethod).assertAbsent();
+
+    // Check that the inner class is removed.
+    inspector.clazz(classFromClass(REMOVED_CLASS)).assertAbsent();
+  }
+}