[committed 1/3] analyzer: use objects for state_machine::state_t

David Malcolm dmalcolm@redhat.com
Wed Sep 9 21:08:06 GMT 2020


This patch is preliminary work towards generalizing sm-malloc.cc so that
it can check APIs other than just malloc/free (and e.g. detect
mismatching alloc/dealloc pairs).

Generalize states in state machines so that, rather than state_t being
just an "unsigned", it becomes a "const state *", where the underlying
state objects are immutable objects managed by the state machine in
question, and can e.g. have vfuncs and extra fields.  The start state
m_start becomes a member of the state_machine base_class.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as 10fc42a8396072912e9d9d940fba25950b3fdfc5.

gcc/analyzer/ChangeLog:
	* checker-path.cc (state_change_event::get_desc): Update
	state_machine::get_state_name calls to state::get_name.
	(warning_event::get_desc): Likewise.
	* diagnostic-manager.cc
	(null_assignment_sm_context::on_transition): Update comparison
	against 0 with comparison with m_sm.get_start_state.
	(diagnostic_manager::prune_for_sm_diagnostic): Update
	state_machine::get_state_name calls to state::get_name.
	* engine.cc (impl_sm_context::on_transition): Likewise.
	(exploded_node::get_dot_fillcolor): Use get_id when summing
	the sm states.
	* program-state.cc (sm_state_map::sm_state_map): Don't hardcode
	0 as the start state when initializing m_global_state.
	(sm_state_map::print): Use dump_to_pp rather than get_state_name
	when dumping states.
	(sm_state_map::is_empty_p): Don't hardcode 0 as the start state
	when examining m_global_state.
	(sm_state_map::hash): Use get_id when hashing states.
	(selftest::test_sm_state_map): Use state objects rather than
	arbitrary hardcoded integers.
	(selftest::test_program_state_merging): Likewise.
	(selftest::test_program_state_merging_2): Likewise.
	* sm-file.cc (fileptr_state_machine::m_start): Move to base class.
	(file_diagnostic::describe_state_change): Use get_start_state.
	(fileptr_state_machine::fileptr_state_machine): Drop m_start
	initialization.
	* sm-malloc.cc (malloc_state_machine::m_start): Move to base
	class.
	(malloc_diagnostic::describe_state_change): Use get_start_state.
	(possible_null::describe_state_change): Likewise.
	(malloc_state_machine::malloc_state_machine): Drop m_start
	initialization.
	* sm-pattern-test.cc (pattern_test_state_machine::m_start): Move
	to base class.
	(pattern_test_state_machine::pattern_test_state_machine): Drop
	m_start initialization.
	* sm-sensitive.cc (sensitive_state_machine::m_start): Move to base
	class.
	(sensitive_state_machine::sensitive_state_machine): Drop m_start
	initialization.
	* sm-signal.cc (signal_state_machine::m_start): Move to base
	class.
	(signal_state_machine::signal_state_machine): Drop m_start
	initialization.
	* sm-taint.cc (taint_state_machine::m_start): Move to base class.
	(taint_state_machine::taint_state_machine): Drop m_start
	initialization.
	* sm.cc (state_machine::state::dump_to_pp): New.
	(state_machine::state_machine): Move here from sm.h.  Initialize
	m_next_state_id and m_start.
	(state_machine::add_state): Reimplement in terms of state objects.
	(state_machine::get_state_name): Delete.
	(state_machine::get_state_by_name): Reimplement in terms of state
	objects.  Make const.
	(state_machine::validate): Delete.
	(state_machine::dump_to_pp): Reimplement in terms of state
	objects.
	* sm.h (state_machine::state): New class.
	(state_machine::state_t): Convert typedef from "unsigned" to
	"const state_machine::state *".
	(state_machine::state_machine): Move to sm.cc.
	(state_machine::get_default_state): Use m_start rather than
	hardcoding 0.
	(state_machine::get_state_name): Delete.
	(state_machine::get_state_by_name): Make const.
	(state_machine::get_start_state): New accessor.
	(state_machine::alloc_state_id): New.
	(state_machine::m_state_names): Drop in favor of...
	(state_machine::m_states): New field
	(state_machine::m_start): New field
	(start_start_p): Delete.
---
 gcc/analyzer/checker-path.cc       | 25 +++++------
 gcc/analyzer/diagnostic-manager.cc | 10 ++---
 gcc/analyzer/engine.cc             |  8 ++--
 gcc/analyzer/program-state.cc      | 72 ++++++++++++++++++------------
 gcc/analyzer/sm-file.cc            |  6 +--
 gcc/analyzer/sm-malloc.cc          |  8 +---
 gcc/analyzer/sm-pattern-test.cc    |  4 --
 gcc/analyzer/sm-sensitive.cc       |  4 --
 gcc/analyzer/sm-signal.cc          |  4 --
 gcc/analyzer/sm-taint.cc           |  4 --
 gcc/analyzer/sm.cc                 | 62 ++++++++++++++-----------
 gcc/analyzer/sm.h                  | 48 +++++++++++++-------
 12 files changed, 136 insertions(+), 119 deletions(-)

diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 5f2b8346485..2503d024a83 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -265,8 +265,8 @@ state_change_event::get_desc (bool can_colorize) const
 		   "%s (state of %qE: %qs -> %qs, origin: %qE)",
 		   custom_desc.m_buffer,
 		   var,
-		   m_sm.get_state_name (m_from),
-		   m_sm.get_state_name (m_to),
+		   m_from->get_name (),
+		   m_to->get_name (),
 		   origin);
 	      else
 		result = make_label_text
@@ -274,8 +274,8 @@ state_change_event::get_desc (bool can_colorize) const
 		   "%s (state of %qE: %qs -> %qs, NULL origin)",
 		   custom_desc.m_buffer,
 		   var,
-		   m_sm.get_state_name (m_from),
-		   m_sm.get_state_name (m_to));
+		   m_from->get_name (),
+		   m_to->get_name ());
 	      custom_desc.maybe_free ();
 	      return result;
 	    }
@@ -295,8 +295,8 @@ state_change_event::get_desc (bool can_colorize) const
 	    (can_colorize,
 	     "state of %qs: %qs -> %qs (origin: %qs)",
 	     sval_desc.m_buffer,
-	     m_sm.get_state_name (m_from),
-	     m_sm.get_state_name (m_to),
+	     m_from->get_name (),
+	     m_to->get_name (),
 	     origin_desc.m_buffer);
 	}
       else
@@ -304,8 +304,8 @@ state_change_event::get_desc (bool can_colorize) const
 	  (can_colorize,
 	   "state of %qs: %qs -> %qs (NULL origin)",
 	   sval_desc.m_buffer,
-	   m_sm.get_state_name (m_from),
-	   m_sm.get_state_name (m_to));
+	   m_from->get_name (),
+	   m_to->get_name ());
     }
   else
     {
@@ -313,8 +313,8 @@ state_change_event::get_desc (bool can_colorize) const
       return make_label_text
 	(can_colorize,
 	 "global state: %qs -> %qs",
-	 m_sm.get_state_name (m_from),
-	 m_sm.get_state_name (m_to));
+	 m_from->get_name (),
+	 m_to->get_name ());
     }
 }
 
@@ -876,7 +876,7 @@ warning_event::get_desc (bool can_colorize) const
 		= make_label_text (can_colorize,
 				   "%s (%qE is in state %qs)",
 				   ev_desc.m_buffer,
-				   m_var,m_sm->get_state_name (m_state));
+				   m_var, m_state->get_name ());
 	      ev_desc.maybe_free ();
 	      return result;
 	    }
@@ -888,8 +888,7 @@ warning_event::get_desc (bool can_colorize) const
   if (m_sm)
     return make_label_text (can_colorize,
 			    "here (%qE is in state %qs)",
-			    m_var,
-			    m_sm->get_state_name (m_state));
+			    m_var, m_state->get_name ());
   else
     return label_text::borrow ("here");
 }
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index c38395a4b0d..04c7d2ac4d3 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -775,7 +775,7 @@ struct null_assignment_sm_context : public sm_context
 		      state_machine::state_t to,
 		      tree origin ATTRIBUTE_UNUSED) FINAL OVERRIDE
   {
-    if (from != 0)
+    if (from != m_sm.get_start_state ())
       return;
 
     const svalue *var_new_sval
@@ -1207,12 +1207,12 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 		  label_text sval_desc = sval->get_desc ();
 		  log ("considering event %i (%s), with sval: %qs, state: %qs",
 		       idx, event_kind_to_string (base_event->m_kind),
-		       sval_desc.m_buffer, sm->get_state_name (state));
+		       sval_desc.m_buffer, state->get_name ());
 		}
 	      else
 		log ("considering event %i (%s), with global state: %qs",
 		     idx, event_kind_to_string (base_event->m_kind),
-		     sm->get_state_name (state));
+		     state->get_name ());
 	    }
 	  else
 	    log ("considering event %i", idx);
@@ -1275,8 +1275,8 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 		    sval = state_change->m_origin;
 		  }
 		log ("event %i: switching state of interest from %qs to %qs",
-		     idx, sm->get_state_name (state_change->m_to),
-		     sm->get_state_name (state_change->m_from));
+		     idx, state_change->m_to->get_name (),
+		     state_change->m_from->get_name ());
 		state = state_change->m_from;
 	      }
 	    else if (m_verbosity < 4)
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index fdfbb0b9568..05121e34b37 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -236,8 +236,8 @@ public:
 	  logger->log ("%s: state transition of %qE: %s -> %s",
 		       m_sm.get_name (),
 		       var,
-		       m_sm.get_state_name (from),
-		       m_sm.get_state_name (to));
+		       from->get_name (),
+		       to->get_name ());
 	m_new_smap->set_state (m_new_state->m_region_model, var_new_sval,
 			       to, origin_new_sval, m_eg.get_ext_state ());
       }
@@ -815,8 +815,8 @@ exploded_node::get_dot_fillcolor () const
       for (sm_state_map::iterator_t iter = smap->begin ();
 	   iter != smap->end ();
 	   ++iter)
-	total_sm_state += (*iter).second.m_state;
-      total_sm_state += smap->get_global_state ();
+	total_sm_state += (*iter).second.m_state->get_id ();
+      total_sm_state += smap->get_global_state ()->get_id ();
     }
 
   if (total_sm_state > 0)
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index ede20a70cca..71bb2864d6d 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -115,7 +115,7 @@ extrinsic_state::get_model_manager () const
 /* sm_state_map's ctor.  */
 
 sm_state_map::sm_state_map (const state_machine &sm, int sm_idx)
-: m_sm (sm), m_sm_idx (sm_idx), m_map (), m_global_state (0)
+: m_sm (sm), m_sm_idx (sm_idx), m_map (), m_global_state (sm.get_start_state ())
 {
 }
 
@@ -143,7 +143,8 @@ sm_state_map::print (const region_model *model,
     {
       if (multiline)
 	pp_string (pp, "  ");
-      pp_printf (pp, "global: %s", m_sm.get_state_name (m_global_state));
+      pp_string (pp, "global: ");
+      m_global_state->dump_to_pp (pp);
       if (multiline)
 	pp_newline (pp);
       first = false;
@@ -163,7 +164,8 @@ sm_state_map::print (const region_model *model,
       sval->dump_to_pp (pp, simple);
 
       entry_t e = (*iter).second;
-      pp_printf (pp, ": %s", m_sm.get_state_name (e.m_state));
+      pp_string (pp, ": ");
+      e.m_state->dump_to_pp (pp);
       if (model)
 	if (tree rep = model->get_representative_tree (sval))
 	  {
@@ -212,7 +214,7 @@ sm_state_map::dump (bool simple) const
 bool
 sm_state_map::is_empty_p () const
 {
-  return m_map.elements () == 0 && m_global_state == 0;
+  return m_map.elements () == 0 && m_global_state == m_sm.get_start_state ();
 }
 
 /* Generate a hash value for this sm_state_map.  */
@@ -232,11 +234,11 @@ sm_state_map::hash () const
       inchash::hash hstate;
       hstate.add_ptr ((*iter).first);
       entry_t e = (*iter).second;
-      hstate.add_int (e.m_state);
+      hstate.add_int (e.m_state->get_id ());
       hstate.add_ptr (e.m_origin);
       result ^= hstate.end ();
     }
-  result ^= m_global_state;
+  result ^= m_global_state->get_id ();
 
   return result;
 }
@@ -1054,9 +1056,12 @@ test_sm_state_map ()
   auto_delete_vec <state_machine> checkers;
   checkers.safe_push (sm);
   extrinsic_state ext_state (checkers);
+  state_machine::state_t start = sm->get_start_state ();
 
   /* Test setting states on svalue_id instances directly.  */
   {
+    const state_machine::state test_state_42 ("test state 42", 42);
+    const state_machine::state_t TEST_STATE_42 = &test_state_42;
     region_model_manager mgr;
     region_model model (&mgr);
     const svalue *x_sval = model.get_rvalue (x, NULL);
@@ -1065,22 +1070,25 @@ test_sm_state_map ()
 
     sm_state_map map (*sm, 0);
     ASSERT_TRUE (map.is_empty_p ());
-    ASSERT_EQ (map.get_state (x_sval, ext_state), 0);
+    ASSERT_EQ (map.get_state (x_sval, ext_state), start);
 
-    map.impl_set_state (x_sval, 42, z_sval, ext_state);
-    ASSERT_EQ (map.get_state (x_sval, ext_state), 42);
+    map.impl_set_state (x_sval, TEST_STATE_42, z_sval, ext_state);
+    ASSERT_EQ (map.get_state (x_sval, ext_state), TEST_STATE_42);
     ASSERT_EQ (map.get_origin (x_sval, ext_state), z_sval);
-    ASSERT_EQ (map.get_state (y_sval, ext_state), 0);
+    ASSERT_EQ (map.get_state (y_sval, ext_state), start);
     ASSERT_FALSE (map.is_empty_p ());
 
     map.impl_set_state (y_sval, 0, z_sval, ext_state);
-    ASSERT_EQ (map.get_state (y_sval, ext_state), 0);
+    ASSERT_EQ (map.get_state (y_sval, ext_state), start);
 
     map.impl_set_state (x_sval, 0, z_sval, ext_state);
-    ASSERT_EQ (map.get_state (x_sval, ext_state), 0);
+    ASSERT_EQ (map.get_state (x_sval, ext_state), start);
     ASSERT_TRUE (map.is_empty_p ());
   }
 
+  const state_machine::state test_state_5 ("test state 5", 5);
+  const state_machine::state_t TEST_STATE_5 = &test_state_5;
+
   /* Test setting states via equivalence classes.  */
   {
     region_model_manager mgr;
@@ -1091,16 +1099,16 @@ test_sm_state_map ()
 
     sm_state_map map (*sm, 0);
     ASSERT_TRUE (map.is_empty_p ());
-    ASSERT_EQ (map.get_state (x_sval, ext_state), 0);
-    ASSERT_EQ (map.get_state (y_sval, ext_state), 0);
+    ASSERT_EQ (map.get_state (x_sval, ext_state), start);
+    ASSERT_EQ (map.get_state (y_sval, ext_state), start);
 
     model.add_constraint (x, EQ_EXPR, y, NULL);
 
     /* Setting x to a state should also update y, as they
        are in the same equivalence class.  */
-    map.set_state (&model, x_sval, 5, z_sval, ext_state);
-    ASSERT_EQ (map.get_state (x_sval, ext_state), 5);
-    ASSERT_EQ (map.get_state (y_sval, ext_state), 5);
+    map.set_state (&model, x_sval, TEST_STATE_5, z_sval, ext_state);
+    ASSERT_EQ (map.get_state (x_sval, ext_state), TEST_STATE_5);
+    ASSERT_EQ (map.get_state (y_sval, ext_state), TEST_STATE_5);
     ASSERT_EQ (map.get_origin (x_sval, ext_state), z_sval);
     ASSERT_EQ (map.get_origin (y_sval, ext_state), z_sval);
   }
@@ -1119,18 +1127,22 @@ test_sm_state_map ()
     ASSERT_EQ (map0.hash (), map1.hash ());
     ASSERT_EQ (map0, map1);
 
-    map1.impl_set_state (y_sval, 5, z_sval, ext_state);
+    map1.impl_set_state (y_sval, TEST_STATE_5, z_sval, ext_state);
     ASSERT_NE (map0.hash (), map1.hash ());
     ASSERT_NE (map0, map1);
 
     /* Make the same change to map2.  */
-    map2.impl_set_state (y_sval, 5, z_sval, ext_state);
+    map2.impl_set_state (y_sval, TEST_STATE_5, z_sval, ext_state);
     ASSERT_EQ (map1.hash (), map2.hash ());
     ASSERT_EQ (map1, map2);
   }
 
   /* Equality and hashing shouldn't depend on ordering.  */
   {
+    const state_machine::state test_state_2 ("test state 2", 2);
+    const state_machine::state_t TEST_STATE_2 = &test_state_2;
+    const state_machine::state test_state_3 ("test state 3", 3);
+    const state_machine::state_t TEST_STATE_3 = &test_state_3;
     sm_state_map map0 (*sm, 0);
     sm_state_map map1 (*sm, 0);
     sm_state_map map2 (*sm, 0);
@@ -1144,13 +1156,13 @@ test_sm_state_map ()
     const svalue *y_sval = model.get_rvalue (y, NULL);
     const svalue *z_sval = model.get_rvalue (z, NULL);
 
-    map1.impl_set_state (x_sval, 2, NULL, ext_state);
-    map1.impl_set_state (y_sval, 3, NULL, ext_state);
-    map1.impl_set_state (z_sval, 2, NULL, ext_state);
+    map1.impl_set_state (x_sval, TEST_STATE_2, NULL, ext_state);
+    map1.impl_set_state (y_sval, TEST_STATE_3, NULL, ext_state);
+    map1.impl_set_state (z_sval, TEST_STATE_2, NULL, ext_state);
 
-    map2.impl_set_state (z_sval, 2, NULL, ext_state);
-    map2.impl_set_state (y_sval, 3, NULL, ext_state);
-    map2.impl_set_state (x_sval, 2, NULL, ext_state);
+    map2.impl_set_state (z_sval, TEST_STATE_2, NULL, ext_state);
+    map2.impl_set_state (y_sval, TEST_STATE_3, NULL, ext_state);
+    map2.impl_set_state (x_sval, TEST_STATE_2, NULL, ext_state);
 
     ASSERT_EQ (map1.hash (), map2.hash ());
     ASSERT_EQ (map1, map2);
@@ -1241,7 +1253,8 @@ test_program_state_merging ()
   model0->set_value (model0->get_lvalue (p, &ctxt),
 		     ptr_sval, &ctxt);
   sm_state_map *smap = s0.m_checker_states[0];
-  const state_machine::state_t TEST_STATE = 3;
+  const state_machine::state test_state ("test state", 0);
+  const state_machine::state_t TEST_STATE = &test_state;
   smap->impl_set_state (ptr_sval, TEST_STATE, NULL, ext_state);
   ASSERT_EQ (smap->get_state (ptr_sval, ext_state), TEST_STATE);
 
@@ -1293,10 +1306,14 @@ test_program_state_merging_2 ()
   checkers.safe_push (make_signal_state_machine (NULL));
   extrinsic_state ext_state (checkers);
 
+  const state_machine::state test_state_0 ("test state 0", 0);
+  const state_machine::state test_state_1 ("test state 1", 1);
+  const state_machine::state_t TEST_STATE_0 = &test_state_0;
+  const state_machine::state_t TEST_STATE_1 = &test_state_1;
+
   program_state s0 (ext_state);
   {
     sm_state_map *smap0 = s0.m_checker_states[0];
-    const state_machine::state_t TEST_STATE_0 = 0;
     smap0->set_global_state (TEST_STATE_0);
     ASSERT_EQ (smap0->get_global_state (), TEST_STATE_0);
   }
@@ -1304,7 +1321,6 @@ test_program_state_merging_2 ()
   program_state s1 (ext_state);
   {
     sm_state_map *smap1 = s1.m_checker_states[0];
-    const state_machine::state_t TEST_STATE_1 = 1;
     smap1->set_global_state (TEST_STATE_1);
     ASSERT_EQ (smap1->get_global_state (), TEST_STATE_1);
   }
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index eaec176d7bb..33b445195d5 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -83,9 +83,6 @@ public:
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
   pending_diagnostic *on_leak (tree var) const FINAL OVERRIDE;
 
-  /* Start state.  */
-  state_t m_start;
-
   /* State for a FILE * returned from fopen that hasn't been checked for
      NULL.
      It could be an open stream, or could be NULL.  */
@@ -121,7 +118,7 @@ public:
   label_text describe_state_change (const evdesc::state_change &change)
     OVERRIDE
   {
-    if (change.m_old_state == m_sm.m_start
+    if (change.m_old_state == m_sm.get_start_state ()
 	&& change.m_new_state == m_sm.m_unchecked)
       // TODO: verify that it's the fopen stmt, not a copy
       return label_text::borrow ("opened here");
@@ -229,7 +226,6 @@ private:
 fileptr_state_machine::fileptr_state_machine (logger *logger)
 : state_machine ("file", logger)
 {
-  m_start = add_state ("start");
   m_unchecked = add_state ("unchecked");
   m_null = add_state ("null");
   m_nonnull = add_state ("nonnull");
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index ba6d41cf8ee..19afff49519 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -98,9 +98,6 @@ public:
   bool reset_when_passed_to_unknown_fn_p (state_t s,
 					  bool is_mutable) const FINAL OVERRIDE;
 
-  /* Start state.  */
-  state_t m_start;
-
   /* State for a pointer returned from malloc that hasn't been checked for
      NULL.
      It could be a pointer to heap-allocated memory, or could be NULL.  */
@@ -147,7 +144,7 @@ public:
   label_text describe_state_change (const evdesc::state_change &change)
     OVERRIDE
   {
-    if (change.m_old_state == m_sm.m_start
+    if (change.m_old_state == m_sm.get_start_state ()
 	&& change.m_new_state == m_sm.m_unchecked)
       // TODO: verify that it's the allocation stmt, not a copy
       return label_text::borrow ("allocated here");
@@ -258,7 +255,7 @@ public:
   label_text describe_state_change (const evdesc::state_change &change)
     FINAL OVERRIDE
   {
-    if (change.m_old_state == m_sm.m_start
+    if (change.m_old_state == m_sm.get_start_state ()
 	&& change.m_new_state == m_sm.m_unchecked)
       {
 	m_origin_of_unchecked_event = change.m_event_id;
@@ -659,7 +656,6 @@ private:
 malloc_state_machine::malloc_state_machine (logger *logger)
 : state_machine ("malloc", logger)
 {
-  m_start = add_state ("start");
   m_unchecked = add_state ("unchecked");
   m_null = add_state ("null");
   m_nonnull = add_state ("nonnull");
diff --git a/gcc/analyzer/sm-pattern-test.cc b/gcc/analyzer/sm-pattern-test.cc
index cac78e7c041..6a59e8fff83 100644
--- a/gcc/analyzer/sm-pattern-test.cc
+++ b/gcc/analyzer/sm-pattern-test.cc
@@ -65,9 +65,6 @@ public:
 		     tree rhs) const FINAL OVERRIDE;
 
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
-
-private:
-  state_t m_start;
 };
 
 class pattern_match : public pending_diagnostic_subclass<pattern_match>
@@ -100,7 +97,6 @@ private:
 pattern_test_state_machine::pattern_test_state_machine (logger *logger)
 : state_machine ("pattern-test", logger)
 {
-  m_start = add_state ("start");
 }
 
 bool
diff --git a/gcc/analyzer/sm-sensitive.cc b/gcc/analyzer/sm-sensitive.cc
index b14bf147d0c..f10008307af 100644
--- a/gcc/analyzer/sm-sensitive.cc
+++ b/gcc/analyzer/sm-sensitive.cc
@@ -66,9 +66,6 @@ public:
 
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
-  /* Start state.  */
-  state_t m_start;
-
   /* State for "sensitive" data, such as a password.  */
   state_t m_sensitive;
 
@@ -163,7 +160,6 @@ private:
 sensitive_state_machine::sensitive_state_machine (logger *logger)
 : state_machine ("sensitive", logger)
 {
-  m_start = add_state ("start");
   m_sensitive = add_state ("sensitive");
   m_stop = add_state ("stop");
 }
diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 85c4b433d7b..21c9d58f6de 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -91,9 +91,6 @@ public:
 
   /* These states are "global", rather than per-expression.  */
 
-  /* Start state.  */
-  state_t m_start;
-
   /* State for when we're in a signal handler.  */
   state_t m_in_signal_handler;
 
@@ -196,7 +193,6 @@ private:
 signal_state_machine::signal_state_machine (logger *logger)
 : state_machine ("signal", logger)
 {
-  m_start = add_state ("start");
   m_in_signal_handler = add_state ("in_signal_handler");
   m_stop = add_state ("stop");
 }
diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index 28d196505d2..385909ce26c 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -66,9 +66,6 @@ public:
 
   bool can_purge_p (state_t s) const FINAL OVERRIDE;
 
-  /* Start state.  */
-  state_t m_start;
-
   /* State for a "tainted" value: unsanitized data potentially under an
      attacker's control.  */
   state_t m_tainted;
@@ -188,7 +185,6 @@ private:
 taint_state_machine::taint_state_machine (logger *logger)
 : state_machine ("taint", logger)
 {
-  m_start = add_state ("start");
   m_tainted = add_state ("tainted");
   m_has_lb = add_state ("has_lb");
   m_has_ub = add_state ("has_ub");
diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc
index 3504b2e3cbb..a333063c65e 100644
--- a/gcc/analyzer/sm.cc
+++ b/gcc/analyzer/sm.cc
@@ -45,6 +45,27 @@ any_pointer_p (tree var)
   return POINTER_TYPE_P (TREE_TYPE (var));
 }
 
+
+/* class state_machine::state.  */
+
+/* Base implementation of dump_to_pp vfunc.  */
+
+void
+state_machine::state::dump_to_pp (pretty_printer *pp) const
+{
+  pp_string (pp, m_name);
+}
+
+/* class state_machine.  */
+
+/* state_machine's ctor.  */
+
+state_machine::state_machine (const char *name, logger *logger)
+: log_user (logger), m_name (name), m_next_state_id (0),
+  m_start (add_state ("start"))
+{
+}
+
 /* Add a state with name NAME to this state_machine.
    The string is required to outlive the state_machine.
 
@@ -53,50 +74,39 @@ any_pointer_p (tree var)
 state_machine::state_t
 state_machine::add_state (const char *name)
 {
-  m_state_names.safe_push (name);
-  return m_state_names.length () - 1;
-}
-
-/* Get the name of state S within this state_machine.  */
-
-const char *
-state_machine::get_state_name (state_t s) const
-{
-  return m_state_names[s];
+  state *s = new state (name, alloc_state_id ());
+  m_states.safe_push (s);
+  return s;
 }
 
 /* Get the state with name NAME, which must exist.
    This is purely intended for use in selftests.  */
 
 state_machine::state_t
-state_machine::get_state_by_name (const char *name)
+state_machine::get_state_by_name (const char *name) const
 {
   unsigned i;
-  const char *iter_name;
-  FOR_EACH_VEC_ELT (m_state_names, i, iter_name)
-    if (!strcmp (name, iter_name))
-      return i;
+  state *s;
+  FOR_EACH_VEC_ELT (m_states, i, s)
+    if (!strcmp (name, s->get_name ()))
+      return s;
   /* Name not found.  */
   gcc_unreachable ();
 }
 
-/* Assert that S is a valid state for this state_machine.  */
-
-void
-state_machine::validate (state_t s) const
-{
-  gcc_assert (s < m_state_names.length ());
-}
-
 /* Dump a multiline representation of this state machine to PP.  */
 
 void
 state_machine::dump_to_pp (pretty_printer *pp) const
 {
   unsigned i;
-  const char *name;
-  FOR_EACH_VEC_ELT (m_state_names, i, name)
-    pp_printf (pp, "  state %i: %qs\n", i, name);
+  state *s;
+  FOR_EACH_VEC_ELT (m_states, i, s)
+    {
+      pp_printf (pp, "  state %i: ", i);
+      s->dump_to_pp (pp);
+      pp_newline (pp);
+    }
 }
 
 /* Create instances of the various state machines, each using LOGGER,
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index fa65f527399..769d2a46767 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -32,17 +32,32 @@ class pending_diagnostic;
 extern bool any_pointer_p (tree var);
 
 /* An abstract base class for a state machine describing an API.
-   A mapping from state IDs to names, and various virtual functions
+   Manages a set of state objects, and has various virtual functions
    for pattern-matching on statements.  */
 
 class state_machine : public log_user
 {
 public:
-  typedef unsigned state_t;
+  /* States are represented by immutable objects, owned by the state
+     machine.  */
+  class state
+  {
+  public:
+    state (const char *name, unsigned id) : m_name (name), m_id (id) {}
+    virtual ~state () {}
+
+    const char *get_name () const { return m_name; }
+    virtual void dump_to_pp (pretty_printer *pp) const;
+
+    unsigned get_id () const { return m_id; }
 
-  state_machine (const char *name, logger *logger)
-  : log_user (logger), m_name (name) {}
+  private:
+    const char *m_name;
+    unsigned m_id;
+  };
+  typedef const state_machine::state *state_t;
 
+  state_machine (const char *name, logger *logger);
   virtual ~state_machine () {}
 
   /* Should states be inherited from a parent region to a child region,
@@ -54,14 +69,12 @@ public:
 
   virtual state_machine::state_t get_default_state (const svalue *) const
   {
-    return 0;
+    return m_start;
   }
 
   const char *get_name () const { return m_name; }
 
-  const char *get_state_name (state_t s) const;
-
-  state_t get_state_by_name (const char *name);
+  state_t get_state_by_name (const char *name) const;
 
   /* Return true if STMT is a function call recognized by this sm.  */
   virtual bool on_stmt (sm_context *sm_ctxt,
@@ -108,23 +121,26 @@ public:
 
   void dump_to_pp (pretty_printer *pp) const;
 
+  state_t get_start_state () const { return m_start; }
+
 protected:
   state_t add_state (const char *name);
+  unsigned alloc_state_id () { return m_next_state_id++; }
 
 private:
   DISABLE_COPY_AND_ASSIGN (state_machine);
 
   const char *m_name;
-  auto_vec<const char *> m_state_names;
-};
 
-/* Is STATE the start state?  (zero is hardcoded as the start state).  */
+  /* States are owned by the state_machine.  */
+  auto_delete_vec<state> m_states;
 
-static inline bool
-start_start_p (state_machine::state_t state)
-{
-  return state == 0;
-}
+  unsigned m_next_state_id;
+
+protected:
+  /* Must be inited after m_next_state_id.  */
+  state_t m_start;
+};
 
 /* Abstract base class for state machines to pass to
    sm_context::on_custom_transition for handling non-standard transitions
-- 
2.26.2



More information about the Gcc-patches mailing list