Ensure lookupDescriptor do not introduce duplicate minified types
This should ideally not happen however, if we do not correctly visit
live types, this assert will catch an error before verification errors
on load time.
Change-Id: I29ee105702ca520ddb01b37da52fe14b8fc1c708
diff --git a/src/main/java/com/android/tools/r8/naming/Minifier.java b/src/main/java/com/android/tools/r8/naming/Minifier.java
index becc643..47137e5 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -24,6 +24,7 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableMap;
+import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
@@ -91,6 +92,8 @@
private final AppInfo appInfo;
private final Map<String, String> packageRenaming;
private final Map<DexItem, DexString> renaming = new IdentityHashMap<>();
+ // This set is only used for asserting no duplicated names.
+ private final Map<DexString, DexType> renamedTypesForVerification;
private MinifiedRenaming(
ClassRenaming classRenaming,
@@ -103,6 +106,10 @@
renaming.putAll(methodRenaming.renaming);
renaming.putAll(methodRenaming.callSiteRenaming);
renaming.putAll(fieldRenaming.renaming);
+ renamedTypesForVerification = new HashMap<>();
+ for (Map.Entry<DexType, DexString> entry : classRenaming.classRenaming.entrySet()) {
+ renamedTypesForVerification.put(entry.getValue(), entry.getKey());
+ }
}
@Override
@@ -112,7 +119,18 @@
@Override
public DexString lookupDescriptor(DexType type) {
- return renaming.getOrDefault(type, type.descriptor);
+ DexString dexString = renaming.get(type);
+ if (dexString != null) {
+ return dexString;
+ }
+ assert type.isPrimitiveType()
+ || type.isVoidType()
+ || !renamedTypesForVerification.containsKey(type.descriptor)
+ : "Duplicate minified type '"
+ + type.descriptor
+ + "' already mapped for: "
+ + renamedTypesForVerification.get(type.descriptor);
+ return type.descriptor;
}
@Override
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierTest.java b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
index 473ca53..adf74ce 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
@@ -3,13 +3,16 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.naming;
+import static junit.framework.TestCase.assertTrue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.fail;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.naming.Minifier.MinifiedRenaming;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Timing;
import java.nio.file.Paths;
@@ -40,6 +43,25 @@
inspection.accept(dexItemFactory, naming);
}
+ @Test
+ public void ensureClassesAddedToRenamingOrNoClashTest() throws Exception {
+ MinifiedRenaming naming =
+ (MinifiedRenaming) runMinifier(ListUtils.map(keepRulesFiles, Paths::get));
+ // Create a type that exists.
+ String existingType = "La/c;";
+ DexType d = dexItemFactory.createType(existingType);
+ try {
+ naming.lookupDescriptor(d);
+ } catch (AssertionError ae) {
+ assertTrue(
+ ae.getMessage()
+ .startsWith(
+ "Duplicate minified type '" + existingType + "' already mapped for: naming001."));
+ return;
+ }
+ fail("Should have thrown an error.");
+ }
+
@Parameters(name = "test: {0} keep: {1}")
public static Collection<Object[]> data() {
List<String> tests = Arrays.asList("naming001");
