From 42b61efb2d986e3af7cc292089d2502d4fd5efdf Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Thu, 18 Jul 2019 08:11:16 -0400
Subject: [PATCH 099/108] FIXME: more cleanups

---
 gcc/analyzer/impl-4.cc       |   6 +-
 gcc/analyzer/region-model.cc | 396 +++++++++++++++++++++++++++----------------
 gcc/analyzer/region-model.h  |  29 +++-
 3 files changed, 280 insertions(+), 151 deletions(-)

diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index 7152772..8985428 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -4224,9 +4224,9 @@ program_state::on_edge (exploded_graph &eg,
      This also updates frame information for call/return superedges.  */
 
   impl_region_model_context ctxt (eg, &enode, this, NULL);
-  if (!m_region_model->apply_constraints_for_edge (*succ,
-						   last_stmt,
-						   &ctxt))
+  if (!m_region_model->maybe_update_for_edge (*succ,
+					      last_stmt,
+					      &ctxt))
     {
       eg.log ("edge to SN: %i is impossible due to region_model constraints",
 	      succ->m_dest->m_index);
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 3aa5ab3..7f71e78 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -119,6 +119,8 @@ class impl_constraint_manager : public constraint_manager
 
 ////////////////////////////////////////////////////////////////////////////
 
+/* class svalue_id.  */
+
 /* Print this svalue_id to PP.  */
 
 void
@@ -151,6 +153,8 @@ svalue_id::to_str () const
 
 ////////////////////////////////////////////////////////////////////////////
 
+/* class region_id.  */
+
 /* Print this region_id to PP.  */
 
 void
@@ -222,7 +226,7 @@ id_map<T>::get_src_for_dst (T dst) const
   return m_dst_to_src[dst.as_int ()];
 }
 
-/* FIXME.  */
+/* Dump this id_map to PP.  */
 
 template <typename T>
 void
@@ -258,7 +262,7 @@ id_map<T>::dump_to_pp (pretty_printer *pp) const
   pp_newline (pp);
 }
 
-/* FIXME.  */
+/* Dump this id_map to stderr.  */
 
 template <typename T>
 DEBUG_FUNCTION void
@@ -270,7 +274,7 @@ id_map<T>::dump () const
   pp_flush (&pp);
 }
 
-/* FIXME.  */
+/* Make a std::string based on the dump of this id_map.  */
 
 template <typename T>
 std::string
@@ -310,11 +314,12 @@ id_set<region_id>::id_set (const region_model *model)
 
 ////////////////////////////////////////////////////////////////////////////
 
+/* class svalue and its various subclasses.  */
 
 /* class svalue.  */
 
 /* svalue's equality operator.  Most of the work is done by the
-   compare_fields vfunc.  */
+   a "compare_fields" implementation on each subclass.  */
 
 bool
 svalue::operator== (const svalue &other) const
@@ -429,7 +434,7 @@ svalue::remap_region_ids (const region_id_map &)
   /* Empty.  */
 }
 
-/* FIXME.  */
+/* Base implementation of svalue::walk_for_canonicalization vfunc.  */
 
 void
 svalue::walk_for_canonicalization (canonicalization *) const
@@ -439,7 +444,9 @@ svalue::walk_for_canonicalization (canonicalization *) const
 
 /* class region_svalue : public svalue.  */
 
-/* FIXME.  */
+/* Compare the fields of this region_svalue with OTHER, returning true
+   if they are equal.
+   For use by svalue::operator==.  */
 
 bool
 region_svalue::compare_fields (const region_svalue &other) const
@@ -447,7 +454,7 @@ region_svalue::compare_fields (const region_svalue &other) const
   return m_rid == other.m_rid;
 }
 
-/* FIXME.  */
+/* Implementation of svalue::hash vfunc for region_svalue.  */
 
 hashval_t
 region_svalue::hash () const
@@ -676,7 +683,8 @@ region_svalue::can_merge_values_p (const region_svalue &region_sval_a,
 	}
 }
 
-/* FIXME.  */
+/* Implementation of svalue::walk_for_canonicalization vfunc for
+   region_svalue.  */
 
 void
 region_svalue::walk_for_canonicalization (canonicalization *c) const
@@ -684,9 +692,60 @@ region_svalue::walk_for_canonicalization (canonicalization *c) const
   c->walk_rid (m_rid);
 }
 
+/* Evaluate the condition LHS OP RHS.
+   Subroutine of region_model::eval_condition for when we have a pair of
+   pointers.  */
+
+tristate
+region_svalue::eval_condition (region_svalue *lhs,
+			       enum tree_code op,
+			       region_svalue *rhs)
+{
+  /* See if they point to the same region.  */
+  /* FIXME: what about child regions where the child is the first child
+     (or descendent)?  */
+  region_id lhs_rid = lhs->get_pointee ();
+  region_id rhs_rid = rhs->get_pointee ();
+  switch (op)
+    {
+    default:
+      gcc_unreachable ();
+
+    case EQ_EXPR:
+      if (lhs_rid == rhs_rid)
+	return tristate::TS_TRUE;
+      else
+	return tristate::TS_FALSE;
+      break;
+
+    case NE_EXPR:
+      if (lhs_rid != rhs_rid)
+	return tristate::TS_TRUE;
+      else
+	return tristate::TS_FALSE;
+      break;
+
+    case GE_EXPR:
+    case LE_EXPR:
+      if (lhs_rid == rhs_rid)
+	return tristate::TS_TRUE;
+      break;
+
+    case GT_EXPR:
+    case LT_EXPR:
+      if (lhs_rid == rhs_rid)
+	return tristate::TS_FALSE;
+      break;
+    }
+
+  return tristate::TS_UNKNOWN;
+}
+
 /* class constant_svalue : public svalue.  */
 
-/* FIXME.  */
+/* Compare the fields of this constant_svalue with OTHER, returning true
+   if they are equal.
+   For use by svalue::operator==.  */
 
 bool
 constant_svalue::compare_fields (const constant_svalue &other) const
@@ -695,7 +754,7 @@ constant_svalue::compare_fields (const constant_svalue &other) const
   return m_cst_expr == other.m_cst_expr;
 }
 
-/* FIXME.  */
+/* Implementation of svalue::hash vfunc for constant_svalue.  */
 
 hashval_t
 constant_svalue::hash () const
@@ -736,6 +795,63 @@ constant_svalue::can_merge_values_p (const constant_svalue &cst_sval_a,
   return true;
 }
 
+/* Evaluate the condition LHS OP RHS.
+   Subroutine of region_model::eval_condition for when we have a pair of
+   constants.  */
+
+tristate
+constant_svalue::eval_condition (constant_svalue *lhs,
+				 enum tree_code op,
+				 constant_svalue *rhs)
+{
+  tree lhs_const = lhs->get_constant ();
+  tree rhs_const = rhs->get_constant ();
+
+  /* If one or both of the "constants" are function pointers, then
+     we have equality iff the trees are equal.  */
+  if (TREE_CODE (lhs_const) == FUNCTION_DECL
+      || TREE_CODE (rhs_const) == FUNCTION_DECL)
+    {
+      bool eq = (lhs_const == rhs_const);
+      switch (op)
+	{
+	default:
+	  gcc_unreachable ();
+
+	case EQ_EXPR:
+	  return tristate (eq);
+
+	case GE_EXPR:
+	case LE_EXPR:
+	  if (eq)
+	    return tristate (true);
+	  else
+	    return tristate (tristate::TS_UNKNOWN);
+
+	case NE_EXPR:
+	  return tristate (lhs_const != rhs_const);
+
+	case GT_EXPR:
+	case LT_EXPR:
+	  if (eq)
+	    return tristate (false);
+	  return tristate::TS_UNKNOWN;
+	}
+    }
+
+  /* Otherwise, we have a pair of constants; compare them.  */
+  gcc_assert (CONSTANT_CLASS_P (lhs_const));
+  gcc_assert (CONSTANT_CLASS_P (rhs_const));
+
+  tree comparison
+    = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
+  if (comparison == boolean_true_node)
+    return tristate (tristate::TS_TRUE);
+  if (comparison == boolean_false_node)
+    return tristate (tristate::TS_FALSE);
+  return tristate::TS_UNKNOWN;
+}
+
 /* FIXME.  */
 
 void
@@ -753,7 +869,9 @@ POP_IGNORE_WFORMAT
 
 /* class unknown_svalue : public svalue.  */
 
-/* FIXME.  */
+/* Compare the fields of this unknown_svalue with OTHER, returning true
+   if they are equal.
+   For use by svalue::operator==.  */
 
 bool
 unknown_svalue::compare_fields (const unknown_svalue &) const
@@ -764,7 +882,7 @@ unknown_svalue::compare_fields (const unknown_svalue &) const
   return true;
 }
 
-/* FIXME.  */
+/* Implementation of svalue::hash vfunc for unknown_svalue.  */
 
 hashval_t
 unknown_svalue::hash () const
@@ -802,7 +920,9 @@ poison_kind_to_str (enum poison_kind kind)
 
 /* class poisoned_svalue : public svalue.  */
 
-/* FIXME.  */
+/* Compare the fields of this poisoned_svalue with OTHER, returning true
+   if they are equal.
+   For use by svalue::operator==.  */
 
 bool
 poisoned_svalue::compare_fields (const poisoned_svalue &other) const
@@ -810,7 +930,7 @@ poisoned_svalue::compare_fields (const poisoned_svalue &other) const
   return m_kind == other.m_kind;
 }
 
-/* FIXME.  */
+/* Implementation of svalue::hash vfunc for poisoned_svalue.  */
 
 hashval_t
 poisoned_svalue::hash () const
@@ -830,6 +950,8 @@ poisoned_svalue::print_details (const region_model &model ATTRIBUTE_UNUSED,
 
 ////////////////////////////////////////////////////////////////////////////
 
+/* class region and its various subclasses.  */
+
 /* FIXME.  */
 
 const char *
@@ -854,6 +976,8 @@ region_kind_to_str (enum region_kind kind)
     }
 }
 
+/* class region.  */
+
 /* FIXME.  */
 
 bool
@@ -926,8 +1050,6 @@ region::operator== (const region &other) const
     }
 }
 
-/* class region.  */
-
 void
 region::set_value (const region_model &model ATTRIBUTE_UNUSED,
 		   svalue_id rhs_sid)
@@ -990,7 +1112,8 @@ region::get_value (region_model &model, bool non_null,
   return svalue_id::null ();
 }
 
-/* FIXME.  */
+/* Generate a hash value for this region.  The work is done by the
+   add_to_hash vfunc.  */
 
 hashval_t
 region::hash () const
@@ -1201,7 +1324,9 @@ map_region::map_region (const map_region &other)
 {
 }
 
-/* FIXME.  */
+/* Compare the fields of this map_region with OTHER, returning true
+   if they are equal.
+   For use by region::operator==.  */
 
 bool
 map_region::compare_fields (const map_region &other) const
@@ -1325,7 +1450,7 @@ map_region::get_or_create (region_model *model,
 			   tree expr,
 			   tree type)
 {
-  // TODO
+  gcc_assert (expr);
   region_id *slot = m_map.get (expr);
   if (slot)
     return *slot;
@@ -1364,6 +1489,7 @@ map_region::remap_region_ids (const region_id_map &map)
 void
 map_region::unbind (tree expr)
 {
+  gcc_assert (expr);
   m_map.remove (expr);
 }
 
@@ -1474,7 +1600,8 @@ tree_cmp (const_tree t1, const_tree t2)
   return 0;
 }
 
-/* FIXME.  */
+/* qsort comparator for trees to impose a deterministic ordering on
+   P1 and P2.  */
 
 static int
 tree_cmp (const void *p1, const void *p2)
@@ -1592,7 +1719,8 @@ map_region::can_merge_p (const map_region *map_region_a,
 }
 
 
-/* FIXME.  */
+/* Implementation of region::walk_for_canonicalization vfunc for
+   map_region.  */
 
 void
 map_region::walk_for_canonicalization (canonicalization *c) const
@@ -1647,7 +1775,9 @@ map_region::get_value_by_name (tree identifier,
 
 /* class frame_region : public map_region.  */
 
-/* FIXME.  */
+/* Compare the fields of this frame_region with OTHER, returning true
+   if they are equal.
+   For use by region::operator==.  */
 
 bool
 frame_region::compare_fields (const frame_region &other) const
@@ -1704,7 +1834,9 @@ stack_region::stack_region (const stack_region &other)
     m_frame_rids.safe_push (*frame_rid);
 }
 
-/* FIXME.  */
+/* Compare the fields of this stack_region with OTHER, returning true
+   if they are equal.
+   For use by region::operator==.  */
 
 bool
 stack_region::compare_fields (const stack_region &other) const
@@ -1879,7 +2011,8 @@ stack_region::can_merge_p (const stack_region *stack_region_a,
   return true;
 }
 
-/* FIXME.  */
+/* Implementation of region::walk_for_canonicalization vfunc for
+   stack_region.  */
 
 void
 stack_region::walk_for_canonicalization (canonicalization *c) const
@@ -1892,8 +2025,13 @@ stack_region::walk_for_canonicalization (canonicalization *c) const
 
 /* For debugging purposes: look for a grandchild region within one of
    the child frame regions, where the grandchild is for a decl named
-   IDENTIFIER (or an SSA_NAME for such a decl), returning its value,
-   or svalue_id::null if none are found.  */
+   IDENTIFIER (or an SSA_NAME for such a decl):
+
+     stack_region
+     `-frame_region
+       `-region for decl named IDENTIFIER
+
+   returning its value, or svalue_id::null if none are found.  */
 
 svalue_id
 stack_region::get_value_by_name (tree identifier,
@@ -1921,7 +2059,9 @@ heap_region::heap_region (const heap_region &other)
 {
 }
 
-/* FIXME.  */
+/* Compare the fields of this heap_region with OTHER, returning true
+   if they are equal.
+   For use by region::operator==.  */
 
 bool
 heap_region::compare_fields (const heap_region &) const
@@ -1962,12 +2102,13 @@ heap_region::can_merge_p (const heap_region *heap_a ATTRIBUTE_UNUSED,
   return true;
 }
 
-/* FIXME.  */
+/* Implementation of region::walk_for_canonicalization vfunc for
+   heap_region.  */
 
 void
 heap_region::walk_for_canonicalization (canonicalization *) const
 {
-  // empty
+  /* Empty.  */
 }
 
 /* class root_region : public region.  */
@@ -1991,7 +2132,9 @@ root_region::root_region (const root_region &other)
 {
 }
 
-/* FIXME.  */
+/* Compare the fields of this root_region with OTHER, returning true
+   if they are equal.
+   For use by region::operator==.  */
 
 bool
 root_region::compare_fields (const root_region &other) const
@@ -2140,7 +2283,8 @@ root_region::pop_frame (region_model *model, purge_stats *out,
   return stack->pop_frame (model, out, ctxt);
 }
 
-/* FIXME.  */
+/* Return the region_id of the stack region, creating it if doesn't
+   already exist.  */
 
 region_id
 root_region::ensure_stack_region (region_model *model)
@@ -2156,7 +2300,7 @@ root_region::ensure_stack_region (region_model *model)
   return m_stack_rid;
 }
 
-/* FIXME.  */
+/* Return the stack region (which could be NULL).  */
 
 stack_region *
 root_region::get_stack_region (const region_model *model) const
@@ -2164,7 +2308,8 @@ root_region::get_stack_region (const region_model *model) const
   return model->get_region <stack_region> (m_stack_rid);
 }
 
-/* FIXME.  */
+/* Return the region_id of the globals region, creating it if doesn't
+   already exist.  */
 
 region_id
 root_region::ensure_globals_region (region_model *model)
@@ -2175,7 +2320,7 @@ root_region::ensure_globals_region (region_model *model)
   return m_globals_rid;
 }
 
-/* FIXME.  */
+/* Return the globals region (which could be NULL).  */
 
 map_region *
 root_region::get_globals_region (const region_model *model) const
@@ -2183,7 +2328,8 @@ root_region::get_globals_region (const region_model *model) const
   return model->get_region <map_region> (m_globals_rid);
 }
 
-/* FIXME.  */
+/* Return the region_id of the heap region, creating it if doesn't
+   already exist.  */
 
 region_id
 root_region::ensure_heap_region (region_model *model)
@@ -2199,7 +2345,7 @@ root_region::ensure_heap_region (region_model *model)
   return m_heap_rid;
 }
 
-/* FIXME.  */
+/* Return the heap region (which could be NULL).  */
 
 heap_region *
 root_region::get_heap_region (const region_model *model) const
@@ -2288,7 +2434,8 @@ root_region::add_to_hash (inchash::hash &hstate) const
   inchash::add (m_heap_rid, hstate);
 }
 
-/* FIXME.  */
+/* Implementation of region::walk_for_canonicalization vfunc for
+   root_region.  */
 
 void
 root_region::walk_for_canonicalization (canonicalization *c) const
@@ -2321,7 +2468,6 @@ root_region::get_value_by_name (tree identifier,
   return svalue_id::null ();
 }
 
-
 /* class symbolic_region : public map_region.  */
 
 /* symbolic_region's copy ctor.  */
@@ -2331,7 +2477,9 @@ symbolic_region::symbolic_region (const symbolic_region &other)
 {
 }
 
-/* FIXME.  */
+/* Compare the fields of this symbolic_region with OTHER, returning true
+   if they are equal.
+   For use by region::operator==.  */
 
 bool
 symbolic_region::compare_fields (const symbolic_region &) const
@@ -2455,7 +2603,7 @@ region_model::operator== (const region_model &other) const
   return true;
 }
 
-/* FIXME.  */
+/* Generate a hash value for this region_model.  */
 
 hashval_t
 region_model::hash () const
@@ -3351,7 +3499,10 @@ region_model::get_or_create_ptr_svalue (tree ptr_type, region_id rid)
 #endif
 }
 
-/* FIXME.  */
+/* Return an svalue_id for a constant_svalue for CST_EXPR,
+   creating the constant_svalue if necessary.
+   The constant_svalue instances are reused, based on pointer equality
+   of trees  */
 
 svalue_id
 region_model::get_or_create_constant_svalue (tree cst_expr)
@@ -3369,7 +3520,10 @@ region_model::get_or_create_constant_svalue (tree cst_expr)
   return cst_sid;
 }
 
-/* FIXME.  */
+/* Return an svalue_id for a constant_svalue for FNDECL,
+   creating the constant_svalue if necessary.
+   The constant_svalue instances are reused, based on pointer equality
+   of trees  */
 
 svalue_id
 region_model::get_svalue_for_fndecl (tree fndecl)
@@ -3520,7 +3674,8 @@ region_model::deref_rvalue (tree ptr, region_model_context *ctxt)
   return region_id::null ();
 }
 
-/* FIXME.  */
+/* Set the value of the region given by LHS_RID to the value given
+   by RHS_SID.  */
 
 void
 region_model::set_value (region_id lhs_rid, svalue_id rhs_sid)
@@ -3530,7 +3685,8 @@ region_model::set_value (region_id lhs_rid, svalue_id rhs_sid)
   get_region (lhs_rid)->set_value (*this, rhs_sid);
 }
 
-/* FIXME.  */
+/* Determine what is known about the condition "LHS_SID OP RHS_SID" within
+   this model.  */
 
 tristate
 region_model::eval_condition (svalue_id lhs_sid,
@@ -3542,6 +3698,7 @@ region_model::eval_condition (svalue_id lhs_sid,
   gcc_assert (lhs);
   gcc_assert (rhs);
 
+  /* See what we know based on the values.  */
   if (lhs && rhs)
     {
       if (lhs == rhs)
@@ -3565,103 +3722,30 @@ region_model::eval_condition (svalue_id lhs_sid,
 	    }
 	}
 
-      /* If we have a pair of region_svalues, see if they point to the same
-	 region.  */
+      /* If we have a pair of region_svalues, compare them.  */
       if (region_svalue *lhs_ptr = lhs->dyn_cast_region_svalue ())
 	if (region_svalue *rhs_ptr = rhs->dyn_cast_region_svalue ())
 	  {
-	    switch (op)
-	      {
-	      default:
-		gcc_unreachable ();
-
-	      case EQ_EXPR:
-		if (lhs_ptr->get_pointee () == rhs_ptr->get_pointee ())
-		  return tristate::TS_TRUE;
-		else
-		  return tristate::TS_FALSE;
-		break;
-
-	      case NE_EXPR:
-		if (lhs_ptr->get_pointee () != rhs_ptr->get_pointee ())
-		  return tristate::TS_TRUE;
-		else
-		  return tristate::TS_FALSE;
-		break;
-
-	      case GE_EXPR:
-	      case LE_EXPR:
-		if (lhs_ptr->get_pointee () == rhs_ptr->get_pointee ())
-		  return tristate::TS_TRUE;
-		break;
-
-	      case GT_EXPR:
-	      case LT_EXPR:
-		if (lhs_ptr->get_pointee () == rhs_ptr->get_pointee ())
-		  return tristate::TS_FALSE;
-		break;
-	      }
+	    tristate res = region_svalue::eval_condition (lhs_ptr, op, rhs_ptr);
+	    if (res.is_known ())
+	      return res;
 	    /* Otherwise, try constraints.  */
 	  }
 
       /* If we have a pair of constants, compare them.  */
       if (constant_svalue *cst_lhs = lhs->dyn_cast_constant_svalue ())
 	if (constant_svalue *cst_rhs = rhs->dyn_cast_constant_svalue ())
-	  {
-	    tree lhs_const = cst_lhs->get_constant ();
-	    tree rhs_const = cst_rhs->get_constant ();
-
-	    /* If one or both of the "constants" are function pointers, then
-	       we have equality iff the trees are equal.  */
-	    if (TREE_CODE (lhs_const) == FUNCTION_DECL
-		|| TREE_CODE (rhs_const) == FUNCTION_DECL)
-	      {
-		bool eq = (lhs_const == rhs_const);
-		switch (op)
-		  {
-		  default:
-		    gcc_unreachable ();
-
-		  case EQ_EXPR:
-		    return tristate (eq);
-
-		  case GE_EXPR:
-		  case LE_EXPR:
-		    if (eq)
-		      return tristate (true);
-		    else
-		      return tristate (tristate::TS_UNKNOWN);
-
-		  case NE_EXPR:
-		    return tristate (lhs_const != rhs_const);
-
-		  case GT_EXPR:
-		  case LT_EXPR:
-		    if (eq)
-		      return tristate (false);
-		    return tristate::TS_UNKNOWN;
-		  }
-	      }
-
-	    /* Otherwise, we have a pair of constants; compare them.  */
-	    gcc_assert (CONSTANT_CLASS_P (lhs_const));
-	    gcc_assert (CONSTANT_CLASS_P (rhs_const));
-
-	    tree comparison
-	      = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
-	    if (comparison == boolean_true_node)
-	      return tristate (tristate::TS_TRUE);
-	    if (comparison == boolean_false_node)
-	      return tristate (tristate::TS_FALSE);
-	    return tristate::TS_UNKNOWN;
-	  }
+	  return constant_svalue::eval_condition (cst_lhs, op, cst_rhs);
     }
 
   /* Otherwise, try constraints.  */
   return m_constraints->eval_condition (lhs_sid, op, rhs_sid);
 }
 
-/* FIXME.  */
+/* Attempt to add the constraint "LHS OP RHS" to this region_model.
+   If it is consistent with existing constraints, add it, and return true.
+   Return false if it contradicts existing constraints.
+   Use CTXT for reporting any diagnostics associated with the accesses.  */
 
 bool
 region_model::add_constraint (tree lhs, enum tree_code op, tree rhs,
@@ -3687,7 +3771,9 @@ region_model::add_constraint (tree lhs, enum tree_code op, tree rhs,
   return true;
 }
 
-/* FIXME.  */
+/* Determine what is known about the condition "LHS OP RHS" within
+   this model.
+   Use CTXT for reporting any diagnostics associated with the accesses.  */
 
 tristate
 region_model::eval_condition (tree lhs,
@@ -3711,7 +3797,9 @@ region_model::maybe_get_constant (svalue_id sid)
   return NULL_TREE;
 }
 
-/* FIXME.  */
+/* Create a new child region of the heap (creating the heap region if
+   necessary).
+   Return the region_id of the new child region.  */
 
 region_id
 region_model::add_new_malloc_region ()
@@ -3817,13 +3905,29 @@ region_model::set_to_new_unknown_value (region_id dst_rid, tree type)
   return new_sid;
 }
 
-/* FIXME.  */
-// FIXME: this also updates frame information
+/* Attempt to update this model for taking EDGE (where the last statement
+   was LAST_STMT), returning true if the edge can be taken, false
+   otherwise.
+
+   For CFG superedges where LAST_STMT is a conditional or a switch
+   statement, attempt to add the relevant conditions for EDGE to this
+   model, returning true if they are feasible, or false if they are
+   impossible.
+
+   For call superedges, push frame information and store arguments
+   into parameters.
+
+   For return superedges, pop frame information and store return
+   values into any lhs.
+
+   Rejection of call/return superedges happens elsewhere, in
+   program_point::on_edge (i.e. based on program point, rather
+   than program state).  */
 
 bool
-region_model::apply_constraints_for_edge (const superedge &edge,
-					  const gimple *last_stmt,
-					  region_model_context *ctxt)
+region_model::maybe_update_for_edge (const superedge &edge,
+				     const gimple *last_stmt,
+				     region_model_context *ctxt)
 {
   /* Handle frame updates for interprocedural edges.  */
   switch (edge.m_kind)
@@ -3859,8 +3963,9 @@ region_model::apply_constraints_for_edge (const superedge &edge,
     return apply_constraints_for_gcond (edge, cond_stmt, ctxt);
 
   // FIXME: some linker issues with is_a_helper const
-  if (const gswitch *switch_stmt = dyn_cast <gswitch *> (const_cast <gimple *> (last_stmt)))
-    return apply_constraints_for_gswitch (edge, switch_stmt,ctxt);
+  if (const gswitch *switch_stmt
+	= dyn_cast <gswitch *> (const_cast <gimple *> (last_stmt)))
+    return apply_constraints_for_gswitch (edge, switch_stmt, ctxt);
 
   return true;
 }
@@ -4781,7 +4886,7 @@ svalue_id_merger_mapping::dump () const
 
 /* struct canonicalization.  */
 
-/* FIXME.  */
+/* canonicalization's ctor.  */
 
 canonicalization::canonicalization (const region_model &model)
 : m_model (model),
@@ -4792,7 +4897,8 @@ canonicalization::canonicalization (const region_model &model)
 {
 }
 
-/* FIXME.  */
+/* If we've not seen RID yet, assign it a canonicalized region_id,
+   and walk the region's svalue and then the region.  */
 
 void
 canonicalization::walk_rid (region_id rid)
@@ -4810,7 +4916,8 @@ canonicalization::walk_rid (region_id rid)
     }
 }
 
-/* FIXME.  */
+/* If we've not seen SID yet, assign it a canonicalized svalue_id,
+   and walk the svalue (and potentially regions e.g. for ptr values).  */
 
 void
 canonicalization::walk_sid (svalue_id sid)
@@ -4828,7 +4935,7 @@ canonicalization::walk_sid (svalue_id sid)
     }
 }
 
-/* FIXME.  */
+/* Dump a multiline representation of this to PP.  */
 
 void
 canonicalization::dump_to_pp (pretty_printer *pp) const
@@ -5205,7 +5312,8 @@ test_stack_frames ()
   ASSERT_EQ (purged.m_num_equiv_classes, 1);
   ASSERT_EQ (purged.m_num_constraints, 1);
 
-  /* Verify that p has been poisoned.  */
+  /* Verify that p (which was pointing at the local "x" in the popped
+     frame) has been poisoned.  */
   svalue *new_p_sval = model.get_svalue (model.get_rvalue (p, &ctxt));
   ASSERT_EQ (new_p_sval->get_kind (), SK_POISONED);
   ASSERT_EQ (new_p_sval->dyn_cast_poisoned_svalue ()->get_poison_kind (),
@@ -5383,7 +5491,8 @@ test_get_representative_path_var ()
     }
 }
 
-/* FIXME.  */
+/* Verify that the core regions within a region_model are in a consistent
+   order after canonicalization.  */
 
 static void
 test_canonicalization_1 ()
@@ -5514,7 +5623,7 @@ assert_region_models_merge (tree expr, tree val_a, tree val_b,
 
 }
 
-/* FIXME.  */
+/* Verify that we can merge region_model instances.  */
 
 static void
 test_state_merging ()
@@ -5804,7 +5913,8 @@ test_state_merging ()
    */
 }
 
-/* FIXME.  */
+/* Verify that constraints are correctly merged when merging region_model
+   instances.  */
 
 static void
 test_constraint_merging ()
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index b8330f0..bca5408 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -413,6 +413,10 @@ public:
 
   void walk_for_canonicalization (canonicalization *c) const FINAL OVERRIDE;
 
+  static tristate eval_condition (region_svalue *lhs_ptr,
+				  enum tree_code op,
+				  region_svalue *rhs_ptr);
+
  private:
   void print_details (const region_model &model,
 		      svalue_id this_sid,
@@ -430,7 +434,11 @@ class constant_svalue : public svalue
 public:
   constant_svalue (tree cst_expr)
   : svalue (TREE_TYPE (cst_expr)), m_cst_expr (cst_expr)
-  {}
+  {
+    gcc_assert (cst_expr);
+    gcc_assert (TREE_CODE (cst_expr) == FUNCTION_DECL
+		|| CONSTANT_CLASS_P (cst_expr));
+  }
 
   bool compare_fields (const constant_svalue &other) const;
 
@@ -452,6 +460,10 @@ public:
 				  svalue_id *merged_sid,
 				  model_merger *merger);
 
+  static tristate eval_condition (constant_svalue *lhs,
+				  enum tree_code op,
+				  constant_svalue *rhs);
+
  private:
   void print_details (const region_model &model,
 		      svalue_id this_sid,
@@ -948,9 +960,9 @@ class region_model
   void handle_phi (tree lhs, tree rhs, bool is_back_edge,
 		   region_model_context *ctxt);
 
-  bool apply_constraints_for_edge (const superedge &edge,
-				   const gimple *last_stmt,
-				   region_model_context *ctxt);
+  bool maybe_update_for_edge (const superedge &edge,
+			      const gimple *last_stmt,
+			      region_model_context *ctxt);
 
   root_region *get_root_region () const;
 
@@ -1202,7 +1214,12 @@ struct svalue_id_merger_mapping
 
 /* A bundle of data used when canonicalizing a region_model so that the
    order of regions and svalues is in a predictable order (thus increasing
-   the chance of two region_models being equal).  */
+   the chance of two region_models being equal).
+
+   This object is used to keep track of a recursive traversal across the
+   svalues and regions within the model, made in a deterministic order,
+   assigning new ids the first time each region or svalue is
+   encountered.  */
 
 struct canonicalization
 {
@@ -1215,8 +1232,10 @@ struct canonicalization
   void dump () const;
 
   const region_model &m_model;
+  /* Maps from existing IDs to new IDs.  */
   region_id_map m_rid_map;
   svalue_id_map m_sid_map;
+  /* The next IDs to hand out.  */
   int m_next_rid_int;
   int m_next_sid_int;
 };
-- 
1.8.5.3

