analyzer: improvements to state dumping

This patch fixes a bug in which summarized state dumps involving a
non-NULL pointer to a region for which get_representative_path_var
returned NULL were erroneously dumped as "NULL".

It also extends sm-state dumps so that they show representative tree
values, where available.

Finally, it adds some selftest coverage for such dumps.  Doing so
requires replacing some %qE with a dump_quoted_tree, to avoid
C vs C++ differences between "make selftest-c" and "make selftest-c++".

gcc/analyzer/ChangeLog:
	* analyzer.h (dump_quoted_tree): New decl.
	* engine.cc (exploded_node::dump_dot): Pass region model to
	sm_state_map::print.
	* program-state.cc: Include diagnostic-core.h.
	(sm_state_map::print): Add "model" param and use it to print
	representative trees.  Only print origin information if non-null.
	(sm_state_map::dump): Pass NULL for model to print call.
	(program_state::print): Pass region model to sm_state_map::print.
	(program_state::dump_to_pp): Use spaces rather than newlines when
	summarizing.  Pass region_model to sm_state_map::print.
	(ana::selftest::assert_dump_eq): New function.
	(ASSERT_DUMP_EQ): New macro.
	(ana::selftest::test_program_state_dumping): New function.
	(ana::selftest::analyzer_program_state_cc_tests): Call it.
	* program-state.h (program_state::print): Add model param.
	* region-model.cc (dump_quoted_tree): New function.
	(map_region::print_fields): Use dump_quoted_tree rather than
	%qE to avoid lang-dependent output.
	(map_region::dump_child_label): Likewise.
	(region_model::dump_summary_of_map): For SK_REGION, when
	get_representative_path_var fails, print the region id rather than
	erroneously printing NULL.
	* sm.cc (state_machine::get_state_by_name): New function.
	* sm.h (state_machine::get_state_by_name): New decl.

gcc/analyzer/ChangeLog

+2020-03-06  David Malcolm  <dmalcolm@redhat.com>
+
+	* analyzer.h (dump_quoted_tree): New decl.
+	* engine.cc (exploded_node::dump_dot): Pass region model to
+	sm_state_map::print.
+	* program-state.cc: Include diagnostic-core.h.
+	(sm_state_map::print): Add "model" param and use it to print
+	representative trees.  Only print origin information if non-null.
+	(sm_state_map::dump): Pass NULL for model to print call.
+	(program_state::print): Pass region model to sm_state_map::print.
+	(program_state::dump_to_pp): Use spaces rather than newlines when
+	summarizing.  Pass region_model to sm_state_map::print.
+	(ana::selftest::assert_dump_eq): New function.
+	(ASSERT_DUMP_EQ): New macro.
+	(ana::selftest::test_program_state_dumping): New function.
+	(ana::selftest::analyzer_program_state_cc_tests): Call it.
+	* program-state.h (program_state::print): Add model param.
+	* region-model.cc (dump_quoted_tree): New function.
+	(map_region::print_fields): Use dump_quoted_tree rather than
+	%qE to avoid lang-dependent output.
+	(map_region::dump_child_label): Likewise.
+	(region_model::dump_summary_of_map): For SK_REGION, when
+	get_representative_path_var fails, print the region id rather than
+	erroneously printing NULL.
+	* sm.cc (state_machine::get_state_by_name): New function.
+	* sm.h (state_machine::get_state_by_name): New decl.
+
 2020-03-04  David Malcolm  <dmalcolm@redhat.com>
 
 	* region-model.cc (region::validate): Convert model param from ptr

gcc/analyzer/analyzer.h

@@ -21,12 +21,12 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_ANALYZER_ANALYZER_H
 #define GCC_ANALYZER_ANALYZER_H
 
-/* Forward decls of common types, with indentation to show inheritance.  */
-
 class graphviz_out;
 
 namespace ana {
 
+/* Forward decls of common types, with indentation to show inheritance.  */
+
 class supergraph;
 class supernode;
 class superedge;
@@ -71,6 +71,10 @@ class state_purge_per_ssa_name;
 class state_change;
 class rewind_info_t;
 
+/* Forward decls of functions.  */
+
+extern void dump_quoted_tree (pretty_printer *pp, tree t);
+
 } // namespace ana
 
 extern bool is_special_named_call_p (const gcall *call, const char *funcname,

gcc/analyzer/engine.cc

@@ -869,7 +869,7 @@ exploded_node::dump_dot (graphviz_out *gv, const dump_args_t &args) const
 	if (!smap->is_empty_p ())
 	  {
 	    pp_printf (pp, "%s: ", ext_state.get_name (i));
-	    smap->print (ext_state.get_sm (i), pp);
+	    smap->print (ext_state.get_sm (i), state.m_region_model, pp);
 	    pp_newline (pp);
 	  }
       }

gcc/analyzer/program-state.cc

@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "coretypes.h"
 #include "tree.h"
+#include "diagnostic-core.h"
 #include "diagnostic.h"
 #include "function.h"
 #include "analyzer/analyzer.h"
@@ -147,10 +148,13 @@ sm_state_map::clone_with_remapping (const one_way_svalue_id_map &id_map) const
   return result;
 }
 
-/* Print this sm_state_map (for SM) to PP.  */
+/* Print this sm_state_map (for SM) to PP.
+   If MODEL is non-NULL, print representative tree values where
+   available.  */
 
 void
-sm_state_map::print (const state_machine &sm, pretty_printer *pp) const
+sm_state_map::print (const state_machine &sm, const region_model *model,
+		     pretty_printer *pp) const
 {
   bool first = true;
   pp_string (pp, "{");
@@ -170,10 +174,27 @@ sm_state_map::print (const state_machine &sm, pretty_printer *pp) const
       sid.print (pp);
 
       entry_t e = (*iter).second;
-      pp_printf (pp, ": %s (origin: ",
-		 sm.get_state_name (e.m_state));
-      e.m_origin.print (pp);
-      pp_string (pp, ")");
+      pp_printf (pp, ": %s", sm.get_state_name (e.m_state));
+      if (model)
+	if (tree rep = model->get_representative_tree (sid))
+	  {
+	    pp_string (pp, " (");
+	    dump_quoted_tree (pp, rep);
+	    pp_character (pp, ')');
+	  }
+      if (!e.m_origin.null_p ())
+	{
+	  pp_string (pp, " (origin: ");
+	  e.m_origin.print (pp);
+	  if (model)
+	    if (tree rep = model->get_representative_tree (e.m_origin))
+	      {
+		pp_string (pp, " (");
+		dump_quoted_tree (pp, rep);
+		pp_character (pp, ')');
+	      }
+	  pp_string (pp, ")");
+	}
     }
   pp_string (pp, "}");
 }
@@ -186,7 +207,7 @@ sm_state_map::dump (const state_machine &sm) const
   pretty_printer pp;
   pp_show_color (&pp) = pp_show_color (global_dc->printer);
   pp.buffer->stream = stderr;
-  print (sm, &pp);
+  print (sm, NULL, &pp);
   pp_newline (&pp);
   pp_flush (&pp);
 }
@@ -696,7 +717,7 @@ program_state::print (const extrinsic_state &ext_state,
       if (!smap->is_empty_p ())
 	{
 	  pp_printf (pp, "%s: ", ext_state.get_name (i));
-	  smap->print (ext_state.get_sm (i), pp);
+	  smap->print (ext_state.get_sm (i), m_region_model, pp);
 	  pp_newline (pp);
 	}
     }
@@ -707,7 +728,9 @@ program_state::print (const extrinsic_state &ext_state,
     }
 }
 
-/* Dump a multiline representation of this state to PP.  */
+/* Dump a representation of this state to PP.
+   If SUMMARIZE is true, print a one-line summary;
+   if false, print a detailed multiline representation.  */
 
 void
 program_state::dump_to_pp (const extrinsic_state &ext_state,
@@ -723,16 +746,22 @@ program_state::dump_to_pp (const extrinsic_state &ext_state,
     {
       if (!smap->is_empty_p ())
 	{
+	  if (summarize)
+	    pp_space (pp);
 	  pp_printf (pp, "%s: ", ext_state.get_name (i));
-	  smap->print (ext_state.get_sm (i), pp);
-	  pp_newline (pp);
+	  smap->print (ext_state.get_sm (i), m_region_model, pp);
+	  if (!summarize)
+	    pp_newline (pp);
 	}
     }
 
   if (!m_valid)
     {
+      if (summarize)
+	pp_space (pp);
       pp_printf (pp, "invalid state");
-      pp_newline (pp);
+      if (!summarize)
+	pp_newline (pp);
     }
 }
 
@@ -1231,6 +1260,30 @@ state_change::validate (const program_state &new_state,
 
 namespace selftest {
 
+/* Implementation detail of ASSERT_DUMP_EQ.  */
+
+static void
+assert_dump_eq (const location &loc,
+		const program_state &state,
+		const extrinsic_state &ext_state,
+		bool summarize,
+		const char *expected)
+{
+  auto_fix_quotes sentinel;
+  pretty_printer pp;
+  pp_format_decoder (&pp) = default_tree_printer;
+  state.dump_to_pp (ext_state, summarize, &pp);
+  ASSERT_STREQ_AT (loc, pp_formatted_text (&pp), expected);
+}
+
+/* Assert that STATE.dump_to_pp (SUMMARIZE) is EXPECTED.  */
+
+#define ASSERT_DUMP_EQ(STATE, EXT_STATE, SUMMARIZE, EXPECTED)		\
+  SELFTEST_BEGIN_STMT							\
+  assert_dump_eq ((SELFTEST_LOCATION), (STATE), (EXT_STATE), (SUMMARIZE), \
+		  (EXPECTED));						\
+  SELFTEST_END_STMT
+
 /* Tests for sm_state_map.  */
 
 static void
@@ -1364,6 +1417,56 @@ test_sm_state_map ()
   // TODO: coverage for purging
 }
 
+/* Verify that program_state::dump_to_pp works as expected.  */
+
+static void
+test_program_state_dumping ()
+{
+  /* Create a program_state for a global ptr "p" that has
+     malloc sm-state, pointing to a region on the heap.  */
+  tree p = build_global_decl ("p", ptr_type_node);
+
+  state_machine *sm = make_malloc_state_machine (NULL);
+  const state_machine::state_t UNCHECKED_STATE
+    = sm->get_state_by_name ("unchecked");
+  auto_delete_vec <state_machine> checkers;
+  checkers.safe_push (sm);
+  extrinsic_state ext_state (checkers);
+
+  program_state s (ext_state);
+  region_model *model = s.m_region_model;
+  region_id new_rid = model->add_new_malloc_region ();
+  svalue_id ptr_sid
+      = model->get_or_create_ptr_svalue (ptr_type_node, new_rid);
+  model->set_value (model->get_lvalue (p, NULL),
+		    ptr_sid, NULL);
+  sm_state_map *smap = s.m_checker_states[0];
+
+  smap->impl_set_state (ptr_sid, UNCHECKED_STATE, svalue_id::null ());
+  ASSERT_EQ (smap->get_state (ptr_sid), UNCHECKED_STATE);
+
+  ASSERT_DUMP_EQ
+    (s, ext_state, false,
+     "rmodel: r0: {kind: `root', parent: null, sval: null}\n"
+     "|-heap: r1: {kind: `heap', parent: r0, sval: sv0}\n"
+     "|  |: sval: sv0: {poisoned: uninit}\n"
+     "|  `-r2: {kind: `symbolic', parent: r1, sval: null}\n"
+     "`-globals: r3: {kind: `globals', parent: r0, sval: null, map: {`p': r4}}\n"
+     "  `-`p': r4: {kind: `primitive', parent: r3, sval: sv1, type: `void *'}\n"
+     "    |: sval: sv1: {type: `void *', &r2}\n"
+     "    |: type: `void *'\n"
+     "svalues:\n"
+     "  sv0: {poisoned: uninit}\n"
+     "  sv1: {type: `void *', &r2}\n"
+     "constraint manager:\n"
+     "  equiv classes:\n"
+     "  constraints:\n"
+     "malloc: {sv1: unchecked (`p')}\n");
+
+  ASSERT_DUMP_EQ (s, ext_state, true,
+		  "rmodel: p: &r2 malloc: {sv1: unchecked (`p')}");
+}
+
 /* Verify that program_states with identical sm-state can be merged,
    and that the merged program_state preserves the sm-state.  */
 
@@ -1466,6 +1569,7 @@ void
 analyzer_program_state_cc_tests ()
 {
   test_sm_state_map ();
+  test_program_state_dumping ();
   test_program_state_merging ();
   test_program_state_merging_2 ();
 }

gcc/analyzer/program-state.h

@@ -146,7 +146,8 @@ public:
   sm_state_map *
   clone_with_remapping (const one_way_svalue_id_map &id_map) const;
 
-  void print (const state_machine &sm, pretty_printer *pp) const;
+  void print (const state_machine &sm, const region_model *model,
+	      pretty_printer *pp) const;
   void dump (const state_machine &sm) const;
 
   bool is_empty_p () const;

gcc/analyzer/region-model.cc

@@ -73,6 +73,17 @@ dump_tree (pretty_printer *pp, tree t)
   dump_generic_node (pp, t, 0, TDF_SLIM, 0);
 }
 
+/* Dump T to PP in language-independent form in quotes, for
+   debugging/logging/dumping purposes.  */
+
+void
+dump_quoted_tree (pretty_printer *pp, tree t)
+{
+  pp_begin_quote (pp, pp_show_color (pp));
+  dump_tree (pp, t);
+  pp_end_quote (pp, pp_show_color (pp));
+}
+
 /* Equivalent to pp_printf (pp, "%qT", t), to avoid nesting pp_printf
    calls within other pp_printf calls.
 
@@ -1595,7 +1606,8 @@ map_region::print_fields (const region_model &model,
 	pp_string (pp, ", ");
       tree expr = (*iter).first;
       region_id child_rid = (*iter).second;
-      pp_printf (pp, "%qE: ", expr);
+      dump_quoted_tree (pp, expr);
+      pp_string (pp, ": ");
       child_rid.print (pp);
     }
   pp_string (pp, "}");
@@ -1665,10 +1677,8 @@ map_region::dump_child_label (const region_model &model,
       if (child_rid == (*iter).second)
 	{
 	  tree key = (*iter).first;
-	  if (DECL_P (key))
-	    pp_printf (pp, "%qD: ", key);
-	  else
-	    pp_printf (pp, "%qE: ", key);
+	  dump_quoted_tree (pp, key);
+	  pp_string (pp, ": ");
 	}
     }
 }
@@ -3706,17 +3716,16 @@ region_model::dump_summary_of_map (pretty_printer *pp,
 	  {
 	    region_svalue *region_sval = as_a <region_svalue *> (sval);
 	    region_id pointee_rid = region_sval->get_pointee ();
+	    gcc_assert (!pointee_rid.null_p ());
 	    tree pointee = get_representative_path_var (pointee_rid).m_tree;
 	    dump_separator (pp, is_first);
 	    dump_tree (pp, key);
 	    pp_string (pp, ": ");
+	    pp_character (pp, '&');
 	    if (pointee)
-	      {
-		pp_character (pp, '&');
-		dump_tree (pp, pointee);
-	      }
+	      dump_tree (pp, pointee);
 	    else
-	      pp_string (pp, "NULL");
+	      pointee_rid.print (pp);
 	  }
 	  break;
 	case SK_CONSTANT:

gcc/analyzer/sm.cc

@@ -84,6 +84,21 @@ state_machine::get_state_name (state_t s) const
   return m_state_names[s];
 }
 
+/* Get the state with name NAME, which must exist.
+   This is purely intended for use in selftests.  */
+
+state_machine::state_t
+state_machine::get_state_by_name (const char *name)
+{
+  unsigned i;
+  const char *iter_name;
+  FOR_EACH_VEC_ELT (m_state_names, i, iter_name)
+    if (!strcmp (name, iter_name))
+      return i;
+  /* Name not found.  */
+  gcc_unreachable ();
+}
+
 /* Assert that S is a valid state for this state_machine.  */
 
 void

gcc/analyzer/sm.h

@@ -57,6 +57,8 @@ public:
 
   const char *get_state_name (state_t s) const;
 
+  state_t get_state_by_name (const char *name);
+
   /* Return true if STMT is a function call recognized by this sm.  */
   virtual bool on_stmt (sm_context *sm_ctxt,
 			const supernode *node,