From 80877c49eb1376f6792b305ec983770b466e5fe1 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 1 Aug 2019 12:02:24 -0400
Subject: [PATCH 61/96] FIXME: make it clear enode is just for diagnostics;
 pass around old_state and stmt where they're needed

---
 gcc/analyzer/impl-4.cc | 97 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 37 deletions(-)

diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index 4831e33..36f325b 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -3092,14 +3092,20 @@ class impl_region_model_context : public region_model_context, public log_user
 {
  public:
   impl_region_model_context (exploded_graph &eg,
-			     const exploded_node *enode,
+			     const exploded_node *enode_for_diag,
+
 			     /* FIXME: should we be getting the ECs from the
 				old state, rather than the new?  */
+			     const program_state *old_state,
 			     program_state *new_state,
 
+			     const gimple *stmt,
 			     stmt_finder *stmt_finder = NULL)
-  : log_user (eg.get_logger ()), m_eg (&eg), m_enode (enode),
+  : log_user (eg.get_logger ()), m_eg (&eg),
+    m_enode_for_diag (enode_for_diag),
+    m_old_state (old_state),
     m_new_state (new_state),
+    m_stmt (stmt),
     m_stmt_finder (stmt_finder),
     m_ext_state (eg.get_ext_state ())
   {
@@ -3107,8 +3113,10 @@ class impl_region_model_context : public region_model_context, public log_user
 
   impl_region_model_context (program_state *state,
 			     const extrinsic_state &ext_state)
-  : log_user (NULL), m_eg (NULL), m_enode (NULL),
+  : log_user (NULL), m_eg (NULL), m_enode_for_diag (NULL),
+    m_old_state (NULL),
     m_new_state (state),
+    m_stmt (NULL),
     m_stmt_finder (NULL),
     m_ext_state (ext_state)
   {
@@ -3119,9 +3127,8 @@ class impl_region_model_context : public region_model_context, public log_user
     JIT_LOG_FUNC (get_logger ());
     if (m_eg)
       m_eg->get_diagnostic_manager ().add_diagnostic
-	(m_enode, m_enode->get_supernode (),
-	 m_enode->get_stmt (),
-	 m_stmt_finder, d);
+	(m_enode_for_diag, m_enode_for_diag->get_supernode (),
+	 m_stmt, m_stmt_finder, d);
   }
 
   void remap_svalue_ids (const svalue_id_map &map) FINAL OVERRIDE
@@ -3177,8 +3184,10 @@ class impl_region_model_context : public region_model_context, public log_user
   void on_condition (tree lhs, enum tree_code op, tree rhs) FINAL OVERRIDE;
 
   exploded_graph *m_eg;
-  const exploded_node *m_enode;
+  const exploded_node *m_enode_for_diag;
+  const program_state *m_old_state;
   program_state *m_new_state;
+  const gimple *m_stmt;
   impl_4::stmt_finder *m_stmt_finder;
   const extrinsic_state &m_ext_state;
 };
@@ -3192,10 +3201,11 @@ class impl_sm_context : public sm_context, public log_user
 public:
   impl_sm_context (exploded_graph &eg,
 		   const state_machine &sm,
-		   const exploded_node *enode,
+		   const exploded_node *enode_for_diag,
 
 		   /* FIXME: should we be getting the ECs from the
 		      old state, rather than the new?  */
+		   const program_state *old_state,
 		   program_state *new_state,
 
 		   const sm_state_map *old_smap,
@@ -3204,8 +3214,8 @@ public:
 		   stmt_finder *stmt_finder = NULL)
   : sm_context (sm),
     log_user (eg.get_logger ()),
-    m_eg (eg), m_enode (enode),
-    m_new_state (new_state),
+    m_eg (eg), m_enode_for_diag (enode_for_diag),
+    m_old_state (old_state), m_new_state (new_state),
     m_old_smap (old_smap), m_new_smap (new_smap),
     m_stmt_finder (stmt_finder)
   {
@@ -3220,11 +3230,12 @@ public:
   {
     JIT_LOG_FUNC (get_logger ());
     impl_region_model_context old_ctxt
-      (m_eg, m_enode, NULL/*m_enode->get_state ()*/, NULL);
+      (m_eg, m_enode_for_diag, NULL, NULL/*m_enode->get_state ()*/, NULL);
     svalue_id var_old_sid
-      = m_enode->get_state ().m_region_model->get_rvalue (var, &old_ctxt);
+      = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
 
-    impl_region_model_context new_ctxt (m_eg, m_enode, m_new_state, NULL);
+    impl_region_model_context new_ctxt (m_eg, m_enode_for_diag,
+					m_old_state, m_new_state, NULL);
     svalue_id var_new_sid
       = m_new_state->m_region_model->get_rvalue (var, &new_ctxt);
     svalue_id origin_new_sid
@@ -3252,14 +3263,14 @@ public:
     gcc_assert (d); // take ownership
 
     impl_region_model_context old_ctxt
-      (m_eg, m_enode, NULL/*m_enode->get_state ()*/, NULL);
+      (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL);
     svalue_id var_old_sid
-      = m_enode->get_state ().m_region_model->get_rvalue (var, &old_ctxt);
+      = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
     state_machine::state_t current = m_old_smap->get_state (var_old_sid);
     if (state == current)
       {
 	m_eg.get_diagnostic_manager ().add_diagnostic
-	  (&m_sm, m_enode, snode, stmt, m_stmt_finder,
+	  (&m_sm, m_enode_for_diag, snode, stmt, m_stmt_finder,
 	   var, state, d);
       }
     else
@@ -3295,7 +3306,8 @@ public:
   }
 
   exploded_graph &m_eg;
-  const exploded_node *m_enode;
+  const exploded_node *m_enode_for_diag;
+  const program_state *m_old_state;
   program_state *m_new_state;
   const sm_state_map *m_old_smap;
   sm_state_map *m_new_smap;
@@ -3463,9 +3475,9 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
   if (!m_eg)
     return;
 
-  /* FIXME: m_enode also needs to be non-NULL so warn_for_state can look up the
-     state of the sid.  */
-  gcc_assert (m_enode);
+  /* FIXME: m_old_state also needs to be non-NULL so warn_for_state can look
+     up the state of the sid.  */
+  gcc_assert (m_old_state);
 
   /* SID has leaked within the new state: no regions use it.
      We need to convert it back to a tree, but since no regions use it, we
@@ -3473,12 +3485,10 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
      We can then look that svalue_id up to locate regions and thus tree(s)
      that use it.  */
 
-  gcc_assert (m_enode);
-  const program_state &old_state = m_enode->get_state ();
   svalue_id old_sid = map.get_src_for_dst (sid);
 
   auto_vec<path_var> leaked_pvs;
-  old_state.m_region_model->get_path_vars_for_svalue (old_sid, &leaked_pvs);
+  m_old_state->m_region_model->get_path_vars_for_svalue (old_sid, &leaked_pvs);
 
   // FIXME: find "best" leaked tree
   // TODO: should we also consider (and consolidate) equiv classes?
@@ -3498,12 +3508,13 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
     log ("best leaked_tree: %qE", leaked_tree);
 
   leak_stmt_finder stmt_finder (*m_eg, leaked_tree);
-  impl_sm_context sm_ctxt (*m_eg, sm, m_enode, NULL,
-			   m_enode->get_state ().m_checker_states[sm_idx],
+  impl_sm_context sm_ctxt (*m_eg, sm, m_enode_for_diag,
+			   m_old_state, m_new_state,
+			   m_old_state->m_checker_states[sm_idx],
 			   m_new_state->m_checker_states[sm_idx],
 			   &stmt_finder);
 
-  sm.on_leak (&sm_ctxt, m_enode->get_supernode (), m_enode->get_stmt (),
+  sm.on_leak (&sm_ctxt, m_enode_for_diag->get_supernode (), m_stmt,
 	      leaked_tree, state);
 }
 
@@ -3539,11 +3550,12 @@ impl_region_model_context::on_condition (tree lhs, enum tree_code op, tree rhs)
   FOR_EACH_VEC_ELT (m_new_state->m_checker_states, sm_idx, smap)
     {
       const state_machine &sm = m_ext_state.get_sm (sm_idx);
-      impl_sm_context sm_ctxt (*m_eg, sm, m_enode, m_new_state,
-			       m_enode->get_state ().m_checker_states[sm_idx],
+      impl_sm_context sm_ctxt (*m_eg, sm, m_enode_for_diag,
+			       m_old_state, m_new_state,
+			       m_old_state->m_checker_states[sm_idx],
 			       m_new_state->m_checker_states[sm_idx]);
       sm.on_condition (&sm_ctxt,
-		       m_enode->get_supernode (), m_enode->get_stmt (),
+		       m_enode_for_diag->get_supernode (), m_stmt,
 		       lhs, op, rhs);
     }
 }
@@ -3769,7 +3781,10 @@ program_state::on_edge (exploded_graph &eg,
      sm-state transitions (e.g. transitions due to ptrs becoming known
      to be NULL or non-NULL) */
 
-  impl_region_model_context ctxt (eg, &enode, this, NULL);
+  impl_region_model_context ctxt (eg, &enode,
+				  &enode.get_state (),
+				  this,
+				  last_stmt);
   if (!m_region_model->maybe_update_for_edge (*succ,
 					      last_stmt,
 					      &ctxt))
@@ -3936,7 +3951,8 @@ program_state::prune_for_point (exploded_graph &eg,
 
   /* Purge unused svalues.  */
   // FIXME: which enode to use, if any?
-  impl_region_model_context ctxt (eg, NULL /* FIXME: old_state*/,
+  impl_region_model_context ctxt (eg, NULL,
+				  this,
 				  &new_state, NULL);
   new_state.m_region_model->purge_unused_svalues (&stats, &ctxt);
   eg.log ("num svalues purged: %i", stats.m_num_svalues);
@@ -4367,11 +4383,15 @@ exploded_node::on_stmt (exploded_graph &eg,
 			const gimple *stmt,
 			program_state *state) const
 {
-  /* FIXME: the only thing the old_state is used for is for looking
-     up old checker states, for determining state transitions.  */
+  /* Preserve the old state.  It is used here for looking
+     up old checker states, for determining state transitions, and
+     also within impl_region_model_context and impl_sm_context for
+     going from tree to svalue_id.  */
   const program_state old_state (*state);
 
-  impl_region_model_context ctxt (eg, this, state, NULL);
+  impl_region_model_context ctxt (eg, this,
+				  &old_state, state,
+				  stmt);
 
   if (const gassign *assign = dyn_cast <const gassign *> (stmt))
     state->m_region_model->on_assignment (assign, &ctxt);
@@ -4432,7 +4452,8 @@ exploded_node::on_stmt (exploded_graph &eg,
       const sm_state_map *old_smap
 	= old_state.m_checker_states[sm_idx];
       sm_state_map *new_smap = state->m_checker_states[sm_idx];
-      impl_sm_context ctxt (eg, sm, this, state, old_smap, new_smap);
+      impl_sm_context ctxt (eg, sm, this, &old_state, state,
+			    old_smap, new_smap);
       /* Allow the state_machine to handle the stmt.  */
       if (!sm.on_stmt (&ctxt, snode, stmt))
 	{
@@ -4517,7 +4538,8 @@ exploded_node::detect_leaks (exploded_graph &eg) const
   gcc_assert (new_state.m_region_model);
 
   purge_stats stats;
-  impl_region_model_context ctxt(eg, this, &new_state);
+  impl_region_model_context ctxt (eg, this,
+				  &old_state, &new_state, get_stmt ());
   new_state.m_region_model->pop_frame (&stats, &ctxt);
 }
 
@@ -6319,7 +6341,8 @@ exploded_graph::process_node (exploded_node *node)
 
 	if (point.m_from_edge)
 	  {
-	    impl_region_model_context ctxt (*this, node, &next_state, NULL);
+	    impl_region_model_context ctxt (*this, node,
+					    &state, &next_state, NULL);
 	    const cfg_superedge *last_cfg_superedge
 	      = point.m_from_edge->dyn_cast_cfg_superedge ();
 	    if (last_cfg_superedge)
-- 
1.8.5.3

