Don't class-inline synchronized methods (even for CF).
Bug:112295630
Change-Id: I07600cf9229ef3aa258f8980ed393527232ed5fa
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 359223d..30712e6 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
@@ -1055,8 +1055,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/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/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);
