Package obfuscation w/o access modification.
Bug: 62048823
Change-Id: I6b8e81b88bca923a72a641c96ea20d24747861e8
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 c1e8515..3ef67e6 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -40,6 +40,8 @@
private final AppInfoWithLiveness appInfo;
private final RootSet rootSet;
private final PackageObfuscationMode packageObfuscationMode;
+ private final boolean isAccessModificationAllowed;
+ private final Set<String> noObfuscationPrefixes = Sets.newHashSet();
private final Set<String> usedPackagePrefixes = Sets.newHashSet();
private final Set<DexString> usedTypeNames = Sets.newIdentityHashSet();
@@ -63,6 +65,7 @@
this.appInfo = appInfo;
this.rootSet = rootSet;
this.packageObfuscationMode = options.proguardConfiguration.getPackageObfuscationMode();
+ this.isAccessModificationAllowed = options.proguardConfiguration.isAccessModificationAllowed();
this.packageDictionary = options.proguardConfiguration.getPackageObfuscationDictionary();
this.classDictionary = options.proguardConfiguration.getClassObfuscationDictionary();
this.keepInnerClassStructure = options.attributeRemoval.signature;
@@ -164,6 +167,11 @@
* Registers the given package prefix and all of parent packages as used.
*/
private void registerPackagePrefixesAsUsed(String packagePrefix) {
+ // If -allowaccessmodification is not set, we may keep classes in their original packages,
+ // accounting for package-private accesses.
+ if (!isAccessModificationAllowed) {
+ noObfuscationPrefixes.add(packagePrefix);
+ }
String usedPrefix = packagePrefix;
while (usedPrefix.length() > 0) {
usedPackagePrefixes.add(usedPrefix);
@@ -209,7 +217,9 @@
private Namespace getStateForClass(DexClass clazz) {
String packageName = getPackageBinaryNameFromJavaType(clazz.type.getPackageDescriptor());
// Check whether the given class should be kept.
- if (rootSet.keepPackageName.contains(clazz)) {
+ // or check whether the given class belongs to a package that is kept for another class.
+ if (rootSet.keepPackageName.contains(clazz)
+ || noObfuscationPrefixes.contains(packageName)) {
return states.computeIfAbsent(packageName, Namespace::new);
}
Namespace state = topLevelState;
@@ -320,6 +330,7 @@
do {
candidate = appInfo.dexItemFactory.createString(nextSuggestedNameForClass());
} while (usedTypeNames.contains(candidate));
+ usedTypeNames.add(candidate);
return candidate;
}
@@ -342,6 +353,7 @@
do {
candidate = nextSuggestedNameForSubpackage();
} while (usedPackagePrefixes.contains(candidate));
+ usedPackagePrefixes.add(candidate);
return candidate;
}
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 951790c..a26e135 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -36,10 +36,6 @@
public NamingLens run(Timing timing) {
assert !options.skipMinification;
- // TODO(b/62048823): Minifier should not depend on -allowaccessmodification.
- if (!options.proguardConfiguration.isAccessModificationAllowed()) {
- throw new CompilationError("Minification requires allowaccessmodification.");
- }
timing.begin("MinifyClasses");
Map<DexType, DexString> classRenaming =
new ClassNameMinifier(appInfo, rootSet, options).computeRenaming(timing);
diff --git a/src/test/examples/naming101/keep-rules-102.txt b/src/test/examples/naming101/keep-rules-102.txt
new file mode 100644
index 0000000..b1c0cda
--- /dev/null
+++ b/src/test/examples/naming101/keep-rules-102.txt
@@ -0,0 +1,13 @@
+# Copyright (c) 2017, 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.
+
+-repackageclasses 'naming101.a'
+
+-keep public class **.a {
+ *;
+}
+
+-keep,allowobfuscation class * {
+ *;
+}
diff --git a/src/test/examples/naming101/keep-rules-104.txt b/src/test/examples/naming101/keep-rules-104.txt
new file mode 100644
index 0000000..6866dee
--- /dev/null
+++ b/src/test/examples/naming101/keep-rules-104.txt
@@ -0,0 +1,13 @@
+# Copyright (c) 2017, 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.
+
+-flattenpackagehierarchy 'naming101'
+
+-keep public class **.a {
+ *;
+}
+
+-keep,allowobfuscation class * {
+ *;
+}
diff --git a/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java b/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
index b8e22d2..5975483 100644
--- a/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
+++ b/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
@@ -17,12 +17,14 @@
import com.android.tools.r8.utils.Timing;
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.function.BiConsumer;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -45,6 +47,16 @@
inspection.accept(dexItemFactory, naming);
}
+ private static final List<String> CLASSES_IN_NAMING044 = ImmutableList.of(
+ "Lnaming044/A;", "Lnaming044/B;", "Lnaming044/sub/SubA;", "Lnaming044/sub/SubB;");
+
+ private static final List<String> CLASSES_IN_NAMING101 = ImmutableList.of(
+ "Lnaming101/c;", "Lnaming101/d;",
+ "Lnaming101/a/a;", "Lnaming101/a/c;",
+ "Lnaming101/a/b/c",
+ "Lnaming101/b/a'", "Lnaming101/b/b;"
+ );
+
@Parameters(name = "test: {0} keep: {1}")
public static Collection<Object[]> data() {
List<String> tests = Arrays.asList("naming044", "naming101");
@@ -60,6 +72,8 @@
inspections.put("naming101:keep-rules-003.txt", PackageNamingTest::test101_rule003);
inspections.put("naming101:keep-rules-004.txt", PackageNamingTest::test101_rule004);
inspections.put("naming101:keep-rules-005.txt", PackageNamingTest::test101_rule005);
+ inspections.put("naming101:keep-rules-102.txt", PackageNamingTest::test101_rule102);
+ inspections.put("naming101:keep-rules-104.txt", PackageNamingTest::test101_rule104);
return createTests(tests, inspections);
}
@@ -68,8 +82,23 @@
return CharMatcher.is('/').countIn(descriptor);
}
+ private static void verifyUniqueNaming(
+ DexItemFactory dexItemFactory,
+ NamingLens naming,
+ List<String> klasses) {
+ Set<String> renamedNames = Sets.newHashSet();
+ for (String klass : klasses) {
+ DexType k = dexItemFactory.createType(klass);
+ String rename = naming.lookupDescriptor(k).toSourceString();
+ assertFalse(renamedNames.contains(rename));
+ renamedNames.add(rename);
+ }
+ }
+
// repackageclasses ''
private static void test044_rule001(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING044);
+
// All classes are moved to the top-level package, hence no package separator.
DexType b = dexItemFactory.createType("Lnaming044/B;");
assertFalse(naming.lookupDescriptor(b).toSourceString().contains("/"));
@@ -86,6 +115,8 @@
// repackageclasses 'p44.x'
private static void test044_rule002(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING044);
+
// All classes are moved to a single package, so they all have the same package prefix.
DexType a = dexItemFactory.createType("Lnaming044/A;");
assertTrue(naming.lookupDescriptor(a).toSourceString().startsWith("Lp44/x/"));
@@ -103,6 +134,8 @@
// flattenpackagehierarchy ''
private static void test044_rule003(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING044);
+
// All packages are moved to the top-level package, hence only one package separator.
DexType b = dexItemFactory.createType("Lnaming044/B;");
assertEquals(1, countPackageDepth(naming.lookupDescriptor(b).toSourceString()));
@@ -145,6 +178,8 @@
}
private static void test044_rule005(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING044);
+
// All packages are renamed somehow. Need to check package hierarchy is consistent.
DexType a = dexItemFactory.createType("Lnaming044/A;");
assertEquals(1, countPackageDepth(naming.lookupDescriptor(a).toSourceString()));
@@ -171,6 +206,8 @@
}
private static void test101_rule001(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING101);
+
// All classes are moved to the top-level package, hence no package separator.
DexType c = dexItemFactory.createType("Lnaming101/c;");
assertFalse(naming.lookupDescriptor(c).toSourceString().contains("/"));
@@ -183,9 +220,14 @@
}
private static void test101_rule002(DexItemFactory dexItemFactory, NamingLens naming) {
- // Check naming101.a.a is kept due to **.a
- DexType a = dexItemFactory.createType("Lnaming101/a/a;");
- assertEquals("Lnaming101/a/a;", naming.lookupDescriptor(a).toSourceString());
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING101);
+
+ // Check naming101.*.a is kept due to **.a
+ DexType aa = dexItemFactory.createType("Lnaming101/a/a;");
+ assertEquals("Lnaming101/a/a;", naming.lookupDescriptor(aa).toSourceString());
+ DexType ba = dexItemFactory.createType("Lnaming101/b/a;");
+ assertEquals("Lnaming101/b/a;", naming.lookupDescriptor(ba).toSourceString());
+
// Repackaged to naming101.a, but naming101.a.a exists to make a name conflict.
// Thus, everything else should not be renamed to 'a',
// except for naming101.b.a, which is also kept due to **.a
@@ -204,6 +246,8 @@
}
private static void test101_rule003(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING101);
+
// All packages are moved to the top-level package, hence only one package separator.
DexType aa = dexItemFactory.createType("Lnaming101/a/a;");
assertEquals(1, countPackageDepth(naming.lookupDescriptor(aa).toSourceString()));
@@ -217,9 +261,14 @@
}
private static void test101_rule004(DexItemFactory dexItemFactory, NamingLens naming) {
- // Check naming101.a.a is kept due to **.a
- DexType a = dexItemFactory.createType("Lnaming101/a/a;");
- assertEquals("Lnaming101/a/a;", naming.lookupDescriptor(a).toSourceString());
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING101);
+
+ // Check naming101.*.a is kept due to **.a
+ DexType aa = dexItemFactory.createType("Lnaming101/a/a;");
+ assertEquals("Lnaming101/a/a;", naming.lookupDescriptor(aa).toSourceString());
+ DexType ba = dexItemFactory.createType("Lnaming101/b/a;");
+ assertEquals("Lnaming101/b/a;", naming.lookupDescriptor(ba).toSourceString());
+
// Flattened to naming101, hence all other classes will be in naming101.* package.
// Due to naming101.a.a, prefix naming101.a is already used. So, any other classes,
// except for naming101.a.c, should not have naming101.a as package.
@@ -237,6 +286,8 @@
}
private static void test101_rule005(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING101);
+
// All packages are renamed somehow. Need to check package hierarchy is consistent.
DexType aa = dexItemFactory.createType("Lnaming101/a/a;");
assertEquals(2, countPackageDepth(naming.lookupDescriptor(aa).toSourceString()));
@@ -250,4 +301,60 @@
getParentPackagePrefix(
getParentPackagePrefix(naming.lookupDescriptor(abc).toSourceString())));
}
+
+ private static void test101_rule102(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING101);
+
+ // Check naming101.*.a is kept due to **.a
+ DexType aa = dexItemFactory.createType("Lnaming101/a/a;");
+ assertEquals("Lnaming101/a/a;", naming.lookupDescriptor(aa).toSourceString());
+ DexType ba = dexItemFactory.createType("Lnaming101/b/a;");
+ assertEquals("Lnaming101/b/a;", naming.lookupDescriptor(ba).toSourceString());
+ // Due to package-private access, classes in the same package should remain as-is.
+ DexType ab = dexItemFactory.createType("Lnaming101/a/b;");
+ assertEquals("Lnaming101/a/b;", naming.lookupDescriptor(ab).toSourceString());
+ DexType bb = dexItemFactory.createType("Lnaming101/b/b;");
+ assertEquals("Lnaming101/b/b;", naming.lookupDescriptor(bb).toSourceString());
+
+ // All other classes can be repackaged to naming101.a, but naming101.a.a exists to make a name
+ // conflict. Thus, those should not be renamed to 'a'.
+ List<String> klasses = ImmutableList.of(
+ "Lnaming101/c;",
+ "Lnaming101/d;",
+ "Lnaming101/a/b/c;");
+ for (String klass : klasses) {
+ DexType k = dexItemFactory.createType(klass);
+ String renamedName = naming.lookupDescriptor(k).toSourceString();
+ assertEquals("naming101.a", getPackageNameFromDescriptor(renamedName));
+ assertNotEquals("Lnaming101/a/a;", renamedName);
+ }
+ }
+
+ private static void test101_rule104(DexItemFactory dexItemFactory, NamingLens naming) {
+ verifyUniqueNaming(dexItemFactory, naming, CLASSES_IN_NAMING101);
+
+ // Check naming101.*.a is kept due to **.a
+ DexType aa = dexItemFactory.createType("Lnaming101/a/a;");
+ assertEquals("Lnaming101/a/a;", naming.lookupDescriptor(aa).toSourceString());
+ DexType ba = dexItemFactory.createType("Lnaming101/b/a;");
+ assertEquals("Lnaming101/b/a;", naming.lookupDescriptor(ba).toSourceString());
+ // Due to package-private access, classes in the same package should remain as-is.
+ DexType ab = dexItemFactory.createType("Lnaming101/a/b;");
+ assertEquals("Lnaming101/a/b;", naming.lookupDescriptor(ab).toSourceString());
+ DexType bb = dexItemFactory.createType("Lnaming101/b/b;");
+ assertEquals("Lnaming101/b/b;", naming.lookupDescriptor(bb).toSourceString());
+
+ // All other packages are flattened to naming101, hence all other classes will be in
+ // naming101.* package. Due to naming101.a.a, prefix naming101.a is already used. So,
+ // remaining classes should not have naming101.a as package.
+ List<String> klasses = ImmutableList.of(
+ "Lnaming101/c;",
+ "Lnaming101/d;",
+ "Lnaming101/a/b/c;");
+ for (String klass : klasses) {
+ DexType k = dexItemFactory.createType(klass);
+ String renamedName = naming.lookupDescriptor(k).toSourceString();
+ assertNotEquals("naming101.a", getPackageNameFromDescriptor(renamedName));
+ }
+ }
}
