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