Control dependency computation fix

Previously in transform() only the control dependencies in the set control_deps
(plus gotos and assumes at the from location) were considered candidates for the
control dependencies at the to location. With this scheme however one misses
control dependencies when there are nested if statements.

We introduce a new set control_dep_candidates in addition to control_deps, which
contains all control statements encountered so far.

Author: danpoe

regression/goto-analyzer/dependence-graph4/main.c

@@ -0,0 +1,16 @@
+int g_in1, g_in2;
+int g_out;
+
+void main(void)
+{
+  int t;
+  if(g_in1)
+  {
+    if(g_in2)
+      t = 0;
+    else
+      t = 1;
+
+    g_out = 1; // depends on "if(g_in1)
+  }
+}

regression/goto-analyzer/dependence-graph4/test.desc

@@ -0,0 +1,9 @@
+CORE
+main.c
+--show --dependence-graph --text -
+activate-multi-line-match
+EXIT=0
+SIGNAL=0
+\/\/ ([0-9]+) file.*\n.*IF.*g_in1.*THEN GOTO(.*\n)*Control dependencies: \1\n\n.*\n.*g_out = 1
+--
+^warning: ignoring

src/analyses/dependence_graph.cpp

@@ -37,6 +37,8 @@ bool dep_graph_domaint::merge(
   }
 
   changed |= util_inplace_set_union(control_deps, src.control_deps);
+  changed |=
+    util_inplace_set_union(control_dep_candidates, src.control_dep_candidates);
 
   return changed;
 }
@@ -48,42 +50,54 @@ void dep_graph_domaint::control_dependencies(
 {
   // Better Slicing of Programs with Jumps and Switches
   // Kumar and Horwitz, FASE'02:
-  // Node N is control dependent on node M iff N postdominates one
-  // but not all of M's CFG successors.
+  // "Node N is control dependent on node M iff N postdominates, in
+  // the CFG, one but not all of M's CFG successors."
   //
-  // candidates for M are from and all existing control-depended on
-  // nodes; from is added if it is a goto or assume instruction
-  if(from->is_goto() ||
-     from->is_assume())
-    control_deps.insert(from);
+  // The "successor" above refers to an immediate successor of M.
+  //
+  // When computing the control dependencies of a node N (i.e., "to"
+  // being N), candidates for M are all control statements (gotos or
+  // assumes) from which there is a path in the CFG to N.
+
+  // Add new candidates
+
+  if(from->is_goto() || from->is_assume())
+    control_dep_candidates.insert(from);
+  else if(from->is_end_function())
+  {
+    control_dep_candidates.clear();
+    return;
+  }
+
+  if(control_dep_candidates.empty())
+    return;
+
+  // Get postdominators
 
   const irep_idt id=from->function;
   const cfg_post_dominatorst &pd=dep_graph.cfg_post_dominators().at(id);
 
-  // check all candidates for M
-  for(depst::iterator
-      it=control_deps.begin();
-      it!=control_deps.end();
-      ) // no ++it
-  {
-    depst::iterator next=it;
-    ++next;
+  // Check all candidates
 
-    // check all CFG successors
+  for(const auto &control_dep_candidate : control_dep_candidates)
+  {
+    // check all CFG successors of M
     // special case: assumptions also introduce a control dependency
-    bool post_dom_all=!(*it)->is_assume();
+    bool post_dom_all = !control_dep_candidate->is_assume();
     bool post_dom_one=false;
 
     // we could hard-code assume and goto handling here to improve
     // performance
-    cfg_post_dominatorst::cfgt::entry_mapt::const_iterator e=
-      pd.cfg.entry_map.find(*it);
+    cfg_post_dominatorst::cfgt::entry_mapt::const_iterator e =
+      pd.cfg.entry_map.find(control_dep_candidate);
 
-    assert(e!=pd.cfg.entry_map.end());
+    INVARIANT(
+      e != pd.cfg.entry_map.end(), "cfg must have an entry for every location");
 
     const cfg_post_dominatorst::cfgt::nodet &m=
       pd.cfg[e->second];
 
+    // successors of M
     for(const auto &edge : m.out)
     {
       const cfg_post_dominatorst::cfgt::nodet &m_s=
@@ -95,11 +109,14 @@ void dep_graph_domaint::control_dependencies(
         post_dom_all=false;
     }
 
-    if(post_dom_all ||
-       !post_dom_one)
-      control_deps.erase(it);
-
-    it=next;
+    if(post_dom_all || !post_dom_one)
+    {
+      control_deps.erase(control_dep_candidate);
+    }
+    else
+    {
+      control_deps.insert(control_dep_candidate);
+    }
   }
 }
 
@@ -190,7 +207,10 @@ void dep_graph_domaint::transform(
       assert(s!=nullptr);
 
       util_inplace_set_union(s->control_deps, control_deps);
+      util_inplace_set_union(s->control_dep_candidates, control_dep_candidates);
+
       control_deps.clear();
+      control_dep_candidates.clear();
     }
   }
   else

src/analyses/dependence_graph.h

@@ -162,7 +162,19 @@ class dep_graph_domaint:public ai_domain_baset
   node_indext node_id;
 
   typedef std::set<goto_programt::const_targett> depst;
-  depst control_deps, data_deps;
+
+  // Set of locations with control instructions on which the instruction at this
+  // location has a control dependency on
+  depst control_deps;
+
+  // Set of locations with control instructions from which there is a path in
+  // the CFG to the current location (with the locations being in the same
+  // function). The set control_deps is a subset of this set.
+  depst control_dep_candidates;
+
+  // Set of locations with instructions on which the instruction at this
+  // location has a data dependency on
+  depst data_deps;
 
   friend const depst &
     dependence_graph_test_get_control_deps(const dep_graph_domaint &);