Version 1.4.96
The following changes is made directly on the branch since this
version of applymapping is no longer on newer branches.
- Allow for duplicate names for static fields and methods in mapping.
- Rewrite Signature annotation when doing applymapping.
The tests are also present on the master branch and cherry-picked:
Cherry pick: Add test for rewriting signatures when using applymapping
https://r8-review.googlesource.com/c/r8/+/38232
Cherry pick: Test using -applymapping with same static methods in type
hierarchy
https://r8-review.googlesource.com/c/r8/+/38280
Bug: 131532229
Bug: 131710444
Bug: 132593471
Change-Id: Ic89436f623e8ee12321105e1cbb722f1d0686252
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 356d63d..0190082 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.95";
+ public static final String LABEL = "1.4.96";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
index 78dad02..7b7a67f 100644
--- a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
+++ b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.naming;
+import com.android.tools.r8.graph.AppInfo.ResolutionResult;
+import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexAnnotation;
import com.android.tools.r8.graph.DexAnnotationSet;
@@ -20,6 +22,7 @@
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.naming.MemberNaming.FieldSignature;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.naming.signature.GenericSignatureRewriter;
import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
import com.android.tools.r8.utils.ArrayUtils;
import com.android.tools.r8.utils.ThrowingConsumer;
@@ -34,12 +37,14 @@
public class ProguardMapApplier {
+ private final AppView<AppInfoWithLiveness> appView;
private final AppInfoWithLiveness appInfo;
private final GraphLense previousLense;
private final SeedMapper seedMapper;
public ProguardMapApplier(AppView<AppInfoWithLiveness> appView, SeedMapper seedMapper) {
assert appView.graphLense().isContextFreeForMethods();
+ this.appView = appView;
this.appInfo = appView.appInfo();
this.previousLense = appView.graphLense();
this.seedMapper = seedMapper;
@@ -61,7 +66,7 @@
private final Map<DexProto, DexProto> protoFixupCache = new IdentityHashMap<>();
MapToLenseConverter() {
- lenseBuilder = new ConflictFreeBuilder();
+ lenseBuilder = new ConflictFreeBuilder(appInfo);
}
private GraphLense run() {
@@ -303,10 +308,26 @@
private final ConflictFreeBuilder lenseBuilder;
private final GraphLense appliedLense;
private final Map<DexProto, DexProto> protoFixupCache = new IdentityHashMap<>();
+ private final GenericSignatureRewriter signatureRewriter;
TreeFixer(GraphLense appliedLense) {
- this.lenseBuilder = new ConflictFreeBuilder();
+ this.lenseBuilder = new ConflictFreeBuilder(appInfo);
this.appliedLense = appliedLense;
+ this.signatureRewriter =
+ new GenericSignatureRewriter(
+ appView,
+ (t, ignored) -> {
+ // The second argument is different from descriptor only when parsing innner type
+ // names, where ignored == NULL. When that is the case the GenericSignatureRewriter
+ // will use the read name and not perform a rewrite. By ignoring the second argument
+ // and always returning the looked-up descriptor, we basically force a rewrite with
+ // the identity name in the null case, which is equivalent, as long as b/110085899
+ // is not reported in the null case.
+ assert t != null;
+ DexType dexType = appliedLense.lookupType(t);
+ assert dexType != null;
+ return dexType.descriptor;
+ });
}
private GraphLense run() {
@@ -317,6 +338,7 @@
// PrgA#foo signature: from LibA to a renamed name.
appInfo.classes().forEach(this::fixClass);
appInfo.libraryClasses().forEach(this::fixClass);
+ signatureRewriter.run();
return lenseBuilder.build(appInfo.dexItemFactory, appliedLense);
}
@@ -442,8 +464,12 @@
}
private static class ConflictFreeBuilder extends GraphLense.Builder {
- ConflictFreeBuilder() {
+
+ AppInfoWithSubtyping appInfo;
+
+ ConflictFreeBuilder(AppInfoWithSubtyping appInfo) {
super();
+ this.appInfo = appInfo;
}
@Override
@@ -458,25 +484,35 @@
}
@Override
- public void map(DexMethod from, DexMethod to) {
+ public void move(DexMethod from, DexMethod to) {
if (methodMap.containsKey(from)) {
String keptName = methodMap.get(from).name.toString();
if (!keptName.equals(to.name.toString())) {
+ ResolutionResult resolutionResult = appInfo.resolveMethod(to.holder, to);
+ // Static methods shadow parent implementations but can be mapped to different names.
+ if (resolutionResult.hasSingleTarget() && resolutionResult.asSingleTarget().isStatic()) {
+ return;
+ }
throw ProguardMapError.keptMethodWasRenamed(from, keptName, to.name.toString());
}
}
- super.map(from, to);
+ super.move(from, to);
}
@Override
- public void map(DexField from, DexField to) {
+ public void move(DexField from, DexField to) {
if (fieldMap.containsKey(from)) {
String keptName = fieldMap.get(from).name.toString();
if (!keptName.equals(to.name.toString())) {
+ DexEncodedField field = appInfo.resolveFieldOn(to.type, to);
+ // Static fields shadow parent implementations but can be mapped to different names.
+ if (field != null && field.isStatic()) {
+ return;
+ }
throw ProguardMapError.keptFieldWasRenamed(from, keptName, to.name.toString());
}
}
- super.map(from, to);
+ super.move(from, to);
}
// Helper to determine whether to apply the class mapping on-the-fly or applied already.
diff --git a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
index 20f4bc2..3f331ff 100644
--- a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
+++ b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
@@ -19,10 +19,11 @@
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.StringDiagnostic;
-import com.google.common.collect.Maps;
+import com.google.common.collect.ImmutableMap;
import java.lang.reflect.GenericSignatureFormatError;
import java.util.Map;
import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Supplier;
@@ -30,15 +31,24 @@
private final AppView<AppInfoWithLiveness> appView;
private final AppInfoWithLiveness appInfo;
- private final Map<DexType, DexString> renaming;
+ private final BiFunction<DexType, DexString, DexString> renaming;
private final Reporter reporter;
+ private final GenericSignatureCollector genericSignatureCollector =
+ new GenericSignatureCollector();
+ private final GenericSignatureParser<DexType> genericSignatureParser =
+ new GenericSignatureParser<>(genericSignatureCollector);
public GenericSignatureRewriter(AppView<AppInfoWithLiveness> appView) {
- this(appView, Maps.newIdentityHashMap());
+ this(appView, ImmutableMap.of());
}
public GenericSignatureRewriter(
AppView<AppInfoWithLiveness> appView, Map<DexType, DexString> renaming) {
+ this(appView, renaming::getOrDefault);
+ }
+
+ public GenericSignatureRewriter(
+ AppView<AppInfoWithLiveness> appView, BiFunction<DexType, DexString, DexString> renaming) {
this.appView = appView;
this.appInfo = appView.appInfo();
this.renaming = renaming;
@@ -46,10 +56,6 @@
}
public void run() {
- final GenericSignatureCollector genericSignatureCollector = new GenericSignatureCollector();
- final GenericSignatureParser<DexType> genericSignatureParser =
- new GenericSignatureParser<>(genericSignatureCollector);
-
for (DexClass clazz : appInfo.classes()) {
clazz.annotations =
rewriteGenericSignatures(
@@ -164,7 +170,7 @@
if (appInfo.wasPruned(type)) {
type = appInfo.dexItemFactory.objectType;
}
- DexString renamedDescriptor = renaming.getOrDefault(type, type.descriptor);
+ DexString renamedDescriptor = renaming.apply(type, type.descriptor);
renamedSignature.append(getClassBinaryNameFromDescriptor(renamedDescriptor.toString()));
return type;
}
@@ -181,9 +187,9 @@
+ name));
String enclosingRenamedBinaryName =
getClassBinaryNameFromDescriptor(
- renaming.getOrDefault(enclosingType, enclosingType.descriptor).toString());
+ renaming.apply(enclosingType, enclosingType.descriptor).toString());
type = appView.graphLense().lookupType(type);
- DexString renamedDescriptor = renaming.get(type);
+ DexString renamedDescriptor = renaming.apply(type, null);
if (renamedDescriptor != null) {
// Pick the renamed inner class from the fully renamed binary name.
String fullRenamedBinaryName =
diff --git a/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingSameStaticNameTest.java b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingSameStaticNameTest.java
new file mode 100644
index 0000000..d74692b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingSameStaticNameTest.java
@@ -0,0 +1,67 @@
+// 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 com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** This test reproduces b/131532229 on d8-1.4. */
+@RunWith(Parameterized.class)
+public class ApplyMappingSameStaticNameTest extends TestBase {
+
+ public static final String pgMap =
+ StringUtils.lines(
+ A.class.getTypeName() + " -> " + A.class.getTypeName() + ":",
+ " boolean[] jacocoInit() -> a",
+ " int f1 -> c",
+ B.class.getTypeName() + " -> " + B.class.getTypeName() + ":",
+ " boolean[] jacocoInit() -> b",
+ " int f1 -> d");
+
+ public static class A {
+ public static boolean[] a() {
+ return null;
+ }
+
+ public static int c = 1;
+ }
+
+ public static class B extends A {
+ public static boolean[] b() {
+ return null;
+ }
+
+ public static int d = 2;
+ }
+
+ public static class C {}
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ private Backend backend;
+
+ public ApplyMappingSameStaticNameTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void test_b131532229() throws CompilationFailedException {
+ testForR8(backend)
+ .noTreeShaking()
+ .noMinification()
+ .addLibraryClasses(A.class, B.class)
+ .addLibraryFiles(runtimeJar(backend))
+ .addProgramClasses(C.class)
+ .addApplyMapping(pgMap)
+ .compile();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingSignatureTest.java b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingSignatureTest.java
new file mode 100644
index 0000000..f36eef2
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/applymapping/ApplyMappingSignatureTest.java
@@ -0,0 +1,73 @@
+// 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.hamcrest.core.StringContains.containsString;
+
+import com.android.tools.r8.CompilationFailedException;
+import com.android.tools.r8.R8TestBuilder;
+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.FieldSubject;
+import java.io.IOException;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** This test reproduces b/132593471 on d8-1.4. */
+@RunWith(Parameterized.class)
+public class ApplyMappingSignatureTest extends TestBase {
+
+ public interface A {}
+
+ public static class SignatureTest {
+ Set<A> field;
+
+ public static void main(String[] args) {
+ System.out.print("HELLO");
+ }
+ }
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Backend[] data() {
+ return Backend.values();
+ }
+
+ private Backend backend;
+
+ public ApplyMappingSignatureTest(Backend backend) {
+ this.backend = backend;
+ }
+
+ @Test
+ public void testApplyMappingWithSignatureRenaming()
+ throws IOException, CompilationFailedException, ExecutionException {
+ R8TestBuilder r8TestBuilder =
+ testForR8(backend)
+ .addProgramClasses(A.class, SignatureTest.class)
+ .addKeepAllClassesRule()
+ .addKeepMainRule(SignatureTest.class)
+ .addKeepAttributes("Signature", "InnerClasses", "EnclosingMethod")
+ .addApplyMapping(A.class.getTypeName() + " -> foo:");
+ if (backend == Backend.DEX) {
+ r8TestBuilder.setMinApi(ToolHelper.getMinApiLevelForDexVm());
+ }
+ r8TestBuilder
+ .run(SignatureTest.class)
+ .assertSuccessWithOutput("HELLO")
+ .inspect(
+ inspector -> {
+ ClassSubject clazz = inspector.clazz(SignatureTest.class);
+ assertThat(clazz, isPresent());
+ FieldSubject field = clazz.uniqueFieldWithName("field");
+ assertThat(field, isPresent());
+ assertThat(field.getSignatureAnnotationValue(), containsString("foo"));
+ });
+ }
+}
