Check that string literals refer to original types when minifying
Bug: 210825394
Bug: 210699098
Change-Id: I158ae11672eaa3ce03543b61da3130680b66ea6a
diff --git a/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java b/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java
index a775a72..4decfe8 100644
--- a/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java
@@ -13,14 +13,17 @@
import com.android.tools.r8.code.Instruction;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.Code;
+import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.graph.DexValue;
import com.android.tools.r8.graph.DexValue.DexItemBasedValueString;
import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.ProguardClassFilter;
+import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.ThreadUtils;
import java.util.ArrayList;
import java.util.List;
@@ -102,7 +105,23 @@
}
private DexString getRenamedStringLiteral(DexString originalLiteral) {
- DexString rewrittenString = lens.lookupDescriptorForJavaTypeName(originalLiteral.toString());
+ String descriptor =
+ DescriptorUtils.javaTypeToDescriptorIfValidJavaType(originalLiteral.toString());
+ if (descriptor == null) {
+ return originalLiteral;
+ }
+ DexType type = appView.dexItemFactory().createType(descriptor);
+ DexType originalType = appView.graphLens().getOriginalType(type);
+ if (originalType != type) {
+ // The type has changed to something clashing with the string.
+ return originalLiteral;
+ }
+ DexType rewrittenType = appView.graphLens().lookupType(type);
+ DexClass clazz = appView.appInfo().definitionForWithoutExistenceAssert(rewrittenType);
+ if (clazz == null || clazz.isNotProgramClass()) {
+ return originalLiteral;
+ }
+ DexString rewrittenString = lens.lookupClassDescriptor(rewrittenType);
return rewrittenString == null
? originalLiteral
: appView.dexItemFactory().createString(descriptorToJavaType(rewrittenString.toString()));
diff --git a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
index cc64864..bf18111 100644
--- a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
+++ b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
@@ -34,7 +34,7 @@
ClassRenaming classRenaming,
MethodRenaming methodRenaming,
FieldRenaming fieldRenaming) {
- super(appView.dexItemFactory(), classRenaming.classRenaming);
+ super(appView.dexItemFactory());
this.appView = appView;
this.packageRenaming = classRenaming.packageRenaming;
renaming.putAll(classRenaming.classRenaming);
diff --git a/src/main/java/com/android/tools/r8/naming/NamingLens.java b/src/main/java/com/android/tools/r8/naming/NamingLens.java
index b6e94ee..5b24eb9 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingLens.java
+++ b/src/main/java/com/android/tools/r8/naming/NamingLens.java
@@ -21,8 +21,6 @@
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Map;
import java.util.Set;
/**
@@ -42,8 +40,6 @@
public abstract DexString lookupDescriptor(DexType type);
- public abstract DexString lookupDescriptorForJavaTypeName(String typeName);
-
public DexString lookupClassDescriptor(DexType type) {
assert type.isClassType();
return internalLookupClassDescriptor(type);
@@ -185,13 +181,9 @@
public abstract static class NonIdentityNamingLens extends NamingLens {
private final DexItemFactory dexItemFactory;
- private final Map<String, DexString> typeStringMapping;
- protected NonIdentityNamingLens(
- DexItemFactory dexItemFactory, Map<DexType, DexString> typeMapping) {
+ protected NonIdentityNamingLens(DexItemFactory dexItemFactory) {
this.dexItemFactory = dexItemFactory;
- typeStringMapping = new HashMap<>();
- typeMapping.forEach((k, v) -> typeStringMapping.put(k.toSourceString(), v));
}
protected DexItemFactory dexItemFactory() {
@@ -211,11 +203,6 @@
assert type.isClassType();
return lookupClassDescriptor(type);
}
-
- @Override
- public DexString lookupDescriptorForJavaTypeName(String typeName) {
- return typeStringMapping.get(typeName);
- }
}
private static final class IdentityLens extends NamingLens {
@@ -230,11 +217,6 @@
}
@Override
- public DexString lookupDescriptorForJavaTypeName(String typeName) {
- return null;
- }
-
- @Override
protected DexString internalLookupClassDescriptor(DexType type) {
return type.descriptor;
}
diff --git a/src/main/java/com/android/tools/r8/naming/PrefixRewritingNamingLens.java b/src/main/java/com/android/tools/r8/naming/PrefixRewritingNamingLens.java
index 04f235f..5ab9772 100644
--- a/src/main/java/com/android/tools/r8/naming/PrefixRewritingNamingLens.java
+++ b/src/main/java/com/android/tools/r8/naming/PrefixRewritingNamingLens.java
@@ -12,7 +12,6 @@
import com.android.tools.r8.graph.InnerClassAttribute;
import com.android.tools.r8.naming.NamingLens.NonIdentityNamingLens;
import com.android.tools.r8.utils.InternalOptions;
-import java.util.IdentityHashMap;
// Naming lens for rewriting type prefixes.
public class PrefixRewritingNamingLens extends NonIdentityNamingLens {
@@ -34,7 +33,7 @@
}
public PrefixRewritingNamingLens(NamingLens namingLens, AppView<?> appView) {
- super(appView.dexItemFactory(), new IdentityHashMap<>());
+ super(appView.dexItemFactory());
this.appView = appView;
this.namingLens = namingLens;
this.options = appView.options();
@@ -96,18 +95,6 @@
}
@Override
- public DexString lookupDescriptorForJavaTypeName(String typeName) {
- if (appView.rewritePrefix.shouldRewriteTypeName(typeName)) {
- DexType rewrittenType =
- appView.rewritePrefix.rewrittenType(dexItemFactory().createType(typeName), appView);
- if (rewrittenType != null) {
- return rewrittenType.descriptor;
- }
- }
- return namingLens.lookupDescriptorForJavaTypeName(typeName);
- }
-
- @Override
public String lookupPackageName(String packageName) {
// Used for resource shrinking.
// Desugared libraries do not have resources.
diff --git a/src/main/java/com/android/tools/r8/naming/RecordRewritingNamingLens.java b/src/main/java/com/android/tools/r8/naming/RecordRewritingNamingLens.java
index e809317..f2a4e8b 100644
--- a/src/main/java/com/android/tools/r8/naming/RecordRewritingNamingLens.java
+++ b/src/main/java/com/android/tools/r8/naming/RecordRewritingNamingLens.java
@@ -13,7 +13,6 @@
import com.android.tools.r8.graph.InnerClassAttribute;
import com.android.tools.r8.naming.NamingLens.NonIdentityNamingLens;
import com.android.tools.r8.utils.InternalOptions;
-import java.util.IdentityHashMap;
// Naming lens for rewriting java.lang.Record to the internal RecordTag type.
public class RecordRewritingNamingLens extends NonIdentityNamingLens {
@@ -34,7 +33,7 @@
}
public RecordRewritingNamingLens(NamingLens namingLens, AppView<?> appView) {
- super(appView.dexItemFactory(), new IdentityHashMap<>());
+ super(appView.dexItemFactory());
this.namingLens = namingLens;
factory = appView.dexItemFactory();
}
@@ -75,14 +74,6 @@
}
@Override
- public DexString lookupDescriptorForJavaTypeName(String typeName) {
- if (typeName.equals(factory.recordType.toSourceString())) {
- return factory.recordTagType.descriptor;
- }
- return namingLens.lookupDescriptorForJavaTypeName(typeName);
- }
-
- @Override
public String lookupPackageName(String packageName) {
return namingLens.lookupPackageName(packageName);
}
diff --git a/src/main/java/com/android/tools/r8/relocator/SimplePackagesRewritingMapper.java b/src/main/java/com/android/tools/r8/relocator/SimplePackagesRewritingMapper.java
index 3a5902a..7afb7e3 100644
--- a/src/main/java/com/android/tools/r8/relocator/SimplePackagesRewritingMapper.java
+++ b/src/main/java/com/android/tools/r8/relocator/SimplePackagesRewritingMapper.java
@@ -102,7 +102,7 @@
Map<DexType, DexString> typeMappings,
Map<String, String> packageMappings,
DexItemFactory factory) {
- super(factory, typeMappings);
+ super(factory);
this.typeMappings = typeMappings;
this.packageMappings = packageMappings;
}
diff --git a/src/test/java/com/android/tools/r8/naming/adaptclassstrings/AdaptClassStringWithRepackagingTest.java b/src/test/java/com/android/tools/r8/naming/adaptclassstrings/AdaptClassStringWithRepackagingTest.java
index 6bcb1a5..06557fa 100644
--- a/src/test/java/com/android/tools/r8/naming/adaptclassstrings/AdaptClassStringWithRepackagingTest.java
+++ b/src/test/java/com/android/tools/r8/naming/adaptclassstrings/AdaptClassStringWithRepackagingTest.java
@@ -36,8 +36,7 @@
.addKeepRules("-adaptclassstrings")
.addKeepClassRulesWithAllowObfuscation(Foo.class)
.run(parameters.getRuntime(), Main.class)
- // TODO(b/210699098): The class is renamed and so should the output
- .assertSuccessWithOutputLines(Foo.class.getName())
+ .assertSuccessWithOutputLines("a")
.inspect(inspector -> assertThat(inspector.clazz(Foo.class), isPresentAndRenamed()));
}
diff --git a/src/test/java/com/android/tools/r8/naming/adaptclassstrings/RepackageMinificationNameClashTest.java b/src/test/java/com/android/tools/r8/naming/adaptclassstrings/RepackageMinificationNameClashTest.java
index 00dc064..1f23283 100644
--- a/src/test/java/com/android/tools/r8/naming/adaptclassstrings/RepackageMinificationNameClashTest.java
+++ b/src/test/java/com/android/tools/r8/naming/adaptclassstrings/RepackageMinificationNameClashTest.java
@@ -34,8 +34,7 @@
.addKeepClassRulesWithAllowObfuscation(Foo.class)
.addKeepMainRule(Main.class)
.run(parameters.getRuntime(), Main.class)
- // TODO(b/210699098): Should be RepackageMinificationNameClashTest$Foo.
- .assertSuccessWithOutputLines("a");
+ .assertSuccessWithOutputLines("RepackageMinificationNameClashTest$Foo");
}
public static class Foo {}
