Reproduce unused argument removal bug in presence of annotations
Bug: 131663970
Change-Id: Ia6572d253920766bbae2a755237052dcdf559e55
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index 1fb8f0e..8757da5 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -4,6 +4,7 @@
package com.android.tools.r8;
import static com.android.tools.r8.D8Command.USAGE_MESSAGE;
+import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.dex.ApplicationWriter;
@@ -206,7 +207,7 @@
.write(executor);
options.printWarnings();
} catch (ExecutionException e) {
- throw R8.unwrapExecutionException(e);
+ throw unwrapExecutionException(e);
} finally {
options.signalFinishedToConsumers();
// Dump timings.
diff --git a/src/main/java/com/android/tools/r8/DexFileMergerHelper.java b/src/main/java/com/android/tools/r8/DexFileMergerHelper.java
index f6abea5..404aa4b 100644
--- a/src/main/java/com/android/tools/r8/DexFileMergerHelper.java
+++ b/src/main/java/com/android/tools/r8/DexFileMergerHelper.java
@@ -4,6 +4,8 @@
package com.android.tools.r8;
+import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
+
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.dex.ApplicationWriter;
import com.android.tools.r8.dex.Marker;
@@ -106,7 +108,7 @@
writer.write(executor);
options.printWarnings();
} catch (ExecutionException e) {
- throw R8.unwrapExecutionException(e);
+ throw unwrapExecutionException(e);
} finally {
options.signalFinishedToConsumers();
}
diff --git a/src/main/java/com/android/tools/r8/DexSplitterHelper.java b/src/main/java/com/android/tools/r8/DexSplitterHelper.java
index 19e84d8..34e67e6 100644
--- a/src/main/java/com/android/tools/r8/DexSplitterHelper.java
+++ b/src/main/java/com/android/tools/r8/DexSplitterHelper.java
@@ -4,6 +4,8 @@
package com.android.tools.r8;
+import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
+
import com.android.tools.r8.DexIndexedConsumer.DirectoryConsumer;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.dex.ApplicationWriter;
@@ -112,7 +114,7 @@
}
}
} catch (ExecutionException e) {
- throw R8.unwrapExecutionException(e);
+ throw unwrapExecutionException(e);
} catch (FeatureMappingException e) {
options.reporter.error(e.getMessage());
} finally {
diff --git a/src/main/java/com/android/tools/r8/GenerateMainDexList.java b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
index e5842df..6c5ed9a 100644
--- a/src/main/java/com/android/tools/r8/GenerateMainDexList.java
+++ b/src/main/java/com/android/tools/r8/GenerateMainDexList.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
+
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.experimental.graphinfo.GraphConsumer;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
@@ -99,7 +101,7 @@
return result;
} catch (ExecutionException e) {
- throw R8.unwrapExecutionException(e);
+ throw unwrapExecutionException(e);
}
}
diff --git a/src/main/java/com/android/tools/r8/PrintSeeds.java b/src/main/java/com/android/tools/r8/PrintSeeds.java
index 050e69f..d151496 100644
--- a/src/main/java/com/android/tools/r8/PrintSeeds.java
+++ b/src/main/java/com/android/tools/r8/PrintSeeds.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8;
+import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
+
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppServices;
@@ -96,7 +98,7 @@
RootSetBuilder.writeSeeds(
appInfo, System.out, type -> descriptors.contains(type.toDescriptorString()));
} catch (ExecutionException e) {
- throw R8.unwrapExecutionException(e);
+ throw unwrapExecutionException(e);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 458972d..16b75f5 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -4,6 +4,7 @@
package com.android.tools.r8;
import static com.android.tools.r8.R8Command.USAGE_MESSAGE;
+import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.dex.ApplicationWriter;
@@ -821,20 +822,6 @@
}
}
- static RuntimeException unwrapExecutionException(ExecutionException executionException) {
- Throwable cause = executionException.getCause();
- if (cause instanceof Error) {
- // add original exception as suppressed exception to provide the original stack trace
- cause.addSuppressed(executionException);
- throw (Error) cause;
- } else if (cause instanceof RuntimeException) {
- cause.addSuppressed(executionException);
- throw (RuntimeException) cause;
- } else {
- throw new RuntimeException(executionException.getMessage(), cause);
- }
- }
-
private static void run(String[] args) throws CompilationFailedException {
R8Command command = R8Command.parse(args, CommandLineOrigin.INSTANCE).build();
if (command.isPrintHelp()) {
diff --git a/src/main/java/com/android/tools/r8/dex/ApplicationReader.java b/src/main/java/com/android/tools/r8/dex/ApplicationReader.java
index 20eafd2..3c239c8 100644
--- a/src/main/java/com/android/tools/r8/dex/ApplicationReader.java
+++ b/src/main/java/com/android/tools/r8/dex/ApplicationReader.java
@@ -6,6 +6,7 @@
import static com.android.tools.r8.graph.ClassKind.CLASSPATH;
import static com.android.tools.r8.graph.ClassKind.LIBRARY;
import static com.android.tools.r8.graph.ClassKind.PROGRAM;
+import static com.android.tools.r8.utils.ExceptionUtils.unwrapExecutionException;
import com.android.tools.r8.ClassFileResourceProvider;
import com.android.tools.r8.DataResourceProvider;
@@ -123,6 +124,8 @@
builder.addDataResourceProvider(dataResourceProvider);
}
}
+ } catch (ExecutionException e) {
+ throw unwrapExecutionException(e);
} catch (ResourceException e) {
throw options.reporter.fatalError(new StringDiagnostic(e.getMessage(), e.getOrigin()));
} finally {
diff --git a/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java b/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
index d77adfd..c56ecf4 100644
--- a/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
@@ -13,6 +13,7 @@
import java.io.IOException;
import java.nio.file.FileSystemException;
import java.nio.file.Paths;
+import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
public abstract class ExceptionUtils {
@@ -105,4 +106,17 @@
return Origin.unknown();
}
+ public static RuntimeException unwrapExecutionException(ExecutionException executionException) {
+ Throwable cause = executionException.getCause();
+ if (cause instanceof Error) {
+ // add original exception as suppressed exception to provide the original stack trace
+ cause.addSuppressed(executionException);
+ throw (Error) cause;
+ } else if (cause instanceof RuntimeException) {
+ cause.addSuppressed(executionException);
+ throw (RuntimeException) cause;
+ } else {
+ throw new RuntimeException(executionException.getMessage(), cause);
+ }
+ }
}
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index 0c441a5..14d6d72 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -232,10 +232,18 @@
}
public T enableUnusedArgumentAnnotations() {
- if (!enableUnusedArgumentAnnotations) {
- enableUnusedArgumentAnnotations = true;
- addInternalKeepRules(
- "-keepunusedarguments class * { @com.android.tools.r8.KeepUnusedArguments *; }");
+ return enableUnusedArgumentAnnotations(true);
+ }
+
+ public T enableUnusedArgumentAnnotations(boolean value) {
+ if (value) {
+ if (!enableUnusedArgumentAnnotations) {
+ enableUnusedArgumentAnnotations = true;
+ addInternalKeepRules(
+ "-keepunusedarguments class * { @com.android.tools.r8.KeepUnusedArguments *; }");
+ }
+ } else {
+ assert !enableUnusedArgumentAnnotations;
}
return self();
}
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index 8b7dbbb..cfb3c11 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -132,8 +132,12 @@
return self();
}
+ public T addKeepAttributes(String... attributes) {
+ return addKeepRules("-keepattributes " + String.join(",", attributes));
+ }
+
public T addKeepAllAttributes() {
- return addKeepRules("-keepattributes *");
+ return addKeepAttributes("*");
}
public abstract T addApplyMapping(String proguardMap);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedAnnotatedArgumentsTest.java b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedAnnotatedArgumentsTest.java
new file mode 100644
index 0000000..c0eaca3
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedAnnotatedArgumentsTest.java
@@ -0,0 +1,105 @@
+// 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.ir.optimize.unusedarguments;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.KeepUnusedArguments;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+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 UnusedAnnotatedArgumentsTest extends TestBase {
+
+ private final boolean keepUnusedArguments;
+ private final TestParameters parameters;
+
+ @Parameters(name = "{1}, keep unused arguments: {0}")
+ public static List<Object[]> params() {
+ return buildParameters(BooleanUtils.values(), getTestParameters().withAllRuntimes().build());
+ }
+
+ public UnusedAnnotatedArgumentsTest(boolean keepUnusedArguments, TestParameters parameters) {
+ this.keepUnusedArguments = keepUnusedArguments;
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ try {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(UnusedAnnotatedArgumentsTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addKeepClassRules(Unused.class)
+ .addKeepAttributes("RuntimeVisibleParameterAnnotations")
+ .enableInliningAnnotations()
+ .enableUnusedArgumentAnnotations(keepUnusedArguments)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifyOutput)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello world!");
+ assertTrue(keepUnusedArguments);
+ } catch (ArrayIndexOutOfBoundsException | AssertionError e) {
+ // TODO(b/131663970): Fix unused argument removal.
+ assertFalse(keepUnusedArguments);
+ }
+ }
+
+ private void verifyOutput(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(TestClass.class);
+ assertThat(classSubject, isPresent());
+
+ MethodSubject methodSubject =
+ // TODO(b/123060011): Mapping not working in presence of unused argument removal.
+ classSubject.uniqueMethodWithName(keepUnusedArguments ? "test" : "a");
+ assertThat(methodSubject, isPresent());
+
+ if (keepUnusedArguments) {
+ assertEquals(1, methodSubject.getMethod().method.proto.parameters.size());
+ assertEquals(1, methodSubject.getMethod().parameterAnnotationsList.size());
+ } else {
+ assertEquals(0, methodSubject.getMethod().method.proto.parameters.size());
+ assertEquals(0, methodSubject.getMethod().parameterAnnotationsList.size());
+ }
+
+ System.out.println();
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ test(null);
+ }
+
+ @KeepUnusedArguments
+ @NeverInline
+ static void test(@Unused Object unused) {
+ System.out.println("Hello world!");
+ }
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.PARAMETER)
+ @interface Unused {}
+}
