Version 1.2.40
Merge: Mark argument types as live.
CL: https://r8-review.googlesource.com/c/r8/+/25120
Merge: Reproduce b/112452064.
CL: https://r8-review.googlesource.com/c/r8/+/25000
Bug: 112452064
Change-Id: Icb24b6f5d7a18b1a917da64af94fb14f818fdedc
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 10d19a5..7fdd393 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.2.39";
+ public static final String LABEL = "1.2.40";
private Version() {
}
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 2b51720..42f9174 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -785,8 +785,12 @@
// Mark the type live here, so that the class exists at runtime. Note that this also marks all
// supertypes as live, so even if the field is actually on a supertype, its class will be live.
markTypeAsLive(field.clazz);
- if (field.type.isClassType()) {
- markTypeAsLive(field.type);
+ DexType fieldType = field.type;
+ if (fieldType.isArrayType()) {
+ fieldType = fieldType.toBaseType(appInfo.dexItemFactory);
+ }
+ if (fieldType.isClassType()) {
+ markTypeAsLive(fieldType);
}
// Find the actual field.
DexEncodedField encodedField = appInfo.resolveFieldOn(field.clazz, field);
@@ -817,8 +821,12 @@
private void markInstanceFieldAsLive(DexEncodedField field, KeepReason reason) {
assert field != null;
markTypeAsLive(field.field.clazz);
- if (field.field.type.isClassType()) {
- markTypeAsLive(field.field.type);
+ DexType fieldType = field.field.type;
+ if (fieldType.isArrayType()) {
+ fieldType = fieldType.toBaseType(appInfo.dexItemFactory);
+ }
+ if (fieldType.isClassType()) {
+ markTypeAsLive(fieldType);
}
if (Log.ENABLED) {
Log.verbose(getClass(), "Adding instance field `%s` to live set.", field.field);
@@ -1246,6 +1254,14 @@
markVirtualMethodAsLive(superCallTarget, KeepReason.invokedViaSuperFrom(method));
}
}
+ for (DexType parameterType : method.method.proto.parameters.values) {
+ if (parameterType.isArrayType()) {
+ parameterType = parameterType.toBaseType(appInfo.dexItemFactory);
+ }
+ if (parameterType.isClassType()) {
+ markTypeAsLive(parameterType);
+ }
+ }
processAnnotations(method.annotations.annotations);
method.parameterAnnotationsList.forEachAnnotation(this::processAnnotation);
if (protoLiteExtension != null && protoLiteExtension.appliesTo(method)) {
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index d5d0558..cfeb11d 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -63,6 +63,7 @@
import java.util.concurrent.Executors;
import java.util.function.Consumer;
import java.util.function.Function;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.junit.Assume;
import org.junit.rules.TemporaryFolder;
@@ -747,9 +748,17 @@
}
public static List<Path> getClassFilesForTestPackage(Package pkg) throws IOException {
- Path dir = ToolHelper.getPackageDirectoryForTestPackage(pkg);
- return Files.walk(dir)
- .filter(path -> path.toString().endsWith(".class"))
+ return getClassFilesForTestDirectory(ToolHelper.getPackageDirectoryForTestPackage(pkg));
+ }
+
+ public static List<Path> getClassFilesForTestDirectory(Path directory) throws IOException {
+ return getClassFilesForTestDirectory(directory, null);
+ }
+
+ public static List<Path> getClassFilesForTestDirectory(
+ Path directory, Predicate<Path> filter) throws IOException {
+ return Files.walk(directory)
+ .filter(path -> path.toString().endsWith(".class") && (filter == null || filter.test(path)))
.collect(Collectors.toList());
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ParameterTypeTest.java b/src/test/java/com/android/tools/r8/shaking/ParameterTypeTest.java
new file mode 100644
index 0000000..00af5f7
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ParameterTypeTest.java
@@ -0,0 +1,288 @@
+// Copyright (c) 2018, 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;
+
+import static com.android.tools.r8.utils.DexInspectorMatchers.isPresent;
+import static com.android.tools.r8.utils.DexInspectorMatchers.isRenamed;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.DexIndexedConsumer;
+import com.android.tools.r8.OutputMode;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.DexInspector;
+import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.DexInspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+interface B112452064SuperInterface1 {
+ void foo();
+}
+
+interface B112452064SuperInterface2 {
+ void bar();
+}
+
+interface B112452064SubInterface extends B112452064SuperInterface1, B112452064SuperInterface2 {
+}
+
+class B112452064TestMain {
+
+ static void bazSuper1(B112452064SuperInterface1 instance) {
+ instance.foo();
+ }
+
+ static void bazSub(B112452064SubInterface instance) {
+ instance.foo();
+ }
+
+ public static void main(String[] args) {
+ bazSuper1(new B112452064SuperInterface1() {
+ @Override
+ public void foo() {
+ System.out.println("Anonymous1::foo");
+ }
+ });
+ bazSub(new B112452064SubInterface() {
+ @Override
+ public void foo() {
+ System.out.println("Anonymous2::foo");
+ }
+ @Override
+ public void bar() {
+ System.out.println("Anonymous2::bar");
+ }
+ });
+ }
+}
+
+@RunWith(VmTestRunner.class)
+public class ParameterTypeTest extends TestBase {
+
+ @Test
+ public void test_fromJavac() throws Exception {
+ String mainName = B112452064TestMain.class.getCanonicalName();
+ ProcessResult javaResult = ToolHelper.runJava(ToolHelper.getClassPathForTests(), mainName);
+ assertEquals(0, javaResult.exitCode);
+ assertThat(javaResult.stdout, containsString("Anonymous"));
+ assertThat(javaResult.stdout, containsString("::foo"));
+ assertEquals(-1, javaResult.stderr.indexOf("ClassNotFoundException"));
+
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ "-keep class " + mainName + " {",
+ " public static void main(...);",
+ "}"
+ );
+ R8Command.Builder builder = R8Command.builder();
+ builder.addProgramFiles(ToolHelper.getClassFilesForTestDirectory(
+ ToolHelper.getPackageDirectoryForTestPackage(B112452064TestMain.class.getPackage()),
+ path -> path.getFileName().toString().startsWith("B112452064")));
+ builder.setProgramConsumer(DexIndexedConsumer.emptyConsumer());
+ builder.setMinApiLevel(ToolHelper.getMinApiLevelForDexVm().getLevel());
+ builder.addProguardConfiguration(config, Origin.unknown());
+ AndroidApp processedApp = ToolHelper.runR8(builder.build(), options -> {
+ options.enableInlining = false;
+ });
+
+ Path outDex = temp.getRoot().toPath().resolve("dex.zip");
+ processedApp.writeToZip(outDex, OutputMode.DexIndexed);
+ ProcessResult artResult = ToolHelper.runArtNoVerificationErrorsRaw(outDex.toString(), mainName);
+ assertEquals(0, artResult.exitCode);
+ assertEquals(javaResult.stdout, artResult.stdout);
+ assertEquals(-1, artResult.stderr.indexOf("ClassNotFoundException"));
+
+ DexInspector inspector = new DexInspector(processedApp);
+ ClassSubject superInterface1 = inspector.clazz(B112452064SuperInterface1.class);
+ assertThat(superInterface1, isRenamed());
+ MethodSubject foo = superInterface1.method("void", "foo", ImmutableList.of());
+ assertThat(foo, isRenamed());
+ ClassSubject superInterface2 = inspector.clazz(B112452064SuperInterface2.class);
+ assertThat(superInterface2, isRenamed());
+ MethodSubject bar = superInterface1.method("void", "bar", ImmutableList.of());
+ assertThat(bar, not(isPresent()));
+ ClassSubject subInterface = inspector.clazz(B112452064SubInterface.class);
+ assertThat(subInterface, isRenamed());
+ }
+
+ @Test
+ public void test_brokenTypeHierarchy_singleInterface() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ // interface SuperInterface {
+ // void foo();
+ // }
+ ClassBuilder sup = jasminBuilder.addInterface("SuperInterface");
+ MethodSignature foo = sup.addAbstractMethod("foo", ImmutableList.of(), "V");
+ // interface SubInterface extends SuperInterface
+ ClassBuilder sub = jasminBuilder.addInterface("SubInterface", sup.name);
+
+ // class Foo implements SuperInterface /* supposed to implement SubInterface */
+ ClassBuilder impl = jasminBuilder.addClass("Foo", "java/lang/Object", sup.name);
+ impl.addDefaultConstructor();
+ impl.addVirtualMethod(foo.name, ImmutableList.of(), "V",
+ ".limit locals 2",
+ ".limit stack 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "ldc \"" + foo.name + "\"",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return");
+
+ // class TestMain {
+ // static bar(SubInterface instance) {
+ // // instance.foo();
+ // }
+ // public static void main(String[] args) {
+ // // ART verifies the argument (Foo) is an instance of the parameter type (SubInterface).
+ // bar(new Foo());
+ // }
+ // }
+ ClassBuilder mainClass = jasminBuilder.addClass("Main");
+ MethodSignature bar =
+ mainClass.addStaticMethod("bar", ImmutableList.of(sub.getDescriptor()), "V",
+ ".limit locals 2",
+ ".limit stack 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "ldc \"" + "bar" + "\"",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return");
+ mainClass.addMainMethod(
+ ".limit locals 1",
+ ".limit stack 2",
+ "new " + impl.name,
+ "dup",
+ "invokespecial " + impl.name + "/<init>()V",
+ "invokestatic " + mainClass.name + "/bar(" + sub.getDescriptor() + ")V",
+ "return");
+
+
+ final String mainClassName = mainClass.name;
+ String proguardConfig = keepMainProguardConfiguration(mainClassName, false, false);
+
+ // Run input program on java.
+ Path outputDirectory = temp.newFolder().toPath();
+ jasminBuilder.writeClassFiles(outputDirectory);
+ ProcessResult javaResult = ToolHelper.runJava(outputDirectory, mainClassName);
+ assertEquals(0, javaResult.exitCode);
+ assertThat(javaResult.stdout, containsString(bar.name));
+ assertEquals(-1, javaResult.stderr.indexOf("ClassNotFoundException"));
+
+ AndroidApp processedApp = compileWithR8(jasminBuilder.build(), proguardConfig,
+ // Disable inlining to avoid the (short) tested method from being inlined and then removed.
+ internalOptions -> internalOptions.enableInlining = false);
+
+ // Run processed (output) program on ART
+ ProcessResult artResult = runOnArtRaw(processedApp, mainClassName);
+ assertEquals(0, artResult.exitCode);
+ assertThat(artResult.stdout, containsString(bar.name));
+ assertEquals(-1, artResult.stderr.indexOf("ClassNotFoundException"));
+
+ DexInspector inspector = new DexInspector(processedApp);
+ ClassSubject subSubject = inspector.clazz(sub.name);
+ assertThat(subSubject, isPresent());
+ }
+
+ @Test
+ public void test_brokenTypeHierarchy_doubleInterfaces() throws Exception {
+ JasminBuilder jasminBuilder = new JasminBuilder();
+ // interface SuperInterface1 {
+ // void foo();
+ // }
+ ClassBuilder sup1 = jasminBuilder.addInterface("SuperInterface1");
+ MethodSignature foo = sup1.addAbstractMethod("foo", ImmutableList.of(), "V");
+ // interface SuperInterface2 {
+ // void bar();
+ // }
+ ClassBuilder sup2 = jasminBuilder.addInterface("SuperInterface2");
+ MethodSignature bar = sup1.addAbstractMethod("bar", ImmutableList.of(), "V");
+ // interface SubInterface extends SuperInterface1, SuperInterface2
+ ClassBuilder sub = jasminBuilder.addInterface("SubInterface", sup1.name, sup2.name);
+
+ // class Foo implements SuperInterface1, SuperInterface2
+ // /* supposed to implement SubInterface */
+ ClassBuilder impl = jasminBuilder.addClass("Foo", "java/lang/Object", sup1.name, sup2.name);
+ impl.addDefaultConstructor();
+ impl.addVirtualMethod(foo.name, ImmutableList.of(), "V",
+ ".limit locals 2",
+ ".limit stack 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "ldc \"" + foo.name + "\"",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return");
+ impl.addVirtualMethod(bar.name, ImmutableList.of(), "V",
+ ".limit locals 2",
+ ".limit stack 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "ldc \"" + bar.name + "\"",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return");
+
+ // class TestMain {
+ // static bar(SubInterface instance) {
+ // // instance.foo();
+ // }
+ // public static void main(String[] args) {
+ // // ART verifies the argument (Foo) is an instance of the parameter type (SubInterface).
+ // bar(new Foo());
+ // }
+ // }
+ ClassBuilder mainClass = jasminBuilder.addClass("Main");
+ MethodSignature baz =
+ mainClass.addStaticMethod("baz", ImmutableList.of(sub.getDescriptor()), "V",
+ ".limit locals 2",
+ ".limit stack 2",
+ "getstatic java/lang/System/out Ljava/io/PrintStream;",
+ "ldc \"" + "baz" + "\"",
+ "invokevirtual java/io/PrintStream/print(Ljava/lang/String;)V",
+ "return");
+ mainClass.addMainMethod(
+ ".limit locals 1",
+ ".limit stack 2",
+ "new " + impl.name,
+ "dup",
+ "invokespecial " + impl.name + "/<init>()V",
+ "invokestatic " + mainClass.name + "/baz(" + sub.getDescriptor() + ")V",
+ "return");
+
+ final String mainClassName = mainClass.name;
+ String proguardConfig = keepMainProguardConfiguration(mainClassName, false, false);
+
+ // Run input program on java.
+ Path outputDirectory = temp.newFolder().toPath();
+ jasminBuilder.writeClassFiles(outputDirectory);
+ ProcessResult javaResult = ToolHelper.runJava(outputDirectory, mainClassName);
+ assertEquals(0, javaResult.exitCode);
+ assertThat(javaResult.stdout, containsString(baz.name));
+ assertEquals(-1, javaResult.stderr.indexOf("ClassNotFoundException"));
+
+ AndroidApp processedApp = compileWithR8(jasminBuilder.build(), proguardConfig,
+ // Disable inlining to avoid the (short) tested method from being inlined and then removed.
+ internalOptions -> internalOptions.enableInlining = false);
+
+ // Run processed (output) program on ART
+ ProcessResult artResult = runOnArtRaw(processedApp, mainClassName);
+ assertEquals(0, artResult.exitCode);
+ assertThat(artResult.stdout, containsString(baz.name));
+ assertEquals(javaResult.stdout, artResult.stdout);
+ assertEquals(-1, artResult.stderr.indexOf("ClassNotFoundException"));
+
+ DexInspector inspector = new DexInspector(processedApp);
+ ClassSubject subSubject = inspector.clazz(sub.name);
+ assertThat(subSubject, isPresent());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java b/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
index cae3b06..81fd6b0 100644
--- a/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/includedescriptorclasses/IncludeDescriptorClassesTest.java
@@ -94,10 +94,11 @@
Result result = runTest(mainClass, proguardConfig);
- // Without includedescriptorclasses return type and argument type are removed.
+ // Without includedescriptorclasses return type is removed.
result.assertKept(ClassWithNativeMethods.class);
- result.assertRemoved(NativeArgumentType.class);
result.assertRemoved(NativeReturnType.class);
+ // Argument type is not removed due to the concern about the broken type hierarchy.
+ result.assertRenamed(NativeArgumentType.class);
// Field type is not removed due to the concern about the broken type hierarchy.
result.assertRenamed(InstanceFieldType.class);
result.assertRenamed(StaticFieldType.class);
