From 01b97e4e94c91796b36788af18f59df13ee8fdf4 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 27 Jun 2019 17:08:16 -0400
Subject: [PATCH 048/108] FIXME: WIP on state merging

---
 gcc/analyzer/impl-4.cc                  |  91 +++++++++++++--
 gcc/analyzer/region-model.cc            | 201 ++++++++++++++++++++++++++++++++
 gcc/analyzer/region-model.h             |  12 +-
 gcc/testsuite/gcc.dg/analyzer/paths-1.c |   4 +-
 gcc/testsuite/gcc.dg/analyzer/paths-2.c |  10 +-
 gcc/testsuite/gcc.dg/analyzer/zlib-1.c  |  15 ++-
 6 files changed, 308 insertions(+), 25 deletions(-)

diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index 7ad0516..2e9ec03 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -700,6 +700,10 @@ public:
 
   hashval_t hash () const;
   bool operator== (const program_state &other) const;
+  bool operator!= (const program_state &other) const
+  {
+    return !(*this == other);
+  }
 
   void print (const extrinsic_state &ext_state,
 	      pretty_printer *pp) const;
@@ -739,7 +743,9 @@ public:
     return true;
   }
 
-  bool can_merge_with_p (const program_state &other) const;
+  bool can_merge_with_p (const program_state &other,
+			 const extrinsic_state &ext_state,
+			 program_state *out) const;
 
   void validate (const extrinsic_state &ext_state) const;
 
@@ -2926,11 +2932,12 @@ program_state::operator= (const program_state &other)
   delete m_region_model;
   m_region_model = new region_model (*other.m_region_model);
 
-  // TODO: eliminate existing things
-  gcc_assert (m_checker_states.length () == 0);
-
   int i;
   sm_state_map *smap;
+  FOR_EACH_VEC_ELT (m_checker_states, i, smap)
+    delete smap;
+  m_checker_states.truncate (0);
+
   FOR_EACH_VEC_ELT (other.m_checker_states, i, smap)
     m_checker_states.safe_push (smap->clone ());
 
@@ -2944,6 +2951,9 @@ program_state::program_state (program_state &&other)
 {
   other.m_region_model = NULL;
 
+  // FIXME:
+  gcc_assert (m_checker_states.length () == 0);
+
   int i;
   sm_state_map *smap;
   FOR_EACH_VEC_ELT (other.m_checker_states, i, smap)
@@ -3246,10 +3256,23 @@ program_state::get_representative_tree (svalue_id sid) const
 /* FIXME.  */
 
 bool
-program_state::can_merge_with_p (const program_state &other) const
+program_state::can_merge_with_p (const program_state &other,
+				 const extrinsic_state &ext_state,
+				 program_state *out) const
 {
   // TODO: are the states similar enough?
-  return false;
+
+  program_state result (ext_state);
+  if (!m_region_model->can_merge_with_p (*other.m_region_model,
+					 result.m_region_model))
+    return false;
+
+  // TODO: check sm states
+
+  if (out)
+    *out = result;
+
+  return true;
 }
 
 /* FIXME.  */
@@ -5182,10 +5205,60 @@ exploded_graph::process_worklist ()
 			 node->get_point ().to_str (f).c_str ());
 		  }
 
-		if (node->get_state ().can_merge_with_p (node_2->get_state ()))
+		const program_state &state = node->get_state ();
+		const program_state &state_2 = node_2->get_state ();
+
+		/* They shouldn't be equal, or we wouldn't have two
+		   separate nodes.  */
+		gcc_assert (state != state_2);
+
+		program_state merged_state (m_ext_state);
+		if (state.can_merge_with_p (state_2, m_ext_state,
+					    &merged_state))
 		  {
-		    log ("would merge here");
-		    // TODO
+		    log ("merging EN: %i and EN: %i",
+			 node->m_index, node_2->m_index);
+
+		    if (merged_state == state)
+		      {
+			/* Then merge node_2 into node.  */
+			add_edge (node_2, node, NULL);
+
+			/* Consume node_2.  */
+			m_worklist.take_next ();
+
+			/* Continue processing node.  */
+		      }
+		    else if (merged_state == state_2)
+		      {
+			/* Then merge node into node_2, and continue
+			   processing node_2 (it's still in the worklist).  */
+			add_edge (node, node_2, NULL);
+			continue;
+		      }
+		    else
+		      {
+			/* We have a merged state that differs from
+			   both state and state_2.  */
+
+			/* Consume node_2.  */
+			m_worklist.take_next ();
+
+			/* Create an exploded node for the merged states,
+			   adding to the worklist.  */
+			exploded_node *merged_enode
+			  = get_or_create_node (node->get_point (), merged_state);
+
+			log ("merged EN: %i and EN: %i into EN: %i",
+			     node->m_index, node_2->m_index, merged_enode->m_index);
+
+			/* Add edges from the unmerged nodes to the merged node.  */
+			add_edge (node, merged_enode, NULL);
+			add_edge (node_2, merged_enode, NULL);
+
+			/* Don't process the two merged nodes.  */
+			continue;
+		      }
 		  }
 
 		// TODO: are the states similar enough?
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 2993798..2ff23a2 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1669,6 +1669,40 @@ region_model::~region_model ()
 
 /* FIXME.  */
 
+region_model &
+region_model::operator= (const region_model &other)
+{
+  unsigned i;
+  svalue *svalue;
+  region *region;
+
+  /* Delete existing content.  */
+  FOR_EACH_VEC_ELT (m_svalues, i, svalue)
+    delete svalue;
+  m_svalues.truncate (0);
+
+  FOR_EACH_VEC_ELT (m_regions, i, region)
+    delete region;
+  m_regions.truncate (0);
+
+  delete m_constraints;
+
+  /* Clone the svalues and regions.  */
+  FOR_EACH_VEC_ELT (other.m_svalues, i, svalue)
+    m_svalues.safe_push (svalue->clone ());
+
+  FOR_EACH_VEC_ELT (other.m_regions, i, region)
+    m_regions.safe_push (region->clone ());
+
+  m_root_rid = other.m_root_rid;
+
+  m_constraints = other.m_constraints->clone (this);
+
+  return *this;
+}
+
+/* FIXME.  */
+
 bool
 region_model::operator== (const region_model &other) const
 {
@@ -3520,6 +3554,108 @@ region_model::poison_any_pointers_to_bad_regions (const region_id_set &
 
 /* FIXME.  */
 
+bool
+region_model::can_merge_with_p (const region_model &other,
+				region_model *out) const
+{
+  /* TODO: assert they have the same frames.  */
+
+#if 0
+  if (m_regions.length () != other.m_regions.length ())
+    return false;
+
+  if (m_svalues.length () != other.m_svalues.length ())
+    return false;
+
+  /* Build up result, based on the things in common between the two
+     input models.  */
+  region_model result;
+  result.m_regions.truncate (0);
+  result.m_root_rid = region_id::null ();
+
+  /* FIXME: we may want to be less strict here, to allow for more
+     merging.  e.g. if one model has a value for a field, and
+     the other doesn't, then they'll have different m_regions.length ()
+     but they ought to be still mergable, given a sufficiently smart
+     implementation...
+
+     What would a "sufficiently smart implementation" look like?
+     Perhaps have each one do a mark and sweep over the others, to find
+     the regions that are common between the two
+     (with a pair of region_id_map, one for each original model),
+     based on the hierarchical "paths" within the model.
+     Build a pair of region_id_set describing the regions that are being
+     discarded.
+
+     Start at the root regions, finding common children based on identity.
+     Clone them into the result.
+
+     I think this always succeeds; it just might throw away more information
+     than the sm_state_map should keep (and hence the program_state can check
+     this, and then reject the merger for those cases)
+
+     class region_model_merger_peer
+     {
+       region_id_map m_region_map;
+       region_id_set m_discarded_regions;
+     };
+
+     class region_model_merger
+     {
+       region_model_merger_peer m_from_1;
+       region_model_merger_peer m_from_2;
+     };
+  */
+
+  unsigned i;
+  region *r;
+  FOR_EACH_VEC_ELT (m_regions, i, r)
+    {
+      if (*r != *other.m_regions[i])
+	return false;
+      result.add_region (r->clone ());
+      /* This won't be valid yet, as result doesn't have any svalues yet.  */
+
+      /* FIXME: update root_rid values, etc.  */
+      if (i == 
+    }
+
+  svalue *sval;
+  FOR_EACH_VEC_ELT (m_svalues, i, sval)
+    {
+      if (*sval != *other.m_svalues[i])
+	return false;
+      // FIXME: what about constants, though?
+      result.add_svalue (sval->clone ());
+    }
+
+  // TODO: constraint manager
+
+  result.validate ();
+#else
+  /* FIXME: for now, forget everything.  */
+  region_model result;
+
+  // FIXME: but we do need the same frames:
+  if (stack_region *stack = get_root_region ()->get_stack_region (this))
+    {
+      for (unsigned i = 0; i < stack->get_num_frames (); i++)
+	{
+	  region_id rid = stack->get_frame_rid (i);
+	  frame_region *frame = get_region <frame_region> (rid);
+	  result.push_frame (frame->get_function (), NULL);
+	}
+    }
+#endif
+
+  if (out)
+    *out = result;
+
+  return true;
+}
+
+/* FIXME.  */
+
 void
 inchash::add (svalue_id sid, inchash::hash &hstate)
 {
@@ -3974,6 +4110,70 @@ test_purging ()
 }
 #endif
 
+/* FIXME.  */
+
+static void
+test_state_merging ()
+{
+  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);
+
+  {
+    region_model model0;
+    region_model model1;
+    region_model merged;
+    /* Verify empty models can be merged.  */
+    ASSERT_TRUE (model0.can_merge_with_p (model1, &merged));
+    ASSERT_EQ (model0, merged);
+  }
+
+  /* Verify that we can merge two contradictory constraints on the
+     value for a global.  */
+  /* TODO: verify that the merged model doesn't have a value for
+     the global  */
+  {
+    region_model model0;
+    region_model model1;
+    region_model merged;
+    test_region_model_context ctxt;
+    model0.add_constraint (x, EQ_EXPR, int_42, &ctxt);
+    model1.add_constraint (x, EQ_EXPR, int_113, &ctxt);
+    ASSERT_TRUE (model0.can_merge_with_p (model1, &merged));
+    ASSERT_NE (model0, merged);
+    ASSERT_NE (model1, merged);
+
+#if 0
+    // FIXME: hack to trigger a failure:
+    ASSERT_FALSE (model0.can_merge_with_p (model1));
+#endif
+  }
+
+#if 0
+  {
+    region_model model0;
+    region_model model1;
+    test_region_model_context ctxt;
+    model0.add_constraint (x, EQ_EXPR, int_42, &ctxt);
+    model1.add_constraint (x, NE_EXPR, int_42, &ctxt);
+    ASSERT_TRUE (model0.can_merge_with_p (model1));
+  }
+
+  {
+    region_model model0;
+    region_model model1;
+    test_region_model_context ctxt;
+    model0.add_constraint (x, EQ_EXPR, int_42, &ctxt);
+    model1.add_constraint (x, NE_EXPR, int_42, &ctxt);
+    model1.add_constraint (x, EQ_EXPR, int_113, &ctxt);
+    ASSERT_TRUE (model0.can_merge_with_p (model1));
+  }
+#endif
+
+  // TODO: what can't we merge? need at least one such test
+
+  // TODO: various things
+}
 
 /* Run all of the selftests within this file.  */
 
@@ -3993,6 +4193,7 @@ analyzer_region_model_cc_tests ()
   test_purging ();
 #endif
   // TODO: test coverage for get_representative_tree / get_trees_for_svalue
+  test_state_merging ();
 }
 
 } // namespace selftest
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index a756cfd..a68dfa7 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -616,6 +616,9 @@ public:
 
   void remap_region_ids (const region_id_map &map) FINAL OVERRIDE;
 
+  unsigned get_num_frames () const { return m_frame_rids.length (); }
+  region_id get_frame_rid (unsigned i) const { return m_frame_rids[i]; }
+
  private:
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
   void print_fields (const region_model &model,
@@ -727,10 +730,12 @@ class region_model
   region_model (const region_model &other);
   ~region_model ();
 
-#if __cplusplus >= 201103
-  region_model (region_model &&other); // not needed
+#if 0//__cplusplus >= 201103
+  region_model (region_model &&other);
 #endif
 
+  region_model &operator= (const region_model &other);
+
   bool operator== (const region_model &other) const;
   bool operator!= (const region_model &other) const
   {
@@ -844,6 +849,9 @@ class region_model
 				      purge_stats *stats,
 				      logger *logger);
 
+  bool can_merge_with_p (const region_model &other,
+			 region_model *out) const;
+
  private:
   void update_for_call_superedge (const call_superedge &call_edge,
 				  region_model_context *ctxt);
diff --git a/gcc/testsuite/gcc.dg/analyzer/paths-1.c b/gcc/testsuite/gcc.dg/analyzer/paths-1.c
index 966ecb5..7c299c4 100644
--- a/gcc/testsuite/gcc.dg/analyzer/paths-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/paths-1.c
@@ -13,6 +13,6 @@ void test (struct foo *pf)
     bar (0);
   else
     bar (1);
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
-  // TODO: make this be 1 node, if merging paths
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
+  /* TODO: consolidating 2 nodes into 1 leads to 2 + 1 == 3 resulting nodes.  */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/paths-2.c b/gcc/testsuite/gcc.dg/analyzer/paths-2.c
index 0b017b5..f1013e8 100644
--- a/gcc/testsuite/gcc.dg/analyzer/paths-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/paths-2.c
@@ -1,11 +1,13 @@
+/* { dg-additional-options "-fanalyzer-state-merge" } */
+
 int test (int a)
 {
   if (a != 42 && a != 113) {
     return (-2);
   }
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
-  /* TODO: consolidate them into one.  */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
+  /* TODO: consolidating 2 nodes into 1 leads to 2 + 1 == 3 resulting nodes.  */
 
   return 0;
 }
@@ -16,8 +18,8 @@ int test_2 (int a)
     return (-2);
   }
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
-  /* TODO: consolidate them into one.  */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "4 exploded nodes" } */
+  /* TODO: consolidating 3 nodes into 1 leads to 3 + 1 == 4.  */
 
   return 0;
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/zlib-1.c b/gcc/testsuite/gcc.dg/analyzer/zlib-1.c
index d40a178..5d8f543 100644
--- a/gcc/testsuite/gcc.dg/analyzer/zlib-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/zlib-1.c
@@ -1,4 +1,4 @@
-/* { dg-additional-options "--param analyzer-bb-explosion-factor=20" } */
+/* { dg-additional-options "--param analyzer-bb-explosion-factor=20 -fanalyzer-state-merge" } */
 /* TODO: we ought go be able to fully analyze this function without needing
    the limit.  */
 
@@ -35,38 +35,37 @@ int deflateEnd(z_stream *strm)
     return (-2);
   }
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "4 exploded nodes" } */
   /* TODO: consolidate them; see paths-2.c.  */
 
   if (strm->state->pending_buf)
     (*(strm->zfree))(strm->opaque, (void *)(strm->state->pending_buf));
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "6 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
   /* TODO: consolidate them.  */
 
   if (strm->state->head)
       (*(strm->zfree))(strm->opaque, (void *)(strm->state->head));
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "12 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
   /* TODO: consolidate them.  */
 
   if (strm->state->prev)
     (*(strm->zfree))(strm->opaque, (void *)(strm->state->prev));
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "24 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
   /* TODO: consolidate them.  */
 
   if (strm->state->window)
     (*(strm->zfree))(strm->opaque, (void *)(strm->state->window));
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "48 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "3 exploded nodes" } */
   /* TODO: consolidate them.  */
 
   (*(strm->zfree))(strm->opaque, (void *)(strm->state));
   strm->state = 0;
 
-  __analyzer_dump_exploded_nodes (0); /* { dg-warning "48 exploded nodes" } */
-  /* TODO: consolidate them; we ought to be able to get away with just 1.  */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 exploded node" } */
 
   return status == 113 ? (-3) : 0;
 }
-- 
1.8.5.3

