Version 1.6.41
Cherry pick: Add test asserting no renaming of interface clinits using
applymapping
CL: https://r8-review.googlesource.com/c/r8/+/44541
Cherry pick: Reserve clinits in proguardmapminifier name strategy
CL: https://r8-review.googlesource.com/c/r8/+/44542
Cherry pick: Do not statically merge classes with synchronized methods
CL: https://r8-review.googlesource.com/c/r8/+/44227
Cherry pick: Classes with synchronized methods should not be merged
vertically
CL: https://r8-review.googlesource.com/c/r8/+/44226
Cherry pick: Update isStrict to isStatic
CL: https://r8-review.googlesource.com/c/r8/+/44360
Cherry pick: Negate assertion regarding synchronized method
CL: https://r8-review.googlesource.com/c/r8/+/44362
Cherry pick: Refactor static class merger merge into a single method
CL: https://r8-review.googlesource.com/c/r8/+/44464
Cherry pick: Reland "Find set of lock-candidates from const-class to disallow in merging"
CL: https://r8-review.googlesource.com/c/r8/+/44742
Cherry pick: Add utility to testbase for testing on current runtime
CL: https://r8-review.googlesource.com/c/r8/+/44182
Bug: 142438687
Bug: 142909857
Change-Id: I9371099fccff6f480711774aca69aeb32dc0f873
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 38f3cb0..d8c530b 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.6.40";
+ public static final String LABEL = "1.6.41";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index 6f5df28..7ce2685 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -924,4 +924,13 @@
assert verifyNoDuplicateMethods();
return true;
}
+
+ public boolean hasStaticSynchronizedMethods() {
+ for (DexEncodedMethod encodedMethod : directMethods()) {
+ if (encodedMethod.accessFlags.isStatic() && encodedMethod.accessFlags.isSynchronized()) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java b/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
index dc90b8f..47a8d3f 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapMinifier.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexClass;
+import com.android.tools.r8.graph.DexDefinition;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
@@ -440,7 +441,20 @@
DexMethod method,
InternalNamingState internalState,
BiPredicate<DexString, DexMethod> isAvailable) {
- return nextName(method, method.name, method.holder, internalState, isAvailable, super::next);
+ DexEncodedMethod definition = appView.definitionFor(method);
+ DexString nextName =
+ nextName(
+ method,
+ definition,
+ method.name,
+ method.holder,
+ internalState,
+ isAvailable,
+ super::next);
+ assert nextName == method.name || !definition.isClassInitializer();
+ assert nextName == method.name
+ || !appView.definitionFor(method.holder).accessFlags.isAnnotation();
+ return nextName;
}
@Override
@@ -448,22 +462,32 @@
DexField field,
InternalNamingState internalState,
BiPredicate<DexString, DexField> isAvailable) {
- return nextName(field, field.name, field.holder, internalState, isAvailable, super::next);
+ return nextName(
+ field,
+ appView.definitionFor(field),
+ field.name,
+ field.holder,
+ internalState,
+ isAvailable,
+ super::next);
}
private <T extends DexReference> DexString nextName(
T reference,
+ DexDefinition definition,
DexString name,
DexType holderType,
InternalNamingState internalState,
BiPredicate<DexString, T> isAvailable,
TriFunction<T, InternalNamingState, BiPredicate<DexString, T>, DexString> generateName) {
+ assert definition.isDexEncodedMethod() || definition.isDexEncodedField();
+ assert definition.toReference() == reference;
DexClass holder = appView.definitionFor(holderType);
assert holder != null;
- DexString reservedName = getReservedName(reference, name, holder);
+ DexString reservedName = getReservedName(definition, name, holder);
if (reservedName != null) {
if (!isAvailable.test(reservedName, reference)) {
- reportReservationError(reference, reservedName);
+ reportReservationError(definition.asDexReference(), reservedName);
}
return reservedName;
}
@@ -474,26 +498,36 @@
@Override
public DexString getReservedName(DexEncodedMethod method, DexClass holder) {
- return getReservedName(method.method, method.method.name, holder);
+ return getReservedName(method, method.method.name, holder);
}
@Override
public DexString getReservedName(DexEncodedField field, DexClass holder) {
- return getReservedName(field.field, field.field.name, holder);
+ return getReservedName(field, field.field.name, holder);
}
- private DexString getReservedName(DexReference reference, DexString name, DexClass holder) {
+ private DexString getReservedName(DexDefinition definition, DexString name, DexClass holder) {
+ assert definition.isDexEncodedMethod() || definition.isDexEncodedField();
// Always consult the mapping for renamed members that are not on program path.
- if (holder.isNotProgramClass() && mappedNames.containsKey(reference)) {
- return factory.createString(mappedNames.get(reference).getRenamedName());
- }
- if (holder.isProgramClass() && appView.rootSet().mayBeMinified(reference, appView)) {
+ DexReference reference = definition.toReference();
+ if (holder.isNotProgramClass()) {
if (mappedNames.containsKey(reference)) {
return factory.createString(mappedNames.get(reference).getRenamedName());
}
- return null;
+ return name;
}
- return name;
+ assert holder.isProgramClass();
+ DexString reservedName =
+ definition.isDexEncodedMethod()
+ ? super.getReservedName(definition.asDexEncodedMethod(), holder)
+ : super.getReservedName(definition.asDexEncodedField(), holder);
+ if (reservedName != null) {
+ return reservedName;
+ }
+ if (mappedNames.containsKey(reference)) {
+ return factory.createString(mappedNames.get(reference).getRenamedName());
+ }
+ return null;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 9676a05..c7776f2 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -151,6 +151,8 @@
public final Set<DexType> neverClassInline;
/** All types that *must* never be merged due to a configuration directive (testing only). */
public final Set<DexType> neverMerge;
+ /** Set of const-class references. */
+ public final Set<DexType> constClassReferences;
/**
* All methods and fields whose value *must* never be propagated due to a configuration directive.
* (testing only).
@@ -220,7 +222,8 @@
Set<DexType> prunedTypes,
Map<DexField, Int2ReferenceMap<DexField>> switchMaps,
Map<DexType, Map<DexField, EnumValueInfo>> enumValueInfoMaps,
- Set<DexType> instantiatedLambdas) {
+ Set<DexType> instantiatedLambdas,
+ Set<DexType> constClassReferences) {
super(application);
this.liveTypes = liveTypes;
this.instantiatedAnnotationTypes = instantiatedAnnotationTypes;
@@ -260,6 +263,7 @@
this.switchMaps = switchMaps;
this.enumValueInfoMaps = enumValueInfoMaps;
this.instantiatedLambdas = instantiatedLambdas;
+ this.constClassReferences = constClassReferences;
}
public AppInfoWithLiveness(
@@ -299,7 +303,8 @@
Set<DexType> prunedTypes,
Map<DexField, Int2ReferenceMap<DexField>> switchMaps,
Map<DexType, Map<DexField, EnumValueInfo>> enumValueInfoMaps,
- Set<DexType> instantiatedLambdas) {
+ Set<DexType> instantiatedLambdas,
+ Set<DexType> constClassReferences) {
super(appInfoWithSubtyping);
this.liveTypes = liveTypes;
this.instantiatedAnnotationTypes = instantiatedAnnotationTypes;
@@ -339,6 +344,7 @@
this.switchMaps = switchMaps;
this.enumValueInfoMaps = enumValueInfoMaps;
this.instantiatedLambdas = instantiatedLambdas;
+ this.constClassReferences = constClassReferences;
}
private AppInfoWithLiveness(AppInfoWithLiveness previous) {
@@ -391,7 +397,8 @@
: CollectionUtils.mergeSets(previous.prunedTypes, removedClasses),
previous.switchMaps,
previous.enumValueInfoMaps,
- previous.instantiatedLambdas);
+ previous.instantiatedLambdas,
+ previous.constClassReferences);
copyMetadataFromPrevious(previous);
assert removedClasses == null || assertNoItemRemoved(previous.pinnedItems, removedClasses);
}
@@ -476,6 +483,7 @@
.collect(Collectors.toList()));
this.switchMaps = rewriteReferenceKeys(previous.switchMaps, lense::lookupField);
this.enumValueInfoMaps = rewriteReferenceKeys(previous.enumValueInfoMaps, lense::lookupType);
+ this.constClassReferences = previous.constClassReferences;
}
public AppInfoWithLiveness(
@@ -521,6 +529,7 @@
this.prunedTypes = previous.prunedTypes;
this.switchMaps = switchMaps;
this.enumValueInfoMaps = enumValueInfoMaps;
+ this.constClassReferences = previous.constClassReferences;
previous.markObsolete();
}
@@ -582,6 +591,16 @@
return interfaces;
}
+ /**
+ * Const-classes is a conservative set of types that may be lock-candidates and cannot be merged.
+ * When using synchronized blocks, we cannot ensure that const-class locks will not flow in. This
+ * can potentially cause incorrect behavior when merging classes. A conservative choice is to not
+ * merge any const-class classes. More info at b/142438687.
+ */
+ public boolean isLockCandidate(DexType type) {
+ return constClassReferences.contains(type);
+ }
+
public AppInfoWithLiveness withoutStaticFieldsWrites(Set<DexField> noLongerWrittenFields) {
assert checkIfObsolete();
if (noLongerWrittenFields.isEmpty()) {
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index b38471a..f602541 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -299,6 +299,12 @@
private final Set<DexReference> pinnedItems = Sets.newIdentityHashSet();
/**
+ * A set of seen const-class references that both serve as an initial lock-candidate set and will
+ * prevent statically merging the classes referenced.
+ */
+ private final Set<DexType> constClassReferences = Sets.newIdentityHashSet();
+
+ /**
* A map from classes to annotations that need to be processed should the classes ever become
* live.
*/
@@ -910,6 +916,15 @@
@Override
public boolean registerConstClass(DexType type) {
+ // We conservatively group T.class and T[].class to ensure that we do not merge T with S if
+ // potential locks on T[].class and S[].class exists.
+ DexType baseType = type.toBaseType(appView.dexItemFactory());
+ if (baseType.isClassType()) {
+ DexProgramClass baseClass = getProgramClassOrNull(baseType);
+ if (baseClass != null) {
+ constClassReferences.add(baseType);
+ }
+ }
return registerConstClassOrCheckCast(type);
}
@@ -2167,7 +2182,8 @@
Collections.emptyMap(),
Collections.emptyMap(),
SetUtils.mapIdentityHashSet(
- instantiatedInterfaceTypes.getItems(), DexProgramClass::getType));
+ instantiatedInterfaceTypes.getItems(), DexProgramClass::getType),
+ constClassReferences);
appInfo.markObsolete();
return appInfoWithLiveness;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
index 7f16e07..dc69005 100644
--- a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -157,6 +157,8 @@
private final HashMultiset<Wrapper<DexField>> fieldBuckets = HashMultiset.create();
private final HashMultiset<Wrapper<DexMethod>> methodBuckets = HashMultiset.create();
+ private boolean hasSynchronizedMethods = false;
+
public Representative(DexProgramClass clazz) {
this.clazz = clazz;
include(clazz);
@@ -168,10 +170,14 @@
Wrapper<DexField> wrapper = fieldEquivalence.wrap(field.field);
fieldBuckets.add(wrapper);
}
+ boolean classHasSynchronizedMethods = false;
for (DexEncodedMethod method : clazz.methods()) {
+ assert !hasSynchronizedMethods || !method.accessFlags.isSynchronized();
+ classHasSynchronizedMethods |= method.accessFlags.isSynchronized();
Wrapper<DexMethod> wrapper = methodEquivalence.wrap(method.method);
methodBuckets.add(wrapper);
}
+ hasSynchronizedMethods |= classHasSynchronizedMethods;
}
// Returns true if this representative should no longer be used. The current heuristic is to
@@ -317,83 +323,47 @@
assert satisfiesMergeCriteria(clazz) == group;
assert group != MergeGroup.DONT_MERGE;
- String pkg = clazz.type.getPackageDescriptor();
- return mayMergeAcrossPackageBoundaries(clazz)
- ? mergeGlobally(clazz, pkg, group)
- : mergeInsidePackage(clazz, pkg, group);
+ return merge(
+ clazz,
+ mayMergeAcrossPackageBoundaries(clazz)
+ ? group.globalKey()
+ : group.key(clazz.type.getPackageDescriptor()));
}
- private boolean mergeGlobally(DexProgramClass clazz, String pkg, MergeGroup group) {
- Representative globalRepresentative = representatives.get(group.globalKey());
- if (globalRepresentative == null) {
- if (isValidRepresentative(clazz)) {
- // Make the current class the global representative.
- setRepresentative(group.globalKey(), getOrCreateRepresentative(group.key(pkg), clazz));
- } else {
- clearRepresentative(group.globalKey());
- }
-
- // Do not attempt to merge this class inside its own package, because that could lead to
- // an increase in the global representative, which is not desirable.
- return false;
- } else {
- // Check if we can merge the current class into the current global representative.
- globalRepresentative.include(clazz);
-
- if (globalRepresentative.isFull()) {
- if (isValidRepresentative(clazz)) {
- // Make the current class the global representative instead.
- setRepresentative(group.globalKey(), getOrCreateRepresentative(group.key(pkg), clazz));
- } else {
- clearRepresentative(group.globalKey());
- }
-
- // Do not attempt to merge this class inside its own package, because that could lead to
- // an increase in the global representative, which is not desirable.
+ private boolean merge(DexProgramClass clazz, MergeGroup.Key key) {
+ Representative representative = representatives.get(key);
+ if (representative != null) {
+ if (representative.hasSynchronizedMethods && clazz.hasStaticSynchronizedMethods()) {
+ // We are not allowed to merge synchronized classes with synchronized methods.
return false;
- } else {
- // Merge this class into the global representative.
- moveMembersFromSourceToTarget(clazz, globalRepresentative.clazz);
- return true;
}
- }
- }
-
- private boolean mergeInsidePackage(DexProgramClass clazz, String pkg, MergeGroup group) {
- MergeGroup.Key key = group.key(pkg);
- Representative packageRepresentative = representatives.get(key);
- if (packageRepresentative != null) {
+ if (appView.appInfo().constClassReferences.contains(clazz.type)) {
+ // Since the type is const-class referenced (and the static merger does not create a lense
+ // to map the merged type) the class will likely remain and there is no gain from merging.
+ return false;
+ }
+ // Check if current candidate is a better choice depending on visibility. For package private
+ // or protected, the key is parameterized by the package name already, so we just have to
+ // check accessibility-flags. For global this is no-op.
if (isValidRepresentative(clazz)
- && clazz.accessFlags.isMoreVisibleThan(
- packageRepresentative.clazz.accessFlags,
- clazz.type.getPackageName(),
- packageRepresentative.clazz.type.getPackageName())) {
- // Use `clazz` as a representative for this package instead.
+ && !representative.clazz.accessFlags.isAtLeastAsVisibleAs(clazz.accessFlags)) {
+ assert clazz.type.getPackageDescriptor().equals(key.packageOrGlobal);
+ assert representative.clazz.type.getPackageDescriptor().equals(key.packageOrGlobal);
Representative newRepresentative = getOrCreateRepresentative(key, clazz);
- newRepresentative.include(packageRepresentative.clazz);
-
+ newRepresentative.include(representative.clazz);
if (!newRepresentative.isFull()) {
- setRepresentative(group.key(pkg), newRepresentative);
- moveMembersFromSourceToTarget(packageRepresentative.clazz, clazz);
+ setRepresentative(key, newRepresentative);
+ moveMembersFromSourceToTarget(representative.clazz, clazz);
return true;
}
-
- // We are not allowed to merge members into a class that is less visible.
- return false;
- }
-
- // Merge current class into the representative of this package if it has room.
- packageRepresentative.include(clazz);
-
- // If there is room, then merge, otherwise fall-through to update the representative of this
- // package.
- if (!packageRepresentative.isFull()) {
- moveMembersFromSourceToTarget(clazz, packageRepresentative.clazz);
- return true;
+ } else {
+ representative.include(clazz);
+ if (!representative.isFull()) {
+ moveMembersFromSourceToTarget(clazz, representative.clazz);
+ return true;
+ }
}
}
-
- // We were unable to use the current representative for this package (if any).
if (isValidRepresentative(clazz)) {
setRepresentative(key, getOrCreateRepresentative(key, clazz));
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index ef9a378..53d2179 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -137,6 +137,7 @@
RESOLUTION_FOR_FIELDS_MAY_CHANGE,
RESOLUTION_FOR_METHODS_MAY_CHANGE,
SERVICE_LOADER,
+ SOURCE_AND_TARGET_LOCK_CANDIDATES,
STATIC_INITIALIZERS,
UNHANDLED_INVOKE_DIRECT,
UNHANDLED_INVOKE_SUPER,
@@ -183,6 +184,9 @@
case SERVICE_LOADER:
message = "it is used by a service loader";
break;
+ case SOURCE_AND_TARGET_LOCK_CANDIDATES:
+ message = "source and target are both lock-candidates";
+ break;
case STATIC_INITIALIZERS:
message = "merging of static initializers are not supported";
break;
@@ -463,6 +467,17 @@
}
return false;
}
+ boolean sourceCanBeSynchronizedOn =
+ appView.appInfo().isLockCandidate(clazz.type) || clazz.hasStaticSynchronizedMethods();
+ boolean targetCanBeSynchronizedOn =
+ appView.appInfo().isLockCandidate(targetClass.type)
+ || targetClass.hasStaticSynchronizedMethods();
+ if (sourceCanBeSynchronizedOn && targetCanBeSynchronizedOn) {
+ if (Log.ENABLED) {
+ AbortReason.SOURCE_AND_TARGET_LOCK_CANDIDATES.printLogMessageForClass(clazz);
+ }
+ return false;
+ }
if (targetClass.getEnclosingMethod() != null || !targetClass.getInnerClasses().isEmpty()) {
// TODO(herhut): Consider supporting merging of enclosing-method and inner-class attributes.
if (Log.ENABLED) {
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 9b8abe6..797d185 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -147,6 +147,17 @@
return testForJvm(temp);
}
+ public TestBuilder<?, ?> testForRuntime(TestRuntime runtime, AndroidApiLevel apiLevel) {
+ if (runtime.isCf()) {
+ return testForJvm();
+ } else {
+ assert runtime.isDex();
+ D8TestBuilder d8TestBuilder = testForD8();
+ d8TestBuilder.setMinApi(apiLevel);
+ return d8TestBuilder;
+ }
+ }
+
public ProguardTestBuilder testForProguard() {
return testForProguard(temp);
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/HorizontalClassMergerShouldMergeSynchronizedMethodTest.java b/src/test/java/com/android/tools/r8/classmerging/HorizontalClassMergerShouldMergeSynchronizedMethodTest.java
new file mode 100644
index 0000000..7007821
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/HorizontalClassMergerShouldMergeSynchronizedMethodTest.java
@@ -0,0 +1,82 @@
+// 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.classmerging;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+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 java.io.IOException;
+import java.util.concurrent.ExecutionException;
+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 HorizontalClassMergerShouldMergeSynchronizedMethodTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public HorizontalClassMergerShouldMergeSynchronizedMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testNoMergingOfClassUsedInMonitor()
+ throws IOException, CompilationFailedException, ExecutionException {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(HorizontalClassMergerShouldMergeSynchronizedMethodTest.class)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("1", "2", "3")
+ .inspect(
+ inspector -> {
+ assertFalse(
+ inspector.clazz(LockOne.class).isPresent()
+ && inspector.clazz(LockTwo.class).isPresent());
+ assertTrue(
+ inspector.clazz(LockOne.class).isPresent()
+ || inspector.clazz(LockTwo.class).isPresent());
+ });
+ }
+
+ // Will be merged with LockTwo.
+ static class LockOne {
+
+ static synchronized void print1() {
+ System.out.println("1");
+ }
+
+ static synchronized void print2() {
+ System.out.println("2");
+ }
+ }
+
+ public static class LockTwo {
+
+ static void print3() {
+ System.out.println("3");
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ LockOne.print1();
+ LockOne.print2();
+ LockTwo.print3();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerShouldMergeSynchronizedMethodTest.java b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerShouldMergeSynchronizedMethodTest.java
new file mode 100644
index 0000000..5f6e255
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerShouldMergeSynchronizedMethodTest.java
@@ -0,0 +1,85 @@
+// 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.classmerging;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+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 java.io.IOException;
+import java.util.concurrent.ExecutionException;
+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 VerticalClassMergerShouldMergeSynchronizedMethodTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public VerticalClassMergerShouldMergeSynchronizedMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testNoMergingOfClassUsedInMonitor()
+ throws IOException, CompilationFailedException, ExecutionException {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(VerticalClassMergerShouldMergeSynchronizedMethodTest.class)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("1", "2", "3", "4")
+ .inspect(
+ inspector -> {
+ assertThat(inspector.clazz(LockOne.class), not(isPresent()));
+ assertThat(inspector.clazz(LockTwo.class), isPresent());
+ });
+ }
+
+ // Will be merged with LockTwo.
+ static class LockOne {
+
+ synchronized void print1() {
+ System.out.println("1");
+ print2();
+ }
+
+ private synchronized void print2() {
+ System.out.println("2");
+ }
+ }
+
+ public static class LockTwo extends LockOne {
+
+ synchronized void print3() {
+ System.out.println("3");
+ }
+
+ static void print4() {
+ System.out.println("4");
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ LockTwo lockTwo = new LockTwo();
+ lockTwo.print1();
+ lockTwo.print3();
+ LockTwo.print4();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerSynchronizedBlockWithArraysTest.java b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerSynchronizedBlockWithArraysTest.java
new file mode 100644
index 0000000..b6039db
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/classmerging/VerticalClassMergerSynchronizedBlockWithArraysTest.java
@@ -0,0 +1,121 @@
+// 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.classmerging;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+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.ToolHelper.DexVm.Version;
+import java.io.IOException;
+import java.lang.Thread.State;
+import java.util.concurrent.ExecutionException;
+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 VerticalClassMergerSynchronizedBlockWithArraysTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ // The 4.0.4 runtime will flakily mark threads as blocking and report DEADLOCKED.
+ return getTestParameters()
+ .withDexRuntimesStartingFromExcluding(Version.V4_0_4)
+ .withAllApiLevels()
+ .build();
+ }
+
+ public VerticalClassMergerSynchronizedBlockWithArraysTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testOnRuntime() throws IOException, CompilationFailedException, ExecutionException {
+ testForRuntime(parameters.getRuntime(), parameters.getApiLevel())
+ .addInnerClasses(VerticalClassMergerSynchronizedBlockWithArraysTest.class)
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutput("Hello World!");
+ }
+
+ @Test
+ public void testNoMergingOfClassUsedInMonitor()
+ throws IOException, CompilationFailedException, ExecutionException {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(VerticalClassMergerSynchronizedBlockWithArraysTest.class)
+ .addKeepMainRule(Main.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutput("Hello World!")
+ .inspect(inspector -> assertThat(inspector.clazz(LockOne.class), isPresent()));
+ }
+
+ abstract static class LockOne {}
+
+ static class LockTwo extends LockOne {}
+
+ static class LockThree {}
+
+ public static class Main {
+
+ private static volatile boolean inLockThreeCritical = false;
+ private static volatile boolean inLockTwoCritical = false;
+ private static volatile boolean arnoldWillNotBeBack = false;
+
+ private static volatile Thread t1 = new Thread(Main::lockThreeThenOne);
+ private static volatile Thread t2 = new Thread(Main::lockTwoThenThree);
+ private static volatile Thread t3 = new Thread(Main::arnold);
+
+ static void synchronizedAccessThroughLocks(String arg) {
+ System.out.print(arg);
+ }
+
+ public static void main(String[] args) {
+ t1.start();
+ t2.start();
+ // This thread is started to ensure termination in case we are rewriting incorrectly.
+ t3.start();
+
+ while (!arnoldWillNotBeBack) {}
+ }
+
+ static void lockThreeThenOne() {
+ synchronized (LockThree[].class) {
+ inLockThreeCritical = true;
+ while (!inLockTwoCritical) {}
+ synchronized (LockOne[].class) {
+ synchronizedAccessThroughLocks("Hello ");
+ }
+ }
+ }
+
+ static void lockTwoThenThree() {
+ synchronized (LockTwo[].class) {
+ inLockTwoCritical = true;
+ while (!inLockThreeCritical) {}
+ synchronized (LockThree[].class) {
+ synchronizedAccessThroughLocks("World!");
+ }
+ }
+ }
+
+ static void arnold() {
+ while (t1.getState() != State.TERMINATED || t2.getState() != State.TERMINATED) {
+ if (t1.getState() == State.BLOCKED && t2.getState() == State.BLOCKED) {
+ System.err.println("DEADLOCKED!");
+ System.exit(1);
+ break;
+ }
+ }
+ arnoldWillNotBeBack = true;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
index a044d3f..f8aa866 100644
--- a/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/SimplifyIfNotNullKotlinTest.java
@@ -34,7 +34,10 @@
final String mainClassName = ex1.getClassName();
final String extraRules =
keepMainMethod(mainClassName) + neverInlineMethod(mainClassName, testMethodSignature);
- runTest(FOLDER, mainClassName, extraRules,
+ runTest(
+ FOLDER,
+ mainClassName,
+ extraRules,
InternalOptions::enableCallSiteOptimizationInfoPropagation,
app -> {
CodeInspector codeInspector = new CodeInspector(app);
@@ -43,8 +46,7 @@
MethodSubject testMethod = checkMethodIsKept(clazz, testMethodSignature);
long ifzCount =
testMethod.streamInstructions().filter(i -> i.isIfEqz() || i.isIfNez()).count();
- long paramNullCheckCount =
- countCall(testMethod, "ArrayIteratorKt", "checkParameterIsNotNull");
+ long paramNullCheckCount = countCall(testMethod, "Intrinsics", "checkParameterIsNotNull");
// One after Iterator#hasNext, and another in the filter predicate: sinceYear != null.
assertEquals(2, ifzCount);
assertEquals(allowAccessModification ? 0 : 5, paramNullCheckCount);
diff --git a/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingInterfaceClInitTest.java b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingInterfaceClInitTest.java
new file mode 100644
index 0000000..04081f8
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingInterfaceClInitTest.java
@@ -0,0 +1,101 @@
+// 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.naming.applymapping;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+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 com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+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 ApplyMappingInterfaceClInitTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public ApplyMappingInterfaceClInitTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testNotRenamingClInitIfNotInMap()
+ throws ExecutionException, CompilationFailedException, IOException {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ApplyMappingInterfaceClInitTest.class)
+ .addKeepMainRule(Main.class)
+ .addKeepClassAndMembersRules(TestInterface.class)
+ .addApplyMapping("")
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .inspect(this::verifyNoRenamingOfClInit);
+ }
+
+ @Test
+ public void testNotRenamingClInitIfInMap()
+ throws ExecutionException, CompilationFailedException, IOException {
+ String interfaceName = TestInterface.class.getTypeName();
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ApplyMappingInterfaceClInitTest.class)
+ .addKeepMainRule(Main.class)
+ .addKeepClassAndMembersRules(TestInterface.class)
+ .addApplyMapping(
+ StringUtils.lines(
+ interfaceName + " -> " + interfaceName + ":", " void <clinit>() -> a"))
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .inspect(this::verifyNoRenamingOfClInit);
+ }
+
+ private void verifyNoRenamingOfClInit(CodeInspector inspector) {
+ ClassSubject interfaceSubject = inspector.clazz(TestInterface.class);
+ assertThat(interfaceSubject, isPresent());
+ interfaceSubject.allMethods().stream()
+ .allMatch(
+ method -> {
+ boolean classInitNotRenamed = !method.isClassInitializer() || !method.isRenamed();
+ assertTrue(classInitNotRenamed);
+ return classInitNotRenamed;
+ });
+ }
+
+ public interface TestInterface {
+ Throwable t = new Throwable();
+
+ void foo();
+ }
+
+ @NeverClassInline
+ public static class Main implements TestInterface {
+
+ public static void main(String[] args) {
+ new Main().foo();
+ }
+
+ @Override
+ @NeverInline
+ public void foo() {
+ System.out.println("Hello World!");
+ }
+ }
+}
