From 8dde66a1d7c5d9360ec76cb819e5889a4c6018e7 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 19 Feb 2021 16:00:33 -0500
Subject: [PATCH 19/23] FIXME: don't use ordered_hash_map; fix leak of
 dedupe_sets; add logging

---
 gcc/analyzer/diagnostic-manager.cc | 126 ++++++++++++++++++++---------
 gcc/analyzer/diagnostic-manager.h  |  17 +++-
 2 files changed, 99 insertions(+), 44 deletions(-)

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 0eec1f27133..f7230b8d3e0 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -252,8 +252,8 @@ dedupe_key::get_location () const
 
 /* FIXME.  */
 
-dedupe_set::dedupe_set (const dedupe_key &key)
-: m_key (key), m_best_sd (NULL), m_best_epath (NULL)
+dedupe_set::dedupe_set (const dedupe_key &key, unsigned index)
+: m_key (key), m_index (index), m_best_sd (NULL), m_best_epath (NULL)
 {
 }
 
@@ -270,10 +270,19 @@ void
 dedupe_set::add_diagnostic (const exploded_node *enode,
 			    stmt_finder *finder,
 			    const svalue *sval,
-			    pending_diagnostic *d)
+			    pending_diagnostic *d,
+			    logger *logger)
 {
   /* FIXME: we take owndership of D here, I think
      The first one in the dedupe_set is also borrowed for the key.  */
+
+  if (logger)
+    logger->log ("adding saved diagnostic %i"
+		 " at EN %i to dedupe_set %i (SN %i, %qs)",
+		 m_saved_diagnostics.length (),
+		 enode->m_index, m_index,
+		 m_key.m_snode->m_index, d->get_kind ());
+
   saved_diagnostic *sd = new saved_diagnostic (&m_key, enode, finder, sval, d);
   m_saved_diagnostics.safe_push (sd);
 }
@@ -293,6 +302,9 @@ dedupe_set::calc_best_candidate (diagnostic_manager *dm,
 
   logger *logger = dm->get_logger ();
   LOG_SCOPE (logger);
+  if (logger)
+    logger->log ("dedupe_set %i (SN %i, %qs)",
+		 m_index, get_snode ()->m_index, m_key.m_d->get_kind ());
 
   /* Find the best candidate in this set.  */
   unsigned i;
@@ -398,6 +410,17 @@ dedupe_set::calc_best_candidate (diagnostic_manager *dm,
       }
   /* Any set for which we have a best candidate should now have an m_stmt.  */
   gcc_assert (m_key.m_stmt != NULL || m_best_sd == NULL);
+
+  if (logger)
+    {
+      if (m_best_sd)
+	logger->log ("best candidate for dedupe_set %i (SN %i, %qs): EN %i",
+		     m_index, get_snode ()->m_index, m_key.m_d->get_kind (),
+		     m_best_sd->m_enode->m_index);
+      else
+	logger->log ("no best candidate for dedupe_set %i (SN %i, %qs)",
+		     m_index, get_snode ()->m_index, m_key.m_d->get_kind ());
+    }
 }
 
 /* Set SD and EPATH as the best diagnostic (so far) within this set.
@@ -420,6 +443,9 @@ dedupe_set::comparator (const void *p1, const void *p2)
   const dedupe_set *ps1 = *(const dedupe_set * const *)p1;
   const dedupe_set *ps2 = *(const dedupe_set * const *)p2;
 
+  gcc_assert (ps1->m_best_sd);
+  gcc_assert (ps2->m_best_sd);
+
   const dedupe_key *pk1 = &ps1->m_key;
   const dedupe_key *pk2 = &ps2->m_key;
 
@@ -437,7 +463,7 @@ dedupe_set::comparator (const void *p1, const void *p2)
   return 0;
 }
 
-/* Emit the best diagnostic within this set, if any.  */
+/* Emit the best diagnostic within this set (which must exist).  */
 
 void
 dedupe_set::emit_best (diagnostic_manager *dm,
@@ -445,17 +471,20 @@ dedupe_set::emit_best (diagnostic_manager *dm,
 {
   logger *logger = dm->get_logger ();
   LOG_SCOPE (logger);
-
-  /* Emit the best candidate, if any.  */
-  if (m_best_sd)
-    {
-      gcc_assert (m_best_epath);
-      dm->emit_saved_diagnostic (eg,
-				 *m_best_sd,
-				 *m_best_epath,
-				 m_key.m_stmt, // FIXME: or stmt finder?
-				 m_saved_diagnostics.length ()); // or -1 ?
-    }
+  gcc_assert (m_best_sd);
+  gcc_assert (m_best_epath);
+  gcc_assert (m_key.m_stmt);
+  if (logger)
+    logger->log ("dedupe_set %i (SN %i, %qs): EN %i",
+		 m_index, get_snode ()->m_index, m_key.m_d->get_kind (),
+		 m_best_sd->m_enode->m_index);
+
+  /* Emit the best candidate.  */
+  dm->emit_saved_diagnostic (eg,
+			     *m_best_sd,
+			     *m_best_epath,
+			     m_key.m_stmt,
+			     m_saved_diagnostics.length ()); // or -1 ?
 }
 
 /* class diagnostic_manager.  */
@@ -488,15 +517,7 @@ diagnostic_manager::add_diagnostic (const state_machine *sm,
 
   dedupe_set *set
     = get_or_create_dedupe_set (sm, snode, stmt, var, state, d);
-  set->add_diagnostic (enode, finder, sval, d);
-  // FIXME: add logging
-#if 0
-  m_saved_diagnostics.safe_push (sd);
-  if (get_logger ())
-    log ("adding saved diagnostic %i at SN %i: %qs",
-	 m_saved_diagnostics.length () - 1,
-	 snode->m_index, d->get_kind ());
-#endif
+  set->add_diagnostic (enode, finder, sval, d, get_logger ());
 }
 
 /* FIXME.  */
@@ -511,12 +532,22 @@ diagnostic_manager::get_or_create_dedupe_set (const state_machine *sm,
 {
   const dedupe_key key (sm, snode, stmt, var, state, d);
   if (dedupe_set **slot = m_dedupe_map.get (&key))
-    return *slot;
+    {
+      dedupe_set *set = *slot;
+      if (get_logger ())
+	log ("reusing dedupe_set %i (SN %i, %qs)",
+	     set->get_index (), set->get_snode ()->m_index, d->get_kind ());
+
+      return set;
+    }
 
-  dedupe_set *set = new dedupe_set (key);
+  dedupe_set *set = new dedupe_set (key, m_dedupe_set_vec.length ());
   m_dedupe_map.put (set->get_key (), set);
+  m_dedupe_set_vec.safe_push (set);
 
-  // FIXME: add logging
+  if (get_logger ())
+    log ("created dedupe_set %i (SN %i, %qs)",
+	 set->get_index (), set->get_snode ()->m_index, d->get_kind ());
 
   return set;
 }
@@ -838,25 +869,40 @@ diagnostic_manager::emit_saved_diagnostics (const exploded_graph &eg)
   shortest_exploded_paths sp (eg, eg.get_origin ());
 
   /* Determine the best saved_diagnostic in each dedupe_set.  */
-  for (dedupe_map_t::iterator iter = m_dedupe_map.begin ();
-       iter != m_dedupe_map.end (); ++iter)
-    (*iter).second->calc_best_candidate (this, eg, &sp);
+  {
+    unsigned i;
+    dedupe_set *set;
+    FOR_EACH_VEC_ELT (m_dedupe_set_vec, i, set)
+      set->calc_best_candidate (this, eg, &sp);
+  }
 
-  /* Sort into a good emission order.
+  /* Sort the valid ones into a good emission order.
      This needs to happen after the best candidates (and their exploded_paths)
      are determined, since the sort order uses the location_t, and any cases
      that need to use a stmt_finder need an exploded_path to work on.  */
-  auto_vec<dedupe_set *> sorted_sets (m_dedupe_map.elements ());
-  for (dedupe_map_t::iterator iter = m_dedupe_map.begin ();
-       iter != m_dedupe_map.end (); ++iter)
-    sorted_sets.quick_push ((*iter).second);
-  sorted_sets.qsort (dedupe_set::comparator);
+  auto_vec<const dedupe_set *> sorted_filtered_sets
+    (m_dedupe_set_vec.length ());
+  {
+    unsigned i;
+    dedupe_set *set;
+    FOR_EACH_VEC_ELT (m_dedupe_set_vec, i, set)
+      {
+	/* Only consider those which have a valid diagnostic (the comparator
+	   assumes this, and there's no point sorting ones that would be
+	   discarded later).  */
+	if (set->get_best_diagnostic ())
+	  sorted_filtered_sets.quick_push (set);
+      }
+  }
+  sorted_filtered_sets.qsort (dedupe_set::comparator);
 
   /* Emit the sorted best candidates for each dedupe_set.  */
-  int i;
-  dedupe_set *set;
-  FOR_EACH_VEC_ELT (sorted_sets, i, set)
-    set->emit_best (this, eg);
+  {
+    unsigned i;
+    const dedupe_set *set;
+    FOR_EACH_VEC_ELT (sorted_filtered_sets, i, set)
+      set->emit_best (this, eg);
+  }
 
   // FIXME: update internal docs if necessary
 }
diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h
index 57986a2fb34..cc147119b51 100644
--- a/gcc/analyzer/diagnostic-manager.h
+++ b/gcc/analyzer/diagnostic-manager.h
@@ -138,14 +138,17 @@ private:
 class dedupe_set
 {
 public:
-  dedupe_set (const dedupe_key &key);
+  dedupe_set (const dedupe_key &key, unsigned index);
   ~dedupe_set ();
 
   void add_diagnostic (const exploded_node *enode,
 		       stmt_finder *finder,
 		       const svalue *sval,
-		       pending_diagnostic *d);
+		       pending_diagnostic *d,
+		       logger *logger);
   const dedupe_key *get_key () const { return &m_key; }
+  unsigned get_index () const { return m_index; }
+  const supernode *get_snode () const { return m_key.m_snode; }
 
   void calc_best_candidate (diagnostic_manager *dm,
 			    const exploded_graph &eg,
@@ -158,12 +161,14 @@ public:
 
   // FIXME: add dump methods
 
+  const saved_diagnostic *get_best_diagnostic () const { return m_best_sd; }
   const exploded_path *get_best_epath () const { return m_best_epath; }
 
 private:
   void set_best (const saved_diagnostic *sd, exploded_path *epath);
 
   dedupe_key m_key;
+  const unsigned m_index; /* for debugging/logging.  */
   auto_delete_vec<saved_diagnostic> m_saved_diagnostics;
   const saved_diagnostic *m_best_sd; /* borrowed.  */
   exploded_path *m_best_epath; /* owned.  */
@@ -317,10 +322,14 @@ private:
   engine *m_eng;
 
   /* This maps from each dedupe_key to a dedupe_set.  */
-  typedef ordered_hash_map<const dedupe_key *, dedupe_set *,
-			   dedupe_hash_map_traits> dedupe_map_t;
+  typedef hash_map<const dedupe_key *, dedupe_set *,
+		   dedupe_hash_map_traits> dedupe_map_t;
   dedupe_map_t m_dedupe_map;
 
+  /* Handle ownership of the sets, and impose a deterministic
+     ordering when iterating through them.  */
+  auto_delete_vec<dedupe_set> m_dedupe_set_vec;
+
   const int m_verbosity;
 };
 
-- 
2.26.2

