commit 4df8a61e70e41721412df96030979e8d630c3536
author Christoffer Quist Adamsen <christofferqa@google.com> Wed Oct 09 17:06:24 2019 +0200
committer Christoffer Quist Adamsen <christofferqa@google.com> Thu Oct 10 09:42:20 2019 +0000
tree e8ebe89dbb5740bd5ac7a0687320b7fc8f79686c
parent 9409965220b0380cbc17559d8b3d2c0c4f08c5d7

Mark all proto fields as being read using reflection

Bug: 141599586
Change-Id: I6cf982c2115b01149a1b06b53cd4bc42f7a48bd1

src/main/java/com/android/tools/r8/graph/FieldAccessInfo.java

diff --git a/src/main/java/com/android/tools/r8/graph/FieldAccessInfo.java b/src/main/java/com/android/tools/r8/graph/FieldAccessInfo.java
index 2af1aa5..079b8bb 100644
--- a/src/main/java/com/android/tools/r8/graph/FieldAccessInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/FieldAccessInfo.java
@@ -28,4 +28,6 @@
   boolean isReadOnlyIn(DexEncodedMethod method);
 
   boolean isWritten();
+
+  boolean isWrittenOutside(DexEncodedMethod method);
 }

src/main/java/com/android/tools/r8/graph/FieldAccessInfoImpl.java

diff --git a/src/main/java/com/android/tools/r8/graph/FieldAccessInfoImpl.java b/src/main/java/com/android/tools/r8/graph/FieldAccessInfoImpl.java
index b6c54cb..fcd4f5c 100644
--- a/src/main/java/com/android/tools/r8/graph/FieldAccessInfoImpl.java
+++ b/src/main/java/com/android/tools/r8/graph/FieldAccessInfoImpl.java
@@ -146,6 +146,23 @@
     return writesWithContexts != null && !writesWithContexts.isEmpty();
   }
 
+  /**
+   * Returns true if this field is written by a method in the program other than {@param method}.
+   */
+  @Override
+  public boolean isWrittenOutside(DexEncodedMethod method) {
+    if (writesWithContexts != null) {
+      for (Set<DexEncodedMethod> encodedWriteContexts : writesWithContexts.values()) {
+        for (DexEncodedMethod encodedWriteContext : encodedWriteContexts) {
+          if (encodedWriteContext != method) {
+            return true;
+          }
+        }
+      }
+    }
+    return false;
+  }
+
   public boolean recordRead(DexField access, DexEncodedMethod context) {
     if (readsWithContexts == null) {
       readsWithContexts = new IdentityHashMap<>();

src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java

diff --git a/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java b/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java
index ac6bcde..0ca8fac 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/proto/schema/ProtoEnqueuerExtension.java
@@ -144,13 +144,13 @@
 
         boolean encodedValueStorageIsLive;
         if (enqueuer.isFieldLive(encodedValueStorage)) {
-          // Mark that the field is written by reflection such that we do not optimize field reads
-          // into loading the default value of the field.
-          enqueuer.registerFieldWrite(encodedValueStorage.field, dynamicMethod);
-          // Map/required fields cannot be removed. Therefore, we mark such fields as both read and
-          // written such that we cannot optimize any field reads or writes.
-          if (reachesMapOrRequiredField(protoFieldInfo)) {
-            enqueuer.registerFieldRead(encodedValueStorage.field, dynamicMethod);
+          if (enqueuer.isFieldRead(encodedValueStorage)
+              || enqueuer.isFieldWrittenOutsideDefaultConstructor(encodedValueStorage)) {
+            // Mark that the field is both read and written by reflection such that we do not
+            // (i) optimize field reads into loading the default value of the field or (ii) remove
+            // field writes to proto fields that could be read using reflection by the proto
+            // library.
+            enqueuer.registerFieldAccess(encodedValueStorage.field, dynamicMethod);
           }
           encodedValueStorageIsLive = true;
         } else if (reachesMapOrRequiredField(protoFieldInfo)) {

src/main/java/com/android/tools/r8/shaking/Enqueuer.java

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 1d209ea..0689533 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1786,6 +1786,23 @@
     return liveFields.contains(field);
   }
 
+  public boolean isFieldRead(DexEncodedField field) {
+    FieldAccessInfoImpl info = fieldAccessInfoCollection.get(field.field);
+    return info != null && info.isRead();
+  }
+
+  public boolean isFieldWrittenOutsideDefaultConstructor(DexEncodedField field) {
+    FieldAccessInfoImpl info = fieldAccessInfoCollection.get(field.field);
+    if (info == null) {
+      return false;
+    }
+    DexClass clazz = appView.definitionFor(field.field.holder);
+    DexEncodedMethod defaultInitializer = clazz.getDefaultInitializer();
+    return defaultInitializer != null
+        ? info.isWrittenOutside(defaultInitializer)
+        : info.isWritten();
+  }
+
   private boolean isInstantiatedOrHasInstantiatedSubtype(DexProgramClass clazz) {
     return directAndIndirectlyInstantiatedTypes.contains(clazz);
   }