Version 1.4.89
Cherry pick: Raise parse error when type name is negated
CL: https://r8-review.googlesource.com/c/r8/+/37421
Bug: 130665986
Change-Id: Ic5f3a449ccf78f6eb64b5114448006b26a82ab5c
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 18c648d..3c1c551 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.4.88";
+ public static final String LABEL = "1.4.89";
private Version() {
}
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 10d3695..4a4a182 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -942,8 +942,9 @@
boolean found = true;
while (found && !eof()) {
found = false;
+ boolean negated = parseNegation();
ProguardAccessFlags flags =
- parseNegation() ? ruleBuilder.getNegatedAccessFlags() : ruleBuilder.getAccessFlags();
+ negated ? ruleBuilder.getNegatedAccessFlags() : ruleBuilder.getAccessFlags();
skipWhitespace();
switch (peekChar()) {
case 'a':
@@ -999,6 +1000,12 @@
default:
// Intentionally left empty.
}
+
+ // Ensure that we do not consume a negation character '!' when the subsequent identifier
+ // does not match a valid access flag name (e.g., "private !int x").
+ if (!found && negated) {
+ unacceptString("!");
+ }
}
}
@@ -1006,6 +1013,11 @@
ProguardMemberRule.Builder ruleBuilder, boolean allowValueSpecification)
throws ProguardRuleParserException {
skipWhitespace();
+ if (!eof() && peekChar() == '!') {
+ throw parseError(
+ "Unexpected character '!': "
+ + "The negation character can only be used to negate access flags");
+ }
if (acceptString("<methods>")) {
ruleBuilder.setRuleType(ProguardMemberType.ALL_METHODS);
} else if (acceptString("<fields>")) {
diff --git a/src/test/java/com/android/tools/r8/proguard/rules/NegatedClassMemberTest.java b/src/test/java/com/android/tools/r8/proguard/rules/NegatedClassMemberTest.java
new file mode 100644
index 0000000..27747f5
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/proguard/rules/NegatedClassMemberTest.java
@@ -0,0 +1,107 @@
+// 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.proguard.rules;
+
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.fail;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class NegatedClassMemberTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withNoneRuntime().build();
+ }
+
+ public NegatedClassMemberTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testProguard() {
+ try {
+ testForProguard()
+ .addProgramClasses(
+ NegatedClassMemberTestClassA.class,
+ NegatedClassMemberTestClassB.class,
+ NegatedClassMemberTestClassC.class)
+ .addKeepRules(getKeepRule())
+ .setMinApi(parameters.getRuntime())
+ .compile();
+
+ // For some reason, Proguard fails with "The output jar is empty". One likely explanation is
+ // that Proguard only keeps classes that have a field y with type "!long", which is not a
+ // valid type name, and therefore never matches anything.
+ fail("Expected Proguard to fail with \"The output jar is empty\"");
+ } catch (CompilationFailedException e) {
+ assertThat(e.getMessage(), containsString("The output jar is empty"));
+ }
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ try {
+ testForR8(Backend.DEX)
+ .addProgramClasses(
+ NegatedClassMemberTestClassA.class,
+ NegatedClassMemberTestClassB.class,
+ NegatedClassMemberTestClassC.class)
+ .addKeepRules(getKeepRule())
+ .compile();
+ fail("Expected R8 to fail during parsing of the Proguard configuration file");
+ } catch (CompilationFailedException e) {
+ int expectedOffset = getKeepRule().indexOf("!");
+ int expectedColumn = expectedOffset + 1;
+ assertThat(
+ e.getCause().getMessage(),
+ allOf(
+ containsString(
+ "Error: offset: "
+ + expectedOffset
+ + ", line: 1, column: "
+ + expectedColumn
+ + ", Unexpected character '!': "
+ + "The negation character can only be used to negate access flags"),
+ containsString(
+ StringUtils.joinLines(
+ "-keepclasseswithmembers class ** { long x; !long y; }",
+ " ^"))));
+ }
+ }
+
+ private String getKeepRule() {
+ return "-keepclasseswithmembers class ** { long x; !long y; }";
+ }
+}
+
+class NegatedClassMemberTestClassA {
+
+ long x = System.currentTimeMillis();
+}
+
+class NegatedClassMemberTestClassB {
+
+ long x = System.currentTimeMillis();
+ private long y = System.currentTimeMillis();
+}
+
+class NegatedClassMemberTestClassC {
+
+ long x = System.currentTimeMillis();
+ int y = (int) System.currentTimeMillis();
+}
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 7516576..7805217 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -2464,7 +2464,7 @@
}
@Test
- public void b124181032() throws Exception {
+ public void b124181032() {
// Test spaces and quotes in class name list.
ProguardConfigurationParser parser;
parser = new ProguardConfigurationParser(new DexItemFactory(), reporter);
@@ -2535,4 +2535,22 @@
assertEquals("*", rule.getClassNames().toString());
}
}
+
+ @Test
+ public void negatedMemberTypeTest() throws IOException {
+ Path proguardConfigurationFile = writeTextToTempFile("-keepclassmembers class ** { !int x; }");
+ try {
+ new ProguardConfigurationParser(new DexItemFactory(), reporter)
+ .parse(proguardConfigurationFile);
+ fail("Expected to fail since the type name cannot be negated.");
+ } catch (AbortException e) {
+ checkDiagnostics(
+ handler.errors,
+ proguardConfigurationFile,
+ 1,
+ 30,
+ "Unexpected character '!': "
+ + "The negation character can only be used to negate access flags");
+ }
+ }
}
