From 8494a38c5ded12287aa5209cefe480ea1f46cf6c Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 21 Jun 2019 13:54:12 -0400
Subject: [PATCH 42/47] FIXME: rename pending_diagnostic description vfuncs

---
 gcc/analyzer/impl-2.cc    |   7 ++-
 gcc/analyzer/impl-3.cc    |   5 +-
 gcc/analyzer/impl-4.cc    |  13 ++---
 gcc/analyzer/sm-malloc.cc |  26 ++++-----
 gcc/analyzer/sm.h         | 144 ++++++++++++++++++++++++++++------------------
 notes.txt                 |   6 ++
 6 files changed, 120 insertions(+), 81 deletions(-)

diff --git a/gcc/analyzer/impl-2.cc b/gcc/analyzer/impl-2.cc
index ab12c1d..57b3d9f 100644
--- a/gcc/analyzer/impl-2.cc
+++ b/gcc/analyzer/impl-2.cc
@@ -698,7 +698,7 @@ public:
 	if (m_pending_diagnostic)
 	  {
 	    label_text custom_desc
-	      = m_pending_diagnostic->describe
+	      = m_pending_diagnostic->describe_state_change
 		  (evdesc::state_change (m_var, m_from, m_to, m_emission_id));
 	    if (custom_desc.m_buffer)
 	      {
@@ -927,7 +927,7 @@ public:
     if (!logger)
       return;
     logger->log ("%s: %s", desc, to_str ().c_str ());
-    for (int i = 0; i < m_events.length (); i++)
+    for (unsigned i = 0; i < m_events.length (); i++)
       logger->log ("%s[%i]: %s",
 		   desc, i, m_events[i]->to_str ().c_str ());
   }
@@ -3435,7 +3435,8 @@ engine::maybe_emit (const candidate_diagnostic &cd)
 
   char *msg = NULL;
   label_text ev_desc
-    = cd.m_pd->describe (evdesc::final_event (cd.m_var, cd.m_state));
+    = cd.m_pd->describe_final_event (evdesc::final_event (cd.m_var,
+							  cd.m_state));
   if (ev_desc.m_buffer)
     {
       if (flag_analyzer_verbose_state_changes)
diff --git a/gcc/analyzer/impl-3.cc b/gcc/analyzer/impl-3.cc
index 9e31dfd..83b927d 100644
--- a/gcc/analyzer/impl-3.cc
+++ b/gcc/analyzer/impl-3.cc
@@ -346,7 +346,7 @@ public:
 	if (m_pending_diagnostic)
 	  {
 	    label_text custom_desc
-	      = m_pending_diagnostic->describe
+	      = m_pending_diagnostic->describe_state_change
 		  (evdesc::state_change (m_var, m_from, m_to, m_emission_id));
 	    if (custom_desc.m_buffer)
 	      {
@@ -664,7 +664,8 @@ public:
 			pending_diagnostic *pd)
   {
     char *msg = NULL;
-    label_text ev_desc = pd->describe (evdesc::final_event (var, state));
+    label_text ev_desc
+      = pd->describe_final_event (evdesc::final_event (var, state));
     if (ev_desc.m_buffer)
       {
 	if (flag_analyzer_verbose_state_changes)
diff --git a/gcc/analyzer/impl-4.cc b/gcc/analyzer/impl-4.cc
index d415ade..7fb6d9e 100644
--- a/gcc/analyzer/impl-4.cc
+++ b/gcc/analyzer/impl-4.cc
@@ -1377,7 +1377,7 @@ public:
 	if (m_pending_diagnostic)
 	  {
 	    label_text custom_desc
-	      = m_pending_diagnostic->describe
+	      = m_pending_diagnostic->describe_state_change
 		  (evdesc::state_change (m_var, m_from, m_to, m_emission_id));
 	    if (custom_desc.m_buffer)
 	      {
@@ -1504,7 +1504,7 @@ public:
 	    {
 	      gcc_assert (m_var);
 	      label_text custom_desc
-		= m_pending_diagnostic->describe
+		= m_pending_diagnostic->describe_call_with_state
 		    (evdesc::call_with_state (m_sedge->m_src->m_fun->decl,
 					      m_sedge->m_dest->m_fun->decl,
 					      m_var,
@@ -1527,7 +1527,7 @@ public:
 	  if (m_critical_state && m_pending_diagnostic)
 	    {
 	      label_text custom_desc
-		= m_pending_diagnostic->describe
+		= m_pending_diagnostic->describe_return_of_state
 		    (evdesc::return_of_state (m_sedge->m_dest->m_fun->decl,
 					      m_sedge->m_src->m_fun->decl,
 					      m_critical_state));
@@ -1624,7 +1624,7 @@ public:
   {
     if (m_pending_diagnostic)
       {
-	label_text ev_desc = m_pending_diagnostic->describe
+	label_text ev_desc = m_pending_diagnostic->describe_final_event
 	  (evdesc::final_event (m_var, m_state));
 	if (ev_desc.m_buffer)
 	  {
@@ -1735,8 +1735,7 @@ public:
 
   void add_final_event (const state_machine *sm,
 			const exploded_node *enode, const gimple *stmt,
-			tree var, state_machine::state_t state,
-			pending_diagnostic *pd)
+			tree var, state_machine::state_t state)
   {
     checker4_event *end_of_path
       = new warning_event (stmt->location,
@@ -4033,7 +4032,7 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg,
      the sd.m_enode, as the dedupe code doesn't care about enodes, just
      snodes.  */
   emission_path.add_final_event (sd.m_sm, epath.get_final_enode (), stmt,
-				 sd.m_var, sd.m_state, sd.m_d);
+				 sd.m_var, sd.m_state);
 
   emission_path.prepare_for_emission (sd.m_d);
 
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index fd83d3e..fa79f21 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -90,7 +90,7 @@ public:
 #endif
   }
 
-  label_text describe (const evdesc::state_change &change)
+  label_text describe_state_change (const evdesc::state_change &change)
     FINAL OVERRIDE
   {
     if (change.m_new_state == m_sm.m_free)
@@ -101,7 +101,7 @@ public:
     return label_text ();
   }
 
-  label_text describe (const evdesc::call_with_state &info)
+  label_text describe_call_with_state (const evdesc::call_with_state &info)
     FINAL OVERRIDE
   {
     if (info.m_state == m_sm.m_free)
@@ -111,7 +111,7 @@ public:
     return label_text ();
   }
 
-  label_text describe (const evdesc::final_event &) FINAL OVERRIDE
+  label_text describe_final_event (const evdesc::final_event &) FINAL OVERRIDE
   {
     if (m_first_free_event.known_p ())
       return formatted_print ("second %qs here; first %qs was at (%i)",
@@ -142,7 +142,7 @@ public:
   : malloc_diagnostic (sm, arg)
   {}
 
-  label_text describe (const evdesc::state_change &change)
+  label_text describe_state_change (const evdesc::state_change &change)
     FINAL OVERRIDE
   {
     if (change.m_old_state == m_sm.m_start
@@ -151,7 +151,7 @@ public:
     return label_text ();
   }
 
-  label_text describe (const evdesc::return_of_state &info)
+  label_text describe_return_of_state (const evdesc::return_of_state &info)
     FINAL OVERRIDE
   {
     if (info.m_state == m_sm.m_unchecked)
@@ -183,7 +183,7 @@ public:
   }
 
   /* FIXME.  */
-  label_text describe (const evdesc::final_event &ev) FINAL OVERRIDE
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
   {
     return formatted_print ("%qE could be NULL", ev.m_expr);
   }
@@ -243,7 +243,7 @@ public:
   }
 
   /* FIXME.  */
-  label_text describe (const evdesc::final_event &ev) FINAL OVERRIDE
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
   {
     return formatted_print ("argument %u (%qE) could be NULL"
 			    " where non-null expected",
@@ -312,7 +312,7 @@ public:
   }
 
   /* FIXME.  */
-  label_text describe (const evdesc::final_event &ev) FINAL OVERRIDE
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
   {
     return formatted_print ("argument %u (%qE) NULL"
 			    " where non-null expected",
@@ -342,7 +342,7 @@ public:
 		"use after free of %qE", m_arg);
   }
 
-  label_text describe (const evdesc::state_change &change)
+  label_text describe_state_change (const evdesc::state_change &change)
     FINAL OVERRIDE
   {
     if (change.m_new_state == m_sm.m_free)
@@ -367,7 +367,7 @@ public:
 		"leak of %qE", m_arg);
   }
 
-  label_text describe (const evdesc::state_change &change)
+  label_text describe_state_change (const evdesc::state_change &change)
     FINAL OVERRIDE
   {
     if (change.m_new_state == m_sm.m_unchecked)
@@ -378,7 +378,7 @@ public:
     return label_text ();
   }
 
-  label_text describe (const evdesc::final_event &ev) FINAL OVERRIDE
+  label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
   {
     if (m_malloc_event.known_p ())
       return formatted_print ("%qE leaks here; was allocated at (%i)",
@@ -430,7 +430,7 @@ public:
       }
   }
 
-  label_text describe (const evdesc::state_change &change)
+  label_text describe_state_change (const evdesc::state_change &change)
     FINAL OVERRIDE
   {
     /* Attempt to reconstruct what kind of pointer it is.
@@ -453,7 +453,7 @@ public:
   }
 
   /* FIXME.  */
-  label_text describe (const evdesc::final_event &) FINAL OVERRIDE
+  label_text describe_final_event (const evdesc::final_event &) FINAL OVERRIDE
   {
     return formatted_print ("call to %qs here", "free");
   }
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index 9577f09..46a472f 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -105,6 +105,8 @@ start_start_p (state_machine::state_t state)
 
 // TODO: maybe capture which sm emitted the error
 
+// FIXME: is this needed?
+
 class metadata : public diagnostic_metadata
 {
  public:
@@ -134,23 +136,13 @@ class event_id_t
 };
 
 /* Various bundles of information used for generating more precise
-   messages for events within a diagnostic_path, for passing to
-   "describe" vfuncs of pending_diagnostic.  */
+   messages for events within a diagnostic_path, for passing to the
+   various "describe_*" vfuncs of pending_diagnostic.  See those
+   for more information.  */
 
 namespace evdesc {
 
-/* Information for describing a critical state change within the
-   diagnostic_path.
-
-   For example, a double-free diagnostic might use the descriptions:
-   - "first 'free' happens here"
-   - "second 'free' happens here"
-   for the pertinent events, whereas a use-after-free might use the
-   descriptions:
-   - "freed here"
-   - "use after free here"
-   Note how in both cases the first event is a "free": the best
-   description to use depends on the diagnostic.  */
+/* For use by pending_diagnostic::describe_state_change.  */
 
 struct state_change
 {
@@ -168,13 +160,7 @@ struct state_change
   event_id_t m_event_id;
 };
 
-/* Information for describing an interprocedural call carrying
-   critial state for the diagnostic, from caller to callee.
-
-   For example a double-free diagnostic might use:
-     "passing freed pointer 'ptr' in call to 'deallocator' from 'test'"
-   to make it clearer how the unchecked value moves from caller
-   to callee.  */
+/* For use by pending_diagnostic::describe_call_with_state.  */
 
 struct call_with_state
 {
@@ -193,14 +179,7 @@ struct call_with_state
   state_machine::state_t m_state;
 };
 
-/* Information for describing an interprocedural return within the
-   diagnostic_path that carries critial state for the diagnostic,
-   from callee back to caller.
-
-   For example, a deref-of-unchecked-malloc diagnostic might use:
-   - "returning possibly-NULL pointer to 'make_obj' from 'allocator'"
-   to make it clearer how the unchecked value moves from callee
-   back to caller.  */
+/* For use by pending_diagnostic::describe_return_of_state.  */
 
 struct return_of_state
 {
@@ -217,12 +196,7 @@ struct return_of_state
   state_machine::state_t m_state;
 };
 
-/* Information for describing the final event within a diagnostic_path.
-
-   For example a double-free diagnostic might use:
-     "second 'free' here"
-   and a use-after-free might use
-     "use after 'free' here".  */
+/* For use by pending_diagnostic::describe_final_event.  */
 
 struct final_event
 {
@@ -236,20 +210,35 @@ struct final_event
 
 } /* end of namespace evdesc */
 
-/* FIXME:
-   Diagnostics are emitted using va_list, which can't be portably stored
-   for later use, so we have to make it be a vfunc.  */
+/* An abstract base class for capturing information about a diagnostic in
+   a form that is ready to emit at a later point (or be rejected).
+   Each kind of diagnostic will have a concrete subclass of
+   pending_diagnostic.
+
+   Normally, gcc diagnostics are emitted using va_list, which can't be
+   portably stored for later use, so we have to use an "emit" virtual
+   function.
+
+   This class also supports comparison, so that multiple pending_diagnostic
+   instances can be de-duplicated.
+
+   As well as emitting a diagnostic, the class has various "precision of
+   wording" virtual functions, for generating descriptions for events
+   within a diagnostic_path.  These are optional, but implementing these
+   allows for more precise wordings than the more generic
+   implementation.  */
 
 class pending_diagnostic
 {
  public:
   virtual ~pending_diagnostic () {}
 
+  /* Vfunc for emitting the diagnostic.  */
+  virtual void emit (rich_location *) = 0;
+
   /* Hand-coded RTTI: get an ID for the subclass.  */
   virtual const char *get_kind () const = 0;
 
-  virtual void emit (rich_location *) = 0;
-
   /* Compare for equality with OTHER, which might be of a different
      subclass.  */
 
@@ -267,34 +256,71 @@ class pending_diagnostic
      below for a convenience subclass for implementing this.  */
   virtual bool subclass_equal_p (const pending_diagnostic &other) const = 0;
 
-  /* For greatest precision-of-wording, the various following overloads of
-     "describe" are virtual functions that give the pending diagnostic a
-     way to describe events in a diagnostic_path in terms that make sense
-     for that diagnostic.
+  /* For greatest precision-of-wording, the various following "describe_*"
+     virtual functions give the pending diagnostic a way to describe events
+     in a diagnostic_path in terms that make sense for that diagnostic.
 
      In each case, return a non-NULL label_text to give the event a custom
      description; NULL otherwise (falling back on a more generic
-     description).
-
-     Refer to the various "evdesc" struct definitions above for more
-     information on the meanings of each event kind.  */
-
-  virtual label_text describe (const evdesc::state_change &)
+     description).  */
+
+  /* Precision-of-wording vfunc for describing a critical state change
+     within the diagnostic_path.
+
+     For example, a double-free diagnostic might use the descriptions:
+     - "first 'free' happens here"
+     - "second 'free' happens here"
+     for the pertinent events, whereas a use-after-free might use the
+     descriptions:
+     - "freed here"
+     - "use after free here"
+     Note how in both cases the first event is a "free": the best
+     description to use depends on the diagnostic.  */
+
+  virtual label_text describe_state_change (const evdesc::state_change &)
   {
     /* Default no-op implementation.  */
     return label_text ();
   }
-  virtual label_text describe (const evdesc::call_with_state &)
+
+  /* Precision-of-wording vfunc for describing an interprocedural call
+     carrying critial state for the diagnostic, from caller to callee.
+
+     For example a double-free diagnostic might use:
+     - "passing freed pointer 'ptr' in call to 'deallocator' from 'test'"
+     to make it clearer how the unchecked value moves from caller
+     to callee.  */
+
+  virtual label_text describe_call_with_state (const evdesc::call_with_state &)
   {
     /* Default no-op implementation.  */
     return label_text ();
   }
-  virtual label_text describe (const evdesc::return_of_state &)
+
+  /* Precision-of-wording vfunc for describing an interprocedural return
+     within the diagnostic_path that carries critial state for the
+     diagnostic, from callee back to caller.
+
+     For example, a deref-of-unchecked-malloc diagnostic might use:
+     - "returning possibly-NULL pointer to 'make_obj' from 'allocator'"
+     to make it clearer how the unchecked value moves from callee
+     back to caller.  */
+
+  virtual label_text describe_return_of_state (const evdesc::return_of_state &)
   {
     /* Default no-op implementation.  */
     return label_text ();
   }
-  virtual label_text describe (const evdesc::final_event &)
+
+  /* Precision-of-wording vfunc for describing the final event within a
+     diagnostic_path.
+
+     For example a double-free diagnostic might use:
+      - "second 'free' here; first 'free' was at (3)"
+     and a use-after-free might use
+      - "use after 'free' here; memory was freed at (2)".  */
+
+  virtual label_text describe_final_event (const evdesc::final_event &)
   {
     /* Default no-op implementation.  */
     return label_text ();
@@ -306,10 +332,14 @@ class pending_diagnostic
     ATTRIBUTE_GCC_DIAG(1,2);
 };
 
-/* Use of the curiously-recurring template pattern, to implement
-   subclass_equal_p by casting and calling the operator==
+/* A template to make it easier to make subclasses of pending_diagnostic.
+
+   This uses the curiously-recurring template pattern, to implement
+   pending_diagnostic::subclass_equal_p by casting and calling
+   the operator==
+
    This assumes that BASE_OTHER has already been checked to have
-   been of the same subclass.  */
+   been of the same subclass (which pending_diagnostic::equal_p does).  */
 
 template <class Subclass>
 class pending_diagnostic_subclass : public pending_diagnostic
@@ -360,6 +390,8 @@ public:
 
   // etc, what else?
 
+  // FIXME:
+
   virtual tree get_readable_tree (tree expr)
   {
     return expr;
diff --git a/notes.txt b/notes.txt
index 39f0b0a..6c1b343 100644
--- a/notes.txt
+++ b/notes.txt
@@ -31,6 +31,12 @@ will dump the copious information about the analyzer's state each time it
 reaches the call in its traversal of the source.
 
 
+Re pending_diagnostic's describe_* vfuncs:
+     I would have given all these the name "describe" and let overloading
+     distinguish them, but I was getting numerous warnings from
+     -Woverloaded-virtual.
+
+
 User-facing Docs (putting them here to avoid the tedious texinfo rebuild times):
 ================================================================================
 
-- 
1.8.5.3

