From 2b5838af2d8aa47d89f62239d573b9b961af30c0 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 10 Jul 2019 11:27:54 -0400
Subject: [PATCH 071/108] FIXME: more WIP on region-merging

---
 gcc/analyzer/region-model.cc                      | 337 ++++++++++++++++++----
 gcc/analyzer/region-model.h                       |  51 +++-
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c |   8 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c |  69 +++++
 4 files changed, 387 insertions(+), 78 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 9f537dd..5418a8c 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -292,6 +292,85 @@ id_map<T>::update (T *id) const
 
 ////////////////////////////////////////////////////////////////////////////
 
+/* class one_way_id_map.  */
+
+/* one_way_id_map's ctor, which populates the map with dummy null values.  */
+
+template <typename T>
+one_way_id_map<T>::one_way_id_map (int num_svalues)
+: m_src_to_dst (num_svalues)
+{
+  for (int i = 0; i < num_svalues; i++)
+    m_src_to_dst.quick_push (T::null ());
+}
+
+/* Record that SRC is to be mapped to DST.  */
+
+template <typename T>
+void
+one_way_id_map<T>::put (T src, T dst)
+{
+  m_src_to_dst[src.as_int ()] = dst;
+}
+
+/* Get the new value for SRC within the map.  */
+
+template <typename T>
+T
+one_way_id_map<T>::get_dst_for_src (T src) const
+{
+  if (src.null_p ())
+    return src;
+  return m_src_to_dst[src.as_int ()];
+}
+
+/* FIXME.  */
+
+template <typename T>
+void
+one_way_id_map<T>::dump_to_pp (pretty_printer *pp) const
+{
+  pp_string (pp, "src to dst: {");
+  unsigned i;
+  T *dst;
+  FOR_EACH_VEC_ELT (m_src_to_dst, i, dst)
+    {
+      if (i > 0)
+	pp_string (pp, ", ");
+      T src (T::from_int (i));
+      src.print (pp);
+      pp_string (pp, " -> ");
+      dst->print (pp);
+    }
+  pp_string (pp, "}");
+  pp_newline (pp);
+}
+
+/* FIXME.  */
+
+template <typename T>
+DEBUG_FUNCTION void
+one_way_id_map<T>::dump () const
+{
+  pretty_printer pp;
+  pp.buffer->stream = stderr;
+  dump_to_pp (&pp);
+  pp_flush (&pp);
+}
+
+/* FIXME.  */
+
+template <typename T>
+std::string
+one_way_id_map<T>::to_str () const
+{
+  pretty_printer pp;
+  dump_to_pp (&pp);
+  return pp_formatted_text (&pp);
+}
+
+////////////////////////////////////////////////////////////////////////////
+
 /* class id_set.  */
 
 /* id_set<region_id>'s ctor.  */
@@ -492,18 +571,21 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
 				   const region_svalue &region_sval_b,
 				   svalue_id *merged_sid,
 				   tree type,
-				   const model_merger &merger)
+				   model_merger *merger)
 {
   /* FIXME: they must be non-NULL; how could we test the kind otherwise?
      We should have been testing the pointee value.  */
   region_id a_rid = region_sval_a.get_pointee ();
   region_id b_rid = region_sval_b.get_pointee ();
 
+  /* FIXME: currently, I don't think we can ever have null regions.  */
+  gcc_assert (!a_rid.null_p () && !b_rid.null_p ());
+
   if (a_rid.null_p () && b_rid.null_p ())
     {
       /* Both are NULL.  */
       svalue *merged_sval = new region_svalue (type, region_id::null ());
-      *merged_sid = merger.m_merged_model->add_svalue (merged_sval);
+      *merged_sid = merger->m_merged_model->add_svalue (merged_sval);
       return true;
     }
 
@@ -511,13 +593,42 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
     {
       /* If we have NULL vs non-NULL, then merge to unknown.  */
       svalue *merged_sval = new unknown_svalue (type);
-      *merged_sid = merger.m_merged_model->add_svalue (merged_sval);
+      *merged_sid = merger->m_merged_model->add_svalue (merged_sval);
       return true;
     }
 
   /* Both are non-NULL. */
   gcc_assert (!a_rid.null_p () && !b_rid.null_p ());
 
+  /* Have these ptr-values already been merged?  */
+
+  region_id a_rid_in_m
+    = merger->m_map_regions_from_a_to_m.get_dst_for_src (a_rid);
+  region_id b_rid_in_m
+    = merger->m_map_regions_from_b_to_m.get_dst_for_src (b_rid);
+
+  /* "null_p" here means "we've seen this ptr-value before".
+     If we've seen one but not the other, or we have different
+     regions, then the merged ptr has to be "unknown".  */
+  if (a_rid_in_m != b_rid_in_m)
+    {
+      gcc_unreachable ();
+      svalue *merged_sval = new unknown_svalue (type);
+      *merged_sid = merger->m_merged_model->add_svalue (merged_sval);
+      return true;
+    }
+
+  /* Have we seen this yet?  If so, reuse the value.  */
+  if (!a_rid_in_m.null_p ())
+    {
+      *merged_sid
+	= merger->m_merged_model->get_or_create_ptr_svalue (type, a_rid_in_m);
+      return true;
+    }
+
+  /* Otherwise we have A/B regions that haven't been referenced yet.  */
+  // TODO
+
   /* If we have two regions that haven't been referenced yet,
      and they're both in the heap, create a new heap region, and record
      that.
@@ -529,18 +640,19 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
      but presumably we'd need the representative tree to do the lookup in
      the merged model - assuming the region exists - aha: but get_lvalue
      would presumably add it lazily.  */
-  tree tree_a = merger.m_model_a->get_representative_tree (a_rid);
-  tree tree_b = merger.m_model_b->get_representative_tree (b_rid);
+  tree tree_a = merger->m_model_a->get_representative_tree (a_rid);
+  tree tree_b = merger->m_model_b->get_representative_tree (b_rid);
 
   /* FIXME: we probably need a deeper test here e.g. for fields of structs.  */
   if (tree_a
       && tree_a == tree_b)
     {
       region_id merged_pointee_rid
-	= merger.m_merged_model->get_lvalue (tree_a, NULL);
-      svalue *merged_sval
-	= new region_svalue (type, merged_pointee_rid);
-      *merged_sid = merger.m_merged_model->add_svalue (merged_sval);
+	= merger->m_merged_model->get_lvalue (tree_a, NULL);
+      *merged_sid
+	= merger->m_merged_model->get_or_create_ptr_svalue (type,
+								   merged_pointee_rid);
+      merger->record_regions (a_rid, b_rid, merged_pointee_rid);
       return true;
     }
 
@@ -551,7 +663,63 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
      though.
      Or have something like get_repr_tree that returns a richer object that
      "knows" about heap regions.
+     e.g. an expression of a "root", and a tree relative to it
+     (a) root == root_region, a tree (as before)
+     (b) root == heap region 0, a tree relative to that heap region
+     etc
+     But what about e.g. a linked list, or other chain of pointers that
+     reaches a heap region?
   */
+  /* Maybe for now it's good enough to detect if both are heap regions
+     i.e. that their parents are the heap_region.
+   */
+#if 0
+  if (heap_region *heap_a
+      = merger->get_root_a ()->get_heap_region (merger->m_model_a))
+    if (heap_region *heap_b
+	= merger->get_root_b ()->get_heap_region (merger->m_model_b))
+#endif
+#if 1
+  region *region_a = merger->m_model_a->get_region (a_rid);
+  region *region_b = merger->m_model_b->get_region (b_rid);
+  region_id a_parent_rid = region_a->get_parent ();
+  region_id b_parent_rid = region_b->get_parent ();
+  region *parent_region_a = merger->m_model_a->get_region (a_parent_rid);
+  region *parent_region_b = merger->m_model_b->get_region (b_parent_rid);
+  if (parent_region_a
+      && parent_region_b
+      && parent_region_a->get_kind () == RK_HEAP
+      && parent_region_b->get_kind () == RK_HEAP)
+    {
+#if 0
+      tree_a = merger->m_model_a->get_representative_tree (a_sid);
+      tree_b = merger->m_model_b->get_representative_tree (b_sid);
+      // FIXME: again, do we need a deeper test here?
+      if (tree_a
+	  && tree_a == tree_b)
+#endif
+      /* FIXME: but isn't the above guaranteed, because of the way we're
+	 being called?  */
+
+	/* We have an A/B pair of ptrs that both point at heap regions.  */
+	/* presumably we want to see if each A/B heap region already
+	   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);
+	// TODO: merge values within the heap regions
+	*merged_sid
+	  = merger->m_merged_model->get_or_create_ptr_svalue
+	     (type, merged_pointee_rid);
+	merger->record_regions (a_rid, b_rid, merged_pointee_rid);
+	return true;
+    }
+#endif
 
 #if 0
     /* FIXME: is it OK to just check that the region_id is the same?  */
@@ -560,7 +728,7 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
 	  region_id merged_pointee_rid; // FIXME: what should this be???
 	  svalue *merged_sval
 	    = new region_svalue (type, merged_pointee_rid);
-	  *merged_sid = merger.m_merged_model->add_svalue (merged_sval);
+	  *merged_sid = merger->m_merged_model->add_svalue (merged_sval);
 	  return true;
 	}
       else
@@ -572,7 +740,7 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
 	  /* Two different non-NULL pointers?  Merge to unknown.  */
 	  // TODO: add a constraint?
 	  svalue *merged_sval = new unknown_svalue (type);
-	  *merged_sid = merger.m_merged_model->add_svalue (merged_sval);
+	  *merged_sid = merger->m_merged_model->add_svalue (merged_sval);
 	  return true;
 	}
 }
@@ -607,7 +775,7 @@ constant_svalue::can_merge_values_p (const constant_svalue &cst_sval_a,
 				     const constant_svalue &cst_sval_b,
 				     svalue_id sid_b ATTRIBUTE_UNUSED,
 				     svalue_id *merged_sid,
-				     const model_merger &merger)
+				     model_merger *merger)
 {
   tree cst_a = cst_sval_a.get_constant ();
   tree cst_b = cst_sval_b.get_constant ();
@@ -625,7 +793,7 @@ constant_svalue::can_merge_values_p (const constant_svalue &cst_sval_a,
 	 (maybe just based on A, to avoid infinite chains)  */
       merged_sval = new unknown_svalue (TREE_TYPE (cst_a));
     }
-  *merged_sid = merger.m_merged_model->add_svalue (merged_sval);
+  *merged_sid = merger->m_merged_model->add_svalue (merged_sval);
   return true;
 }
 
@@ -1333,7 +1501,7 @@ map_region::can_merge_p (const map_region *map_region_a,
 			 const map_region *map_region_b,
 			 map_region *merged_map_region,
 			 region_id merged_rid,
-			 const model_merger &merger)
+			 model_merger *merger)
 {
   // TODO:
   /* Sort keys into a consistent order, to avoid cache misses
@@ -1367,31 +1535,31 @@ map_region::can_merge_p (const map_region *map_region_a,
 	     We want to recurse.
 	     We also want to do something about values.  */
 	  map_region *child_region_a
-	    = merger.get_region_a <map_region> (rid_a);
+	    = merger->get_region_a <map_region> (rid_a);
 	  map_region *child_region_b
-	    = merger.get_region_b <map_region> (rid_b);
+	    = merger->get_region_b <map_region> (rid_b);
 
 	  gcc_assert (child_region_a->get_type ()
 		      == child_region_b->get_type ());
 
 	  region_id child_merged_rid
-	    = merged_map_region->get_or_create (merger.m_merged_model,
+	    = merged_map_region->get_or_create (merger->m_merged_model,
 						merged_rid,
 						key_a,
 						child_region_a->get_type ());
 
 	  map_region *child_merged_region
-	    = merger.m_merged_model->get_region<map_region> (child_merged_rid);
+	    = merger->m_merged_model->get_region<map_region> (child_merged_rid);
 
 	  /* TODO: consider values.  */
 	  svalue_id child_a_sid = child_region_a->get_value_direct ();
 	  svalue_id child_b_sid = child_region_b->get_value_direct ();
 	  svalue_id child_merged_sid;
-	  if (!merger.can_merge_values_p (child_a_sid, child_b_sid,
-					  &child_merged_sid))
+	  if (!merger->can_merge_values_p (child_a_sid, child_b_sid,
+					   &child_merged_sid))
 	    return false;
 	  if (!child_merged_sid.null_p ())
-	    child_merged_region->set_value (*merger.m_merged_model,
+	    child_merged_region->set_value (*merger->m_merged_model,
 					    child_merged_sid);
 
 	  /* Recurse.  */
@@ -1615,7 +1783,7 @@ stack_region::remap_region_ids (const region_id_map &map)
 bool
 stack_region::can_merge_p (const stack_region *stack_region_a,
 			   const stack_region *stack_region_b,
-			   const model_merger &merger)
+			   model_merger *merger)
 {
   if (stack_region_a->get_num_frames ()
       != stack_region_b->get_num_frames ())
@@ -1624,18 +1792,18 @@ stack_region::can_merge_p (const stack_region *stack_region_a,
   for (unsigned i = 0; i < stack_region_a->get_num_frames (); i++)
     {
       region_id rid_a = stack_region_a->get_frame_rid (i);
-      frame_region *frame_a = merger.get_region_a <frame_region> (rid_a);
+      frame_region *frame_a = merger->get_region_a <frame_region> (rid_a);
 
       region_id rid_b = stack_region_b->get_frame_rid (i);
-      frame_region *frame_b = merger.get_region_b <frame_region> (rid_b);
+      frame_region *frame_b = merger->get_region_b <frame_region> (rid_b);
 
       if (frame_a->get_function () != frame_b->get_function ())
 	return false;
-      merger.m_merged_model->push_frame (frame_a->get_function (), NULL);
+      merger->m_merged_model->push_frame (frame_a->get_function (), NULL);
 
-      region_id rid_merged = merger.m_merged_model->get_current_frame_id ();
+      region_id rid_merged = merger->m_merged_model->get_current_frame_id ();
       frame_region *merged_frame
-	= merger.m_merged_model->get_region <frame_region> (rid_merged);
+	= merger->m_merged_model->get_region <frame_region> (rid_merged);
 
       if (!map_region::can_merge_p (frame_a, frame_b,
 				    merged_frame, rid_merged,
@@ -1681,7 +1849,7 @@ heap_region::can_merge_p (const heap_region *heap_a ATTRIBUTE_UNUSED,
 			  region_id heap_b_rid ATTRIBUTE_UNUSED,
 			  heap_region *merged_heap ATTRIBUTE_UNUSED,
 			  region_id merged_heap_rid ATTRIBUTE_UNUSED,
-			  const model_merger &merger ATTRIBUTE_UNUSED)
+			  model_merger *merger ATTRIBUTE_UNUSED)
 {
   /* TODO:
      iterate through descendents of each, cloning
@@ -1949,15 +2117,15 @@ bool
 root_region::can_merge_p (const root_region *root_region_a,
 			  const root_region *root_region_b,
 			  root_region *merged_root_region,
-			  const model_merger &merger)
+			  model_merger *merger)
 {
   /* We can only merge if the stacks are sufficiently similar.  */
-  stack_region *stack_a = root_region_a->get_stack_region (merger.m_model_a);
-  stack_region *stack_b = root_region_b->get_stack_region (merger.m_model_b);
+  stack_region *stack_a = root_region_a->get_stack_region (merger->m_model_a);
+  stack_region *stack_b = root_region_b->get_stack_region (merger->m_model_b);
   if (stack_a && stack_b)
     {
       /* If the two models both have a stack, attempt to merge them.  */
-      merged_root_region->ensure_stack_region (merger.m_merged_model);
+      merged_root_region->ensure_stack_region (merger->m_merged_model);
       if (!stack_region::can_merge_p (stack_a, stack_b, merger))
 	return false;
     }
@@ -1966,15 +2134,15 @@ root_region::can_merge_p (const root_region *root_region_a,
        doesn't.  */
     return false;
 
-  map_region *globals_a = root_region_a->get_globals_region (merger.m_model_a);
-  map_region *globals_b = root_region_b->get_globals_region (merger.m_model_b);
+  map_region *globals_a = root_region_a->get_globals_region (merger->m_model_a);
+  map_region *globals_b = root_region_b->get_globals_region (merger->m_model_b);
   if (globals_a && globals_b)
     {
       /* If both models have globals regions, attempt to merge them.  */
       region_id merged_globals_rid
-	= merged_root_region->ensure_globals_region (merger.m_merged_model);
+	= merged_root_region->ensure_globals_region (merger->m_merged_model);
       map_region *merged_globals
-	= merged_root_region->get_globals_region (merger.m_merged_model);
+	= merged_root_region->get_globals_region (merger->m_merged_model);
       if (!map_region::can_merge_p (globals_a, globals_b,
 				    merged_globals, merged_globals_rid,
 				    merger))
@@ -1982,14 +2150,14 @@ root_region::can_merge_p (const root_region *root_region_a,
     }
   /* otherwise, merge as "no globals".  */
 
-  heap_region *heap_a = root_region_a->get_heap_region (merger.m_model_a);
-  heap_region *heap_b = root_region_b->get_heap_region (merger.m_model_b);
+  heap_region *heap_a = root_region_a->get_heap_region (merger->m_model_a);
+  heap_region *heap_b = root_region_b->get_heap_region (merger->m_model_b);
   if (heap_a && heap_b)
     {
       region_id merged_heap_rid
-	= merged_root_region->ensure_heap_region (merger.m_merged_model);
+	= merged_root_region->ensure_heap_region (merger->m_merged_model);
       heap_region *merged_heap
-	= merged_root_region->get_heap_region (merger.m_merged_model);
+	= merged_root_region->get_heap_region (merger->m_merged_model);
       if (!heap_region::can_merge_p (heap_a, root_region_a->m_heap_rid,
 				     heap_b, root_region_b->m_heap_rid,
 				     merged_heap, merged_heap_rid,
@@ -2420,7 +2588,8 @@ region_model::on_assignment (const gassign *assign, region_model_context *ctxt)
 	region_id element_rid
 	  = array_region->get_or_create (this, buffer_rid, offset,
 					 element_type);
-	svalue_id element_ptr_sid = get_svalue_for_region (element_rid);
+	svalue_id element_ptr_sid
+	  = get_or_create_ptr_svalue (TREE_TYPE (ptr), element_rid);
 	set_value (lhs_rid, element_ptr_sid);
       }
       break;
@@ -2539,7 +2708,7 @@ region_model::on_assignment (const gassign *assign, region_model_context *ctxt)
 	      if (CONSTANT_CLASS_P (result))
 		{
 		  svalue_id result_sid
-		    = get_svalue_for_constant (result);
+		    = get_or_create_constant_svalue (result);
 		  set_value (lhs_rid, result_sid);
 		  return;
 		}
@@ -2587,7 +2756,7 @@ region_model::on_call (const gcall *call, region_model_context *ctxt)
       if (!lhs_rid.null_p ())
 	{
 	  svalue_id ptr_sid
-	    = add_svalue (new region_svalue (lhs_type, new_rid));
+	    = get_or_create_ptr_svalue (lhs_type, new_rid);
 	  set_value (lhs_rid, ptr_sid);
 	}
       return;
@@ -2625,7 +2794,7 @@ region_model::on_call (const gcall *call, region_model_context *ctxt)
       if (!lhs_rid.null_p ())
 	{
 	  svalue_id ptr_sid
-	    = add_svalue (new region_svalue (lhs_type, new_rid));
+	    = get_or_create_ptr_svalue (lhs_type, new_rid);
 	  set_value (lhs_rid, ptr_sid);
 	}
       return;
@@ -2644,7 +2813,7 @@ region_model::on_call (const gcall *call, region_model_context *ctxt)
 	      int strlen_cst = sizeof_cst - 1;
 	      tree t_cst = build_int_cst (lhs_type, strlen_cst);
 	      svalue_id result_sid
-		= get_svalue_for_constant (t_cst);
+		= get_or_create_constant_svalue (t_cst);
 	      set_value (lhs_rid, result_sid);
 	      return;
 	    }
@@ -2821,7 +2990,7 @@ region_model::get_lvalue (tree expr, region_model_context *ctxt)
 	// FIXME: should map to a unique, custom region, containing the constant
 	tree cst_type = TREE_TYPE (expr);
 	map_region *cst_region = new map_region (m_root_rid, cst_type);
-	svalue_id cst_sid = get_svalue_for_constant (expr);
+	svalue_id cst_sid = get_or_create_constant_svalue (expr);
 	cst_region->set_value (*this, cst_sid);
 	region_id cst_rid = add_region (cst_region);
 	return cst_rid;
@@ -2849,7 +3018,7 @@ region_model::get_rvalue (tree expr, region_model_context *ctxt)
 	if (TREE_CODE (TREE_OPERAND (expr, 0)) == FUNCTION_DECL)
 	  return get_svalue_for_fndecl (TREE_OPERAND (expr, 0));
 	region_id expr_rid = get_lvalue (TREE_OPERAND (expr, 0), ctxt);
-	return get_svalue_for_region (expr_rid);
+	return get_or_create_ptr_svalue (TREE_TYPE (expr), expr_rid);
       }
       break;
 
@@ -2862,7 +3031,7 @@ region_model::get_rvalue (tree expr, region_model_context *ctxt)
     case INTEGER_CST:
     case REAL_CST:
     case STRING_CST:
-      return get_svalue_for_constant (expr);
+      return get_or_create_constant_svalue (expr);
 
     case COMPONENT_REF:
     case MEM_REF:
@@ -2880,18 +3049,39 @@ region_model::get_rvalue (tree expr, region_model_context *ctxt)
 /* FIXME.  */
 
 svalue_id
-region_model::get_svalue_for_region (region_id rid)
+region_model::get_or_create_ptr_svalue (tree ptr_type, region_id rid)
 {
-  // TODO: maybe reuse existing ones? (maybe keep a record in the region?)
-  tree type = get_region (rid)->get_type ();
-  // FIXME: ^^^ should this be a pointer type to type?
-  return add_svalue (new region_svalue (type, rid));
+  /* Reuse existing region_svalue, if one of the right type is
+     available.  */
+  /* In theory we could stash a svalue_id in "region", but differing
+     pointer types muddles things.
+     For now, just do a linear search through all existing svalues.  */
+  int i;
+  svalue *svalue;
+  FOR_EACH_VEC_ELT (m_svalues, i, svalue)
+    if (region_svalue *ptr_svalue = svalue->dyn_cast_region_svalue ())
+      if (ptr_svalue->get_pointee () == rid
+	  && ptr_svalue->get_type () == ptr_type)
+	return svalue_id::from_int (i);
+
+  return add_svalue (new region_svalue (ptr_type, rid));
+
+#if 0
+  // FIXME: but what about types?
+  region *region = get_region (rid);
+  if (region->m_pointer_sid.null_p ())
+    {
+      tree type = region->get_type ();
+      // FIXME: ^^^ should this be a pointer type to type?
+    }
+  return region->m_pointer_sid;
+#endif
 }
 
 /* FIXME.  */
 
 svalue_id
-region_model::get_svalue_for_constant (tree cst_expr)
+region_model::get_or_create_constant_svalue (tree cst_expr)
 {
   /* Reuse one if it already exists.  */
   // FIXME: maybe store a map, rather than do linear search?
@@ -2911,7 +3101,7 @@ region_model::get_svalue_for_constant (tree cst_expr)
 svalue_id
 region_model::get_svalue_for_fndecl (tree fndecl)
 {
-  return get_svalue_for_constant (fndecl);
+  return get_or_create_constant_svalue (fndecl);
 }
 
 class deref_diagnostic : public pending_diagnostic_subclass<deref_diagnostic>
@@ -3986,7 +4176,7 @@ region_model::can_merge_with_p (const region_model &other_model,
   if (!root_region::can_merge_p (get_root_region (),
 				 other_model.get_root_region (),
 				 result.get_root_region (),
-				 merger))
+				 &merger))
     return false;
 #else
   if (m_regions.length () != other.m_regions.length ())
@@ -4107,6 +4297,16 @@ model_merger::dump_to_pp (pretty_printer *pp) const
   pp_newline (pp);
   m_merged_model->dump_to_pp (pp);
   pp_newline (pp);
+
+  pp_string (pp, "region map: model A to merged model:");
+  pp_newline (pp);
+  m_map_regions_from_a_to_m.dump_to_pp (pp);
+  pp_newline (pp);
+
+  pp_string (pp, "region map: model B to merged model:");
+  pp_newline (pp);
+  m_map_regions_from_b_to_m.dump_to_pp (pp);
+  pp_newline (pp);
 }
 
 /* Dump a multiline representation of this merger to FILE.  */
@@ -4138,7 +4338,7 @@ model_merger::dump () const
 bool
 model_merger::can_merge_values_p (svalue_id sid_a,
 				  svalue_id sid_b,
-				  svalue_id *merged_sid) const
+				  svalue_id *merged_sid)
 {
   gcc_assert (merged_sid);
   svalue *sval_a = m_model_a->get_svalue (sid_a);
@@ -4189,7 +4389,7 @@ model_merger::can_merge_values_p (svalue_id sid_a,
 	const region_svalue &region_sval_b = *(const region_svalue *)sval_b;
 	return region_svalue::can_merge_values_p (region_sval_a, region_sval_b,
 						  merged_sid, type,
-						  *this);
+						  this);
       }
       break;
     case SK_CONSTANT:
@@ -4200,7 +4400,7 @@ model_merger::can_merge_values_p (svalue_id sid_a,
 	return constant_svalue::can_merge_values_p (cst_sval_a, sid_a,
 						    cst_sval_b, sid_b,
 						    merged_sid,
-						    *this);
+						    this);
       }
       break;
 
@@ -4219,6 +4419,19 @@ model_merger::can_merge_values_p (svalue_id sid_a,
   return false;
 }
 
+/* Record that A_RID in model A and B_RID in model B
+   correspond to MERGED_RID in the merged model, so
+   that pointers can be accurately merged.  */
+
+void
+model_merger::record_regions (region_id a_rid,
+			      region_id b_rid,
+			      region_id merged_rid)
+{
+  m_map_regions_from_a_to_m.put (a_rid, merged_rid);
+  m_map_regions_from_b_to_m.put (b_rid, merged_rid);
+}
+
 ////////////////////////////////////////////////////////////////////////////
 
 /* FIXME.  */
@@ -4528,12 +4741,14 @@ test_stack_frames ()
   /* Point a global pointer at a local in the child frame:  p = &x.  */
   region_id p_in_globals_rid = model.get_lvalue (p, &ctxt);
   model.set_value (p_in_globals_rid,
-		   model.get_svalue_for_region (x_in_child_rid));
+		   model.get_or_create_ptr_svalue (ptr_type_node,
+						   x_in_child_rid));
 
   /* Point another global pointer at p: q = &p.  */
   region_id q_in_globals_rid = model.get_lvalue (q, &ctxt);
   model.set_value (q_in_globals_rid,
-		   model.get_svalue_for_region (p_in_globals_rid));
+		   model.get_or_create_ptr_svalue (ptr_type_node,
+						   p_in_globals_rid));
 
   /* Test get_descendents.  */
   region_id_set descendents (&model);
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index deebead..bae5fc9 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -142,6 +142,27 @@ class id_map
 typedef id_map<svalue_id> svalue_id_map;
 typedef id_map<region_id> region_id_map;
 
+/* Variant of the above, which only stores things in one direction.
+   (e.g. for merging, when the number of destination regions is not
+   the same of the src regions, and can grow).  */
+
+template <typename T>
+class one_way_id_map
+{
+ public:
+  one_way_id_map (int num_ids);
+  void put (T src, T dst);
+  T get_dst_for_src (T src) const;
+  void dump_to_pp (pretty_printer *pp) const;
+  void dump () const;
+  std::string to_str () const;
+
+ private:
+  auto_vec<T> m_src_to_dst;
+};
+
+typedef one_way_id_map<region_id> one_way_region_id_map;
+
 ////////////////////////////////////////////////////////////////////////////
 
 /* A set of IDs within a region_model (either svalue_id or region_id).  */
@@ -296,7 +317,7 @@ public:
 				  const region_svalue &region_sval_b,
 				  svalue_id *merged_sid,
 				  tree type,
-				  const model_merger &merger);
+				  model_merger *merger);
 
  private:
   void print_details (const region_model &model,
@@ -335,7 +356,7 @@ public:
 				  const constant_svalue &cst_sval_b,
 				  svalue_id sid_b,
 				  svalue_id *merged_sid,
-				  const model_merger &merger);
+				  model_merger *merger);
 
  private:
   void print_details (const region_model &model,
@@ -563,7 +584,7 @@ public:
 			   const map_region *map_region_b,
 			   map_region *merged_map_region,
 			   region_id merged_rid,
-			   const model_merger &merger);
+			   model_merger *merger);
 
  protected:
   void add_to_hash (inchash::hash &hstate) const OVERRIDE;
@@ -648,7 +669,7 @@ public:
 
   static bool can_merge_p (const stack_region *stack_region_a,
 			   const stack_region *stack_region_b,
-			   const model_merger &merger);
+			   model_merger *merger);
 
  private:
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
@@ -679,7 +700,7 @@ public:
   static bool can_merge_p (const heap_region *heap_a, region_id heap_a_rid,
 			   const heap_region *heap_b, region_id heap_b_rid,
 			   heap_region *merged_heap, region_id merged_heap_rid,
-			   const model_merger &merger);
+			   model_merger *merger);
 
 };
 
@@ -726,7 +747,7 @@ public:
   static bool can_merge_p (const root_region *root_region_a,
 			   const root_region *root_region_b,
 			   root_region *merged_root_region,
-			   const model_merger &merger);
+			   model_merger *merger);
 
 private:
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
@@ -844,8 +865,8 @@ class region_model
   region_id get_lvalue (tree expr, region_model_context *ctxt);
   svalue_id get_rvalue (tree expr, region_model_context *ctxt);
 
-  svalue_id get_svalue_for_region (region_id id);
-  svalue_id get_svalue_for_constant (tree cst_expr);
+  svalue_id get_or_create_ptr_svalue (tree ptr_type, region_id id);
+  svalue_id get_or_create_constant_svalue (tree cst_expr);
   svalue_id get_svalue_for_fndecl (tree fndecl);
 
   region_id deref_rvalue (tree ptr, region_model_context *ctxt);
@@ -880,6 +901,7 @@ class region_model
 		      logger *logger);
 
   int get_num_svalues () const { return m_svalues.length (); }
+  int get_num_regions () const { return m_regions.length (); }
 
   /* For selftests.  */
   constraint_manager *get_constraints ()
@@ -985,7 +1007,9 @@ struct model_merger
 		const region_model *model_b,
 		region_model *merged_model)
   : m_model_a (model_a), m_model_b (model_b),
-    m_merged_model (merged_model)
+    m_merged_model (merged_model),
+    m_map_regions_from_a_to_m (model_a->get_num_regions ()),
+    m_map_regions_from_b_to_m (model_b->get_num_regions ())
   {}
 
   void dump_to_pp (pretty_printer *pp) const;
@@ -1006,11 +1030,18 @@ struct model_merger
 
   bool can_merge_values_p (svalue_id sid_a,
 			   svalue_id sid_b,
-			   svalue_id *merged_sid) const;
+			   svalue_id *merged_sid);
+
+  void record_regions (region_id a_rid,
+		       region_id b_rid,
+		       region_id merged_rid);
 
   const region_model *m_model_a;
   const region_model *m_model_b;
   region_model *m_merged_model;
+
+  one_way_region_id_map m_map_regions_from_a_to_m;
+  one_way_region_id_map m_map_regions_from_b_to_m;
 };
 
 namespace inchash
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c
index cabad68..10a5aae 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-2.c
@@ -10,13 +10,7 @@ do_stuff_2 (int *p, int n)
   return 0;
 }
 
-/* Various examples of functions that use either a malloc buffer
-   or a local buffer, do something, then conditionally free the
-   buffer, tracking whether "free" is necessary in various
-   ways.
-
-   In each case, there ought to be only two paths through the function,
-   not four.  */
+/* As malloc-vs-local.c, but hand-inlining the logic.  */
 
 /* Repeated (n > 10) predicate.  */
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
new file mode 100644
index 0000000..bf665b9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local-3.c
@@ -0,0 +1,69 @@
+/* { dg-additional-options "-fanalyzer-state-merge" } */
+
+#include <stdlib.h>
+
+extern int foo (int);
+
+static int __attribute__((noinline))
+do_stuff_2 (int *p, int n)
+{
+  return 0;
+}
+
+/* As malloc-vs-local-2.c, but with a memory leak for the "on the heap case"
+   by not attempting to free at the end.  */
+
+int test_1 (int n)
+{
+  int buf[10];
+  int *ptr;
+  int result;
+
+  if (n > 10)
+    ptr = (int *)malloc (sizeof (int) * n);
+  else
+    ptr = buf;
+
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+
+  {
+    int *p = ptr;
+    int sum = 0;
+    int i;
+    for (i = 0; i < n; i++)
+      p[i] = i;
+    for (i = 0; i < n; i++)
+      sum += foo (p[i]); /* { dg-bogus "uninitialized" } */
+    result = sum;
+  }
+
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" "" { xfail *-*-* } } */
+  /* { dg-bogus "1 exploded node" "" { xfail *-*-* } .-1 } */
+  /* TODO(xfail): should be 2 nodes here, for the malloc-vs-non-malloc case; erroneously getting 1.  */
+
+  return result; /* { dg-message "leak of 'ptr'" "" { xfail *-*-* } } */
+  /* TODO(xfail): should report a leak here for the malloc-ed path.  */  
+}
+
+/* A simpler version of the above.  */
+
+int test_2 (int n)
+{
+  int buf[10];
+  int *ptr;
+  int result;
+
+  if (n > 10)
+    ptr = (int *)malloc (sizeof (int) * n);
+  else
+    ptr = buf;
+
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+
+  result = do_stuff_2 (ptr, n);
+
+  __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 exploded nodes" } */
+
+  return result; /* { dg-message "leak of 'ptr'" } */
+}
-- 
1.8.5.3

