add "relaxed_keep_class_members " option

Summary:
Both *RMU and DelInit remove "unreachable" fields and methods. (RMU also removes unreachable classes, but that's not important for this story.)
They differ in what they consider "unreachable".
RMU somewhat straightforwardly visits everything that reachable from some roots, and conditionally holds on root fields and methods in reachable classes. (There's a little quirk where it keeps all volatile ifields, some of which DelInit will actually happily remove, but that's a minor detail.)
DelInit is... less intuitive. On one side, it's more aggressive when it comes to discarding root fields and methods, as it only considers some ways in which their class is referenced --- I think the intuition is that it only consider those kind of references which are "actually" (= historically observed to be) used to access fields and methods via reflection or from native code. On the other side, it's more conservative than RMU, as it also considers the `marked_by_string` flag in addition to the `root` flag, and it may keep extra constructors for undisclosed reasons, and it keeps all ifields and vmethods of instantiable classes. However, RMU might cut some or all those before or after DelInit.

This diffs introduces a new option to RMU that relaxes what members need to be kept, tightening what's considered reachable in RMU, in a way that exactly cuts out what DelInit considers unreachable. We effectively get the intersection between both schemes, but combined in a single reachability visitor, which then identifies a bit more unreachable objects in a single run.

This is a behavior preserving diff, as the new option is disabled.

Reviewed By: agampe

Differential Revision: D45628244

fbshipit-source-id: 55a2c24878370cd675a3a4eaed022d4d518e0163

libredex/Reachability.h

@@ -168,24 +168,50 @@ class ReachableObjects {
   friend class TransitiveClosureMarker;
 };
 
+enum class Condition {
+  ClassRetained,
+  ClassDynamicallyReferenced,
+  ClassInstantiable,
+};
+
 struct ConditionallyMarked {
-  ConcurrentSet<const DexField*> fields;
-  ConcurrentSet<const DexMethod*> methods;
-  ConcurrentSet<const DexMethod*> methods_if_instantiable;
+  struct ConcurrentMethodsAndFields {
+    ConcurrentSet<const DexField*> fields;
+    ConcurrentSet<const DexMethod*> methods;
+  };
+
+  // If any reference to the class if retained as part of any reachable
+  // structure.
+  ConcurrentMethodsAndFields if_class_retained;
+
+  // If the class is referenced in a certain way that makes it discoverable via
+  // reflection, using the rules of the DelInitPass.
+  ConcurrentMethodsAndFields if_class_dynamically_referenced;
+
+  // If the class is not abstract and has a constructor, or has a derived class
+  // that does.
+  ConcurrentMethodsAndFields if_class_instantiable;
 };
 
-using InstantiableTypes = ConcurrentSet<DexClass*>;
+using InstantiableTypes = ConcurrentSet<const DexClass*>;
+using DynamicallyReferencedClasses = ConcurrentSet<const DexClass*>;
 
 struct References {
   std::vector<const DexString*> strings;
   std::vector<DexType*> types;
   std::vector<DexFieldRef*> fields;
   std::vector<DexMethodRef*> methods;
-  // Conditional method references. They are already resolved DexMethods
+  // Conditional virtual method references. They are already resolved DexMethods
   // conditionally reachable at virtual call sites.
-  std::vector<const DexMethod*> cond_methods;
+  std::vector<const DexMethod*> vmethods_if_class_instantiable;
+  std::unordered_set<const DexClass*> classes_dynamically_referenced;
 };
 
+void gather_dynamic_references(const DexAnnotation* item,
+                               References* references);
+
+void gather_dynamic_references(const IRCode* item, References* references);
+
 // Each thread will have its own instance of Stats, so align it in order to
 // avoid false sharing.
 struct alignas(CACHE_LINE_SIZE) Stats {
@@ -213,11 +239,15 @@ class RootSetMarker {
  public:
   RootSetMarker(const method_override_graph::Graph& method_override_graph,
                 bool record_reachability,
+                bool relaxed_keep_class_members,
+                bool remove_no_argument_constructors,
                 ConditionallyMarked* cond_marked,
                 ReachableObjects* reachable_objects,
                 ConcurrentSet<ReachableObject, ReachableObjectHash>* root_set)
       : m_method_override_graph(method_override_graph),
         m_record_reachability(record_reachability),
+        m_relaxed_keep_class_members(relaxed_keep_class_members),
+        m_remove_no_argument_constructors(remove_no_argument_constructors),
         m_cond_marked(cond_marked),
         m_reachable_objects(reachable_objects),
         m_root_set(root_set) {}
@@ -246,11 +276,9 @@ class RootSetMarker {
  private:
   void push_seed(const DexClass* cls);
 
-  void push_seed(const DexField* field);
+  void push_seed(const DexField* field, Condition condition);
 
-  void push_seed(const DexMethod* method);
-
-  void push_seed_if_instantiable(const DexMethod* method);
+  void push_seed(const DexMethod* metho, Condition condition);
 
   template <class Seed>
   void record_is_seed(Seed* seed);
@@ -260,8 +288,14 @@ class RootSetMarker {
    */
   void mark_external_method_overriders();
 
+  static bool is_rootlike_clinit(const DexMethod* m);
+
+  bool is_rootlike_init(const DexMethod* m) const;
+
   const method_override_graph::Graph& m_method_override_graph;
   bool m_record_reachability;
+  bool m_relaxed_keep_class_members;
+  bool m_remove_no_argument_constructors;
   ConditionallyMarked* m_cond_marked;
   ReachableObjects* m_reachable_objects;
   ConcurrentSet<ReachableObject, ReachableObjectHash>* m_root_set;
@@ -273,21 +307,24 @@ class TransitiveClosureMarker {
       const IgnoreSets& ignore_sets,
       const method_override_graph::Graph& method_override_graph,
       bool record_reachability,
+      bool relaxed_keep_class_members,
       ConditionallyMarked* cond_marked,
       ReachableObjects* reachable_objects,
       InstantiableTypes* instantiable_types,
+      reachability::DynamicallyReferencedClasses*
+          dynamically_referenced_classes,
       MarkWorkerState* worker_state,
-      Stats* stats,
-      bool remove_no_argument_constructors = false)
+      Stats* stats)
       : m_ignore_sets(ignore_sets),
         m_method_override_graph(method_override_graph),
         m_record_reachability(record_reachability),
+        m_relaxed_keep_class_members(relaxed_keep_class_members),
         m_cond_marked(cond_marked),
         m_reachable_objects(reachable_objects),
         m_instantiable_types(instantiable_types),
+        m_dynamically_referenced_classes(dynamically_referenced_classes),
         m_worker_state(worker_state),
-        m_stats(stats),
-        m_remove_no_argument_constructors(remove_no_argument_constructors) {
+        m_stats(stats) {
     if (s_class_forname == nullptr) {
       s_class_forname = DexMethod::get_method(
           "Ljava/lang/Class;.forName:(Ljava/lang/String;)Ljava/lang/Class;");
@@ -334,7 +371,13 @@ class TransitiveClosureMarker {
 
   void push(const DexMethodRef* parent, const DexMethodRef* method);
 
-  void push_if_instantiable(const DexMethod* method);
+  void push_if_class_instantiable(const DexField* field);
+
+  void push_if_class_instantiable(const DexMethod* method);
+
+  void push_if_class_retained(const DexField* field);
+
+  void push_if_class_retained(const DexMethod* method);
 
   bool has_class_forname(DexMethod* meth);
 
@@ -359,15 +402,24 @@ class TransitiveClosureMarker {
 
   void instantiable(DexType* type);
 
+  void dynamically_referenced(const DexClass* cls);
+  void dynamically_referenced(
+      const std::unordered_set<const DexClass*>& classes) {
+    for (auto* cls : classes) {
+      dynamically_referenced(cls);
+    }
+  }
+
   const IgnoreSets& m_ignore_sets;
   const method_override_graph::Graph& m_method_override_graph;
   bool m_record_reachability;
+  bool m_relaxed_keep_class_members;
   ConditionallyMarked* m_cond_marked;
   ReachableObjects* m_reachable_objects;
   InstantiableTypes* m_instantiable_types;
+  DynamicallyReferencedClasses* m_dynamically_referenced_classes;
   MarkWorkerState* m_worker_state;
   Stats* m_stats;
-  bool m_remove_no_argument_constructors;
 
   static DexMethodRef* s_class_forname;
 };
@@ -381,6 +433,7 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects(
     const IgnoreSets& ignore_sets,
     int* num_ignore_check_strings,
     bool record_reachability = false,
+    bool relaxed_keep_class_members = false,
     bool should_mark_all_as_seed = false,
     std::unique_ptr<const method_override_graph::Graph>*
         out_method_override_graph = nullptr,

opt/reachable-natives/ReachableNatives.cpp

@@ -71,17 +71,16 @@ void ReachableNativesPass::run_pass(DexStoresVector& stores,
   auto scope = build_class_scope(stores);
   auto reachable_objects = std::make_unique<reachability::ReachableObjects>();
   reachability::InstantiableTypes instantiable_types;
+  reachability::DynamicallyReferencedClasses dynamically_referenced_classes;
   reachability::ConditionallyMarked cond_marked;
   auto method_override_graph = method_override_graph::build_graph(scope);
 
   ConcurrentSet<reachability::ReachableObject,
                 reachability::ReachableObjectHash>
       root_set;
-  reachability::RootSetMarker root_set_marker(*method_override_graph,
-                                              false,
-                                              &cond_marked,
-                                              reachable_objects.get(),
-                                              &root_set);
+  reachability::RootSetMarker root_set_marker(
+      *method_override_graph, false, false, false, &cond_marked,
+      reachable_objects.get(), &root_set);
 
   TRACE(NATIVE, 2, "Blanket Native Classes: %zu",
         g_redex->blanket_native_root_classes.size());
@@ -99,9 +98,10 @@ void ReachableNativesPass::run_pass(DexStoresVector& stores,
       [&](reachability::MarkWorkerState* worker_state,
           const reachability::ReachableObject& obj) {
         reachability::TransitiveClosureMarker transitive_closure_marker(
-            ignore_sets, *method_override_graph, false, &cond_marked,
-            reachable_objects.get(), &instantiable_types, worker_state,
-            &stats_arr[worker_state->worker_id()], false);
+            ignore_sets, *method_override_graph, false, false, &cond_marked,
+            reachable_objects.get(), &instantiable_types,
+            &dynamically_referenced_classes, worker_state,
+            &stats_arr[worker_state->worker_id()]);
         transitive_closure_marker.visit(obj);
         return nullptr;
       },

opt/remove-unreachable/RemoveUnreachable.cpp

@@ -194,7 +194,7 @@ void RemoveUnreachablePassBase::run_pass(DexStoresVector& stores,
   int num_ignore_check_strings = 0;
   auto reachables = this->compute_reachable_objects(
       stores, pm, &num_ignore_check_strings, emit_graph_this_run,
-      m_remove_no_argument_constructors);
+      m_relaxed_keep_class_members, m_remove_no_argument_constructors);
 
   reachability::ObjectCounts before = reachability::count_objects(stores);
   TRACE(RMU, 1, "before: %lu classes, %lu fields, %lu methods",
@@ -281,10 +281,12 @@ RemoveUnreachablePass::compute_reachable_objects(
     PassManager& /* pm */,
     int* num_ignore_check_strings,
     bool emit_graph_this_run,
+    bool relaxed_keep_class_members,
     bool remove_no_argument_constructors) {
   return reachability::compute_reachable_objects(
       stores, m_ignore_sets, num_ignore_check_strings, emit_graph_this_run,
-      false, nullptr, remove_no_argument_constructors);
+      relaxed_keep_class_members, false, nullptr,
+      remove_no_argument_constructors);
 }
 
 static RemoveUnreachablePass s_pass;

opt/remove-unreachable/RemoveUnreachable.h

@@ -41,6 +41,7 @@ class RemoveUnreachablePassBase : public Pass {
          false,
          m_remove_no_argument_constructors);
     bind("output_full_removed_symbols", false, m_output_full_removed_symbols);
+    bind("relaxed_keep_class_members", false, m_relaxed_keep_class_members);
     after_configuration([this] {
       // To keep the backward compatability of this code, ensure that the
       // "MemberClasses" annotation is always in system_annos.
@@ -56,6 +57,7 @@ class RemoveUnreachablePassBase : public Pass {
                             PassManager& pm,
                             int* num_ignore_check_strings,
                             bool emit_graph_this_run,
+                            bool relaxed_keep_class_members,
                             bool remove_no_argument_constructors) = 0;
 
   void write_out_removed_symbols(
@@ -69,6 +71,7 @@ class RemoveUnreachablePassBase : public Pass {
   bool m_always_emit_unreachable_symbols = false;
   bool m_emit_removed_symbols_references = false;
   bool m_output_full_removed_symbols = false;
+  bool m_relaxed_keep_class_members = false;
 };
 
 class RemoveUnreachablePass : public RemoveUnreachablePassBase {
@@ -81,5 +84,6 @@ class RemoveUnreachablePass : public RemoveUnreachablePassBase {
       PassManager& pm,
       int* num_ignore_check_strings,
       bool emit_graph_this_run,
+      bool relaxed_keep_class_members,
       bool remove_no_argument_constructors) override;
 };

opt/remove-unreachable/TypeAnalysisAwareRemoveUnreachable.cpp

@@ -44,18 +44,22 @@ class TypeAnaysisAwareClosureMarker final
       const IgnoreSets& ignore_sets,
       const method_override_graph::Graph& method_override_graph,
       bool record_reachability,
+      bool relaxed_keep_class_members,
       ConditionallyMarked* cond_marked,
       ReachableObjects* reachable_objects,
       InstantiableTypes* instantiable_types,
+      DynamicallyReferencedClasses* dynamically_referenced_classes,
       MarkWorkerState* worker_state,
       Stats* stats,
       std::shared_ptr<type_analyzer::global::GlobalTypeAnalyzer> gta)
       : reachability::TransitiveClosureMarker(ignore_sets,
                                               method_override_graph,
                                               record_reachability,
+                                              relaxed_keep_class_members,
                                               cond_marked,
                                               reachable_objects,
                                               instantiable_types,
+                                              dynamically_referenced_classes,
                                               worker_state,
                                               stats),
         m_gta(std::move(gta)) {}
@@ -69,6 +73,9 @@ class TypeAnaysisAwareClosureMarker final
     gather_methods_on_code(method, refs);
     // Gather from annotations
     method->gather_methods_from_annos(refs.methods);
+    if (m_relaxed_keep_class_members) {
+      gather_dynamic_references(method->get_code(), &refs);
+    }
     return refs;
   }
 
@@ -104,7 +111,7 @@ class TypeAnaysisAwareClosureMarker final
               overriding_methods.size(), SHOW(m));
       }
       for (auto* overriding : overriding_methods) {
-        push_if_instantiable(overriding);
+        push_if_class_instantiable(overriding);
         TRACE(REACH, 3, "marking root override: %s", SHOW(overriding));
       }
     }
@@ -178,7 +185,8 @@ class TypeAnaysisAwareClosureMarker final
     for (auto overriding_method : overriding_methods) {
       TRACE(TRMU, 5, "Gather conditional method ref %s",
             SHOW(overriding_method));
-      refs.cond_methods.push_back(overriding_method);
+      always_assert(overriding_method->is_virtual());
+      refs.vmethods_if_class_instantiable.push_back(overriding_method);
     }
   }
 
@@ -229,6 +237,7 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(
     const IgnoreSets& ignore_sets,
     int* num_ignore_check_strings,
     bool record_reachability,
+    bool relaxed_keep_class_members,
     std::shared_ptr<type_analyzer::global::GlobalTypeAnalyzer> gta,
     bool /*unused*/) {
   Timer t("Marking");
@@ -241,13 +250,16 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(
 
   auto reachable_objects = std::make_unique<ReachableObjects>();
   InstantiableTypes instantiable_types;
+  reachability::DynamicallyReferencedClasses dynamically_referenced_classes;
   ConditionallyMarked cond_marked;
   auto method_override_graph = mog::build_graph(scope);
 
   ConcurrentSet<ReachableObject, ReachableObjectHash> root_set;
+  bool remove_no_argument_constructors = false;
   RootSetMarker root_set_marker(*method_override_graph, record_reachability,
-                                &cond_marked, reachable_objects.get(),
-                                &root_set);
+                                relaxed_keep_class_members,
+                                remove_no_argument_constructors, &cond_marked,
+                                reachable_objects.get(), &root_set);
   root_set_marker.mark(scope);
 
   size_t num_threads = redex_parallel::default_num_threads();
@@ -256,8 +268,9 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(
       [&](MarkWorkerState* worker_state, const ReachableObject& obj) {
         TypeAnaysisAwareClosureMarker transitive_closure_marker(
             ignore_sets, *method_override_graph, record_reachability,
-            &cond_marked, reachable_objects.get(), &instantiable_types,
-            worker_state, &stats_arr[worker_state->worker_id()], gta);
+            relaxed_keep_class_members, &cond_marked, reachable_objects.get(),
+            &instantiable_types, &dynamically_referenced_classes, worker_state,
+            &stats_arr[worker_state->worker_id()], gta);
         transitive_closure_marker.visit(obj);
         return nullptr;
       },
@@ -282,6 +295,7 @@ TypeAnalysisAwareRemoveUnreachablePass::compute_reachable_objects(
     PassManager& pm,
     int* num_ignore_check_strings,
     bool emit_graph_this_run,
+    bool relaxed_keep_class_members,
     bool remove_no_argument_constructors) {
   // Fetch analysis result
   auto analysis = pm.template get_preserved_analysis<GlobalTypeAnalysisPass>();
@@ -290,8 +304,8 @@ TypeAnalysisAwareRemoveUnreachablePass::compute_reachable_objects(
   always_assert(gta);
 
   return compute_reachable_objects_with_type_anaysis(
-      stores, m_ignore_sets, num_ignore_check_strings, emit_graph_this_run, gta,
-      remove_no_argument_constructors);
+      stores, m_ignore_sets, num_ignore_check_strings, emit_graph_this_run,
+      relaxed_keep_class_members, gta, remove_no_argument_constructors);
 }
 
 static TypeAnalysisAwareRemoveUnreachablePass s_pass;

opt/remove-unreachable/TypeAnalysisAwareRemoveUnreachable.h

@@ -27,5 +27,6 @@ class TypeAnalysisAwareRemoveUnreachablePass
       PassManager& pm,
       int* num_ignore_check_strings,
       bool emit_graph_this_run,
+      bool relaxed_keep_class_members,
       bool remove_no_argument_constructors) override;
 };

test/integ/ReachabilityTest.cpp

@@ -74,8 +74,8 @@ TEST_F(ReachabilityTest, ReachabilityMarkAllTest) {
   reachability::IgnoreSets ig_sets;
   auto reachable_objects = reachability::compute_reachable_objects(
       stores, ig_sets, &num_ignore_check_strings,
-      /* record_reachability */ false, /* should_mark_all_as_seed */ true,
-      nullptr);
+      /* record_reachability */ false, /* relaxed_keep_class_members */ false,
+      /* should_mark_all_as_seed */ true, nullptr);
 
   reachability::sweep(stores, *reachable_objects, nullptr);