From 13265da93cd9fe173a7cad822ecb2a8ab17b04ab Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 11 Jul 2019 11:49:18 -0400
Subject: [PATCH 073/108] FIXME: more on canonicalization, enabling for now

---
 gcc/analyzer/impl-4.cc       | 71 +++++++++++++++++++++++++++++++++-----------
 gcc/analyzer/region-model.cc | 65 ++++++++++++++++++++++++++++------------
 gcc/analyzer/region-model.h  |  2 ++
 3 files changed, 101 insertions(+), 37 deletions(-)

diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index 147fbdf..98e7be2 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -2873,19 +2873,30 @@ class impl_region_model_context : public region_model_context, public log_user
 			     program_state *new_state,
 
 			     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 (enode),
     m_new_state (new_state),
-    m_stmt_finder (stmt_finder)
+    m_stmt_finder (stmt_finder),
+    m_ext_state (eg.get_ext_state ())
+  {
+  }
+
+  impl_region_model_context (program_state *state,
+			     const extrinsic_state &ext_state)
+  : log_user (NULL), m_eg (NULL), m_enode (NULL),
+    m_new_state (state),
+    m_stmt_finder (NULL),
+    m_ext_state (ext_state)
   {
   }
 
   void warn (pending_diagnostic *d) FINAL OVERRIDE
   {
     JIT_LOG_FUNC (get_logger ());
-    m_eg.get_diagnostic_manager ().add_diagnostic
-      (m_enode, m_enode->get_supernode (),
-       m_enode->get_stmt (),
-       m_stmt_finder, d);
+    if (m_eg)
+      m_eg->get_diagnostic_manager ().add_diagnostic
+	(m_enode, m_enode->get_supernode (),
+	 m_enode->get_stmt (),
+	 m_stmt_finder, d);
   }
 
   void remap_svalue_ids (const svalue_id_map &map) FINAL OVERRIDE
@@ -2902,7 +2913,7 @@ class impl_region_model_context : public region_model_context, public log_user
     sm_state_map *smap;
     FOR_EACH_VEC_ELT (m_new_state->m_checker_states, i, smap)
       {
-	const state_machine &sm = m_eg.get_ext_state ().get_sm (i);
+	const state_machine &sm = m_ext_state.get_sm (i);
 	if (!sm.can_purge_p (smap->get_state (sid)))
 	  return false;
       }
@@ -2917,7 +2928,7 @@ class impl_region_model_context : public region_model_context, public log_user
     sm_state_map *smap;
     FOR_EACH_VEC_ELT (m_new_state->m_checker_states, sm_idx, smap)
       {
-	const state_machine &sm = m_eg.get_ext_state ().get_sm (sm_idx);
+	const state_machine &sm = m_ext_state.get_sm (sm_idx);
 	total += smap->on_svalue_purge (sm, sm_idx, first_unused_sid,
 					map, this);
       }
@@ -2938,10 +2949,11 @@ class impl_region_model_context : public region_model_context, public log_user
   void on_inherited_svalue (svalue_id parent_sid,
 			    svalue_id child_sid) FINAL OVERRIDE;
 
-  exploded_graph &m_eg;
+  exploded_graph *m_eg;
   const exploded_node *m_enode;
   program_state *m_new_state;
   impl_4::stmt_finder *m_stmt_finder;
+  const extrinsic_state &m_ext_state;
 };
 
 ////////////////////////////////////////////////////////////////////////////
@@ -3219,6 +3231,9 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
 					  const svalue_id_map &map,
 					  state_machine::state_t state)
 {
+  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);
@@ -3250,11 +3265,11 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
     return;
 
   tree leaked_tree = leaked_trees[0];
-  if (m_eg.get_logger ())
-    m_eg.log ("best leaked_tree: %qE", leaked_tree);
+  if (get_logger ())
+    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,
+  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],
 			   m_new_state->m_checker_states[sm_idx],
 			   &stmt_finder);
@@ -3277,7 +3292,7 @@ impl_region_model_context::on_inherited_svalue (svalue_id parent_sid,
   sm_state_map *smap;
   FOR_EACH_VEC_ELT (m_new_state->m_checker_states, sm_idx, smap)
     {
-      const state_machine &sm = m_eg.get_ext_state ().get_sm (sm_idx);
+      const state_machine &sm = m_ext_state.get_sm (sm_idx);
       if (sm.inherited_state_p ())
 	smap->on_inherited_svalue (parent_sid, child_sid);
     }
@@ -7525,19 +7540,39 @@ static void
 test_program_state_merging ()
 {
   // TODO
+  tree p = build_global_decl ("p", ptr_type_node);
+
   using namespace impl_4;
   auto_delete_vec <state_machine> checkers;
   checkers.safe_push (make_malloc_state_machine (NULL));
   extrinsic_state ext_state (checkers);
+
   program_state s0 (ext_state);
+  impl_region_model_context ctxt (&s0, ext_state);
+
+  region_model *model0 = s0.m_region_model;
+  region_id new_rid = model0->add_new_malloc_region ();
+  svalue_id ptr_sid
+      = model0->get_or_create_ptr_svalue (ptr_type_node, new_rid);
+  model0->set_value (model0->get_lvalue (p, &ctxt),
+		     ptr_sid);
+  sm_state_map *smap = s0.m_checker_states[0];
+  const state_machine::state_t TEST_STATE = 3;
+  smap->impl_set_state (ptr_sid, TEST_STATE, svalue_id::null ());
+  model0->canonicalize (&ctxt);
+
   program_state s1 (s0);
   ASSERT_EQ (s0, s1);
 
-  /* Two identical states with x pointing to a heap region
-     with sm-states.
+  /* Two identical states with p pointing to a heap region
+     with the given sm-state.
      They ought to be mergeable, preserving the sm-state.  */
-
-  // TODO
+  program_state merged (ext_state);
+  ASSERT_TRUE (s0.can_merge_with_p (s1, ext_state, &merged));
+#if 0
+  // TODO: why is this failing?
+  ASSERT_EQ (s0, merged);
+#endif
 }
 
 /* FIXME.  */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 7df1d7e..779a436 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -714,12 +714,8 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
 	   has a merged region, and, if so, is it the same one.  */
 	// This check is above
 
-	root_region *merged_root = merger->m_merged_model->get_root_region ();
-	region_id merged_heap_rid
-	  = merged_root->ensure_heap_region (merger->m_merged_model);
-	region *new_region = new symbolic_region (merged_heap_rid);
 	region_id merged_pointee_rid
-	  = merger->m_merged_model->add_region (new_region);
+	  = merger->m_merged_model->add_new_malloc_region ();
 	// TODO: merge values within the heap regions
 	*merged_sid
 	  = merger->m_merged_model->get_or_create_ptr_svalue
@@ -2598,8 +2594,10 @@ region_model::validate () const
 void
 region_model::canonicalize (region_model_context *ctxt)
 {
+#if 0
   // FIXME: disabled for now
   return;
+#endif
 
   /* First pass: walk all regions in a deterministic order, generating
      a rid and sid map.  */
@@ -2844,10 +2842,7 @@ region_model::on_call (const gcall *call, region_model_context *ctxt)
       // FIXME: do we want a new subclass for malloc regions?
       // FIXME: possibly capturing size as a svalue?
 
-      region_id heap_rid
-	= get_root_region ()->ensure_heap_region (this);
-      region_id new_rid = add_region (new symbolic_region (heap_rid));
-
+      region_id new_rid = add_new_malloc_region ();
       if (!lhs_rid.null_p ())
 	{
 	  svalue_id ptr_sid
@@ -3531,6 +3526,16 @@ region_model::maybe_get_constant (svalue_id sid)
 
 /* FIXME.  */
 
+region_id
+region_model::add_new_malloc_region ()
+{
+  region_id heap_rid
+    = get_root_region ()->ensure_heap_region (this);
+  return add_region (new symbolic_region (heap_rid));
+}
+
+/* FIXME.  */
+
 tree
 region_model::get_representative_tree (svalue_id sid) const
 {
@@ -5084,7 +5089,7 @@ assert_region_models_merge (tree expr, tree val_a, tree val_b,
 /* FIXME.  */
 
 static void
-test_canonicalization ()
+test_canonicalization_1 ()
 {
   region_model model0;
   model0.get_root_region ()->ensure_stack_region (&model0);
@@ -5096,9 +5101,34 @@ test_canonicalization ()
 
   model0.canonicalize (NULL);
   model1.canonicalize (NULL);
-#if 0
   ASSERT_EQ (model0, model1);
-#endif
+}
+
+/* FIXME.  */
+
+static void
+test_canonicalization_2 ()
+{
+  tree int_42 = build_int_cst (integer_type_node, 42);
+  tree int_113 = build_int_cst (integer_type_node, 113);
+  tree x = build_global_decl ("x", integer_type_node);
+  tree y = build_global_decl ("y", integer_type_node);
+
+  region_model model0;
+  model0.set_value (model0.get_lvalue (x, NULL),
+		    model0.get_rvalue (int_42, NULL));
+  model0.set_value (model0.get_lvalue (y, NULL),
+		    model0.get_rvalue (int_113, NULL));
+
+  region_model model1;
+  model1.set_value (model1.get_lvalue (y, NULL),
+		    model1.get_rvalue (int_113, NULL));
+  model1.set_value (model1.get_lvalue (x, NULL),
+		    model1.get_rvalue (int_42, NULL));
+
+  model0.canonicalize (NULL);
+  model1.canonicalize (NULL);
+  ASSERT_EQ (model0, model1);
 }
 
 struct region_model_merger_test
@@ -5303,11 +5333,7 @@ test_state_merging ()
   {
     test_region_model_context ctxt;
     region_model model0;
-    // FIXME: maybe introduce a helper function for this
-    // and share with malloc (and other selftests):
-    region_id heap_rid
-      = model0.get_root_region ()->ensure_heap_region (&model0);
-    region_id new_rid = model0.add_region (new symbolic_region (heap_rid));
+    region_id new_rid = model0.add_new_malloc_region ();
     svalue_id ptr_sid
       = model0.get_or_create_ptr_svalue (ptr_type_node, new_rid);
     model0.set_value (model0.get_lvalue (x, &ctxt),
@@ -5326,7 +5352,7 @@ test_state_merging ()
     /* The merged model ought to be identical.  */
     // FIXME: seems to be failing due to ordering;
     // time to implement canonicalization?
-#if 0
+#if 1
     ASSERT_EQ (model0, merged);
 #endif
   }
@@ -5380,7 +5406,8 @@ analyzer_region_model_cc_tests ()
   test_purging ();
 #endif
   // TODO: test coverage for get_representative_tree / get_trees_for_svalue
-  test_canonicalization ();
+  test_canonicalization_1 ();
+  test_canonicalization_2 ();
   test_state_merging ();
 }
 
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 46c16cb..2309542 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -901,6 +901,8 @@ class region_model
 
   tree maybe_get_constant (svalue_id sid);
 
+  region_id add_new_malloc_region ();
+
   tree get_representative_tree (svalue_id sid) const;
   tree get_representative_tree (region_id rid) const;
   void get_trees_for_svalue (svalue_id sid, vec<tree> *out) const;
-- 
1.8.5.3

