Version 1.4.51
Cherry-pick: Don't class-inline synchronized methods (even for CF).
CL: https://r8-review.googlesource.com/c/r8/+/33867
Cherry-pick: Proguard config parser: Allow space after comma in class name list.
CL: https://r8-review.googlesource.com/c/r8/+/34062
Cherry-pick: Proguard config parser: allow spaces and quotes in class name list.
CL: https://r8-review.googlesource.com/c/r8/+/34080
Bug: 112295630, 124181032
Change-Id: Icd0ba00dfa0dd4bbd5ae7f6ef3d8a9b08a9c35ec
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index a6b94b0..ed6b403 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.50";
+ public static final String LABEL = "1.4.51";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index e723a55..3861c66 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -1037,8 +1037,9 @@
// that it is not the instance being initialized.
//
boolean instanceInitializer = method.isInstanceInitializer();
- if (method.accessFlags.isNative() ||
- (!method.isNonAbstractVirtualMethod() && !instanceInitializer)) {
+ if (method.accessFlags.isNative()
+ || (!method.isNonAbstractVirtualMethod() && !instanceInitializer)
+ || method.accessFlags.isSynchronized()) {
return;
}
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 c2f4849..eaad9a4 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -1431,11 +1431,27 @@
}
private IdentifierPatternWithWildcards acceptIdentifierWithBackreference(IdentifierType kind) {
+ IdentifierPatternWithWildcardsAndNegation pattern =
+ acceptIdentifierWithBackreference(kind, false);
+ if (pattern == null) {
+ return null;
+ }
+ assert !pattern.negated;
+ return pattern.patternWithWildcards;
+ }
+
+ private IdentifierPatternWithWildcardsAndNegation acceptIdentifierWithBackreference(
+ IdentifierType kind, boolean allowNegation) {
ImmutableList.Builder<ProguardWildcard> wildcardsCollector = ImmutableList.builder();
StringBuilder currentAsterisks = null;
int asteriskCount = 0;
StringBuilder currentBackreference = null;
skipWhitespace();
+
+ final char quote = acceptQuoteIfPresent();
+ final boolean quoted = isQuote(quote);
+ final boolean negated = allowNegation ? acceptChar('!') : false;
+
int start = position;
int end = position;
while (!eof(end)) {
@@ -1514,9 +1530,17 @@
currentBackreference = new StringBuilder();
end += Character.charCount(current);
} else {
+ if (quoted && quote != current) {
+ throw reporter.fatalError(
+ new StringDiagnostic(
+ "Invalid character '" + (char) current + "', expected end-quote.",
+ origin,
+ getPosition()));
+ }
break;
}
}
+ position = quoted ? end + 1 : end;
if (currentAsterisks != null) {
wildcardsCollector.add(new ProguardWildcard.Pattern(currentAsterisks.toString()));
}
@@ -1528,10 +1552,8 @@
if (start == end) {
return null;
}
- position = end;
- return new IdentifierPatternWithWildcards(
- contents.substring(start, end),
- wildcardsCollector.build());
+ return new IdentifierPatternWithWildcardsAndNegation(
+ contents.substring(start, end), wildcardsCollector.build(), negated);
}
private String acceptFieldNameOrIntegerForReturn() {
@@ -1625,19 +1647,20 @@
}
}
+ private void parseClassNameAddToBuilder(ProguardClassNameList.Builder builder)
+ throws ProguardRuleParserException {
+ IdentifierPatternWithWildcardsAndNegation name = parseClassName(true);
+ builder.addClassName(
+ name.negated,
+ ProguardTypeMatcher.create(name.patternWithWildcards, ClassOrType.CLASS, dexItemFactory));
+ skipWhitespace();
+ }
+
private ProguardClassNameList parseClassNames() throws ProguardRuleParserException {
ProguardClassNameList.Builder builder = ProguardClassNameList.builder();
- skipWhitespace();
- boolean negated = acceptChar('!');
- builder.addClassName(negated,
- ProguardTypeMatcher.create(parseClassName(), ClassOrType.CLASS, dexItemFactory));
- skipWhitespace();
- while (acceptChar(',')) {
- negated = acceptChar('!');
- builder.addClassName(negated,
- ProguardTypeMatcher.create(parseClassName(), ClassOrType.CLASS, dexItemFactory));
- skipWhitespace();
- }
+ do {
+ parseClassNameAddToBuilder(builder);
+ } while (acceptChar(','));
return builder.build();
}
@@ -1647,8 +1670,15 @@
}
private IdentifierPatternWithWildcards parseClassName() throws ProguardRuleParserException {
- IdentifierPatternWithWildcards name =
- acceptIdentifierWithBackreference(IdentifierType.CLASS_NAME);
+ IdentifierPatternWithWildcardsAndNegation name = parseClassName(false);
+ assert !name.negated;
+ return name.patternWithWildcards;
+ }
+
+ private IdentifierPatternWithWildcardsAndNegation parseClassName(boolean allowNegation)
+ throws ProguardRuleParserException {
+ IdentifierPatternWithWildcardsAndNegation name =
+ acceptIdentifierWithBackreference(IdentifierType.CLASS_NAME, allowNegation);
if (name == null) {
throw parseError("Class name expected");
}
@@ -1835,4 +1865,15 @@
return false;
}
}
+
+ static class IdentifierPatternWithWildcardsAndNegation {
+ final IdentifierPatternWithWildcards patternWithWildcards;
+ final boolean negated;
+
+ IdentifierPatternWithWildcardsAndNegation(
+ String pattern, List<ProguardWildcard> wildcards, boolean negated) {
+ patternWithWildcards = new IdentifierPatternWithWildcards(pattern, wildcards);
+ this.negated = negated;
+ }
+ }
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTest.java
new file mode 100644
index 0000000..519c273
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTest.java
@@ -0,0 +1,102 @@
+// 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.inliner;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.InvokeInstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class InlineSynchronizedTest extends TestBase {
+
+ private static final List<String> METHOD_NAMES =
+ ImmutableList.of(
+ "println",
+ "normalInlinedSynchronized",
+ "classInlinedSynchronized",
+ "normalInlinedControl",
+ "classInlinedControl");
+
+ @Parameterized.Parameters(name = "Backend: {0}, ClassInlining: {1}")
+ public static Collection data() {
+ return buildParameters(Backend.values(), BooleanUtils.values());
+ }
+
+ private final Backend backend;
+ private final boolean classInlining;
+
+ public InlineSynchronizedTest(Backend backend, boolean classInlining) {
+ this.backend = backend;
+ this.classInlining = classInlining;
+ }
+
+ private void configure(InternalOptions options) {
+ options.enableClassInlining = classInlining;
+ }
+
+ @Test
+ public void test() throws Exception {
+ CodeInspector codeInspector =
+ testForR8(backend)
+ .addProgramClasses(InlineSynchronizedTestClass.class)
+ .addKeepMainRule("com.android.tools.r8.ir.optimize.inliner.InlineSynchronizedTestClass")
+ .addKeepRules("-dontobfuscate")
+ .addOptionsModification(o -> o.enableClassInlining = classInlining)
+ .compile()
+ .inspector();
+
+ ClassSubject classSubject = codeInspector.clazz(InlineSynchronizedTestClass.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.mainMethod();
+ Iterator<InstructionSubject> it = methodSubject.iterateInstructions();
+ int[] counts = new int[METHOD_NAMES.size()];
+ while (it.hasNext()) {
+ InstructionSubject instruction = it.next();
+ if (!instruction.isInvoke()) {
+ continue;
+ }
+ DexString invokedName = ((InvokeInstructionSubject) instruction).invokedMethod().name;
+ int idx = METHOD_NAMES.indexOf(invokedName.toString());
+ if (idx >= 0) {
+ ++counts[idx];
+ }
+ }
+ // Synchronized methods can never be inlined.
+ assertCount(counts, "normalInlinedSynchronized", 1);
+ assertCount(counts, "classInlinedSynchronized", 1);
+ // Control methods must be inlined, only the normal one or both, depending on classInlining.
+ assertCount(counts, "normalInlinedControl", 0);
+ assertCount(counts, "classInlinedControl", classInlining ? 0 : 1);
+ // Double check the total.
+ int total = 0;
+ for (int i = 0; i < counts.length; ++i) {
+ total += counts[i];
+ }
+ assertEquals(4, total);
+ }
+
+ private static void assertCount(int counts[], String methodName, int expectedCount) {
+ int idx = METHOD_NAMES.indexOf(methodName);
+ assert idx >= 0;
+ assertEquals(expectedCount, counts[idx]);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTestClass.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTestClass.java
new file mode 100644
index 0000000..4e2cae1
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTestClass.java
@@ -0,0 +1,34 @@
+// 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.inliner;
+
+class InlineSynchronizedTestClass {
+ private synchronized void normalInlinedSynchronized() {
+ System.out.println("InlineSynchronizedTestClass::normalInlinedSynchronized");
+ }
+
+ public synchronized void classInlinedSynchronized() {
+ System.out.println("InlineSynchronizedTestClass::classInlinedSynchronized");
+ }
+
+ private void normalInlinedControl() {
+ System.out.println("InlineSynchronizedTestClass::normalInlinedControl");
+ }
+
+ public void classInlinedControl() {
+ System.out.println("InlineSynchronizedTestClass::classInlinedControl");
+ }
+
+ public static void main(String[] args) {
+ // Test normal inlining.
+ InlineSynchronizedTestClass testClass = new InlineSynchronizedTestClass();
+ testClass.normalInlinedSynchronized();
+ testClass.normalInlinedControl();
+
+ // Test class-inlining.
+ new InlineSynchronizedTestClass().classInlinedSynchronized();
+ new InlineSynchronizedTestClass().classInlinedControl();
+ }
+}
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 26e5356..cac6dd3 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -2399,4 +2399,24 @@
assertThat(result.stderr, containsString(expectedMessage));
}
}
+
+ @Test
+ public void b124181032() throws Exception {
+ // Test spaces and quotes in class name list.
+ ProguardConfigurationParser parser;
+ parser = new ProguardConfigurationParser(new DexItemFactory(), reporter);
+ parser.parse(
+ createConfigurationForTesting(
+ ImmutableList.of(
+ "-keepclassmembers class \"a.b.c.**\" ,"
+ + " !**d , '!**e' , \"!**f\" , g , 'h' , \"i\" { ",
+ "<fields>;",
+ "<init>();",
+ "}")));
+ List<ProguardConfigurationRule> rules = parser.getConfig().getRules();
+ assertEquals(1, rules.size());
+ ProguardConfigurationRule rule = rules.get(0);
+ assertEquals(ProguardKeepRuleType.KEEP_CLASS_MEMBERS.toString(), rule.typeString());
+ assertEquals("a.b.c.**,!**d,!**e,!**f,g,h,i", rule.getClassNames().toString());
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
index d300a4e..da38a47 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
@@ -12,7 +12,6 @@
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatibilityTestBase;
-import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -45,11 +44,6 @@
return ImmutableList.of(Shrinker.R8_CF, Shrinker.PROGUARD6, Shrinker.R8);
}
- private void configure(InternalOptions options) {
- // Disable inlining, otherwise classes can be pruned away if all their methods are inlined.
- options.enableInlining = false;
- }
-
@Test
public void ifOnPublic_noPublicClassForIfRule() throws Exception {
List<String> config = ImmutableList.of(
@@ -64,7 +58,7 @@
" public <methods>;",
"}"
);
- CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config, this::configure);
+ CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config);
ClassSubject classSubject = codeInspector.clazz(ClassForIf.class);
assertThat(classSubject, isPresent());
MethodSubject methodSubject = classSubject.method(publicMethod);
@@ -93,7 +87,7 @@
" public <methods>;",
"}"
);
- CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config, this::configure);
+ CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config);
ClassSubject classSubject = codeInspector.clazz(ClassForIf.class);
assertThat(classSubject, isPresent());
MethodSubject methodSubject = classSubject.method(publicMethod);
@@ -126,7 +120,7 @@
" !public <methods>;",
"}"
);
- CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config, this::configure);
+ CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config);
ClassSubject classSubject = codeInspector.clazz(ClassForIf.class);
assertThat(classSubject, isPresent());
MethodSubject methodSubject = classSubject.method(publicMethod);
@@ -160,7 +154,7 @@
" public <methods>;",
"}"
);
- CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config, this::configure);
+ CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config);
ClassSubject classSubject = codeInspector.clazz(ClassForIf.class);
assertThat(classSubject, isPresent());
MethodSubject methodSubject = classSubject.method(publicMethod);
@@ -192,7 +186,7 @@
" !public <methods>;",
"}"
);
- CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config, this::configure);
+ CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config);
ClassSubject classSubject = codeInspector.clazz(ClassForIf.class);
assertThat(classSubject, isPresent());
MethodSubject methodSubject = classSubject.method(publicMethod);
