Merge pull request diffblue#110 from diffblue/marek/propagation_chains_bug_fix_PR

Bug-fix in removal of dead branches in chains propagation graph

Author: marek-trtik

src/taint-slicer/instrumentation_props.cpp

@@ -62,8 +62,8 @@ static void perform_BFS(
       continue;
     reachable.insert(nid);
     if(do_fwd_search)
-      for(const auto &fns_nid : chains.get_successors_map().at(nid))
-        work.push_back(fns_nid.second);
+      for(const auto &nid_fns : chains.get_successors_map().at(nid))
+        work.push_back(nid_fns.first);
     else
       for(const auto pred_nid : chains.get_predecessors_map().at(nid))
         work.push_back(pred_nid);
@@ -126,8 +126,8 @@ void taint_build_instrumentation_props(
     // 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 &fns_nid : elem.second) // For each out-edge from the node
-        for(const auto &fn : fns_nid.first) // For each label on the edge
+      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);
   }
 

src/taint-slicer/propagation_chains.cpp

@@ -170,8 +170,8 @@ taint_propagation_chainst::taint_propagation_chainst(
   }
   // Build predecessors of nodes of chains.
   for(const auto &elem : successors)
-    for(const auto &fns_nid : elem.second)
-      predecessors[fns_nid.second].insert(elem.first);
+    for(const auto &nid_fns : elem.second)
+      predecessors[nid_fns.first].insert(elem.first);
   resolve_conditional_rule_applications(
     program,
     numbering,
@@ -242,7 +242,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({functions, dst_nid});
+  const auto res=successors[nid].insert({dst_nid, functions});
   const std::vector<taint_rule_idt> &to_sink_rules=
     tokens_propagation_graph.get_backward_rules_from_token(
       tokens_propagation_graph.get_sink_token());
@@ -279,24 +279,45 @@ taint_propagation_chainst::erase_node(const node_idt nid)
   }
 
   for(const auto &pred_nid : get_predecessors_map().at(nodes.size()))
-  {
-    auto &edges=successors.at(pred_nid);
-    std::vector<successors_mapt::mapped_type::iterator> to_remove;
-    for(auto it=edges.begin(); it!=edges.end(); ++it)
-      if(it->second==nodes.size())
-        to_remove.push_back(it);
-    for(const auto &it : to_remove)
+    if(pred_nid!=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.
     {
-      edges.insert({it->first,nid});
-      edges.erase(it);
+      auto &edges=successors.at(pred_nid);
+      edges[nid].insert(edges[nodes.size()].begin(), edges[nodes.size()].end());
+      edges.erase(nodes.size());
     }
-  }
-  for(const auto &fns_nid : get_successors_map().at(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,
+                                    // 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);
+      edges.insert(nid);
+      edges.erase(nodes.size());
+    }
+  // Here we handle the situation, when the node 'nodes.size()' (i.e. the one
+  // replacing the removed node 'nid') is its own predecessor and successor
+  // (i.e. when this node has a loop edge).
+  if(get_predecessors_map().at(nodes.size()).count(nodes.size())!=0UL)
   {
-    auto &edges=predecessors.at(fns_nid.second);
-    edges.insert(nid);
-    edges.erase(nodes.size());
+    {
+      auto &edges=successors.at(nodes.size());
+      edges[nid].insert(edges[nodes.size()].begin(), edges[nodes.size()].end());
+      edges.erase(nodes.size());
+    }
+    {
+      auto &edges=predecessors.at(nodes.size());
+      edges.insert(nid);
+      edges.erase(nodes.size());
+    }
   }
+  // Now we finally remove in- and out-edges of the node 'nid' (by replacing
+  // them by the last node 'nodes.size()', which will become the new node
+  // 'nid').
   successors.at(nid)=successors.at(nodes.size());
   successors.erase(nodes.size());
   predecessors.at(nid)=predecessors.at(nodes.size());
@@ -321,7 +342,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.second);
+          work.push_back(fns_nid.first);
       }
     }
     // Use BFS to find all nodes backward-reachable from sinks.
@@ -448,13 +469,13 @@ std::ostream &to_dot(
   for(const auto &nid : chains.get_sinks())
     ostr << "  " << nid << " -> " << chains.get_nodes().size()+1U << ";\n";
   for(const auto &nid_edge : chains.get_successors_map())
-    for(const auto &elabel_dstnid : nid_edge.second)
+    for(const auto &dstnid_elabel : nid_edge.second)
     {
-      ostr << "  " << nid_edge.first << " -> " << elabel_dstnid.second;
+      ostr << "  " << nid_edge.first << " -> " << dstnid_elabel.first;
       ostr << " [label=\"FUNCTIONS {";
-      if(!elabel_dstnid.first.empty())
+      if(!dstnid_elabel.second.empty())
         ostr << "\\l";
-      for(const auto &fid : elabel_dstnid.first)
+      for(const auto &fid : dstnid_elabel.second)
         ostr << "    " << fid << "\\l";
       ostr << "}\\l\"];\n";
     }

src/taint-slicer/propagation_chains.h

@@ -114,7 +114,7 @@ class taint_propagation_chainst
 
   typedef std::map<
             node_idt,
-            std::set<std::pair<function_ids_sett, node_idt> > >
+            std::map<node_idt, function_ids_sett> >
           successors_mapt;
   typedef std::map<node_idt, std::set<node_idt> > predecessors_mapt;