Prevent minifier to rename into classpath names
Bug: b/414335863
Change-Id: I616d16a7d4a3fd2a4074e97da2735b57b720e55d
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index f6279b7..b148ad9 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -541,6 +541,8 @@
// Collect the already pruned types before creating a new app info without liveness.
// TODO: we should avoid removing liveness.
Set<DexType> prunedTypes = appView.withLiveness().appInfo().getPrunedTypes();
+ Set<DexType> prunedClasspathTypes =
+ appView.withLiveness().appInfo().getClasspathTypesIncludingPruned();
assert ArtProfileCompletenessChecker.verify(appView);
@@ -584,6 +586,7 @@
ImmediateAppSubtypingInfo.create(appView),
keptGraphConsumer,
prunedTypes,
+ prunedClasspathTypes,
finalRuntimeTypeCheckInfoBuilder);
EnqueuerResult enqueuerResult =
enqueuer.traceApplication(appView.rootSet(), executorService, timing);
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index 4d87bcd..c6bc840 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -88,11 +88,18 @@
ClassRenaming computeRenaming(Timing timing) {
// Collect names we have to keep.
timing.begin("reserve");
+ if (appView.options().isMinifying()
+ && !appView.options().getProguardConfiguration().hasApplyMappingFile()) {
+ for (DexType reserved : appView.appInfo().getClasspathTypesIncludingPruned()) {
+ registerClassAsUsed(reserved, reserved.getDescriptor());
+ }
+ }
for (ProgramOrClasspathClass clazz : classes) {
- DexString descriptor = classNamingStrategy.reservedDescriptor(clazz.getType());
+ DexType type = clazz.getType();
+ DexString descriptor = classNamingStrategy.reservedDescriptor(type);
if (descriptor != null) {
- assert !renaming.containsKey(clazz.getType());
- registerClassAsUsed(clazz.getType(), descriptor);
+ assert !renaming.containsKey(type);
+ registerClassAsUsed(type, descriptor);
}
}
appView
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 6828ffd..d1ac802 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -76,7 +76,7 @@
assert new MinifiedRenaming(
appView, classRenaming, MethodRenaming.empty(), FieldRenaming.empty())
- .verifyNoCollisions(appView.appInfo().classes(), appView.dexItemFactory());
+ .verifyNoCollisions(appView.appInfo(), appView.dexItemFactory());
MemberNamingStrategy minifyMembers = new MinifierMemberNamingStrategy(appView);
timing.begin("MinifyMethods");
@@ -86,7 +86,7 @@
timing.end();
assert new MinifiedRenaming(appView, classRenaming, methodRenaming, FieldRenaming.empty())
- .verifyNoCollisions(appView.appInfo().classes(), appView.dexItemFactory());
+ .verifyNoCollisions(appView.appInfo(), appView.dexItemFactory());
timing.begin("MinifyFields");
FieldRenaming fieldRenaming =
@@ -100,7 +100,7 @@
timing.end();
NamingLens lens = new MinifiedRenaming(appView, classRenaming, methodRenaming, fieldRenaming);
- assert lens.verifyNoCollisions(appView.appInfo().classes(), appView.dexItemFactory());
+ assert lens.verifyNoCollisions(appView.appInfo(), appView.dexItemFactory());
appView.testing().namingLensConsumer.accept(appView.dexItemFactory(), lens);
appView.notifyOptimizationFinishedForTesting();
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 bc8b12f..dbe8fa4 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingLens.java
+++ b/src/main/java/com/android/tools/r8/naming/NamingLens.java
@@ -24,6 +24,8 @@
import com.android.tools.r8.utils.collections.DexClassAndMethodSet;
import com.google.common.collect.Sets;
import java.util.Arrays;
+import java.util.IdentityHashMap;
+import java.util.Map;
import java.util.Set;
/**
@@ -157,14 +159,35 @@
public abstract boolean verifyRenamingConsistentWithResolution(DexMethod item);
public final boolean verifyNoCollisions(
- Iterable<DexProgramClass> classes, DexItemFactory dexItemFactory) {
+ AppInfoWithLiveness appInfo, DexItemFactory dexItemFactory) {
Set<DexReference> references = Sets.newIdentityHashSet();
- for (DexProgramClass clazz : classes) {
+ Map<DexType, DexType> classpathTypes = new IdentityHashMap<>();
+ appInfo
+ .getClasspathTypesIncludingPruned()
+ .forEach(
+ type -> {
+ DexType newType = lookupType(type, dexItemFactory);
+ classpathTypes.put(newType, type);
+ });
+ for (DexProgramClass clazz : appInfo.classes()) {
{
DexType newType = lookupType(clazz.type, dexItemFactory);
boolean referencesChanged = references.add(newType);
assert referencesChanged
: "Duplicate definition of type `" + newType.toSourceString() + "`";
+ if (classpathTypes.containsKey(newType)) {
+ // If we have a classpath and program type, then it can be renamed the same way. It is
+ // forbidden to rename a program type into a classpath type, since it would lead to
+ // duplicated types.
+ assert classpathTypes.get(newType).isIdenticalTo(clazz.type)
+ : "Duplicate predecessor of type `"
+ + newType.toSourceString()
+ + "`: `"
+ + classpathTypes.get(newType).toSourceString()
+ + "`,`"
+ + clazz.type.toSourceString()
+ + "`";
+ }
}
for (DexEncodedField field : clazz.fields()) {
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 b9dee9f..3d5de3b 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -65,6 +65,7 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.PredicateSet;
+import com.android.tools.r8.utils.SetUtils;
import com.android.tools.r8.utils.Visibility;
import com.android.tools.r8.utils.collections.DexClassAndMethodSet;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
@@ -171,6 +172,13 @@
* ).
*/
public final Object2BooleanMap<DexMember<?, ?>> identifierNameStrings;
+
+ /**
+ * A set of classpath types that are not referenced from the app, but which names can still lead
+ * to collusions in the minifier.
+ */
+ final Set<DexType> prunedClasspathTypes;
+
/** A set of types that have been removed by the {@link TreePruner}. */
final Set<DexType> prunedTypes;
/** A map from switchmap class types to their corresponding switchmaps. */
@@ -205,6 +213,7 @@
PredicateSet<DexType> alwaysClassInline,
Object2BooleanMap<DexMember<?, ?>> identifierNameStrings,
Set<DexType> prunedTypes,
+ Set<DexType> prunedClasspathTypes,
Map<DexField, Int2ReferenceMap<DexField>> switchMaps,
Set<DexType> lockCandidates,
Map<DexType, Visibility> initClassReferences,
@@ -230,6 +239,7 @@
this.alwaysClassInline = alwaysClassInline;
this.identifierNameStrings = identifierNameStrings;
this.prunedTypes = prunedTypes;
+ this.prunedClasspathTypes = prunedClasspathTypes;
this.switchMaps = switchMaps;
this.lockCandidates = lockCandidates;
this.initClassReferences = initClassReferences;
@@ -263,6 +273,7 @@
previous.alwaysClassInline,
previous.identifierNameStrings,
previous.prunedTypes,
+ previous.prunedClasspathTypes,
previous.switchMaps,
previous.lockCandidates,
previous.initClassReferences,
@@ -299,6 +310,7 @@
prunedItems.hasRemovedClasses()
? CollectionUtils.mergeSets(previous.prunedTypes, prunedItems.getRemovedClasses())
: previous.prunedTypes,
+ previous.prunedClasspathTypes,
previous.switchMaps,
pruneClasses(previous.lockCandidates, prunedItems, tasks),
pruneMapFromClasses(previous.initClassReferences, prunedItems, tasks),
@@ -439,6 +451,7 @@
alwaysClassInline,
identifierNameStrings,
prunedTypes,
+ prunedClasspathTypes,
switchMaps,
lockCandidates,
initClassReferences,
@@ -509,6 +522,7 @@
this.alwaysClassInline = previous.alwaysClassInline;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = previous.prunedTypes;
+ this.prunedClasspathTypes = previous.prunedClasspathTypes;
this.switchMaps = switchMaps;
this.lockCandidates = previous.lockCandidates;
this.initClassReferences = previous.initClassReferences;
@@ -1007,6 +1021,7 @@
lens.rewriteReferenceKeys(identifierNameStrings),
// Don't rewrite pruned types - the removed types are identified by their original name.
prunedTypes,
+ prunedClasspathTypes,
lens.rewriteFieldKeys(switchMaps),
lens.rewriteReferences(lockCandidates),
rewriteInitClassReferences(lens),
@@ -1046,6 +1061,18 @@
return prunedTypes;
}
+ public Set<DexType> getClasspathTypesIncludingPruned() {
+ assert checkIfObsolete();
+ Collection<DexClasspathClass> classpathClasses = app().asDirect().classpathClasses();
+ Set<DexType> classpath =
+ SetUtils.newIdentityHashSet(classpathClasses.size() + prunedClasspathTypes.size());
+ for (DexClasspathClass cp : classpathClasses) {
+ classpath.add(cp.getType());
+ }
+ classpath.addAll(prunedClasspathTypes);
+ return classpath;
+ }
+
public DexClassAndMethod lookupSingleTarget(
AppView<AppInfoWithLiveness> appView,
InvokeType type,
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 f17e905..9788f61 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -334,6 +334,9 @@
/** Set of types that was pruned during the first round of tree shaking. */
private final Set<DexType> initialPrunedTypes;
+ /** List of classpath types that was pruned during the first round of tree shaking. */
+ private final Set<DexType> prunedClasspathTypes;
+
private final Set<DexType> noClassMerging = Sets.newIdentityHashSet();
/** Mapping from each unused interface to the set of live types that implements the interface. */
@@ -477,6 +480,7 @@
keptGraphConsumer,
mode,
null,
+ null,
null);
}
@@ -488,6 +492,7 @@
GraphConsumer keptGraphConsumer,
Mode mode,
Set<DexType> initialPrunedTypes,
+ Set<DexType> prunedClasspathTypes,
RuntimeTypeCheckInfo.Builder runtimeTypeCheckInfoBuilder) {
assert appView.appServices() != null;
InternalOptions options = appView.options();
@@ -512,6 +517,7 @@
? ProguardCompatibilityActions.builder()
: null;
this.initialPrunedTypes = initialPrunedTypes;
+ this.prunedClasspathTypes = prunedClasspathTypes;
if (options.isOptimizedResourceShrinking()) {
R8ResourceShrinkerState resourceShrinkerState = appView.getResourceShrinkerState();
@@ -4530,6 +4536,15 @@
// Add just referenced non-program types. We can't replace the program classes at this point as
// they are needed in tree pruning.
+ ImmutableSet.Builder<DexType> prunedClasspathTypesBuilder = ImmutableSet.builder();
+ if (prunedClasspathTypes != null) {
+ prunedClasspathTypesBuilder.addAll(prunedClasspathTypes);
+ }
+ for (DexClasspathClass classpathClass : appInfo.app().asDirect().classpathClasses()) {
+ if (!classpathClasses.contains(classpathClass)) {
+ prunedClasspathTypesBuilder.add(classpathClass.getType());
+ }
+ }
DirectMappedDexApplication app =
appInfo
.app()
@@ -4585,6 +4600,7 @@
joinIdentifierNameStrings(
rootSet.identifierNameStrings, reflectiveIdentification.getIdentifierNameStrings()),
emptySet(),
+ prunedClasspathTypesBuilder.build(),
Collections.emptyMap(),
lockCandidates,
initClassReferences,
diff --git a/src/main/java/com/android/tools/r8/shaking/EnqueuerFactory.java b/src/main/java/com/android/tools/r8/shaking/EnqueuerFactory.java
index 1095378..b4f2984 100644
--- a/src/main/java/com/android/tools/r8/shaking/EnqueuerFactory.java
+++ b/src/main/java/com/android/tools/r8/shaking/EnqueuerFactory.java
@@ -36,6 +36,7 @@
ImmediateAppSubtypingInfo subtypingInfo,
GraphConsumer keptGraphConsumer,
Set<DexType> initialPrunedTypes,
+ Set<DexType> prunedClasspathTypes,
RuntimeTypeCheckInfo.Builder runtimeTypeCheckInfoBuilder) {
ProfileCollectionAdditions profileCollectionAdditions =
ProfileCollectionAdditions.create(appView);
@@ -48,6 +49,7 @@
keptGraphConsumer,
Mode.FINAL_TREE_SHAKING,
initialPrunedTypes,
+ prunedClasspathTypes,
runtimeTypeCheckInfoBuilder);
appView.withProtoShrinker(
shrinker -> enqueuer.setInitialDeadProtoTypes(shrinker.getDeadProtoTypes()));
diff --git a/src/test/java/com/android/tools/r8/naming/ProgramTypeRenamedAsClasspathTypeTest.java b/src/test/java/com/android/tools/r8/naming/ProgramTypeRenamedAsClasspathTypeTest.java
new file mode 100644
index 0000000..1ba9022
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/ProgramTypeRenamedAsClasspathTypeTest.java
@@ -0,0 +1,115 @@
+// Copyright (c) 2025, 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;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.JdkClassFileProvider;
+import com.android.tools.r8.R8TestCompileResult;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.TestRuntime.CfVm;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
+import java.nio.file.Path;
+import java.util.HashSet;
+import java.util.Set;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ProgramTypeRenamedAsClasspathTypeTest extends TestBase {
+
+ @Parameter public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters()
+ .withCfRuntimesStartingFromIncluding(CfVm.JDK24)
+ .withDexRuntimes()
+ .withAllApiLevelsAlsoForCf()
+ .withPartialCompilation()
+ .build();
+ }
+
+ public static String EXPECTED_OUTPUT = StringUtils.lines("program side", "classpath side");
+
+ @Test
+ public void testJvm() throws Exception {
+ parameters.assumeJvmTestParameters();
+ testForJvm(parameters)
+ .addInnerClassesAndStrippedOuter(getClass())
+ .run(parameters.getRuntime(), ProgramMain.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ @Test
+ public void testR8Split() throws Exception {
+ parameters.assumeR8TestParameters();
+ Set<String> types = new HashSet<>();
+ R8TestCompileResult compile =
+ testForR8(Backend.CF)
+ .addProgramClasses(ClasspathSide.class, ClasspathMain.class)
+ .addLibraryProvider(JdkClassFileProvider.fromSystemJdk())
+ .addKeepMainRule(ClasspathMain.class)
+ .addKeepClassAndMembersRulesWithAllowObfuscation(ClasspathSide.class)
+ .compile();
+ compile.inspect(
+ i -> {
+ assertEquals(2, i.allClasses().size());
+ for (FoundClassSubject clazz : i.allClasses()) {
+ types.add(clazz.getDexProgramClass().getTypeName());
+ }
+ });
+ Path classpath = compile.writeToZip();
+ Path runClasspath = testForD8(parameters).addProgramFiles(classpath).compile().writeToZip();
+ testForR8(parameters)
+ .addProgramClasses(ProgramSide.class, ProgramMain.class)
+ .addClasspathFiles(classpath)
+ .addKeepMainRule(ProgramMain.class)
+ .addKeepClassAndMembersRulesWithAllowObfuscation(ProgramSide.class)
+ .compile()
+ .inspect(
+ i -> {
+ assertEquals(2, i.allClasses().size());
+ for (FoundClassSubject clazz : i.allClasses()) {
+ types.add(clazz.getDexProgramClass().getTypeName());
+ }
+ assertEquals(4, types.size());
+ })
+ .addRunClasspathFiles(runClasspath)
+ .run(parameters.getRuntime(), ProgramMain.class)
+ .assertSuccessWithOutput(EXPECTED_OUTPUT);
+ }
+
+ static class ClasspathMain {
+ public static void main(String[] args) {
+ ClasspathSide.print();
+ }
+ }
+
+ static class ClasspathSide {
+ public static void print() {
+ System.out.println("classpath side");
+ }
+ }
+
+ public static class ProgramMain {
+ public static void main(String[] args) {
+ ProgramSide.print();
+ ClasspathMain.main(args);
+ }
+ }
+
+ static class ProgramSide {
+ public static void print() {
+ System.out.println("program side");
+ }
+ }
+}
diff --git a/src/test/java24/com/android/tools/r8/jdk24/switchpatternmatching/EnumSwitchOldSyntaxV2Test.java b/src/test/java24/com/android/tools/r8/jdk24/switchpatternmatching/EnumSwitchOldSyntaxV2Test.java
index c1e3f1d..fede4cb 100644
--- a/src/test/java24/com/android/tools/r8/jdk24/switchpatternmatching/EnumSwitchOldSyntaxV2Test.java
+++ b/src/test/java24/com/android/tools/r8/jdk24/switchpatternmatching/EnumSwitchOldSyntaxV2Test.java
@@ -12,7 +12,6 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.TestRuntime.CfVm;
import com.android.tools.r8.utils.StringUtils;
-import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -69,7 +68,6 @@
@Test
public void testR8Split() throws Exception {
- Assume.assumeFalse("TODO(b/414335863)", parameters.isRandomPartialCompilation());
parameters.assumeR8TestParameters();
R8TestCompileResult compile =
testForR8(Backend.CF)
