Version 1.2.42.
Merge: Mark argument and return types as live for all targeted methods.
CL: https://r8-review.googlesource.com/c/r8/+/25420
Merge: Remove special case for synthetic constructor argument types.
CL: https://r8-review.googlesource.com/c/r8/+/25422
Merge: Add regression test for b/112517039.
CL: https://r8-review.googlesource.com/c/r8/+/25540
R=sgjesse@google.com
Bug: 112517039
Change-Id: I76998f40002cf7c1b8b4c60b2a5de6745a1a6620
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 3ad2ac7..84ab0fe 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.41";
+ public static final String LABEL = "1.2.42";
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 42f9174..32b03180f 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -638,6 +638,7 @@
private void markMethodAsTargeted(DexEncodedMethod encodedMethod, KeepReason reason) {
markTypeAsLive(encodedMethod.method.holder);
+ markParameterAndReturnTypesAsLive(encodedMethod);
if (Log.ENABLED) {
Log.verbose(getClass(), "Method `%s` is targeted.", encodedMethod.method);
}
@@ -862,22 +863,6 @@
if (!liveMethods.contains(encodedMethod)) {
markTypeAsLive(encodedMethod.method.holder);
markMethodAsTargeted(encodedMethod, reason);
- // For granting inner/outer classes access to their private constructors, javac generates
- // additional synthetic constructors. These constructors take a synthetic class
- // as argument. As it is not possible to express a keep rule for these synthetic classes
- // always keep synthetic arguments to synthetic constructors. See b/69825683.
- if (encodedMethod.isInstanceInitializer() && encodedMethod.isSyntheticMethod()) {
- for (DexType type : encodedMethod.method.proto.parameters.values) {
- type = type.isArrayType() ? type.toBaseType(appInfo.dexItemFactory) : type;
- if (type.isPrimitiveType()) {
- continue;
- }
- DexClass clazz = appInfo.definitionFor(type);
- if (clazz != null && clazz.accessFlags.isSynthetic()) {
- markTypeAsLive(type);
- }
- }
- }
if (Log.ENABLED) {
Log.verbose(getClass(), "Method `%s` has become live due to direct invoke",
encodedMethod.method);
@@ -1254,14 +1239,7 @@
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);
- }
- }
+ markParameterAndReturnTypesAsLive(method);
processAnnotations(method.annotations.annotations);
method.parameterAnnotationsList.forEachAnnotation(this::processAnnotation);
if (protoLiteExtension != null && protoLiteExtension.appliesTo(method)) {
@@ -1274,6 +1252,24 @@
}
}
+ private void markParameterAndReturnTypesAsLive(DexEncodedMethod method) {
+ for (DexType parameterType : method.method.proto.parameters.values) {
+ if (parameterType.isArrayType()) {
+ parameterType = parameterType.toBaseType(appInfo.dexItemFactory);
+ }
+ if (parameterType.isClassType()) {
+ markTypeAsLive(parameterType);
+ }
+ }
+ DexType returnType = method.method.proto.returnType;
+ if (returnType.isArrayType()) {
+ returnType = returnType.toBaseType(appInfo.dexItemFactory);
+ }
+ if (returnType.isClassType()) {
+ markTypeAsLive(returnType);
+ }
+ }
+
private void collectProguardCompatibilityRule(KeepReason reason) {
if (reason.isDueToProguardCompatibility() && compatibility != null) {
compatibility.addRule(reason.getProguardKeepRule());
diff --git a/src/test/java/com/android/tools/r8/shaking/ReturnTypeTest.java b/src/test/java/com/android/tools/r8/shaking/ReturnTypeTest.java
new file mode 100644
index 0000000..ace3bf5
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ReturnTypeTest.java
@@ -0,0 +1,93 @@
+// 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.isRenamed;
+import static junit.framework.TestCase.assertEquals;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.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.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.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+class B112517039ReturnType extends Exception {
+}
+
+interface B112517039I {
+ B112517039ReturnType m();
+ void flaf(Exception e);
+}
+
+class B112517039Caller {
+ public void call(B112517039I i) {
+ System.out.println("Ewwo!");
+ i.flaf(i.m());
+ }
+}
+
+class B112517039Main {
+ public static void main(String[] args) {
+ try {
+ B112517039Caller caller = new B112517039Caller();
+ caller.call(null);
+ } catch (NullPointerException e) {
+ System.out.println("NullPointerException");
+ }
+ }
+}
+
+@RunWith(VmTestRunner.class)
+public class ReturnTypeTest extends TestBase {
+ @Test
+ public void testFromJavac() throws Exception {
+ String mainName = B112517039Main.class.getCanonicalName();
+ ProcessResult javaResult = ToolHelper.runJava(ToolHelper.getClassPathForTests(), mainName);
+ assertEquals(0, javaResult.exitCode);
+ assertThat(javaResult.stdout, containsString("Ewwo!"));
+ assertThat(javaResult.stdout, containsString("NullPointerException"));
+
+ List<String> config = ImmutableList.of(
+ "-printmapping",
+ "-keep class " + mainName + " {",
+ " public static void main(...);",
+ "}"
+ );
+ R8Command.Builder builder = R8Command.builder();
+ builder.addProgramFiles(ToolHelper.getClassFilesForTestDirectory(
+ ToolHelper.getPackageDirectoryForTestPackage(B112517039Main.class.getPackage()),
+ path -> path.getFileName().toString().startsWith("B112517039")));
+ 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);
+ assertThat(javaResult.stdout, containsString("Ewwo!"));
+ assertThat(javaResult.stdout, containsString("NullPointerException"));
+
+ DexInspector inspector = new DexInspector(processedApp);
+ ClassSubject returnType = inspector.clazz(B112517039ReturnType.class);
+ assertThat(returnType, isRenamed());
+ }
+}
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 81fd6b0..3be0feb 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,9 +94,10 @@
Result result = runTest(mainClass, proguardConfig);
- // Without includedescriptorclasses return type is removed.
result.assertKept(ClassWithNativeMethods.class);
- result.assertRemoved(NativeReturnType.class);
+ // Return types are not removed as they can be needed for verification.
+ // See b/112517039.
+ result.assertRenamed(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.
