From 186d20147ebd946f523dc3650920405340efcd97 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 17 Jul 2019 16:46:14 -0400
Subject: [PATCH 097/108] FIXME: use representative path_vars rather than trees
 when merging region pointers, fixing
 malloc-vs-local.c:test_pointer_comparison

---
 gcc/analyzer/impl-4.cc                          |  45 +++--
 gcc/analyzer/path-state.h                       |   1 +
 gcc/analyzer/region-model.cc                    | 248 +++++++++++++++++-------
 gcc/analyzer/region-model.h                     |  18 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c |   3 +-
 5 files changed, 223 insertions(+), 92 deletions(-)

diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index a7b2ffe..7152772 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -3111,14 +3111,14 @@ public:
     gcc_assert (m_new_state);
     svalue_id sid = m_new_state->m_region_model->get_rvalue (expr, NULL);
     /* Find trees for all regions storing the value.  */
-    auto_vec<tree> trees;
-    m_new_state->m_region_model->get_trees_for_svalue (sid, &trees);
-    if (trees.length () < 1)
+    auto_vec<path_var> pvs;
+    m_new_state->m_region_model->get_path_vars_for_svalue (sid, &pvs);
+    if (pvs.length () < 1)
       return expr;
     /* Pick the "best" such tree.  */
     // TODO: should we also consider (and consolidate) equiv classes?
-    trees.qsort (readability_comparator);
-    return trees[0];
+    pvs.qsort (readability_comparator);
+    return pvs[0].m_tree;
   }
 
   exploded_graph &m_eg;
@@ -3233,6 +3233,7 @@ private:
 static int
 readability (const_tree expr)
 {
+  gcc_assert (expr);
   switch (TREE_CODE (expr))
     {
     case COMPONENT_REF:
@@ -3267,11 +3268,12 @@ readability (const_tree expr)
 static int
 readability_comparator (const void *p1, const void *p2)
 {
-  const_tree t1 = *(const_tree const *)p1;
-  const_tree t2 = *(const_tree const *)p2;
+  path_var pv1 = *(path_var const *)p1;
+  path_var pv2 = *(path_var const *)p2;
 
-  int r1 = readability (t1);
-  int r2 = readability (t2);
+  /* FIXME: should we consider stack depths?  */
+  int r1 = readability (pv1.m_tree);
+  int r2 = readability (pv2.m_tree);
 
   return r2 - r1;
 }
@@ -3302,8 +3304,8 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
   const program_state &old_state = m_enode->get_state ();
   svalue_id old_sid = map.get_src_for_dst (sid);
 
-  auto_vec<tree> leaked_trees;
-  old_state.m_region_model->get_trees_for_svalue (old_sid, &leaked_trees);
+  auto_vec<path_var> leaked_pvs;
+  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?
@@ -3313,12 +3315,12 @@ impl_region_model_context::on_state_leak (const state_machine &sm,
      leak per EC, this ought to ensure that we pick the most
      user-readable description of each leaking EC.
      This assumes that all vars in the EC have the same state.  */
-  leaked_trees.qsort (readability_comparator);
+  leaked_pvs.qsort (readability_comparator);
 
-  if (leaked_trees.length () < 1)
+  if (leaked_pvs.length () < 1)
     return;
 
-  tree leaked_tree = leaked_trees[0];
+  tree leaked_tree = leaked_pvs[0].m_tree;
   if (get_logger ())
     log ("best leaked_tree: %qE", leaked_tree);
 
@@ -4881,23 +4883,26 @@ diagnostic_manager::build_emission_path (const exploded_graph &eg,
 	      svalue_id dst_sid = (*iter).first;
 	      state_machine::state_t dst_sm_val = (*iter).second.m_state;
 
-	      auto_vec<tree> dst_trees;
-	      dst_state.m_region_model->get_trees_for_svalue (dst_sid,
-							      &dst_trees);
+	      auto_vec<path_var> dst_pvs;
+	      dst_state.m_region_model->get_path_vars_for_svalue (dst_sid,
+								  &dst_pvs);
 
 #if 1
 	      unsigned j;
-	      tree dst_rep;
-	      FOR_EACH_VEC_ELT (dst_trees, j, dst_rep)
+	      path_var *dst_pv;
+	      FOR_EACH_VEC_ELT (dst_pvs, j, dst_pv)
 		{
+		  tree dst_rep = dst_pv->m_tree;
 		  gcc_assert (dst_rep);
 #else
 	      tree dst_rep = dst_state.get_representative_tree (dst_sid);
 	      if (dst_rep == NULL_TREE)
 		continue;
 #endif
+	      if (dst_pv->m_stack_depth >= src_state.m_region_model->get_stack_depth ())
+		continue;
 	      svalue_id src_sid
-		= src_state.m_region_model->get_rvalue (dst_rep, NULL);
+		= src_state.m_region_model->get_rvalue (*dst_pv, NULL);
 	      if (src_sid.null_p ())
 		continue;
 	      state_machine::state_t src_sm_val = src_smap.get_state (src_sid);
diff --git a/gcc/analyzer/path-state.h b/gcc/analyzer/path-state.h
index 68edf80..b53b511 100644
--- a/gcc/analyzer/path-state.h
+++ b/gcc/analyzer/path-state.h
@@ -65,6 +65,7 @@ struct range
 /* A tree, extended with stack frame information for locals, so that
    we can distinguish between different values of locals within a potentially
    recursive callstack.  */
+// FIXME: would this be better as a new tree code?
 
 class path_var
 {
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index d417908..f445ee6 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -574,18 +574,18 @@ 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);
+  path_var pv_a = merger->m_model_a->get_representative_path_var (a_rid);
+  path_var pv_b = merger->m_model_b->get_representative_path_var (b_rid);
 
   /* FIXME: we probably need a deeper test here e.g. for fields of structs.  */
-  if (tree_a
-      && tree_a == tree_b)
+  if (pv_a.m_tree
+      && pv_a == pv_b)
     {
       region_id merged_pointee_rid
-	= merger->m_merged_model->get_lvalue (tree_a, NULL);
+	= merger->m_merged_model->get_lvalue (pv_a, NULL);
       *merged_sid
 	= merger->m_merged_model->get_or_create_ptr_svalue (type,
-								   merged_pointee_rid);
+							    merged_pointee_rid);
       merger->record_regions (a_rid, b_rid, merged_pointee_rid);
       return true;
     }
@@ -1653,6 +1653,8 @@ frame_region::compare_fields (const frame_region &other) const
     return false;
   if (m_fun != other.m_fun)
     return false;
+  if (m_depth != other.m_depth)
+    return false;
   return true;
 }
 
@@ -1672,7 +1674,7 @@ frame_region::print_fields (const region_model &model,
 			    pretty_printer *pp) const
 {
   map_region::print_fields (model, this_rid, pp);
-  pp_printf (pp, ", function: %qs", function_name (m_fun));
+  pp_printf (pp, ", function: %qs, depth: %i", function_name (m_fun), m_depth);
 }
 
 /* FIXME.  */
@@ -1682,6 +1684,7 @@ frame_region::add_to_hash (inchash::hash &hstate) const
 {
   map_region::add_to_hash (hstate);
   hstate.add_ptr (m_fun);
+  hstate.add_int (m_depth);
 }
 
 /* class stack_region : public region.  */
@@ -2043,7 +2046,10 @@ root_region::push_frame (region_model *model, function *fun,
   /* arg_sids can be NULL.  */
 
   ensure_stack_region (model);
-  frame_region *region = new frame_region (m_stack_rid, fun);
+  stack_region *stack = model->get_region <stack_region> (m_stack_rid);
+
+  frame_region *region = new frame_region (m_stack_rid, fun,
+					   stack->get_num_frames ());
   region_id frame_rid = model->add_region (region);
 
   // TODO: unify these cases by building a vec of unknown?
@@ -2104,7 +2110,6 @@ root_region::push_frame (region_model *model, function *fun,
 	}
     }
 
-  stack_region *stack = model->get_region <stack_region> (m_stack_rid);
   stack->push_frame (frame_rid);
 
   return frame_rid;
@@ -3136,8 +3141,10 @@ region_model::handle_phi (tree lhs, tree rhs, bool is_back_edge,
 /* FIXME.  */
 
 region_id
-region_model::get_lvalue (tree expr, region_model_context *ctxt)
+region_model::get_lvalue (path_var pv, region_model_context *ctxt)
 {
+  tree expr = pv.m_tree; // FIXME: take account of stack depth!
+
   if (expr == NULL_TREE)
     return region_id::null ();
 
@@ -3203,7 +3210,11 @@ region_model::get_lvalue (tree expr, region_model_context *ctxt)
 		    || TREE_CODE (expr) == VAR_DECL
 		    || TREE_CODE (expr) == RESULT_DECL);
 
-	region_id frame_rid = get_current_frame_id ();
+	/* FIXME: this is really stack height, rather than depth.  */
+	int stack_depth = pv.m_stack_depth;
+	stack_region *stack = get_root_region ()->get_stack_region (this);
+	gcc_assert (stack);
+	region_id frame_rid = stack->get_frame_rid (stack_depth);
 	frame_region *frame = get_region <frame_region> (frame_rid);
 	gcc_assert (frame);
 	region_id child_rid = frame->get_or_create (this, frame_rid, expr,
@@ -3240,9 +3251,18 @@ region_model::get_lvalue (tree expr, region_model_context *ctxt)
 
 /* FIXME.  */
 
+region_id
+region_model::get_lvalue (tree expr, region_model_context *ctxt)
+{
+  return get_lvalue (path_var (expr, get_stack_depth () - 1), ctxt);
+}
+
+/* FIXME.  */
+
 svalue_id
-region_model::get_rvalue (tree expr, region_model_context *ctxt)
+region_model::get_rvalue (path_var pv, region_model_context *ctxt)
 {
+  tree expr = pv.m_tree; // FIXME: take account of stack depth!
   if (expr == NULL_TREE)
     return svalue_id::null ();
 
@@ -3263,7 +3283,7 @@ region_model::get_rvalue (tree expr, region_model_context *ctxt)
 
     case ARRAY_REF:
       {
-	region_id element_rid = get_lvalue (expr, ctxt);
+	region_id element_rid = get_lvalue (pv, ctxt);
 	return get_region (element_rid)->get_value (*this, true, ctxt);
       }
 
@@ -3279,7 +3299,7 @@ region_model::get_rvalue (tree expr, region_model_context *ctxt)
     case PARM_DECL:
     case RESULT_DECL:
       {
-	region_id var_rid = get_lvalue (expr, ctxt);
+	region_id var_rid = get_lvalue (pv, ctxt);
 	return get_region (var_rid)->get_value (*this, true, ctxt);
       }
     }
@@ -3288,6 +3308,14 @@ region_model::get_rvalue (tree expr, region_model_context *ctxt)
 /* FIXME.  */
 
 svalue_id
+region_model::get_rvalue (tree expr, region_model_context *ctxt)
+{
+  return get_rvalue (path_var (expr, get_stack_depth () - 1), ctxt);
+}
+
+/* FIXME.  */
+
+svalue_id
 region_model::get_or_create_ptr_svalue (tree ptr_type, region_id rid)
 {
   /* Reuse existing region_svalue, if one of the right type is
@@ -3695,28 +3723,36 @@ region_model::get_representative_tree (svalue_id sid) const
   region *region;
   FOR_EACH_VEC_ELT (m_regions, i, region)
     if (sid == region->get_value_direct ())
-      if (tree t = get_representative_tree (region_id::from_int (i)))
-	return t;
+      {
+	path_var pv = get_representative_path_var (region_id::from_int (i));
+	if (pv.m_tree)
+	  return pv.m_tree;
+      }
 
   return NULL_TREE; // FIXME
 }
 
 /* FIXME.  */
+// FIXME: better name than "path_var"?
 
-tree
-region_model::get_representative_tree (region_id rid) const
+path_var
+region_model::get_representative_path_var (region_id rid) const
 {
   region *reg = get_region (rid);
-  if (reg->get_parent () == get_current_frame_id ())
-    {
-      frame_region *frame = get_region <frame_region> (get_current_frame_id ());
-      return frame->get_tree_for_child_region (rid);
+  region *parent_region = get_region (reg->get_parent ());
+  region_id stack_rid = get_stack_region_id ();
+  if (!stack_rid.null_p ())
+    if (parent_region->get_parent () == stack_rid)
+      {
+	frame_region *parent_frame = (frame_region *)parent_region;
+	tree t = parent_frame->get_tree_for_child_region (rid);
+	return path_var (t, parent_frame->get_depth ());
     }
   if (reg->get_parent () == get_globals_region_id ())
     {
       map_region *globals = get_root_region ()->get_globals_region (this);
       if (globals)
-	return globals->get_tree_for_child_region (rid);
+	return path_var (globals->get_tree_for_child_region (rid), -1);
     }
 
   /* Handle e.g. fields of a local by recursing.  */
@@ -3728,29 +3764,33 @@ region_model::get_representative_tree (region_id rid) const
 	{
 	  map_region *parent_map_region = (map_region *)parent_reg;
 	  tree child_key = parent_map_region->get_tree_for_child_region (rid);
-	  tree parent_tree = get_representative_tree (parent_rid);
+	  path_var parent_pv = get_representative_path_var (parent_rid);
 	  /* FIXME: it is always a COMPONENT_REF?  */
-	  if (parent_tree)
-	    return build3 (COMPONENT_REF,
-			   NULL_TREE, /* FIXME: type */
-			   parent_tree, child_key, NULL_TREE);
+	  if (parent_pv.m_tree)
+	    return path_var (build3 (COMPONENT_REF,
+				     NULL_TREE, /* FIXME: type */
+				     parent_pv.m_tree, child_key, NULL_TREE),
+			     parent_pv.m_stack_depth);
 	}
     }
 
-  return NULL_TREE; // FIXME
+  return path_var (NULL_TREE, 0); // FIXME
 }
 
 /* FIXME.  */
 
 void
-region_model::get_trees_for_svalue (svalue_id sid, vec<tree> *out) const
+region_model::get_path_vars_for_svalue (svalue_id sid, vec<path_var> *out) const
 {
   unsigned i;
   region *region;
   FOR_EACH_VEC_ELT (m_regions, i, region)
     if (sid == region->get_value_direct ())
-      if (tree t = get_representative_tree (region_id::from_int (i)))
-	out->safe_push (t);
+      {
+	path_var pv = get_representative_path_var (region_id::from_int (i));
+	if (pv.m_tree)
+	  out->safe_push (pv);
+      }
 }
 
 /* Set DST_RID value to be a new unknown value of type TYPE.  */
@@ -3969,6 +4009,14 @@ region_model::get_root_region () const
   return get_region<root_region> (m_root_rid);
 }
 
+/* FIXME.  */
+
+region_id
+region_model::get_stack_region_id () const
+{
+  return get_root_region ()->get_stack_region_id ();
+}
+
 /* FIXME.
    Return the region_id of the new frame_region.  */
 
@@ -4006,6 +4054,18 @@ region_model::pop_frame (purge_stats *out, region_model_context *ctxt)
 
 /* FIXME.  */
 
+int
+region_model::get_stack_depth () const
+{
+  stack_region *stack = get_root_region ()->get_stack_region (this);
+  if (stack)
+    return stack->get_num_frames ();
+  else
+    return 0;
+}
+
+/* FIXME.  */
+
 region_id
 region_model::get_globals_region_id () const
 {
@@ -5257,45 +5317,58 @@ test_purging ()
 }
 #endif
 
-/* Assert that if we have two region_model instances
-   with values VAL_A and VAL_B for EXPR that they are
-   mergable.  Write the merged model to *OUT_MERGED_MODEL,
-   and the merged svalue ptr to *OUT_MERGED_SVALUE.
-   If VAL_A or VAL_B are NULL_TREE, don't populate EXPR
-   for that region_model.  */
+/* FIXME.  */
 
 static void
-assert_region_models_merge (tree expr, tree val_a, tree val_b,
-			    region_model *out_merged_model,
-			    svalue **out_merged_svalue)
+test_get_representative_path_var ()
 {
-  // FIXME: break this out into a helper function:
-  /* FIXME: might have to be a a class, so that the values can outlive
-     the setup, so we can have custom tests for them.  */
-  test_region_model_context ctxt;
-  region_model model0;
-  region_model model1;
-  if (val_a)
-    model0.set_value (model0.get_lvalue (expr, &ctxt),
-		      model0.get_rvalue (val_a, &ctxt));
-  if (val_b)
-    model1.set_value (model1.get_lvalue (expr, &ctxt),
-		      model1.get_rvalue (val_b, &ctxt));
+  auto_vec <tree> param_types;
+  tree fndecl = make_fndecl (integer_type_node,
+			     "factorial",
+			     param_types);
+  allocate_struct_function (fndecl, true);
+
+  /* Parm "n".  */
+  tree n = build_decl (UNKNOWN_LOCATION, PARM_DECL,
+		       get_identifier ("n"),
+		       integer_type_node);
 
-  /* They should be mergeable.  */
-  ASSERT_TRUE (model0.can_merge_with_p (model1, out_merged_model));
+  region_model model;
 
-  svalue_id merged_svalue_sid = out_merged_model->get_rvalue (expr, &ctxt);
-  *out_merged_svalue = out_merged_model->get_svalue (merged_svalue_sid);
+  /* Push 5 stack frames for "factorial", each with a param  */
+  auto_vec<region_id> parm_rids;
+  auto_vec<svalue_id> parm_sids;
+  for (int depth = 0; depth < 5; depth++)
+    {
+      region_id frame_rid
+	= model.push_frame (DECL_STRUCT_FUNCTION (fndecl), NULL);
+      region_id rid_n = model.get_lvalue (path_var (n, depth), NULL);
+      parm_rids.safe_push (rid_n);
 
-  // FIXME: leave verifications to the caller
-#if 0
-  /* In particular, there should be a constant value for "expr".  */
-  svalue *merged_expr_sval = merged.get_svalue (merged.get_rvalue (expr, &ctxt));
-  ASSERT_EQ (merged_expr_sval->get_kind (), SK_CONSTANT);
-  // FIXME: check cst
-#endif
+      ASSERT_EQ (model.get_region (rid_n)->get_parent (), frame_rid);
+
+      svalue_id sid_n
+	= model.set_to_new_unknown_value (rid_n, integer_type_node);
+      parm_sids.safe_push (sid_n);
+      // FIXME: do something with these?
+    }
 
+  /* Verify that we can recognize that the regions are the parms,
+     at every depth.  */
+  for (int depth = 0; depth < 5; depth++)
+    {
+      ASSERT_EQ (model.get_representative_path_var (parm_rids[depth]),
+		 path_var (n, depth));
+      /* ...and that we can lookup lvalues for locals for all frames,
+	 not just the top.  */
+      ASSERT_EQ (model.get_lvalue (path_var (n, depth), NULL),
+		 parm_rids[depth]);
+      /* ...and that we can locate the svalues.  */
+      auto_vec<path_var> pvs;
+      model.get_path_vars_for_svalue (parm_sids[depth], &pvs);
+      ASSERT_EQ (pvs.length (), 1);
+      ASSERT_EQ (pvs[0], path_var (n, depth));
+    }
 }
 
 /* FIXME.  */
@@ -5376,6 +5449,8 @@ test_canonicalization_3 ()
 
 // FIXME: test for ordering within an EC
 
+#if 0
+
 struct region_model_merger_test
 {
   test_region_model_context ctxt;
@@ -5384,6 +5459,49 @@ struct region_model_merger_test
   region_model m_merged;
 };
 
+#endif
+
+/* Assert that if we have two region_model instances
+   with values VAL_A and VAL_B for EXPR that they are
+   mergable.  Write the merged model to *OUT_MERGED_MODEL,
+   and the merged svalue ptr to *OUT_MERGED_SVALUE.
+   If VAL_A or VAL_B are NULL_TREE, don't populate EXPR
+   for that region_model.  */
+
+static void
+assert_region_models_merge (tree expr, tree val_a, tree val_b,
+			    region_model *out_merged_model,
+			    svalue **out_merged_svalue)
+{
+  // FIXME: break this out into a helper function:
+  /* FIXME: might have to be a a class, so that the values can outlive
+     the setup, so we can have custom tests for them.  */
+  test_region_model_context ctxt;
+  region_model model0;
+  region_model model1;
+  if (val_a)
+    model0.set_value (model0.get_lvalue (expr, &ctxt),
+		      model0.get_rvalue (val_a, &ctxt));
+  if (val_b)
+    model1.set_value (model1.get_lvalue (expr, &ctxt),
+		      model1.get_rvalue (val_b, &ctxt));
+
+  /* They should be mergeable.  */
+  ASSERT_TRUE (model0.can_merge_with_p (model1, out_merged_model));
+
+  svalue_id merged_svalue_sid = out_merged_model->get_rvalue (expr, &ctxt);
+  *out_merged_svalue = out_merged_model->get_svalue (merged_svalue_sid);
+
+  // FIXME: leave verifications to the caller
+#if 0
+  /* In particular, there should be a constant value for "expr".  */
+  svalue *merged_expr_sval = merged.get_svalue (merged.get_rvalue (expr, &ctxt));
+  ASSERT_EQ (merged_expr_sval->get_kind (), SK_CONSTANT);
+  // FIXME: check cst
+#endif
+
+}
+
 /* FIXME.  */
 
 static void
@@ -5737,7 +5855,7 @@ analyzer_region_model_cc_tests ()
 #if 0
   test_purging ();
 #endif
-  // TODO: test coverage for get_representative_tree / get_trees_for_svalue
+  test_get_representative_path_var ();
   test_canonicalization_1 ();
   test_canonicalization_2 ();
   test_canonicalization_3 ();
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index f1f86b9..b8330f0 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -707,12 +707,12 @@ public:
 class frame_region : public map_region
 {
 public:
-  frame_region (region_id parent_rid, function *fun)
-  : map_region (parent_rid, NULL_TREE), m_fun (fun)
+  frame_region (region_id parent_rid, function *fun, int depth)
+  : map_region (parent_rid, NULL_TREE), m_fun (fun), m_depth (depth)
   {}
 
   frame_region (const frame_region &other)
-  : map_region (other), m_fun (other.m_fun)
+  : map_region (other), m_fun (other.m_fun), m_depth (other.m_depth)
   {
   }
 
@@ -724,6 +724,8 @@ public:
 
   function *get_function () const { return m_fun; }
 
+  int get_depth () const { return m_depth; }
+
  private:
   void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE;
   void print_fields (const region_model &model,
@@ -732,6 +734,7 @@ public:
     FINAL OVERRIDE;
 
   function *m_fun;
+  int m_depth;
 };
 
 /* A region containing all stack frames, and implicitly providing
@@ -841,6 +844,7 @@ public:
 		       region_model_context *ctxt);
 
   region_id ensure_stack_region (region_model *model);
+  region_id get_stack_region_id () const { return m_stack_rid; }
   stack_region *get_stack_region (const region_model *model) const;
 
   region_id ensure_globals_region (region_model *model);
@@ -950,10 +954,12 @@ class region_model
 
   root_region *get_root_region () const;
 
+  region_id get_stack_region_id () const;
   region_id push_frame (function *fun, vec<svalue_id> *arg_sids);
   region_id get_current_frame_id () const;
   function * get_current_function () const;
   svalue_id pop_frame (purge_stats *stats, region_model_context *ctxt);
+  int get_stack_depth () const;
 
   region_id get_globals_region_id () const;
 
@@ -975,7 +981,9 @@ class region_model
   void assign_to_lvalue (lvalue *lval,
 			 rvalue *rval);
 #endif
+  region_id get_lvalue (path_var pv, region_model_context *ctxt);
   region_id get_lvalue (tree expr, region_model_context *ctxt);
+  svalue_id get_rvalue (path_var pv, region_model_context *ctxt);
   svalue_id get_rvalue (tree expr, region_model_context *ctxt);
 
   svalue_id get_or_create_ptr_svalue (tree ptr_type, region_id id);
@@ -1002,8 +1010,8 @@ class region_model
   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;
+  path_var get_representative_path_var (region_id rid) const;
+  void get_path_vars_for_svalue (svalue_id sid, vec<path_var> *out) const;
 
   void purge_unused_svalues (purge_stats *out,
 			     region_model_context *ctxt,
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
index 44ecf53..d1de5cb 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-vs-local.c
@@ -135,8 +135,7 @@ int test_pointer_comparison (int n)
   // FIXME: why 6 here?
 
   if (ptr != buf)
-    free (ptr); /* { dg-bogus "not on the heap" "" { xfail *-*-* } } */
-  /* FIXME: how does this fail?  */
+    free (ptr); /* { dg-bogus "not on the heap" } */
 
   return result; /* { dg-bogus "leak" } */
 }
-- 
1.8.5.3

