Use template argument as intraprocedural base class

Summary: The constructor of the functional analyzer class has to do a lot of bookkeeping. I am thinking about using the base class and a number of helper functions to achieve the same and greatly simplify the constructor declaration (since there could be analysis that doesn't find all parameters useful.) Reviewed By: jeremydubreil Differential Revision: D21629733 fbshipit-source-id: 3125346ed9b872831fa18b0bc99d9a80c38afcec
zkwlx · Jul 15, 2020 · 45c70c7 · 45c70c7
1 parent b0f5e15
commit 45c70c7
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 62 deletions.
diff --git a/analysis/ip-reflection-analysis/IPReflectionAnalysis.cpp b/analysis/ip-reflection-analysis/IPReflectionAnalysis.cpp
@@ -135,17 +135,14 @@ struct AnalysisParameters {
 
 using CallerContext = typename Caller::Domain;
 
-template <typename FunctionSummaries>
-class ReflectionAnalyzer
-    : public Intraprocedural<CallerContext, AnalysisParameters> {
+template <typename Base>
+class ReflectionAnalyzer : public Base {
  private:
   const DexMethod* m_method;
-  FunctionSummaries* m_summaries;
   Summary m_summary;
 
  public:
-  ReflectionAnalyzer(const DexMethod* method, FunctionSummaries* summaries)
-      : m_method(method), m_summaries(summaries) {}
+  explicit ReflectionAnalyzer(const DexMethod* method) : m_method(method) {}
 
   void analyze() override {
     if (!m_method) {
@@ -154,16 +151,18 @@ class ReflectionAnalyzer
 
     reflection::SummaryQueryFn query_fn =
         [&](const DexMethod* callee) -> reflection::AbstractObjectDomain {
-      auto ret = m_summaries->get(callee, Summary::top()).get_return_value();
+      auto ret =
+          this->get_summaries()->get(callee, Summary::top()).get_return_value();
 
       std::unordered_set<const DexMethod*> overriding_methods =
           mog::get_overriding_methods(
               *(this->get_analysis_parameters()->method_override_graph),
               callee);
 
       for (const DexMethod* method : overriding_methods) {
-        ret.join_with(
-            m_summaries->get(method, Summary::top()).get_return_value());
+        ret.join_with(this->get_summaries()
+                          ->get(method, Summary::top())
+                          .get_return_value());
       }
       return ret;
     };
@@ -219,7 +218,7 @@ class ReflectionAnalyzer
     if (!m_method) {
       return;
     }
-    m_summaries->maybe_update(m_method, [&](Summary& old) {
+    this->get_summaries()->maybe_update(m_method, [&](Summary& old) {
       if (old == m_summary) {
         // no change will be made
         return false;
@@ -233,7 +232,9 @@ class ReflectionAnalyzer
 struct ReflectionAnalysisAdaptor : public AnalysisAdaptorBase {
   using Registry = MethodSummaryRegistry<Summary>;
   using FunctionSummary = Summary;
-  using FunctionAnalyzer = ReflectionAnalyzer<Registry>;
+
+  template <typename IntraproceduralBase>
+  using FunctionAnalyzer = ReflectionAnalyzer<IntraproceduralBase>;
 
   template <typename GraphInterface, typename Domain>
   using FixpointIteratorBase =

diff --git a/analysis/max-depth/MaxDepthAnalysis.cpp b/analysis/max-depth/MaxDepthAnalysis.cpp
@@ -112,18 +112,15 @@ struct Caller {
 // Core part of the analysis. This analyzer should be similar to an
 // intraprocedural analysis, except that we have access to the summaries and the
 // calling context.
-template <typename FunctionSummaries>
-class MaxDepthFunctionAnalyzer
-    : public Intraprocedural<typename Caller::Domain> {
+template <typename Base>
+class MaxDepthFunctionAnalyzer : public Base {
  private:
   const DexMethod* m_method;
-  FunctionSummaries* m_summaries;
   DepthDomain m_domain;
 
  public:
-  MaxDepthFunctionAnalyzer(const DexMethod* method,
-                           FunctionSummaries* summaries)
-      : m_method(method), m_summaries(summaries), m_domain(0) {}
+  explicit MaxDepthFunctionAnalyzer(const DexMethod* method)
+      : m_method(method), m_domain(0) {}
 
   void analyze() override {
     if (!m_method) {
@@ -152,7 +149,8 @@ class MaxDepthFunctionAnalyzer
     auto callee_method =
         resolve_method(callee, opcode_to_search(insn), m_method);
     if (callee_method) {
-      auto summary = m_summaries->get(callee_method, DepthDomain::top());
+      auto summary =
+          this->get_summaries()->get(callee_method, DepthDomain::top());
       if (summary.is_value()) {
         m_domain.join_with(DepthDomain(summary.depth() + 1u));
       } else {
@@ -167,7 +165,8 @@ class MaxDepthFunctionAnalyzer
     if (!m_method) {
       return;
     }
-    m_summaries->update(m_method, [&](const DepthDomain&) { return m_domain; });
+    this->get_summaries()->update(m_method,
+                                  [&](const DepthDomain&) { return m_domain; });
   }
 };
 
@@ -179,7 +178,8 @@ struct MaxDepthAnalysisAdaptor : public BottomUpAnalysisAdaptorBase {
   using Registry = MethodSummaryRegistry<DepthDomain>;
   using FunctionSummary = DepthDomain;
 
-  using FunctionAnalyzer = MaxDepthFunctionAnalyzer<Registry>;
+  template <typename IntraproceduralBase>
+  using FunctionAnalyzer = MaxDepthFunctionAnalyzer<IntraproceduralBase>;
   using Callsite = Caller;
 };
 

diff --git a/sparta/include/Analyzer.h b/sparta/include/Analyzer.h
@@ -81,27 +81,38 @@ optionally_analyze_edge_if_exist(Callsite* c,
 // analyzers don't implement the necessary methods. We prefer this over template
 // errors.
 
-template <typename CallerContext, typename AnalysisParameters = void>
+template <typename FunctionSummaries,
+          typename CallerContext,
+          typename CallGraph,
+          typename AnalysisParameters = void>
 class Intraprocedural {
  public:
   virtual void analyze() = 0;
   virtual void summarize() = 0;
   virtual ~Intraprocedural() {}
+  void set_summaries(FunctionSummaries* summaries) {
+    this->m_summaries = summaries;
+  }
   void set_caller_context(CallerContext* context) {
     this->m_caller_context = context;
   }
+  void set_call_graph(const CallGraph* graph) { this->m_call_graph = graph; }
   void set_analysis_parameters(AnalysisParameters* parameters) {
     this->m_analysis_parameters = parameters;
   }
 
  protected:
+  FunctionSummaries* get_summaries() const { return this->m_summaries; }
   CallerContext* get_caller_context() const { return this->m_caller_context; }
+  const CallGraph* get_call_graph() const { return this->m_call_graph; }
   AnalysisParameters* get_analysis_parameters() const {
     return this->m_analysis_parameters;
   }
 
  private:
+  FunctionSummaries* m_summaries;
   CallerContext* m_caller_context;
+  const CallGraph* m_call_graph;
   AnalysisParameters* m_analysis_parameters;
 };
 
@@ -143,11 +154,13 @@ class InterproceduralAnalyzer {
 
   using CallGraphInterface = typename Analysis::CallGraphInterface;
 
-  using FunctionAnalyzer = typename Analysis::FunctionAnalyzer;
   using CallGraph = typename CallGraphInterface::Graph;
   using Callsite = typename Analysis::Callsite;
   using CallerContext = typename Callsite::Domain;
 
+  using FunctionAnalyzer = typename Analysis::template FunctionAnalyzer<
+      Intraprocedural<Registry, CallerContext, CallGraph, AnalysisParameters>>;
+
   using IntraFn = std::function<std::shared_ptr<FunctionAnalyzer>(
       const Function&, Registry*, CallerContext*)>;
 
@@ -174,18 +187,17 @@ class InterproceduralAnalyzer {
 
     virtual void analyze_node(const typename CallGraphInterface::NodeId& node,
                               CallerContext* current_state) const override {
-      m_intraprocedural(
-          Analysis::function_by_node_id(node), this->m_registry, current_state)
+      m_intraprocedural(Analysis::function_by_node_id(node), this->m_registry,
+                        current_state)
           ->summarize();
     }
 
     CallerContext analyze_edge(
         const typename CallGraphInterface::EdgeId& edge,
         const CallerContext& exit_state_at_source) const override {
       return optionally_analyze_edge_if_exist<
-          Callsite,
-          typename CallGraphInterface::EdgeId,
-          CallerContext>(nullptr, edge, exit_state_at_source);
+          Callsite, typename CallGraphInterface::EdgeId, CallerContext>(
+          nullptr, edge, exit_state_at_source);
     }
   };
 
@@ -194,7 +206,8 @@ class InterproceduralAnalyzer {
                   "Registry must inherit from sparta::AbstractRegistry");
 
     static_assert(
-        std::is_base_of<Intraprocedural<CallerContext, AnalysisParameters>,
+        std::is_base_of<Intraprocedural<Registry, CallerContext, CallGraph,
+                                        AnalysisParameters>,
                         FunctionAnalyzer>::value,
         "FunctionAnalyzer must inherit from sparta::Intraprocedural");
 
@@ -230,10 +243,11 @@ class InterproceduralAnalyzer {
         fp = std::make_shared<CallGraphFixpointIterator>(
             *callgraph,
             &this->registry,
-            [this](const Function& func, Registry* reg, CallerContext* context)
-                -> std::shared_ptr<FunctionAnalyzer> {
+            [this, callgraph](
+                const Function& func, Registry* reg,
+                CallerContext* context) -> std::shared_ptr<FunctionAnalyzer> {
               // intraprocedural part
-              return this->run_on_function(func, reg, context);
+              return this->run_on_function(func, reg, context, &*callgraph);
             });
       }
 
@@ -252,10 +266,15 @@ class InterproceduralAnalyzer {
   }
 
   virtual std::shared_ptr<FunctionAnalyzer> run_on_function(
-      const Function& function, Registry* reg, CallerContext* context) {
+      const Function& function,
+      Registry* reg,
+      CallerContext* context,
+      const CallGraph* graph) {
 
-    auto analyzer = std::make_shared<FunctionAnalyzer>(function, reg);
+    auto analyzer = std::make_shared<FunctionAnalyzer>(function);
+    analyzer->set_summaries(reg);
     analyzer->set_caller_context(context);
+    analyzer->set_call_graph(graph);
     analyzer->set_analysis_parameters(m_parameters);
     analyzer->analyze();
     return analyzer;

diff --git a/sparta/test/InterproceduralAnalyzerTest.cpp b/sparta/test/InterproceduralAnalyzerTest.cpp
@@ -398,22 +398,16 @@ struct Callsite {
 
 using CallerContext = typename Callsite::Domain;
 
-template <typename FunctionSummaries>
-class SimpleFunctionAnalyzer : public sparta::Intraprocedural<CallerContext> {
+template <typename Base>
+class SimpleFunctionAnalyzer : public Base {
  private:
   language::Function* m_fun;
   language::ControlFlowGraph m_cfg;
-  FunctionSummaries* m_summaries;
   PurityDomain m_domain;
 
  public:
-  SimpleFunctionAnalyzer(language::Function* fun, FunctionSummaries* summaries)
-      : m_fun(fun),
-        m_cfg(language::build_cfg(m_fun)),
-        m_summaries(summaries),
-        m_domain(PURE) {
-    assert(summaries);
-  }
+  SimpleFunctionAnalyzer(language::Function* fun)
+      : m_fun(fun), m_cfg(language::build_cfg(m_fun)), m_domain(PURE) {}
 
   virtual void analyze() override {
     for (auto entry : m_cfg.statements()) {
@@ -428,7 +422,7 @@ class SimpleFunctionAnalyzer : public sparta::Intraprocedural<CallerContext> {
           // No action needed if the function is already impure at this control
           // point, otherwise we grab the summary for the callee
           auto func = stmt.callee;
-          auto summary = m_summaries->get(func);
+          auto summary = this->get_summaries()->get(func);
 
           boost::optional<bool> maybe_pure = boost::none;
 
@@ -472,32 +466,24 @@ class SimpleFunctionAnalyzer : public sparta::Intraprocedural<CallerContext> {
         conclusion.set_value(false);
       }
     }
-    m_summaries->update(m_fun, [&](const Summary&) { return conclusion; });
+    this->get_summaries()->update(m_fun,
+                                  [&](const Summary&) { return conclusion; });
   }
 };
 
-template <typename FunctionSummaries>
+template <typename Base>
 class FunctionFixpoint final : public sparta::MonotonicFixpointIterator<
                                    language::ControlFlowGraphInterface,
                                    PurityDomain>,
-                               public sparta::Intraprocedural<CallerContext> {
+                               public Base {
 
  private:
   language::Function* m_function;
-  FunctionSummaries* m_summaries;
-
   PurityDomain initial_domain() { return PurityDomain(PURE); }
 
  public:
-  explicit FunctionFixpoint(const language::FunctionId& function,
-                            FunctionSummaries* summaries,
-                            CallerContext* context,
-                            void* /* metadata */)
-      : MonotonicFixpointIterator(language::build_cfg(function)),
-        m_function(function),
-        m_summaries(summaries) {
-    assert(summaries);
-  }
+  explicit FunctionFixpoint(const language::FunctionId& function)
+      : MonotonicFixpointIterator(language::build_cfg(function)) {}
 
   // Introduced by sparta::Intraprocedural
   virtual void analyze() override {
@@ -523,7 +509,7 @@ class FunctionFixpoint final : public sparta::MonotonicFixpointIterator<
         // No action needed if the function is already impure at this control
         // point, otherwise we grab the summary for the callee
         auto func = stmt.callee;
-        auto summary = m_summaries->get(func);
+        auto summary = this->get_summaries()->get(func);
 
         boost::optional<bool> maybe_pure = boost::none;
 
@@ -575,7 +561,8 @@ class FunctionFixpoint final : public sparta::MonotonicFixpointIterator<
         conclusion.set_value(false);
       }
     }
-    m_summaries->update(m_function, [&](const Summary&) { return conclusion; });
+    this->get_summaries()->update(m_function,
+                                  [&](const Summary&) { return conclusion; });
   }
 };
 
@@ -601,7 +588,9 @@ class AnalysisRegistry : public sparta::AbstractRegistry {
 
 struct PurityAnalysisAdaptor : public language::AnalysisAdaptorBase {
   using Registry = AnalysisRegistry;
-  using FunctionAnalyzer = SimpleFunctionAnalyzer<Registry>;
+
+  template <typename IntraproceduralBase>
+  using FunctionAnalyzer = SimpleFunctionAnalyzer<IntraproceduralBase>;
   using Callsite = Callsite;
 
   // Optional: override call graph level fixpoint iterator type