From c0ba2f98f76acf3b0b070715c95a1607130877b9 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 19 Jul 2019 17:49:48 -0400
Subject: [PATCH 108/108] FIXME: cleanups

---
 gcc/analyzer/analyzer-selftests.h  |   2 +-
 gcc/analyzer/constraint-manager.cc |  75 +++++-----
 gcc/analyzer/constraint-manager.h  |   5 +-
 gcc/analyzer/digraph.h             |  20 ++-
 gcc/analyzer/impl-4.cc             | 298 ++++++++++++++++++++-----------------
 gcc/analyzer/pending-diagnostic.h  |   6 +-
 gcc/analyzer/region-model.cc       |  20 ++-
 gcc/analyzer/region-model.h        |  17 ++-
 gcc/analyzer/sm.h                  |   2 +-
 gcc/analyzer/supergraph.cc         |  52 +++++--
 gcc/analyzer/supergraph.h          |  26 +++-
 11 files changed, 297 insertions(+), 226 deletions(-)

diff --git a/gcc/analyzer/analyzer-selftests.h b/gcc/analyzer/analyzer-selftests.h
index e29da5a..f427411 100644
--- a/gcc/analyzer/analyzer-selftests.h
+++ b/gcc/analyzer/analyzer-selftests.h
@@ -1,4 +1,4 @@
-/* FIXME.
+/* Selftests for the analyzer.
    Copyright (C) 2019 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
 
diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index 5138970..640c975 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -205,20 +205,13 @@ equiv_class::equiv_class (const equiv_class &other)
   m_vars (),
   m_escaped (other.m_escaped)
 {
-  // FIXME: copy-ctor for auto_vec!
   int i;
   svalue_id *sid;
   FOR_EACH_VEC_ELT (other.m_vars, i, sid)
     m_vars.safe_push (*sid);
 }
 
-/* FIXME.  */
-
-equiv_class::~equiv_class ()
-{
-}
-
-/* FIXME.  */
+/* Generate a std::string for this equiv_class.  */
 
 std::string
 equiv_class::to_str () const
@@ -243,7 +236,7 @@ equiv_class::to_str () const
   return result + "}";
 }
 
-/* FIXME.  */
+/* Generate a hash value for this equiv_class.  */
 
 hashval_t
 equiv_class::hash () const
@@ -260,7 +253,7 @@ equiv_class::hash () const
   return hstate.end ();
 }
 
-/* FIXME.  */
+/* Equality operator for equiv_class.  */
 
 bool
 equiv_class::operator== (const equiv_class &other)
@@ -284,7 +277,7 @@ equiv_class::operator== (const equiv_class &other)
   return true;
 }
 
-/* FIXME.  */
+/* Add SID to this equiv_class, using CM to check if it's a constant.  */
 
 void
 equiv_class::add (svalue_id sid, const constraint_manager &cm)
@@ -350,7 +343,7 @@ equiv_class::get_representative () const
     }
 }
 
-/* FIXME.  */
+/* Remap all svalue_ids within this equiv_class using MAP.  */
 
 void
 equiv_class::remap_svalue_ids (const svalue_id_map &map)
@@ -382,7 +375,7 @@ equiv_class::canonicalize ()
 
 ////////////////////////////////////////////////////////////////////////////
 
-/* FIXME.  */
+/* Get a debug string for C_OP.  */
 
 const char *
 constraint_op_code (enum constraint_op c_op)
@@ -397,7 +390,7 @@ constraint_op_code (enum constraint_op c_op)
     }
 }
 
-/* FIXME.  */
+/* Convert C_OP to an enum tree_code.  */
 
 enum tree_code
 constraint_tree_code (enum constraint_op c_op)
@@ -449,7 +442,8 @@ eval_constraint_op_for_op (enum constraint_op c_op, enum tree_code t_op)
 
 /* class constraint.  */
 
-/* FIXME.  */
+/* Generate a std::string for this constraint, using CM to look up
+   equiv_class instances from ids.  */
 
 std::string
 constraint::to_str (const constraint_manager &cm) const
@@ -463,7 +457,7 @@ constraint::to_str (const constraint_manager &cm) const
 	  + m_rhs.get_obj (cm).to_str ());
 }
 
-/* FIXME.  */
+/* Generate a hash value for this constraint.  */
 
 hashval_t
 constraint::hash () const
@@ -475,7 +469,7 @@ constraint::hash () const
   return hstate.end ();
 }
 
-/* FIXME.  */
+/* Equality operator for constraints.  */
 
 bool
 constraint::operator== (const constraint &other) const
@@ -509,6 +503,8 @@ equiv_class_id::get_obj (constraint_manager &cm) const
   return cm.get_equiv_class_by_index (m_idx);
 }
 
+/* FIXME.  */
+
 void
 equiv_class_id::print (pretty_printer *pp) const
 {
@@ -518,6 +514,8 @@ equiv_class_id::print (pretty_printer *pp) const
     pp_printf (pp, "ec%i", m_idx);
 }
 
+/* FIXME.  */
+
 std::string
 equiv_class_id::to_str () const
 {
@@ -544,17 +542,7 @@ constraint_manager::constraint_manager (const constraint_manager &other)
     m_constraints.safe_push (*c);
 }
 
-/* FIXME.  */
-
-constraint_manager::~constraint_manager ()
-{
-  int i;
-  equiv_class *ec;
-  FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
-    delete ec;
-}
-
-/* FIXME.  */
+/* constraint_manager's assignment operator.  */
 
 constraint_manager&
 constraint_manager::operator= (const constraint_manager &other)
@@ -573,7 +561,7 @@ constraint_manager::operator= (const constraint_manager &other)
   return *this;
 }
 
-/* FIXME.  */
+/* Generate a hash value for this constraint_manager.  */
 
 hashval_t
 constraint_manager::hash () const
@@ -590,7 +578,7 @@ constraint_manager::hash () const
   return hstate.end ();
 }
 
-/* FIXME.  */
+/* Equality operator for constraint_manager.  */
 
 bool
 constraint_manager::operator== (const constraint_manager &other) const
@@ -699,7 +687,9 @@ constraint_manager::dump () const
   dump (stderr);
 }
 
-/* FIXME.  */
+/* Attempt to add the constraint LHS OP RHS to this constraint_manager.
+   Return true if the constraint could be added (or is already true).
+   Return false if the constraint contradicts existing knowledge.  */
 
 bool
 constraint_manager::add_constraint (svalue_id lhs,
@@ -711,7 +701,10 @@ constraint_manager::add_constraint (svalue_id lhs,
   return add_constraint (lhs_ec_id, op,rhs_ec_id);
 }
 
-/* FIXME.  */
+/* Attempt to add the constraint LHS_EC_ID OP RHS_EC_ID to this
+   constraint_manager.
+   Return true if the constraint could be added (or is already true).
+   Return false if the constraint contradicts existing knowledge.  */
 
 bool
 constraint_manager::add_constraint (equiv_class_id lhs_ec_id,
@@ -805,7 +798,8 @@ constraint_manager::add_constraint (equiv_class_id lhs_ec_id,
 
 ////////////////////////////////////////////////////////////////////////////
 
-/* FIXME.  */
+/* Subroutine of constraint_manager::add_constraint, for handling all
+   operations other than equality (for which equiv classes are merged).  */
 
 void
 constraint_manager::add_constraint_internal (equiv_class_id lhs_id,
@@ -1055,12 +1049,12 @@ constraint_manager::dump_condition (tree lhs, enum tree_code op, tree rhs)
 }
 #endif
 
-/* FIXME.  */
+/* Evaluate the condition LHS_EC OP RHS_EC.  */
 
 tristate
 constraint_manager::eval_condition (equiv_class_id lhs_ec,
-			    enum tree_code op,
-			    equiv_class_id rhs_ec)
+				    enum tree_code op,
+				    equiv_class_id rhs_ec)
 {
   if (lhs_ec == rhs_ec)
     {
@@ -1120,7 +1114,8 @@ constraint_manager::eval_condition (equiv_class_id lhs_ec,
   return tristate (tristate::TS_UNKNOWN);
 }
 
-/* FIXME.  */
+/* Evaluate the condition LHS OP RHS, creating equiv_class instances for
+   LHS and RHS if they aren't already in equiv_classes.  */
 
 tristate
 constraint_manager::eval_condition (svalue_id lhs,
@@ -1326,7 +1321,7 @@ constraint_manager::purge (const purge_criteria &p, purge_stats *stats)
   validate ();
 }
 
-/* FIXME.  */
+/* Remap all svalue_ids within this constraint_manager using MAP.  */
 
 void
 constraint_manager::remap_svalue_ids (const svalue_id_map &map)
@@ -1687,9 +1682,9 @@ constraint_manager::validate () const
   FOR_EACH_VEC_ELT (m_constraints, i, c)
     {
       gcc_assert (!c->m_lhs.null_p ());
-      gcc_assert (c->m_lhs.as_int () <= m_equiv_classes.length ());
+      gcc_assert (c->m_lhs.as_int () <= (int)m_equiv_classes.length ());
       gcc_assert (!c->m_rhs.null_p ());
-      gcc_assert (c->m_rhs.as_int () <= m_equiv_classes.length ());
+      gcc_assert (c->m_rhs.as_int () <= (int)m_equiv_classes.length ());
     }
 }
 
diff --git a/gcc/analyzer/constraint-manager.h b/gcc/analyzer/constraint-manager.h
index 7d11e8b..6acfe35 100644
--- a/gcc/analyzer/constraint-manager.h
+++ b/gcc/analyzer/constraint-manager.h
@@ -47,7 +47,6 @@ class equiv_class
 public:
   equiv_class ();
   equiv_class (const equiv_class &other);
-  ~equiv_class ();
 
   std::string to_str () const;
   hashval_t hash () const;
@@ -171,7 +170,7 @@ class constraint_manager
 public:
   constraint_manager () {}
   constraint_manager (const constraint_manager &other);
-  virtual ~constraint_manager ();
+  virtual ~constraint_manager () {}
 
   virtual constraint_manager *clone (region_model *) const = 0;
   virtual tree maybe_get_constant (svalue_id sid) const = 0;
@@ -244,7 +243,7 @@ public:
 
   void validate () const;
 
-  auto_vec<equiv_class *> m_equiv_classes;
+  auto_delete_vec<equiv_class> m_equiv_classes;
   auto_vec<constraint> m_constraints;
 
  private:
diff --git a/gcc/analyzer/digraph.h b/gcc/analyzer/digraph.h
index 09cc77f..7e7f2e5 100644
--- a/gcc/analyzer/digraph.h
+++ b/gcc/analyzer/digraph.h
@@ -1,4 +1,4 @@
-/* FIXME.
+/* Template classes for directed graphs.
    Copyright (C) 2019 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
 
@@ -28,7 +28,11 @@ along with GCC; see the file COPYING3.  If not see
    node_t: the node class
    edge_t: the edge class
    dump_args_t: additional args for dot-dumps
-   cluster_t: the cluster class (for use when generating .dot files).  */
+   cluster_t: the cluster class (for use when generating .dot files).
+
+   Using a template allows for typesafe nodes and edges: a node's
+   predecessor and successor edges can be of a node-specific edge
+   subclass, without needing casting.  */
 
 /* Abstract base class for a node in a directed graph.  */
 
@@ -133,7 +137,9 @@ class cluster
 
 ////////////////////////////////////////////////////////////////////////////
 
-/* FIXME.  */
+/* Write .dot information for this graph to PP, passing ARGS to the nodes
+   and edges.
+   If ROOT_CLUSTER is non-NULL, use it to organize the nodes into clusters.  */
 
 template <typename GraphTraits>
 inline void
@@ -188,7 +194,9 @@ digraph<GraphTraits>::dump_dot_to_pp (pretty_printer *pp,
   pp_newline (pp);
 }
 
-/* FIXME.  */
+/* Write .dot information for this graph to FP, passing ARGS to the nodes
+   and edges.
+   If ROOT_CLUSTER is non-NULL, use it to organize the nodes into clusters.  */
 
 template <typename GraphTraits>
 inline void
@@ -204,7 +212,9 @@ digraph<GraphTraits>::dump_dot_to_file (FILE *fp,
   pp_flush (&pp);
 }
 
-/* FIXME.  */
+/* Write .dot information for this graph to a file at PATH, passing ARGS
+   to the nodes and edges.
+   If ROOT_CLUSTER is non-NULL, use it to organize the nodes into clusters.  */
 
 template <typename GraphTraits>
 inline void
diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index e10f23e..e06dc46 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -345,7 +345,7 @@ enum point_kind {
   NUM_POINT_KINDS
 };
 
-/* FIXME.  */
+/* Get a string for PK.  */
 
 static const char *
 point_kind_to_string (enum point_kind pk)
@@ -818,13 +818,13 @@ public:
   void dump_to_file (const extrinsic_state &ext_state, FILE *outf) const;
   void dump (const extrinsic_state &ext_state) const;
 
-  program_state prune_for_point (exploded_graph &eg,
-				 const program_point &point) const;
-
   bool on_edge (exploded_graph &eg,
 		const exploded_node &enode,
 		const superedge *succ);
 
+  program_state prune_for_point (exploded_graph &eg,
+				 const program_point &point) const;
+
   void remap_svalue_ids (const svalue_id_map &map);
 
 #if 1
@@ -895,7 +895,7 @@ struct point_and_state
   hashval_t m_hash;
 };
 
-/* FIXME.  */
+/* A traits class for exploded graphs and their nodes and edges.  */
 
 struct eg_traits
 {
@@ -2412,6 +2412,73 @@ program_point::get_stmt () const
     return NULL;
 }
 
+/* Check to see if SUCC is a valid edge to take (ensuring that we have
+   interprocedurally valid paths in the exploded graph, and enforcing
+   recursion limits).
+
+   Return true if SUCC can be taken, or false otherwise.
+
+   This is the "point" half of exploded_node::on_edge.  */
+
+bool
+program_point::on_edge (exploded_graph &eg,
+			const superedge *succ)
+{
+  JIT_LOG_FUNC (eg.get_logger ());
+  switch (succ->m_kind)
+    {
+    case SUPEREDGE_CFG_EDGE:
+      break;
+
+    case SUPEREDGE_CALL:
+      {
+	/* Add the callsite to the call string.  */
+	// FIXME: this should be an as_a<>
+	m_call_string.push_call (eg.get_supergraph (),
+				 (const call_superedge *)succ);
+
+	/* Impose a maximum recursion depth and don't analyze paths
+	   that exceed it further.
+	   This is something of a blunt workaround, but it only
+	   applies to recursion (and mutual recursion), not to
+	   general call stacks.  */
+	// FIXME: make this a param:
+	const int MAX_RECURSION_DEPTH = 2;
+	if (m_call_string.calc_recursion_depth () > MAX_RECURSION_DEPTH)
+	  {
+	    eg.log ("rejecting call edge: recursion limit exceeded");
+	    // TODO: issue a sorry for this?
+	    return false;
+	  }
+      }
+      break;
+
+    case SUPEREDGE_RETURN:
+      {
+	/* Require that we return to the call site in the call string.  */
+	if (m_call_string.empty_p ())
+	  {
+	    eg.log ("rejecting return edge: empty call string");
+	    return false;
+	  }
+	const return_superedge *top_of_stack = m_call_string.pop ();
+	if (top_of_stack != succ)
+	  {
+	    eg.log ("rejecting return edge: return to wrong callsite");
+	    return false;
+	  }
+      }
+      break;
+
+    case SUPEREDGE_INTRAPROCEDURAL_CALL:
+      /* We ignore these edges.  FIXME: why have them?  */
+      eg.log ("rejecting interprocedural edge");
+      return false;
+    }
+
+  return true;
+}
+
 /* Comparator for program points within the same supernode,
    for implementing worklist::key_t comparison operators.
    Return negative if POINT_A is before POINT_B
@@ -3513,6 +3580,70 @@ program_state::dump (const extrinsic_state &ext_state) const
   dump_to_file (ext_state, stderr);
 }
 
+/* Determine if following edge SUCC from ENODE is valid within the graph EG
+   and update this state accordingly in-place.
+
+   Return true if the edge can be followed, or false otherwise.
+
+   Check for relevant conditionals and switch-values for conditionals
+   and switch statements, adding the relevant conditions to this state.
+   Push/pop frames for interprocedural edges and update params/returned
+   values.
+
+   This is the "state" half of exploded_node::on_edge.  */
+
+bool
+program_state::on_edge (exploded_graph &eg,
+			const exploded_node &enode,
+			const superedge *succ)
+{
+  /* Update state.  */
+
+  if (succ->m_kind == SUPEREDGE_INTRAPROCEDURAL_CALL)
+    {
+      /* We ignore these edges.  FIXME: why have them?  */
+      eg.log ("rejecting interprocedural edge");
+      return false;
+    }
+
+  const program_point &point = enode.get_point ();
+  const program_state &old_state = enode.get_state ();
+  const gimple *last_stmt = point.m_supernode->get_last_stmt ();
+
+  /* For conditionals and switch statements, add the
+     relevant conditions (for the specific edge) to new_state;
+     skip edges for which the resulting constraints
+     are impossible.
+     This also updates frame information for call/return superedges.  */
+
+  impl_region_model_context ctxt (eg, &enode, this, NULL);
+  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);
+      return false;
+    }
+
+  /* Pass the edge to the various state machines to see if any transitions
+     or warnings occur (e.g. transitions due to ptrs becoming known to be NULL
+     or non-NULL).  */
+  int i;
+  sm_state_map *old_smap;
+  FOR_EACH_VEC_ELT (old_state.m_checker_states, i, old_smap)
+    {
+      const state_machine &sm = eg.get_ext_state ().get_sm (i);
+      impl_sm_context ctxt (eg, sm, &enode,
+			    this,
+			    old_state.m_checker_states[i],
+			    m_checker_states[i]);
+      sm.on_edge (&ctxt, last_stmt, succ);
+    }
+
+  return true;
+}
+
 /* Implementation of purge_criteria for using state_purge_map.
 
    Purge state within a path_state using PURGE_MAP for POINT.
@@ -3617,8 +3748,8 @@ program_state::prune_for_point (exploded_graph &eg,
 	 etc decls are visible there.  */
 
       auto_vec<tree> ssa_names_to_purge;
-      for (frame_region::map_t::iterator iter = frame->m_map.begin ();
-	   iter != frame->m_map.end ();
+      for (frame_region::map_t::iterator iter = frame->begin ();
+	   iter != frame->end ();
 	   ++iter)
 	{
 	  tree var = (*iter).first;
@@ -4189,8 +4320,7 @@ exploded_node::on_stmt (exploded_graph &eg) const
   return new_state;
 }
 
-/* FIXME.
-   Consider the effect of following superedge SUCC from this node.
+/* Consider the effect of following superedge SUCC from this node.
 
    Return true if it's feasible to follow the edge, or false
    if it's infeasible.
@@ -4201,7 +4331,8 @@ exploded_node::on_stmt (exploded_graph &eg) const
    the edge back to the most recent callsite is feasible.
 
    Update NEXT_STATE accordingly (e.g. to record that a condition was
-   true or false, or that the NULL-ness of a pointer has been checked).
+   true or false, or that the NULL-ness of a pointer has been checked,
+   pushing/popping stack frames, etc).
 
    Update NEXT_POINT accordingly (updating the call string).  */
 
@@ -4287,125 +4418,6 @@ exploded_node::dump_succs_and_preds (FILE *outf) const
   }
 }
 
-
-// TODO: move this fn
-/* Update the call string based on interprocedural
-   edges, ensuring that we have interprocedurally valid paths
-   in the exploded graph.  */
-
-bool
-program_point::on_edge (exploded_graph &eg,
-			const superedge *succ)
-{
-  JIT_LOG_FUNC (eg.get_logger ());
-  switch (succ->m_kind)
-    {
-    case SUPEREDGE_CFG_EDGE:
-      break;
-
-    case SUPEREDGE_CALL:
-      {
-	/* Add the callsite to the call string.  */
-	// FIXME: this should be an as_a<>
-	m_call_string.push_call (eg.get_supergraph (),
-				 (const call_superedge *)succ);
-
-	/* Impose a maximum recursion depth and don't analyze paths
-	   that exceed it further.
-	   This is something of a blunt workaround, but it only
-	   applies to recursion (and mutual recursion), not to
-	   general call stacks.  */
-	// FIXME: make this a param:
-	const int MAX_RECURSION_DEPTH = 2;
-	if (m_call_string.calc_recursion_depth () > MAX_RECURSION_DEPTH)
-	  {
-	    eg.log ("rejecting call edge: recursion limit exceeded");
-	    // TODO: issue a sorry for this?
-	    return false;
-	  }
-      }
-      break;
-
-    case SUPEREDGE_RETURN:
-      {
-	/* Require that we return to the call site in the call string.  */
-	if (m_call_string.empty_p ())
-	  {
-	    eg.log ("rejecting return edge: empty call string");
-	    return false;
-	  }
-	const return_superedge *top_of_stack = m_call_string.pop ();
-	if (top_of_stack != succ)
-	  {
-	    eg.log ("rejecting return edge: return to wrong callsite");
-	    return false;
-	  }
-      }
-      break;
-
-    case SUPEREDGE_INTRAPROCEDURAL_CALL:
-      /* We ignore these edges.  FIXME: why have them?  */
-      eg.log ("rejecting interprocedural edge");
-      return false;
-    }
-
-  return true;
-}
-
-// TODO: move this fn
-/* FIXME.  */
-
-bool
-program_state::on_edge (exploded_graph &eg,
-			const exploded_node &enode,
-			const superedge *succ)
-{
-  /* Update state.  */
-
-  if (succ->m_kind == SUPEREDGE_INTRAPROCEDURAL_CALL)
-    {
-      /* We ignore these edges.  FIXME: why have them?  */
-      eg.log ("rejecting interprocedural edge");
-      return false;
-    }
-
-  const program_point &point = enode.get_point ();
-  const program_state &old_state = enode.get_state ();
-  const gimple *last_stmt = point.m_supernode->get_last_stmt ();
-
-  /* For conditionals and switch statements, add the
-     relevant conditions (for the specific edge) to new_state;
-     skip edges for which the resulting constraints
-     are impossible.
-     This also updates frame information for call/return superedges.  */
-
-  impl_region_model_context ctxt (eg, &enode, this, NULL);
-  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);
-      return false;
-    }
-
-  /* Pass the edge to the various state machines to see if any transitions
-     or warnings occur.  */
-  int i;
-  sm_state_map *old_smap;
-  FOR_EACH_VEC_ELT (old_state.m_checker_states, i, old_smap)
-    {
-      const state_machine &sm = eg.get_ext_state ().get_sm (i);
-      impl_sm_context ctxt (eg, sm, &enode,
-			    this,
-			    old_state.m_checker_states[i],
-			    m_checker_states[i]);
-      sm.on_edge (&ctxt, last_stmt, succ);
-    }
-
-  return true;
-}
-
 ////////////////////////////////////////////////////////////////////////////
 
 /* class exploded_edge : public dedge.  */
@@ -4516,7 +4528,8 @@ stats::dump (FILE *out) const
 
 /* class saved_diagnostic.  */
 
-/* FIXME.  */
+/* saved_diagnostic's ctor.
+   Take ownership of D and STMT_FINDER.  */
 
 saved_diagnostic::saved_diagnostic (const state_machine *sm,
 				    const exploded_node *enode,
@@ -4533,7 +4546,7 @@ saved_diagnostic::saved_diagnostic (const state_machine *sm,
 {
 }
 
-/* FIXME.  */
+/* saved_diagnostic's dtor.  */
 
 saved_diagnostic::~saved_diagnostic ()
 {
@@ -4545,7 +4558,7 @@ saved_diagnostic::~saved_diagnostic ()
 
 /* class diagnostic_manager.  */
 
-/* FIXME.  */
+/* diagnostic_manager's ctor.  */
 
 diagnostic_manager::diagnostic_manager (logger *logger, int verbosity)
 : log_user (logger), m_verbosity (verbosity)
@@ -5455,7 +5468,7 @@ strongly_connected_components (const supergraph &sg, logger *logger)
     dump ();
 }
 
-/* FIXME.  */
+/* Dump this object to stderr.  */
 
 DEBUG_FUNCTION void
 strongly_connected_components::dump () const
@@ -5468,7 +5481,8 @@ strongly_connected_components::dump () const
     }
 }
 
-/* FIXME.  */
+/* Subroutine of strongly_connected_components's ctor, part of Tarjan's
+   SCC algorithm.  */
 
 void
 strongly_connected_components::strong_connect (unsigned index)
@@ -5532,7 +5546,7 @@ worklist::worklist (const exploded_graph &eg)
 {
 }
 
-/* FIXME.  */
+/* Return the number of nodes in the worklist.  */
 
 unsigned
 worklist::length () const
@@ -5540,7 +5554,7 @@ worklist::length () const
   return m_queue.nodes ();
 }
 
-/* FIXME.  */
+/* Return the next node in the worklist, removing it.  */
 
 exploded_node *
 worklist::take_next ()
@@ -5548,7 +5562,7 @@ worklist::take_next ()
   return m_queue.extract_min ();
 }
 
-/* FIXME.  */
+/* Return the next node in the worklist without removing it.  */
 
 exploded_node *
 worklist::peek_next ()
@@ -5556,7 +5570,7 @@ worklist::peek_next ()
   return m_queue.min ();
 }
 
-/* FIXME.  */
+/* Add ENODE to the worklist.  */
 
 void
 worklist::add_node (exploded_node *enode)
@@ -6175,7 +6189,9 @@ exploded_graph::process_node (exploded_node *node)
     }
 }
 
-/* FIXME.  */
+/* Ensure that this graph has a stats instance for FN, return it.
+   FN can be NULL, in which case a stats instances is returned covering
+   "functionless" parts of the graph (the origin node).  */
 
 stats *
 exploded_graph::get_or_create_function_stats (function *fn)
@@ -6195,7 +6211,7 @@ exploded_graph::get_or_create_function_stats (function *fn)
     }
 }
 
-/* FIXME.  */
+/* Write all stats information to this graph's logger, if any.  */
 
 void
 exploded_graph::log_stats () const
@@ -6222,6 +6238,8 @@ exploded_graph::log_stats () const
     }
 }
 
+/* Dump all stats information to OUT.  */
+
 void
 exploded_graph::dump_stats (FILE *out) const
 {
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index 8cdabd2..90ac572 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -36,7 +36,11 @@ class metadata : public diagnostic_metadata
  private:
 };
 
-/* FIXME.  */
+/* A class for referring to events within a diagnostic_path.
+   They are stored as 0-based offsets into the events, but
+   printed as 1-based numbers.  For example, a 3-event path
+   has event offsets 0, 1, and 2, which would be shown to
+   the user as "(1)", "(2)" and "(3)".  */
 
 class event_id_t
 {
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 7f71e78..500744c 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -307,7 +307,7 @@ template class one_way_id_map<svalue_id>;
 
 template<>
 id_set<region_id>::id_set (const region_model *model)
-: m_bitmap (model->m_regions.length ())
+: m_bitmap (model->get_num_regions ())
 {
   bitmap_clear (m_bitmap);
 }
@@ -1219,10 +1219,13 @@ POP_IGNORE_WFORMAT
 
   auto_vec<region_id> child_rids;
   unsigned i;
-  region *child;
-  FOR_EACH_VEC_ELT (model.m_regions, i, child)
-    if (child->m_parent_rid == this_rid)
-      child_rids.safe_push (region_id::from_int (i));
+  for (unsigned i = 0; i < model.get_num_regions (); ++i)
+    {
+      region_id rid = region_id::from_int (i);
+      region *child = model.get_region (rid);
+      if (child->m_parent_rid == this_rid)
+	child_rids.safe_push (rid);
+    }
 
   region_id *child_rid;
   FOR_EACH_VEC_ELT (child_rids, i, child_rid)
@@ -2294,7 +2297,7 @@ root_region::ensure_stack_region (region_model *model)
       svalue_id uninit_sid
 	= model->add_svalue (new poisoned_svalue (POISON_KIND_UNINIT));
       m_stack_rid
-	= model->add_region (new stack_region (model->m_root_rid,
+	= model->add_region (new stack_region (model->get_root_rid (),
 					       uninit_sid));
     }
   return m_stack_rid;
@@ -2316,7 +2319,7 @@ root_region::ensure_globals_region (region_model *model)
 {
   if (m_globals_rid.null_p ())
     m_globals_rid
-      = model->add_region (new map_region (model->m_root_rid, NULL_TREE));
+      = model->add_region (new map_region (model->get_root_rid (), NULL_TREE));
   return m_globals_rid;
 }
 
@@ -2340,7 +2343,8 @@ root_region::ensure_heap_region (region_model *model)
 	= model->add_svalue (new poisoned_svalue (POISON_KIND_UNINIT));
       // FIXME: maybe share uninit values?
       m_heap_rid
-	= model->add_region (new heap_region (model->m_root_rid, uninit_sid));
+	= model->add_region (new heap_region (model->get_root_rid (),
+					      uninit_sid));
     }
   return m_heap_rid;
 }
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index bca5408..d33fc70 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -657,6 +657,9 @@ public:
 class map_region : public region
 {
 public:
+  typedef ordered_hash_map<tree, region_id> map_t;
+  typedef map_t::iterator iterator_t;
+
   map_region (region_id parent_rid, tree type)
   : region (parent_rid, svalue_id::null (), type)
   {}
@@ -699,6 +702,9 @@ public:
   svalue_id get_value_by_name (tree identifier,
 			       const region_model &model) const;
 
+  iterator_t begin () { return m_map.begin (); }
+  iterator_t end () { return m_map.end (); }
+
  protected:
   void add_to_hash (inchash::hash &hstate) const OVERRIDE;
   void print_fields (const region_model &model,
@@ -707,9 +713,7 @@ public:
     OVERRIDE;
 
  private:
- public: // FIXME
-  // FIXME: mapping from tree to child region
-  typedef ordered_hash_map<tree, region_id> map_t;
+  /* Mapping from tree to child region.  */
   map_t m_map;
 };
 
@@ -964,6 +968,7 @@ class region_model
 			      const gimple *last_stmt,
 			      region_model_context *ctxt);
 
+  region_id get_root_rid () const { return m_root_rid; }
   root_region *get_root_region () const;
 
   region_id get_stack_region_id () const;
@@ -1035,8 +1040,8 @@ class region_model
 		      purge_stats *stats,
 		      logger *logger);
 
-  int get_num_svalues () const { return m_svalues.length (); }
-  int get_num_regions () const { return m_regions.length (); }
+  unsigned get_num_svalues () const { return m_svalues.length (); }
+  unsigned get_num_regions () const { return m_regions.length (); }
 
   /* For selftests.  */
   constraint_manager *get_constraints ()
@@ -1074,11 +1079,9 @@ class region_model
   void poison_any_pointers_to_bad_regions (const region_id_set &bad_regions,
 					   enum poison_kind pkind);
 
- public: // FIXME:
   auto_delete_vec<svalue> m_svalues;
   auto_delete_vec<region> m_regions;
   region_id m_root_rid;
- private:
   constraint_manager *m_constraints; // FIXME: embed, rather than dynalloc?
 };
 
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index a973545..7c6905f 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -96,7 +96,7 @@ private:
   auto_vec<const char *> m_state_names;
 };
 
-/* FIXME.  */
+/* Is STATE the start state?  (zero is hardcoded as the start state).  */
 
 static inline bool
 start_start_p (state_machine::state_t state)
diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc
index a9d9549..b6ef813 100644
--- a/gcc/analyzer/supergraph.cc
+++ b/gcc/analyzer/supergraph.cc
@@ -1,4 +1,4 @@
-/* FIXME.
+/* "Supergraph" classes that combine CFGs and callgraph into one digraph.
    Copyright (C) 2019 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
 
@@ -74,7 +74,10 @@ supergraph_call_edge (function *fun, gimple *stmt)
   return edge;
 }
 
-// FIXME
+/* supergraph's ctor.  Walk the callgraph, building supernodes for each
+   CFG basic block, splitting the basic blocks at callsites.  Join
+   together the supernodes with interprocedural and intraprocedural
+   superedges as appropriate.  */
 
 supergraph::supergraph (logger *logger)
 {
@@ -327,6 +330,8 @@ supergraph::dump_dot_to_pp (pretty_printer *pp,
   gv.println ("}");
 }
 
+/* FIXME.  */
+
 void
 supergraph::dump_dot_to_file (FILE *fp, const dump_args_t &dump_args) const
 {
@@ -342,6 +347,8 @@ supergraph::dump_dot_to_file (FILE *fp, const dump_args_t &dump_args) const
   delete pp;
 }
 
+/* FIXME.  */
+
 void
 supergraph::dump_dot (const char *path, const dump_args_t &dump_args) const
 {
@@ -350,6 +357,8 @@ supergraph::dump_dot (const char *path, const dump_args_t &dump_args) const
   fclose (fp);
 }
 
+/* FIXME.  */
+
 supernode *
 supergraph::add_node (function *fun, basic_block bb, gcall *returning_call,
 		      gimple_seq phi_nodes)
@@ -360,6 +369,8 @@ supergraph::add_node (function *fun, basic_block bb, gcall *returning_call,
   return n;
 }
 
+/* FIXME.  */
+
 cfg_superedge *
 supergraph::add_cfg_edge (supernode *src, supernode *dest, ::edge e, int idx)
 {
@@ -375,6 +386,8 @@ supergraph::add_cfg_edge (supernode *src, supernode *dest, ::edge e, int idx)
   return new_edge;
 }
 
+/* FIXME.  */
+
 call_superedge *
 supergraph::add_call_superedge (supernode *src, supernode *dest,
 				cgraph_edge *cedge)
@@ -384,6 +397,8 @@ supergraph::add_call_superedge (supernode *src, supernode *dest,
   return new_edge;
 }
 
+/* FIXME.  */
+
 return_superedge *
 supergraph::add_return_superedge (supernode *src, supernode *dest,
 				  cgraph_edge *cedge)
@@ -393,12 +408,16 @@ supergraph::add_return_superedge (supernode *src, supernode *dest,
   return new_edge;
 }
 
+/* FIXME.  */
+
 std::string
 supernode::to_str () const
 {
   return std::to_string (m_index);
 }
 
+/* FIXME.  */
+
 void
 supernode::dump_dot (graphviz_out *gv, const dump_args_t &args) const
 {
@@ -527,7 +546,8 @@ supernode::dump_dot_id (pretty_printer *pp) const
   pp_printf (pp, "node_%i", m_index);
 }
 
-// FIXME
+/* FIXME.  */
+
 location_t
 supernode::get_start_location () const
 {
@@ -553,7 +573,7 @@ supernode::get_start_location () const
   return UNKNOWN_LOCATION;
 }
 
-// FIXME
+/* FIXME.  */
 
 location_t
 supernode::get_end_location () const
@@ -575,6 +595,8 @@ supernode::get_end_location () const
   return UNKNOWN_LOCATION;
 }
 
+/* FIXME.  */
+
 unsigned int
 supernode::get_stmt_index (const gimple *stmt) const
 {
@@ -586,6 +608,8 @@ supernode::get_stmt_index (const gimple *stmt) const
   gcc_unreachable ();
 }
 
+/* FIXME.  */
+
 void
 superedge::dump_dot (graphviz_out *gv, const dump_args_t &) const
 {
@@ -678,17 +702,19 @@ superedge::dump_dot (graphviz_out *gv, const dump_args_t &) const
   pp_printf (gv->m_pp, "\"];\n");
 }
 
-/* FIXME.  */
+/* If this is an intraprocedural superedge, return the associated
+   CFG edge.  Otherwise, return NULL.  */
 
 ::edge
 superedge::get_any_cfg_edge () const
 {
   if (const cfg_superedge *sub = dyn_cast_cfg_superedge ())
-    return sub->m_cfg_edge;
+    return sub->get_cfg_edge ();
   return NULL;
 }
 
-/* FIXME.  */
+/* If this is an interprocedural superedge, return the associated
+   cgraph_edge *.  Otherwise, return NULL.  */
 
 cgraph_edge *
 superedge::get_any_callgraph_edge () const
@@ -811,7 +837,7 @@ switch_cfg_superedge::dump_label_to_pp (pretty_printer *pp,
     pp_printf (pp, "default:");
 }
 
-/* FIXME.  */
+/* Get the case label for this "switch" superedge.  */
 
 tree
 switch_cfg_superedge::get_case_label () const
@@ -846,7 +872,8 @@ callgraph_superedge::dump_label_to_pp (pretty_printer *pp,
     }
 }
 
-/* FIXME.  */
+/* Get the function that was called at this interprocedural call/return
+   edge.  */
 
 function *
 callgraph_superedge::get_callee_function () const
@@ -854,7 +881,7 @@ callgraph_superedge::get_callee_function () const
   return m_cedge->callee->get_fun ();
 }
 
-/* FIXME.  */
+/* Get the calling function at this interprocedural call/return edge.  */
 
 function *
 callgraph_superedge::get_caller_function () const
@@ -862,7 +889,8 @@ callgraph_superedge::get_caller_function () const
   return m_cedge->caller->get_fun ();
 }
 
-/* FIXME.  */
+/* Get the fndecl that was called at this interprocedural call/return
+   edge.  */
 
 tree
 callgraph_superedge::get_callee_decl () const
@@ -870,7 +898,7 @@ callgraph_superedge::get_callee_decl () const
   return get_callee_function ()->decl;
 }
 
-/* FIXME.  */
+/* Get the calling fndecl at this interprocedural call/return edge.  */
 
 tree
 callgraph_superedge::get_caller_decl () const
diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h
index 7d165d1..0c95246 100644
--- a/gcc/analyzer/supergraph.h
+++ b/gcc/analyzer/supergraph.h
@@ -1,4 +1,4 @@
-/* FIXME.
+/* "Supergraph" classes that combine CFGs and callgraph into one digraph.
    Copyright (C) 2019 Free Software Foundation, Inc.
    Contributed by David Malcolm <dmalcolm@redhat.com>.
 
@@ -75,6 +75,9 @@ enum supergraph_dot_flags
   SUPERGRAPH_DOT_SHOW_BBS = (1 << 0)
 };
 
+/* A traits struct describing the family of node, edge and digraph
+   classes for supergraphs.  */
+
 struct supergraph_traits
 {
   typedef supernode node_t;
@@ -94,7 +97,12 @@ struct supergraph_traits
   typedef supercluster cluster_t;
 };
 
-/* FIXME.  */
+/* A "supergraph" is a directed graph formed by joining together all CFGs,
+   linking them via interprocedural call and return edges.
+
+   Basic blocks are split at callsites, so that a call statement occurs
+   twice: once at the end of a supernode, and a second instance at the
+   start of the next supernode (to handle the return).  */
 
 class supergraph : public digraph<supergraph_traits>
 {
@@ -268,7 +276,8 @@ class supernode : public dnode<supergraph_traits>
   const int m_index; /* unique within the supergraph as a whole.  */
 };
 
-/* An abstract base class encapsulating an edge within a supergraph.  */
+/* An abstract base class encapsulating an edge within a supergraph.
+   Edges can be CFG edges, or calls/returns for callgraph edges.  */
 
 class superedge : public dedge<supergraph_traits>
 {
@@ -340,7 +349,8 @@ class callsite_expr
   int m_val; /* 1-based parm, 0 for return value, or -1 for "unknown".  */
 };
 
-/* A subclass of superedge corresponding to a callgraph edge.  */
+/* A subclass of superedge with an associated callgraph edge (either a
+   call or a return).  */
 
 class callgraph_superedge : public superedge
 {
@@ -368,8 +378,7 @@ class callgraph_superedge : public superedge
   cgraph_edge *const m_cedge;
 };
 
-/* A subclass of superedge corresponding to the edge for an
-   interprocedural call.  */
+/* A subclass of superedge representing an interprocedural call.  */
 
 class call_superedge : public callgraph_superedge
 {
@@ -403,8 +412,7 @@ class call_superedge : public callgraph_superedge
   }
 };
 
-/* A subclass of superedge corresponding to the edge for an
-   interprocedural return.  */
+/* A subclass of superedge represesnting an interprocedural return.  */
 
 class return_superedge : public callgraph_superedge
 {
@@ -447,6 +455,7 @@ class cfg_superedge : public superedge
   cfg_superedge *dyn_cast_cfg_superedge () FINAL OVERRIDE { return this; }
   const cfg_superedge *dyn_cast_cfg_superedge () const FINAL OVERRIDE { return this; }
 
+  ::edge get_cfg_edge () const { return m_cfg_edge; }
   int get_flags () const { return m_cfg_edge->flags; }
   int true_value_p () const { return get_flags () & EDGE_TRUE_VALUE; }
   int false_value_p () const { return get_flags () & EDGE_FALSE_VALUE; }
@@ -454,6 +463,7 @@ class cfg_superedge : public superedge
 
   tree get_phi_arg (const gphi *phi) const;
 
+ private:
   const ::edge m_cfg_edge;
 };
 
-- 
1.8.5.3

