Discard init with wildcards.
If the current member rule is constructor pattern, make sure it does not
have wildcard patterns, since only "<init>" is allowed.
Bug: 130710288
Change-Id: I42822dd8c4702019389975e788d963b06f360edd
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index 1d9de37..492f1f5 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -1045,6 +1045,8 @@
ruleBuilder.setRuleType(ProguardMemberType.ALL);
} else {
if (hasNextChar('(')) {
+ // Parsing "<init>" is already done, and thus neither '<' nor '>' can appear.
+ checkConstructorPattern(first, firstStart);
ruleBuilder.setRuleType(ProguardMemberType.CONSTRUCTOR);
ruleBuilder.setName(first);
ruleBuilder.setArguments(parseArgumentList());
@@ -1055,6 +1057,10 @@
if (second != null) {
skipWhitespace();
if (hasNextChar('(')) {
+ // Neither '<' nor '>' can appear except for "[access-flag]* void <init>(...)".
+ if (!first.pattern.contains("void") || !second.pattern.equals("<init>")) {
+ checkConstructorPattern(second, secondStart);
+ }
ruleBuilder.setRuleType(ProguardMemberType.METHOD);
ruleBuilder.setName(second);
ruleBuilder
@@ -1140,6 +1146,18 @@
}
}
+ private void checkConstructorPattern(
+ IdentifierPatternWithWildcards pattern, TextPosition position)
+ throws ProguardRuleParserException {
+ if (pattern.pattern.contains("<")) {
+ throw parseError("Unexpected character '<' in method name. "
+ + "The character '<' is only allowed in the method name '<init>'.", position);
+ } else if (pattern.pattern.contains(">")) {
+ throw parseError("Unexpected character '>' in method name. "
+ + "The character '>' is only allowed in the method name '<init>'.", position);
+ }
+ }
+
private List<ProguardTypeMatcher> parseArgumentList() throws ProguardRuleParserException {
List<ProguardTypeMatcher> arguments = new ArrayList<>();
skipWhitespace();
diff --git a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
index 62aaf50..d0fb723 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -5,6 +5,7 @@
import static com.android.tools.r8.DiagnosticsChecker.checkDiagnostics;
import static com.android.tools.r8.shaking.ProguardConfigurationSourceStrings.createConfigurationForTesting;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -12,7 +13,6 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -27,6 +27,7 @@
import com.android.tools.r8.position.Position;
import com.android.tools.r8.position.TextRange;
import com.android.tools.r8.shaking.ProguardConfigurationParser.IdentifierPatternWithWildcards;
+import com.android.tools.r8.shaking.constructor.InitMatchingTest;
import com.android.tools.r8.utils.AbortException;
import com.android.tools.r8.utils.FileUtils;
import com.android.tools.r8.utils.InternalOptions.PackageObfuscationMode;
@@ -2381,6 +2382,12 @@
return parser.getConfig();
}
+ private ProguardConfiguration parseAndVerifyParserEndsCleanly(Path config) {
+ parser.parse(config);
+ verifyParserEndsCleanly();
+ return parser.getConfig();
+ }
+
private void verifyParserEndsCleanly() {
assertEquals(0, handler.infos.size());
assertEquals(0, handler.warnings.size());
@@ -2538,4 +2545,44 @@
+ "The negation character can only be used to negate access flags");
}
}
+
+ @Test
+ public void b130710288() throws Exception {
+ // "[[access-flag]* void] <init>" is the only valid format.
+ for (String initName : ImmutableList.of("<init>", "void <init>", "public void <init>")) {
+ Path initConfig = writeTextToTempFile(
+ "-keep class **.MyClass {",
+ " " + initName + "(...);",
+ "}");
+ parseAndVerifyParserEndsCleanly(initConfig);
+ }
+
+ for (String initName : InitMatchingTest.INIT_NAMES) {
+ // Tested above.
+ if (initName.contains("<init>")) {
+ continue;
+ }
+ reset();
+ Path proguardConfig = writeTextToTempFile(
+ "-keep class **.MyClass {",
+ " " + initName + "(...);",
+ "}");
+ try {
+ parser.parse(proguardConfig);
+ fail("Expect to fail due to unsupported constructor name pattern.");
+ } catch (AbortException e) {
+ int column = initName.contains("void") ? initName.indexOf("void") + 8 : 3;
+ checkDiagnostics(
+ handler.errors, proguardConfig, 2, column, "Unexpected character", "method name");
+ }
+ // For some exceptional cases, Proguard accepts the rules but fails with an empty jar message.
+ if (initName.contains("<init>")
+ || initName.contains("<clinit>")
+ || initName.contains("void")) {
+ continue;
+ }
+ verifyFailWithProguard6(
+ proguardConfig, "Expecting type and name instead of just '" + initName + "'");
+ }
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/constructor/InitMatchingTest.java b/src/test/java/com/android/tools/r8/shaking/constructor/InitMatchingTest.java
new file mode 100644
index 0000000..4db8a60
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/constructor/InitMatchingTest.java
@@ -0,0 +1,136 @@
+// 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.constructor;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.ProguardTestCompileResult;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+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 com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+class InitMatchingTestClass {
+ // Trivial <clinit>
+ static {
+ }
+ int field;
+ public InitMatchingTestClass(int arg) {
+ field = arg;
+ }
+}
+
+@RunWith(Parameterized.class)
+public class InitMatchingTest extends TestBase {
+ public static final List<String> INIT_NAMES = ImmutableList.of(
+ "<clinit>", "<init>", "<*init>", "<in*>", "<in*", "*init>",
+ "void <clinit>", "void <init>", "void <*init>", "void <in*>", "void <in*", "void *init>",
+ "public void <init>", "public void <*init>"
+ );
+
+ @Parameterized.Parameters(name = "{0} \"{1}\"")
+ public static Collection<Object[]> data() {
+ return buildParameters(ToolHelper.getBackends(), INIT_NAMES);
+ }
+
+ private final Backend backend;
+ private final String initName;
+
+ public InitMatchingTest(Backend backend, String initName) {
+ this.backend = backend;
+ this.initName = initName;
+ }
+
+ private String createKeepRule() {
+ return "-keep class * { " + initName + "(...); }";
+ }
+
+ @Test
+ public void testProguard() throws Exception {
+ assumeTrue(backend == Backend.CF);
+ ProguardTestCompileResult result;
+ try {
+ result =
+ testForProguard()
+ .addProgramClasses(InitMatchingTestClass.class)
+ .addKeepRules(createKeepRule())
+ .compile();
+ } catch (CompilationFailedException e) {
+ assertNotEquals("<init>", initName);
+ if (initName.equals("void <in*")) {
+ assertThat(e.getMessage(), containsString("Missing closing angular bracket"));
+ } else {
+ assertThat(e.getMessage(),
+ containsString("Expecting type and name instead of just '" + initName + "'"));
+ }
+ return;
+ }
+ result.inspect(this::inspectProguard);
+ }
+
+ private void inspectProguard(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(InitMatchingTestClass.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject init = classSubject.init(ImmutableList.of("int"));
+ if (initName.equals("void <clinit>")) {
+ assertThat(init, not(isPresent()));
+ } else {
+ assertThat(init, isPresent());
+ }
+ MethodSubject clinit = classSubject.clinit();
+ if (initName.equals("void <clinit>")
+ || initName.equals("void <*init>")
+ || initName.equals("void *init>")) {
+ assertThat(clinit, isPresent());
+ } else {
+ assertThat(clinit, not(isPresent()));
+ }
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ R8TestCompileResult result;
+ try {
+ result =
+ testForR8(backend)
+ .addProgramClasses(InitMatchingTestClass.class)
+ .addKeepRules(createKeepRule())
+ .compile();
+ } catch (CompilationFailedException e) {
+ assertNotEquals("<init>", initName);
+ assertThat(e.getCause().getMessage(),
+ containsString("Unexpected character '" + (initName.contains("<") ? "<" : ">") + "'"));
+ assertThat(e.getCause().getMessage(),
+ containsString("only allowed in the method name '<init>'"));
+ return;
+ }
+ result
+ .assertNoMessages()
+ .inspect(this::inspectR8);
+ }
+
+ private void inspectR8(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(InitMatchingTestClass.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject init = classSubject.init(ImmutableList.of("int"));
+ assertThat(init, isPresent());
+ MethodSubject clinit = classSubject.clinit();
+ assertThat(clinit, not(isPresent()));
+ }
+
+}
