cfg_dominatorst: introduce a public interface

This eliminates most use of cfg_dominatorst::cfg, hiding the implementation behind a clean interface.

Author: smowton

src/analyses/cfg_dominators.h

@@ -22,6 +22,17 @@ Author: Georg Weissenbacher, [email protected]
 #include <goto-programs/goto_program.h>
 #include <goto-programs/cfg.h>
 
+/// Dominator graph. This computes a control-flow graph (see \ref cfgt) and
+/// decorates it with dominator sets per program point, following
+/// "A Simple, Fast Dominance Algorithm" by Cooper et al.
+/// Templated over the program type (P) and program point type (T), which need
+/// to be supported by \ref cfgt. Can compute either dominators or
+/// postdominators depending on template parameter `post_dom`.
+/// Use \ref cfg_dominators_templatet::dominates to directly query dominance,
+/// or \ref cfg_dominators_templatet::get_node to get the \ref cfgt graph node
+/// corresponding to a program point, including the in- and out-edges provided
+/// by \ref cfgt as well as the dominator set computed by this class.
+/// See also https://en.wikipedia.org/wiki/Dominator_(graph_theory)
 template <class P, class T, bool post_dom>
 class cfg_dominators_templatet
 {
@@ -38,6 +49,59 @@ class cfg_dominators_templatet
 
   void operator()(P &program);
 
+  /// Get the graph node (which gives dominators, predecessors and successors)
+  /// for \p program_point
+  const typename cfgt::nodet &get_node(const T &program_point) const
+  {
+    return cfg[cfg.entry_map.at(program_point)];
+  }
+
+  /// Get the graph node (which gives dominators, predecessors and successors)
+  /// for \p program_point
+  typename cfgt::nodet &get_node(const T &program_point)
+  {
+    return cfg[cfg.entry_map.at(program_point)];
+  }
+
+  /// Returns true if the program point corresponding to \p rhs_node is
+  /// dominated by program point \p lhs. Saves node lookup compared to the
+  /// dominates overload that takes two program points, so this version is
+  /// preferable if you intend to check more than one potential dominator.
+  /// Note by definition all program points dominate themselves.
+  bool dominates(T lhs, const nodet &rhs_node) const
+  {
+    return rhs_node.dominators.count(lhs);
+  }
+
+  /// Returns true if program point \p lhs dominates \p rhs.
+  /// Note by definition all program points dominate themselves.
+  bool dominates(T lhs, T rhs) const
+  {
+    return dominates(lhs, get_node(rhs));
+  }
+
+  /// Returns true if the program point for \p program_point_node is reachable
+  /// from the entry point. Saves a lookup compared to the overload taking a
+  /// program point, so use this overload if you already have the node.
+  bool program_point_reachable(const nodet &program_point_node) const
+  {
+    // Dominator analysis walks from the entry point, so a side-effect is to
+    // identify unreachable program points (those which don't dominate even
+    // themselves).
+    return !program_point_node.dominators.empty();
+  }
+
+  /// Returns true if the program point for \p program_point_node is reachable
+  /// from the entry point. Saves a lookup compared to the overload taking a
+  /// program point, so use this overload if you already have the node.
+  bool program_point_reachable(T program_point) const
+  {
+    // Dominator analysis walks from the entry point, so a side-effect is to
+    // identify unreachable program points (those which don't dominate even
+    // themselves).
+    return program_point_reachable(get_node(program_point));
+  }
+
   T entry_node;
 
   void output(std::ostream &) const;

src/analyses/dependence_graph.cpp

@@ -97,18 +97,15 @@ void dep_graph_domaint::control_dependencies(
 
     // 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(control_dep_candidate);
-
-    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];
+      pd.get_node(control_dep_candidate);
 
     // successors of M
     for(const auto &edge : m.out)
     {
+      // Could use pd.dominates(to, control_dep_candidate) but this would impose
+      // another dominator node lookup per call to this function, which is too
+      // expensive.
       const cfg_post_dominatorst::cfgt::nodet &m_s=
         pd.cfg[edge.first];
 

src/analyses/natural_loops.h

@@ -122,16 +122,13 @@ void natural_loops_templatet<P, T>::compute(P &program)
 
       if(target->location_number<=m_it->location_number)
       {
-        const nodet &node=
-          cfg_dominators.cfg[cfg_dominators.cfg.entry_map[m_it]];
-
         #ifdef DEBUG
         std::cout << "Computing loop for "
                   << m_it->location_number << " -> "
                   << target->location_number << "\n";
         #endif
 
-        if(node.dominators.find(target)!=node.dominators.end())
+        if(cfg_dominators.dominates(target, m_it))
           compute_natural_loop(m_it, target);
       }
     }
@@ -160,7 +157,7 @@ void natural_loops_templatet<P, T>::compute_natural_loop(T m, T n)
     stack.pop();
 
     const nodet &node=
-      cfg_dominators.cfg[cfg_dominators.cfg.entry_map[p]];
+      cfg_dominators.get_node(p);
 
     for(const auto &edge : node.in)
     {

src/goto-instrument/full_slicer.cpp

@@ -151,16 +151,10 @@ void full_slicert::add_jumps(
     const irep_idt &id = j.function_id;
     const cfg_post_dominatorst &pd=post_dominators.at(id);
 
-    cfg_post_dominatorst::cfgt::entry_mapt::const_iterator e=
-      pd.cfg.entry_map.find(j.PC);
-
-    assert(e!=pd.cfg.entry_map.end());
-
-    const cfg_post_dominatorst::cfgt::nodet &n=
-      pd.cfg[e->second];
+    const auto &j_PC_node = pd.get_node(j.PC);
 
     // find the nearest post-dominator in slice
-    if(n.dominators.find(lex_succ)==n.dominators.end())
+    if(!pd.dominates(lex_succ, j_PC_node))
     {
       add_to_queue(queue, *it, lex_succ);
       jumps.erase(it);
@@ -172,8 +166,8 @@ void full_slicert::add_jumps(
       goto_programt::const_targett nearest=lex_succ;
       std::size_t post_dom_size=0;
       for(cfg_dominatorst::target_sett::const_iterator
-          d_it=n.dominators.begin();
-          d_it!=n.dominators.end();
+          d_it=j_PC_node.dominators.begin();
+          d_it!=j_PC_node.dominators.end();
           ++d_it)
       {
         cfgt::entry_mapt::const_iterator entry=
@@ -186,18 +180,12 @@ void full_slicert::add_jumps(
           INVARIANT(id==id2,
                     "goto/jump expected to be within a single function");
 
-          cfg_post_dominatorst::cfgt::entry_mapt::const_iterator e2=
-            pd.cfg.entry_map.find(*d_it);
-
-          assert(e2!=pd.cfg.entry_map.end());
-
-          const cfg_post_dominatorst::cfgt::nodet &n2=
-            pd.cfg[e2->second];
+          const auto &postdom_node = pd.get_node(*d_it);
 
-          if(n2.dominators.size()>post_dom_size)
+          if(postdom_node.dominators.size()>post_dom_size)
           {
             nearest=*d_it;
-            post_dom_size=n2.dominators.size();
+            post_dom_size=postdom_node.dominators.size();
           }
         }
       }

src/goto-instrument/goto_program2code.cpp

@@ -763,14 +763,10 @@ bool goto_program2codet::set_block_end_points(
         case_end!=upper_bound;
         ++case_end)
     {
-      cfg_dominatorst::cfgt::entry_mapt::const_iterator i_entry=
-        dominators.cfg.entry_map.find(case_end);
-      assert(i_entry!=dominators.cfg.entry_map.end());
-      const cfg_dominatorst::cfgt::nodet &n=
-        dominators.cfg[i_entry->second];
+      const auto &case_end_node = dominators.get_node(case_end);
 
       // ignore dead instructions for the following checks
-      if(n.dominators.empty())
+      if(!dominators.program_point_reachable(case_end_node))
       {
         // simplification may have figured out that a case is unreachable
         // this is possibly getting too weird, abort to be safe
@@ -781,7 +777,7 @@ bool goto_program2codet::set_block_end_points(
       }
 
       // find the last instruction dominated by the case start
-      if(n.dominators.find(it->case_start)==n.dominators.end())
+      if(!dominators.dominates(it->case_start, case_end_node))
         break;
 
       if(!processed_locations.insert(case_end->location_number).second)
@@ -818,13 +814,7 @@ bool goto_program2codet::remove_default(
           next_case!=goto_program.instructions.end();
           ++next_case)
       {
-        cfg_dominatorst::cfgt::entry_mapt::const_iterator i_entry=
-          dominators.cfg.entry_map.find(next_case);
-        assert(i_entry!=dominators.cfg.entry_map.end());
-        const cfg_dominatorst::cfgt::nodet &n=
-          dominators.cfg[i_entry->second];
-
-        if(!n.dominators.empty())
+        if(dominators.program_point_reachable(next_case))
           break;
       }
 
@@ -876,10 +866,7 @@ goto_programt::const_targett goto_program2codet::convert_goto_switch(
 
   // always use convert_goto_if for dead code as the construction below relies
   // on effective dominator information
-  cfg_dominatorst::cfgt::entry_mapt::const_iterator t_entry=
-    dominators.cfg.entry_map.find(target);
-  assert(t_entry!=dominators.cfg.entry_map.end());
-  if(dominators.cfg[t_entry->second].dominators.empty())
+  if(!dominators.program_point_reachable(target))
     return convert_goto_if(target, upper_bound, dest);
 
   // maybe, let's try some more
@@ -1019,13 +1006,7 @@ goto_programt::const_targett goto_program2codet::convert_goto_switch(
     if(processed_locations.find(it->location_number)==
         processed_locations.end())
     {
-      cfg_dominatorst::cfgt::entry_mapt::const_iterator it_entry=
-        dominators.cfg.entry_map.find(it);
-      assert(it_entry!=dominators.cfg.entry_map.end());
-      const cfg_dominatorst::cfgt::nodet &n=
-        dominators.cfg[it_entry->second];
-
-      if(!n.dominators.empty())
+      if(dominators.program_point_reachable(it))
       {
         toplevel_block.swap(toplevel_block_bak);
         return convert_goto_if(orig_target, upper_bound, dest);
@@ -1128,13 +1109,7 @@ goto_programt::const_targett goto_program2codet::convert_goto_break_continue(
       next!=upper_bound && next!=goto_program.instructions.end();
       ++next)
   {
-    cfg_dominatorst::cfgt::entry_mapt::const_iterator i_entry=
-      dominators.cfg.entry_map.find(next);
-    assert(i_entry!=dominators.cfg.entry_map.end());
-    const cfg_dominatorst::cfgt::nodet &n=
-      dominators.cfg[i_entry->second];
-
-    if(!n.dominators.empty())
+    if(dominators.program_point_reachable(next))
       break;
   }
 
@@ -1168,13 +1143,7 @@ goto_programt::const_targett goto_program2codet::convert_goto_break_continue(
       after_loop!=goto_program.instructions.end();
       ++after_loop)
   {
-    cfg_dominatorst::cfgt::entry_mapt::const_iterator i_entry=
-      dominators.cfg.entry_map.find(after_loop);
-    assert(i_entry!=dominators.cfg.entry_map.end());
-    const cfg_dominatorst::cfgt::nodet &n=
-      dominators.cfg[i_entry->second];
-
-    if(!n.dominators.empty())
+    if(dominators.program_point_reachable(after_loop))
       break;
   }
 
@@ -1207,15 +1176,10 @@ goto_programt::const_targett goto_program2codet::convert_goto_goto(
     return target;
 
   const cfg_dominatorst &dominators=loops.get_dominator_info();
-  cfg_dominatorst::cfgt::entry_mapt::const_iterator it_entry=
-    dominators.cfg.entry_map.find(target);
-  assert(it_entry!=dominators.cfg.entry_map.end());
-  const cfg_dominatorst::cfgt::nodet &n=
-    dominators.cfg[it_entry->second];
 
   // skip dead goto L as the label might be skipped if it is dead
   // as well and at the end of a case block
-  if(n.dominators.empty())
+  if(!dominators.program_point_reachable(target))
     return target;
 
   std::stringstream label;

src/goto-programs/cfg.h

@@ -102,6 +102,15 @@ class cfg_baset:public grapht< cfg_base_nodet<T, I> >
 
       return e.first->second;
     }
+
+    entryt &at(const goto_programt::const_targett &t)
+    {
+      return data.at(t);
+    }
+    const entryt &at(const goto_programt::const_targett &t) const
+    {
+      return data.at(t);
+    }
   };
   entry_mapt entry_map;