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);