[4/7,analyzer] Support global states and custom transitions

Message ID 20191204162530.9285-5-dmalcolm@redhat.com
State New
Headers show
Series
  • Add checking for unsafe calls within signal handlers
Related show

Commit Message

David Malcolm Dec. 4, 2019, 4:25 p.m.
This patch generalizes the state machine code to support "global" states
as well as per-expression states.  It also adds a custom_transition hook
for state machines that need to make non-standard transitions (e.g. calling
into a signal handler).

gcc/ChangeLog:
	* analyzer/checker-path.cc (state_change_event::get_desc): Support
	global state.
	* analyzer/diagnostic-manager.cc (get_any_origin): Assert that
	dst_rep is non-NULL.
	(state_change_event_creator::on_global_state_change): New function.
	(for_each_state_change): Handle global state changes.
	(diagnostic_manager::prune_path): Handle global state changes, where
	var is NULL.
	* analyzer/engine.cc (impl_sm_context::warn_for_state): Support
	global states.
	(impl_sm_context::get_global_state): New vfunc implementation.
	(impl_sm_context::set_global_state): New vfunc implementation.
	(impl_sm_context::on_custom_transition): New vfunc implementation.
	(exploded_node::get_dot_fillcolor): Support global states.
	* analyzer/pending-diagnostic.h (state_change::is_global_p): New function.
	* analyzer/program-state.cc (sm_state_map::sm_state_map): New ctor.
	(sm_state_map::print): Support global states.
	(sm_state_map::is_empty_p): Likewise.
	(sm_state_map::hash): Likewise.
	(sm_state_map::operator==): Likewise.
	(sm_state_map::set_global_state): New.
	(sm_state_map::get_global_state): New.
	* analyzer/program-state.h (sm_state_map::sm_state_map): New ctor decl.
	(sm_state_map::set_global_state): New decl.
	(sm_state_map::get_global_state): New decl.
	(sm_state_map::m_global_state): New field.
	(state_change_visitor::on_global_state_change): New vfunc.
	* analyzer/sm.h (class custom_transition): New abstract base class.
	(sm_context::get_global_state): New vfunc.
	(sm_context::set_global_state): New vfunc.
	(sm_context::on_custom_transition): New vfunc.
---
 gcc/analyzer/checker-path.cc       |  40 +++++++----
 gcc/analyzer/diagnostic-manager.cc | 112 ++++++++++++++++++++---------
 gcc/analyzer/engine.cc             |  39 ++++++++--
 gcc/analyzer/pending-diagnostic.h  |   2 +
 gcc/analyzer/program-state.cc      |  38 +++++++++-
 gcc/analyzer/program-state.h       |  11 +++
 gcc/analyzer/sm.h                  |  23 ++++++
 7 files changed, 209 insertions(+), 56 deletions(-)

-- 
2.21.0

Patch

diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index d277c2806308..c0783df9e8e9 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -246,21 +246,33 @@  state_change_event::get_desc (bool can_colorize) const
     }
 
   /* Fallback description.  */
-  if (m_origin)
-    return make_label_text
-      (can_colorize,
-       "state of %qE: %qs -> %qs (origin: %qE)",
-       m_var,
-       m_sm.get_state_name (m_from),
-       m_sm.get_state_name (m_to),
-       m_origin);
+  if (m_var)
+    {
+      if (m_origin)
+	return make_label_text
+	  (can_colorize,
+	   "state of %qE: %qs -> %qs (origin: %qE)",
+	   m_var,
+	   m_sm.get_state_name (m_from),
+	   m_sm.get_state_name (m_to),
+	   m_origin);
+      else
+	return make_label_text
+	  (can_colorize,
+	   "state of %qE: %qs -> %qs (origin: NULL)",
+	   m_var,
+	   m_sm.get_state_name (m_from),
+	   m_sm.get_state_name (m_to));
+    }
   else
-    return make_label_text
-      (can_colorize,
-       "state of %qE: %qs -> %qs (origin: NULL)",
-       m_var,
-       m_sm.get_state_name (m_from),
-       m_sm.get_state_name (m_to));
+    {
+      gcc_assert (m_origin == NULL_TREE);
+      return make_label_text
+	(can_colorize,
+	 "global state: %qs -> %qs",
+	 m_sm.get_state_name (m_from),
+	 m_sm.get_state_name (m_to));
+    }
 }
 
 ////////////////////////////////////////////////////////////////////////////
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 461cc3318b7f..694993cab10b 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -471,6 +471,8 @@  get_any_origin (const gimple *stmt,
   if (!stmt)
     return NULL_TREE;
 
+  gcc_assert (dst_rep);
+
   if (const gassign *assign = dyn_cast <const gassign *> (stmt))
     {
       tree lhs = gimple_assign_lhs (assign);
@@ -523,6 +525,33 @@  public:
       m_emission_path (emission_path)
   {}
 
+  bool on_global_state_change (const state_machine &sm,
+			       state_machine::state_t src_sm_val,
+			       state_machine::state_t dst_sm_val)
+    FINAL OVERRIDE
+  {
+    const exploded_node *src_node = m_eedge.m_src;
+    const program_point &src_point = src_node->get_point ();
+    const int src_stack_depth = src_point.get_stack_depth ();
+    const exploded_node *dst_node = m_eedge.m_dest;
+    const gimple *stmt = src_point.get_stmt ();
+    const supernode *supernode = src_point.get_supernode ();
+    const program_state &dst_state = dst_node->get_state ();
+
+    int stack_depth = src_stack_depth;
+
+    m_emission_path->add_event (new state_change_event (supernode,
+							stmt,
+							stack_depth,
+							sm,
+							NULL_TREE,
+							src_sm_val,
+							dst_sm_val,
+							NULL_TREE,
+							dst_state));
+    return false;
+  }
+
   bool on_state_change (const state_machine &sm,
 			state_machine::state_t src_sm_val,
 			state_machine::state_t dst_sm_val,
@@ -575,18 +604,19 @@  public:
 
 /* Compare SRC_STATE and DST_STATE (which use EXT_STATE), and call
    VISITOR's on_state_change for every sm-state change that occurs
-   to a tree.
+   to a tree, and on_global_state_change for every global state change
+   that occurs.
 
    This determines the state changes that ought to be reported to
    the user: a combination of the effects of changes to sm_state_map
    (which maps svalues to sm-states), and of region_model changes
    (which map trees to svalues).
 
-   Bail out early and return true if any call to on_state_change returns
-   true, otherwise return false.
+   Bail out early and return true if any call to on_global_state_change
+   or on_state_change returns true, otherwise return false.
 
    This is split out to make it easier to experiment with changes to
-   exploded_node granulairy (so that we can observe what state changes
+   exploded_node granularity (so that we can observe what state changes
    lead to state_change_events being emitted).  */
 
 bool
@@ -604,6 +634,15 @@  for_each_state_change (const program_state &src_state,
       const state_machine &sm = ext_state.get_sm (i);
       const sm_state_map &src_smap = *src_state.m_checker_states[i];
       const sm_state_map &dst_smap = *dst_state.m_checker_states[i];
+
+      /* Add events for any global state changes.  */
+      if (src_smap.get_global_state () != dst_smap.get_global_state ())
+	if (visitor->on_global_state_change (sm,
+					     src_smap.get_global_state (),
+					     dst_smap.get_global_state ()))
+	  return true;
+
+      /* Add events for per-svalue state changes.  */
       for (sm_state_map::iterator_t iter = dst_smap.begin ();
 	   iter != dst_smap.end ();
 	   ++iter)
@@ -856,8 +895,14 @@  diagnostic_manager::prune_path (checker_path *path,
       if (get_logger ())
 	{
 	  if (sm)
-	    log ("considering event %i, with var: %qE, state: %qs",
-		 idx, var, sm->get_state_name (state));
+	    {
+	      if (var)
+		log ("considering event %i, with var: %qE, state: %qs",
+		     idx, var, sm->get_state_name (state));
+	      else
+		log ("considering event %i, with global state: %qs",
+		     idx, sm->get_state_name (state));
+	    }
 	  else
 	    log ("considering event %i", idx);
 	}
@@ -877,14 +922,17 @@  diagnostic_manager::prune_path (checker_path *path,
 	case EK_STMT:
 	  {
 	    /* If this stmt is the origin of "var", update var.  */
-	    statement_event *stmt_event = (statement_event *)base_event;
-	    tree new_var = get_any_origin (stmt_event->m_stmt, var,
-					   stmt_event->m_dst_state);
-	    if (new_var)
+	    if (var)
 	      {
-		log ("event %i: switching var of interest from %qE to %qE",
-		     idx, var, new_var);
-		var = new_var;
+		statement_event *stmt_event = (statement_event *)base_event;
+		tree new_var = get_any_origin (stmt_event->m_stmt, var,
+					       stmt_event->m_dst_state);
+		if (new_var)
+		  {
+		    log ("event %i: switching var of interest from %qE to %qE",
+			 idx, var, new_var);
+		    var = new_var;
+		  }
 	      }
 	    if (m_verbosity < 3)
 	      {
@@ -1007,28 +1055,24 @@  diagnostic_manager::prune_path (checker_path *path,
 	  // TODO: potentially update var/state based on return value,
 	  // args etc
 	  {
-	    return_event *event = (return_event *)base_event;
-	    const callgraph_superedge& cg_superedge
-	      = event->get_callgraph_superedge ();
-	    callsite_expr expr;
-	    tree callee_var
-	      = cg_superedge.map_expr_from_caller_to_callee (var, &expr);
-	    if (callee_var)
+	    if (var)
 	      {
-		if (var)
-		  log ("event %i:"
-		       " switching var of interest"
-		       " from %qE in caller to %qE in callee",
-		       idx, var, callee_var);
-		else
-		  // TODO: how does this happen?
-		  log ("event %i:"
-		       " switching var of interest"
-		       " from NULL in caller to %qE in callee",
-		       idx, callee_var);
-		var = callee_var;
-		if (expr.return_value_p ())
-		  event->record_critical_state (var, state);
+		return_event *event = (return_event *)base_event;
+		const callgraph_superedge& cg_superedge
+		  = event->get_callgraph_superedge ();
+		callsite_expr expr;
+		tree callee_var
+		  = cg_superedge.map_expr_from_caller_to_callee (var, &expr);
+		if (callee_var)
+		  {
+		    log ("event %i:"
+			 " switching var of interest"
+			 " from %qE in caller to %qE in callee",
+			 idx, var, callee_var);
+		    var = callee_var;
+		    if (expr.return_value_p ())
+		      event->record_critical_state (var, state);
+		  }
 	      }
 	  }
 	  break;
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index d327340c8c28..f739af779ef3 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -234,9 +234,16 @@  public:
 
     impl_region_model_context old_ctxt
       (m_eg, m_enode_for_diag, m_old_state, m_new_state, m_change, NULL);
-    svalue_id var_old_sid
-      = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
-    state_machine::state_t current = m_old_smap->get_state (var_old_sid);
+    state_machine::state_t current;
+    if (var)
+      {
+	svalue_id var_old_sid
+	  = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
+	current = m_old_smap->get_state (var_old_sid);
+      }
+    else
+      current = m_old_smap->get_global_state ();
+
     if (state == current)
       {
 	m_eg.get_diagnostic_manager ().add_diagnostic
@@ -275,6 +282,23 @@  public:
     return pvs[0].m_tree;
   }
 
+  state_machine::state_t get_global_state () const FINAL OVERRIDE
+  {
+    return m_old_state->m_checker_states[m_sm_idx]->get_global_state ();
+  }
+
+  void set_global_state (state_machine::state_t state) FINAL OVERRIDE
+  {
+    m_new_state->m_checker_states[m_sm_idx]->set_global_state (state);
+  }
+
+  void on_custom_transition (custom_transition *transition) FINAL OVERRIDE
+  {
+    transition->impl_transition (&m_eg,
+				 const_cast<exploded_node *> (m_enode_for_diag),
+				 m_sm_idx);
+  }
+
   exploded_graph &m_eg;
   const exploded_node *m_enode_for_diag;
   const program_state *m_old_state;
@@ -718,10 +742,13 @@  exploded_node::get_dot_fillcolor () const
   int i;
   sm_state_map *smap;
   FOR_EACH_VEC_ELT (state.m_checker_states, i, smap)
-    for (sm_state_map::iterator_t iter = smap->begin ();
+    {
+      for (sm_state_map::iterator_t iter = smap->begin ();
 	 iter != smap->end ();
-	 ++iter)
-      total_sm_state += (*iter).second.m_state;
+	   ++iter)
+	total_sm_state += (*iter).second.m_state;
+      total_sm_state += smap->get_global_state ();
+    }
 
   if (total_sm_state > 0)
     {
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index f1ab484c0570..4103a91e2f56 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -57,6 +57,8 @@  struct state_change : public event_desc
     m_event_id (event_id)
   {}
 
+  bool is_global_p () const { return m_expr == NULL_TREE; }
+
   tree m_expr;
   tree m_origin;
   state_machine::state_t m_old_state;
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index ae15795b58be..6313174e99ad 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -35,6 +35,13 @@  along with GCC; see the file COPYING3.  If not see
 
 /* class sm_state_map.  */
 
+/* sm_state_map's ctor.  */
+
+sm_state_map::sm_state_map ()
+: m_map (), m_global_state (0)
+{
+}
+
 /* Clone the sm_state_map.  */
 
 sm_state_map *
@@ -80,13 +87,20 @@  sm_state_map::clone_with_remapping (const one_way_svalue_id_map &id_map) const
 void
 sm_state_map::print (const state_machine &sm, pretty_printer *pp) const
 {
+  bool first = true;
   pp_string (pp, "{");
+  if (m_global_state != 0)
+    {
+      pp_printf (pp, "global: %s", sm.get_state_name (m_global_state));
+      first = false;
+    }
   for (typename map_t::iterator iter = m_map.begin ();
        iter != m_map.end ();
        ++iter)
     {
-      if (iter != m_map.begin ())
+      if (!first)
 	pp_string (pp, ", ");
+      first = false;
       svalue_id sid = (*iter).first;
       sid.print (pp);
 
@@ -118,7 +132,7 @@  sm_state_map::dump (const state_machine &sm) const
 bool
 sm_state_map::is_empty_p () const
 {
-  return m_map.elements () == 0;
+  return m_map.elements () == 0 && m_global_state == 0;
 }
 
 /* Generate a hash value for this sm_state_map.  */
@@ -142,6 +156,7 @@  sm_state_map::hash () const
       inchash::add (e.m_origin, hstate);
       result ^= hstate.end ();
     }
+  result ^= m_global_state;
 
   return result;
 }
@@ -151,6 +166,9 @@  sm_state_map::hash () const
 bool
 sm_state_map::operator== (const sm_state_map &other) const
 {
+  if (m_global_state != other.m_global_state)
+    return false;
+
   if (m_map.elements () != other.m_map.elements ())
     return false;
 
@@ -261,6 +279,22 @@  sm_state_map::impl_set_state (svalue_id sid, state_machine::state_t state,
   m_map.put (sid, entry_t (state, origin));
 }
 
+/* Set the "global" state within this state map to STATE.  */
+
+void
+sm_state_map::set_global_state (state_machine::state_t state)
+{
+  m_global_state = state;
+}
+
+/* Get the "global" state within this state map.  */
+
+state_machine::state_t
+sm_state_map::get_global_state () const
+{
+  return m_global_state;
+}
+
 /* Handle CALL to unknown FNDECL with an unknown function body, which
    could do anything to the states passed to it.
    Clear any state for SM for the params and any LHS.
diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
index 9c87c1505c71..6e57257df74d 100644
--- a/gcc/analyzer/program-state.h
+++ b/gcc/analyzer/program-state.h
@@ -136,6 +136,8 @@  public:
   typedef hash_map <svalue_id, entry_t> map_t;
   typedef typename map_t::iterator iterator_t;
 
+  sm_state_map ();
+
   sm_state_map *clone () const;
 
   sm_state_map *
@@ -168,6 +170,9 @@  public:
 		       state_machine::state_t state,
 		       svalue_id origin);
 
+  void set_global_state (state_machine::state_t state);
+  state_machine::state_t get_global_state () const;
+
   void purge_for_unknown_fncall (const exploded_graph &eg,
 				 const state_machine &sm,
 				 const gcall *call, tree fndecl,
@@ -194,6 +199,7 @@  public:
 
 private:
   map_t m_map;
+  state_machine::state_t m_global_state;
 };
 
 /* A class for representing the state of interest at a given path of
@@ -284,6 +290,11 @@  class state_change_visitor
 public:
   virtual ~state_change_visitor () {}
 
+  /* Return true for early exit, false to keep iterating.  */
+  virtual bool on_global_state_change (const state_machine &sm,
+				       state_machine::state_t src_sm_val,
+				       state_machine::state_t dst_sm_val) = 0;
+
   /* Return true for early exit, false to keep iterating.  */
   virtual bool on_state_change (const state_machine &sm,
 				state_machine::state_t src_sm_val,
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index fe67c6b4b979..5939a69d1ac9 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -104,6 +104,22 @@  start_start_p (state_machine::state_t state)
 
 ////////////////////////////////////////////////////////////////////////////
 
+/* Abstract base class for state machines to pass to
+   sm_context::on_custom_transition for handling non-standard transitions
+   (e.g. adding a node and edge to simulate registering a callback and having
+   the callback be called later).  */
+
+class custom_transition
+{
+public:
+  virtual ~custom_transition () {}
+  virtual void impl_transition (exploded_graph *eg,
+				exploded_node *src_enode,
+				int sm_idx) = 0;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
 /* Abstract base class giving an interface for the state machine to call
    the checker engine, at a particular stmt.  */
 
@@ -140,6 +156,13 @@  public:
     return expr;
   }
 
+  virtual state_machine::state_t get_global_state () const = 0;
+  virtual void set_global_state (state_machine::state_t) = 0;
+
+  /* A vfunc for handling custom transitions, such as when registering
+     a signal handler.  */
+  virtual void on_custom_transition (custom_transition *transition) = 0;
+
 protected:
   sm_context (int sm_idx, const state_machine &sm)
   : m_sm_idx (sm_idx), m_sm (sm) {}