commit d8bdeb6ab68b50734189110231e934c5b713e53d
author Morten Krogh-Jespersen <mkroghj@google.com> Fri Nov 29 13:15:05 2019 +0100
committer Morten Krogh-Jespersen <mkroghj@google.com> Mon Dec 02 20:24:56 2019 +0000
tree 89e836ff1cacb24f3e79c8531b34c56e603aad7d
parent 9c1433725cb8e703d124329100980479fbe19160

PrintUses: Allow obfuscating members with package-dependent visiblity

It is important that members with package-dependent visiblity stay in
the same package as defined - otherwise, some tests may fail on access
violations.

But instead of keeping without allowing obfuscation, we should just
keep the package names.

Bug: 145310926
Change-Id: Ibbf4aec9c728c32f4f79f902c1147349fe0678b5

src/main/java/com/android/tools/r8/PrintUses.java

diff --git a/src/main/java/com/android/tools/r8/PrintUses.java b/src/main/java/com/android/tools/r8/PrintUses.java
index 0d8f3ba..2864dfb 100644
--- a/src/main/java/com/android/tools/r8/PrintUses.java
+++ b/src/main/java/com/android/tools/r8/PrintUses.java
@@ -24,6 +24,7 @@
 import com.android.tools.r8.ir.desugar.LambdaDescriptor;
 import com.android.tools.r8.utils.AndroidApp;
 import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.StringUtils;
 import com.android.tools.r8.utils.Timing;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -31,6 +32,7 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
@@ -70,6 +72,7 @@
   private Map<DexType, Set<DexMethod>> methods = Maps.newIdentityHashMap();
   private Map<DexType, Set<DexField>> fields = Maps.newIdentityHashMap();
   private Set<DexType> noObfuscationTypes = Sets.newIdentityHashSet();
+  private Set<String> keepPackageNames = Sets.newHashSet();
   private final DexApplication application;
   private final AppInfoWithSubtyping appInfo;
   private int errors;
@@ -165,11 +168,12 @@
     private void addType(DexType type) {
       if (isTargetType(type) && types.add(type)) {
         DexClass clazz = appInfo.definitionFor(type);
-        if (clazz == null
-            || clazz.accessFlags.isVisibilityDependingOnPackage()
-            || !allowObfuscation) {
+        if (clazz == null || !allowObfuscation) {
           noObfuscationTypes.add(type);
         }
+        if (clazz != null && clazz.accessFlags.isVisibilityDependingOnPackage()) {
+          keepPackageNames.add(clazz.type.getPackageName());
+        }
         methods.put(type, Sets.newIdentityHashSet());
         fields.put(type, Sets.newIdentityHashSet());
       }
@@ -192,9 +196,12 @@
       Set<DexField> typeFields = fields.get(field.holder);
       if (typeFields != null) {
         assert baseField != null;
-        if (!allowObfuscation || baseField.accessFlags.isVisibilityDependingOnPackage()) {
+        if (!allowObfuscation) {
           noObfuscationTypes.add(field.holder);
         }
+        if (baseField.accessFlags.isVisibilityDependingOnPackage()) {
+          keepPackageNames.add(baseField.field.holder.getPackageName());
+        }
         typeFields.add(field);
       }
     }
@@ -209,9 +216,12 @@
       if (typeMethods != null) {
         DexEncodedMethod encodedMethod = appInfo.definitionFor(method);
         assert encodedMethod != null;
-        if (!allowObfuscation || encodedMethod.accessFlags.isVisibilityDependingOnPackage()) {
+        if (!allowObfuscation) {
           noObfuscationTypes.add(method.holder);
         }
+        if (encodedMethod.accessFlags.isVisibilityDependingOnPackage()) {
+          keepPackageNames.add(encodedMethod.method.holder.getPackageName());
+        }
         typeMethods.add(method);
       }
     }
@@ -344,7 +354,8 @@
   }
 
   private void print() {
-    errors = printer.print(application, types, noObfuscationTypes, methods, fields);
+    errors =
+        printer.print(application, types, noObfuscationTypes, keepPackageNames, methods, fields);
   }
 
   private abstract static class Printer {
@@ -378,6 +389,8 @@
 
     abstract void printMethod(DexEncodedMethod encodedMethod, String typeName);
 
+    abstract void printPackageNames(List<String> packageNames);
+
     void printNameAndReturn(DexEncodedMethod encodedMethod) {
       if (encodedMethod.accessFlags.isConstructor()) {
         printConstructorName(encodedMethod);
@@ -397,6 +410,7 @@
         DexApplication application,
         Set<DexType> types,
         Set<DexType> noObfuscationTypes,
+        Set<String> keepPackageNames,
         Map<DexType, Set<DexMethod>> methods,
         Map<DexType, Set<DexField>> fields) {
       int errors = 0;
@@ -431,6 +445,9 @@
         }
         printTypeFooter();
       }
+      ArrayList<String> packageNamesToKeep = new ArrayList<>(keepPackageNames);
+      Collections.sort(packageNamesToKeep);
+      printPackageNames(packageNamesToKeep);
       return errors;
     }
   }
@@ -457,6 +474,11 @@
     }
 
     @Override
+    void printPackageNames(List<String> packageNames) {
+      // No need to print package names for text output.
+    }
+
+    @Override
     void printTypeHeader(DexClass dexClass, boolean allowObfuscation) {
       appendLine(dexClass.type.toSourceString());
     }
@@ -520,6 +542,11 @@
     }
 
     @Override
+    void printPackageNames(List<String> packageNames) {
+      append("-keeppackagenames " + StringUtils.join(packageNames, ",") + "\n");
+    }
+
+    @Override
     public void printTypeFooter() {
       appendLine("}");
     }