Merge pull request diffblue#496 from diffblue/resolve_performance_issues_in_instrumenter

SEC-542: Replaced edge labels of propagation chains graph by global set of functions.

Author: marek-trtik

src/taint-slicer/instrumentation_props.cpp

@@ -95,7 +95,7 @@ static void perform_BFS(
     reachable.insert(nid);
     if(do_fwd_search)
       for(const auto &nid_fns : chains.get_successors_map().at(nid))
-        work.push_back(nid_fns.first);
+        work.push_back(nid_fns);
     else
       for(const auto pred_nid : chains.get_predecessors_map().at(nid))
         work.push_back(pred_nid);
@@ -232,28 +232,14 @@ void taint_build_instrumentation_props(
 {
   TMPROF_BLOCK();
 
-  // First we collect all functions mentioned in the graph of chains.
-  std::set<irep_idt> functions;
-  {
-    // Here we collect functions mentioned in individual nodes
-    for(const auto &node : chains.get_nodes())
-      functions.insert(node.get_function_id());
-    // Here we collect functions appearing in sets labelling edges of
-    // chains graph.
-    for(const auto &elem : chains.get_successors_map()) // For each node
-      for(const auto &nid_fns : elem.second) // For each out-edge from the node
-        for(const auto &fn : nid_fns.second) // For each label on the edge
-          functions.insert(fn);
-  }
-
-  // Now we compute root functions - they are exactly all nearest common
+  // First we compute root functions - they are exactly all nearest common
   // callers of all functions collected from the graph of chains.
   // Explanation: A common caller of a set of functions is one that can reach
   //              all of them in the call graph; a nearest common caller has no
   //              callees which are themselves common callers.
   std::set<irep_idt> roots;
   find_nearest_common_callees(
-    program.get_inverted_call_graph(), functions, roots);
+    program.get_inverted_call_graph(), chains.get_functions(), roots);
 
   for(const auto &root : roots)
   {
@@ -277,7 +263,7 @@ void taint_build_instrumentation_props(
         //         (those corrensponding to applications of transition rules.)
         for(const auto &node : chains.get_nodes())
         {
-          if(functions.count(node.get_function_id())!=0UL)
+          if(chains.get_functions().count(node.get_function_id())!=0UL)
           {
             goto_programt::instructiont const&  I=*node.get_instruction_id();
             INVARIANT(I.type==FUNCTION_CALL, "");

src/taint-slicer/propagation_chains.cpp

@@ -141,6 +141,7 @@ taint_propagation_chainst::taint_propagation_chainst(
   // In breadth-first manner from the sources we build propagation chains toward
   // sinks. We only build successor edges of chains in this breadth-first
   // construnction.
+  std::unordered_map<taint_function_idt, function_ids_sett> cfg_reach_cache;
   std::deque<node_idt> bfs_queue(sources.cbegin(),sources.cend());
   std::unordered_set<nodet> visited;
   while(!bfs_queue.empty())
@@ -173,7 +174,8 @@ taint_propagation_chainst::taint_propagation_chainst(
                 fn_vec.first,
                 iit,
                 inverted_call_graph,
-                tokens_propagation_graph);
+                tokens_propagation_graph,
+                cfg_reach_cache);
               if(succ_nid!=nodes.size())
                 bfs_queue.push_back(succ_nid);
             }
@@ -190,7 +192,7 @@ taint_propagation_chainst::taint_propagation_chainst(
   // Build predecessors of nodes of chains.
   for(const auto &elem : successors)
     for(const auto &nid_fns : elem.second)
-      predecessors[nid_fns.first].insert(elem.first);
+      predecessors[nid_fns].insert(elem.first);
   resolve_conditional_rule_applications(
     program,
     numbering,
@@ -208,7 +210,8 @@ taint_propagation_chainst::extend_chain_by_transition(
   const taint_function_idt &fid,
   const taint_instruction_idt &iid,
   const call_grapht &inverted_call_graph,
-  const taint_tokens_propagation_grapht &tokens_propagation_graph)
+  const taint_tokens_propagation_grapht &tokens_propagation_graph,
+  std::unordered_map<taint_function_idt, function_ids_sett> &cfg_reach_cache)
 {
   TMPROF_BLOCK();
 
@@ -218,32 +221,38 @@ taint_propagation_chainst::extend_chain_by_transition(
   // in the chain is generated). Otherwise we record the set of intermediate
   // functions (callers and/or callees), through which the paths must go (we
   // save union of the functions from different paths).
-  function_ids_sett functions;
   {
-    function_ids_sett src_fns, dst_fns;
-    std::unordered_set<irep_idt,dstring_hash> fns;
-    find_direct_or_indirect_callees_of_function(
-      inverted_call_graph,
-      nodes.at(nid).get_function_id(),
-      fns);
-    for(const auto &fname : fns)
-      src_fns.insert(as_string(fname));
-    fns.clear();
-    find_direct_or_indirect_callees_of_function(inverted_call_graph, fid, fns);
-    for(const auto &fname : fns)
-      dst_fns.insert(as_string(fname));
+    const auto collect_callers =
+      [this, &inverted_call_graph](
+        std::unordered_map<taint_function_idt, function_ids_sett> &cache,
+        const taint_function_idt &fid)
+      {
+        if(cache.count(fid) == 0UL)
+        {
+          std::unordered_set<irep_idt,dstring_hash> fns;
+          find_direct_or_indirect_callees_of_function(
+            inverted_call_graph, fid, fns);
+          function_ids_sett &dst_set = cache[fid];
+          for(const auto &fname : fns)
+            dst_set.insert(fname);
+        }
+      };
+    collect_callers(cfg_reach_cache, nodes.at(nid).get_function_id());
+    collect_callers(cfg_reach_cache, fid);
+    const function_ids_sett &src_fns =
+      cfg_reach_cache.at(nodes.at(nid).get_function_id());
+    const function_ids_sett &dst_fns = cfg_reach_cache.at(fid);
+    function_ids_sett intersect_functions;
     std::set_intersection(
       src_fns.cbegin(), src_fns.cend(),
       dst_fns.cbegin(), dst_fns.cend(),
-      std::inserter(functions, functions.cend()));
-    if(functions.empty())
+      std::inserter(intersect_functions, intersect_functions.cend()));
+    if(intersect_functions.empty())
       return nodes.size();
     std::set_union(
       src_fns.cbegin(), src_fns.cend(),
       dst_fns.cbegin(), dst_fns.cend(),
       std::inserter(functions, functions.cend()));
-    functions.erase(nodes.at(nid).get_function_id());
-    functions.erase(fid);
   }
   // Now we compute the target instrumentation node.
   node_idt dst_nid;
@@ -263,7 +272,7 @@ taint_propagation_chainst::extend_chain_by_transition(
   }
   // Now we perform the the insertion of the transition to the new node and
   // we consider the case of the target node being sink.
-  const auto res=successors[nid].insert({dst_nid, functions});
+  const auto res=successors[nid].insert(dst_nid);
   const std::vector<taint_rule_idt> &to_sink_rules=
     tokens_propagation_graph.get_backward_rules_from_token(
       tokens_propagation_graph.get_sink_token());
@@ -319,17 +328,17 @@ taint_propagation_chainst::erase_node(const node_idt nid)
                                // predecessor. We handle that situation later.
     {
       auto &edges=successors.at(pred_nid);
-      edges[nid].insert(edges[nodes.size()].begin(), edges[nodes.size()].end());
+      edges.insert(nid);
       edges.erase(nodes.size());
     }
   for(const auto &nid_fns : get_successors_map().at(nodes.size()))
-    if(nid_fns.first!=nodes.size()) // We have to exclude here the situation,
+    if(nid_fns!=nodes.size())       // We have to exclude here the situation,
                                     // when the node 'nodes.size()' (i.e. the
                                     // one replacing the removed node 'nid')
                                     // is its own predecessor. We handle that
                                     // situation later.
     {
-      auto &edges=predecessors.at(nid_fns.first);
+      auto &edges=predecessors.at(nid_fns);
       edges.insert(nid);
       edges.erase(nodes.size());
     }
@@ -340,7 +349,7 @@ taint_propagation_chainst::erase_node(const node_idt nid)
   {
     {
       auto &edges=successors.at(nodes.size());
-      edges[nid].insert(edges[nodes.size()].begin(), edges[nodes.size()].end());
+      edges.insert(nid);
       edges.erase(nodes.size());
     }
     {
@@ -378,7 +387,7 @@ void taint_propagation_chainst::erase_dead_branches()
           continue;
         fwd_reachable.insert(nid);
         for(const auto &fns_nid : get_successors_map().at(nid))
-          work.push_back(fns_nid.first);
+          work.push_back(fns_nid);
       }
     }
     // Use BFS to find all nodes backward-reachable from sinks.
@@ -514,15 +523,7 @@ std::ostream &to_dot(
     ostr << "  " << nid << " -> " << chains.get_nodes().size()+1U << ";\n";
   for(const auto &nid_edge : chains.get_successors_map())
     for(const auto &dstnid_elabel : nid_edge.second)
-    {
-      ostr << "  " << nid_edge.first << " -> " << dstnid_elabel.first;
-      ostr << " [label=\"FUNCTIONS {";
-      if(!dstnid_elabel.second.empty())
-        ostr << "\\l";
-      for(const auto &fid : dstnid_elabel.second)
-        ostr << "    " << fid << "\\l";
-      ostr << "}\\l\"];\n";
-    }
+      ostr << "  " << nid_edge.first << " -> " << dstnid_elabel << ";\n";
 
   ostr << "}\n";
   return ostr;

src/taint-slicer/propagation_chains.h

@@ -117,12 +117,9 @@ class taint_propagation_chainst
   typedef node_vectort::size_type  node_idt;
   typedef std::set<node_idt>  node_id_sett;
 
-  typedef std::set<taint_function_idt> function_ids_sett;
+  typedef std::set<irep_idt> function_ids_sett;
 
-  typedef std::map<
-            node_idt,
-            std::map<node_idt, function_ids_sett> >
-          successors_mapt;
+  typedef std::map<node_idt, std::set<node_idt> > successors_mapt;
   typedef std::map<node_idt, std::set<node_idt> > predecessors_mapt;
 
   /*******************************************************************\
@@ -155,6 +152,7 @@ class taint_propagation_chainst
   const node_id_sett &get_sinks() const { return sinks; }
   const successors_mapt &get_successors_map() const { return successors; }
   const predecessors_mapt &get_predecessors_map() const { return predecessors; }
+  const function_ids_sett &get_functions() const { return functions; }
 
 private:
   taint_propagation_chainst(const taint_propagation_chainst &)=delete;
@@ -182,7 +180,8 @@ class taint_propagation_chainst
     const taint_function_idt &fid,
     const taint_instruction_idt &iid,
     const call_grapht &inverted_call_graph,
-    const taint_tokens_propagation_grapht &tokens_propagation_graph);
+    const taint_tokens_propagation_grapht &tokens_propagation_graph,
+    std::unordered_map<taint_function_idt, function_ids_sett> &cfg_reach_cache);
 
   /*******************************************************************\
 
@@ -228,6 +227,7 @@ class taint_propagation_chainst
   node_id_sett sinks;
   successors_mapt successors;
   predecessors_mapt predecessors;
+  function_ids_sett functions;
 };
 
 inline const taint_propagation_chainst::successors_mapt::mapped_type &