[pushed,2/6] analyzer: add ability for context to add events to a saved_diagnostic

Message ID 20230822012127.2817996-2-dmalcolm@redhat.com
State Accepted
Headers
Series [pushed,1/6] analyzer: convert note_adding_context to annotating_context |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

David Malcolm Aug. 22, 2023, 1:21 a.m. UTC
  Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-3372-g2503dd59b588d3.

gcc/analyzer/ChangeLog:
	* diagnostic-manager.cc (saved_diagnostic::add_event): New.
	(saved_diagnostic::add_any_saved_events): New.
	(diagnostic_manager::add_event): New.
	(dedupe_winners::emit_best): New.
	(diagnostic_manager::emit_saved_diagnostic): Make "sd" param
	non-const.  Call saved_diagnostic::add_any_saved_events.
	* diagnostic-manager.h (saved_diagnostic::add_event): New decl.
	(saved_diagnostic::add_any_saved_events): New decl.
	(saved_diagnostic::m_saved_events): New field.
	(diagnostic_manager::add_event): New decl.
	(diagnostic_manager::emit_saved_diagnostic): Make "sd" param
	non-const.
	* engine.cc (impl_region_model_context::add_event): New.
	* exploded-graph.h (impl_region_model_context::add_event): New decl.
	* region-model.cc
	(noop_region_model_context::add_event): New.
	(region_model_context_decorator::add_event): New.
	* region-model.h (region_model_context::add_event): New vfunc.
	(noop_region_model_context::add_event): New decl.
	(region_model_context_decorator::add_event): New decl.
---
 gcc/analyzer/diagnostic-manager.cc | 45 ++++++++++++++++++++++++++++--
 gcc/analyzer/diagnostic-manager.h  | 12 +++++++-
 gcc/analyzer/engine.cc             |  8 ++++++
 gcc/analyzer/exploded-graph.h      |  1 +
 gcc/analyzer/region-model.cc       | 13 +++++++++
 gcc/analyzer/region-model.h        |  6 ++++
 6 files changed, 82 insertions(+), 3 deletions(-)
  

Patch

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 62f78f35dc08..10fea486b8c8 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -721,6 +721,15 @@  saved_diagnostic::add_note (std::unique_ptr<pending_note> pn)
   m_notes.safe_push (pn.release ());
 }
 
+/* Add EVENT to this diagnostic.  */
+
+void
+saved_diagnostic::add_event (std::unique_ptr<checker_event> event)
+{
+  gcc_assert (event);
+  m_saved_events.safe_push (event.release ());
+}
+
 /* Return a new json::object of the form
    {"sm": optional str,
     "enode": int,
@@ -890,6 +899,19 @@  saved_diagnostic::supercedes_p (const saved_diagnostic &other) const
   return m_d->supercedes_p (*other.m_d);
 }
 
+/* Move any saved checker_events from this saved_diagnostic to
+   the end of DST_PATH.  */
+
+void
+saved_diagnostic::add_any_saved_events (checker_path &dst_path)
+{
+  for (auto &event : m_saved_events)
+    {
+      dst_path.add_event (std::unique_ptr<checker_event> (event));
+      event = nullptr;
+    }
+}
+
 /* Emit any pending notes owned by this diagnostic.  */
 
 void
@@ -1057,6 +1079,20 @@  diagnostic_manager::add_note (std::unique_ptr<pending_note> pn)
   sd->add_note (std::move (pn));
 }
 
+/* Add EVENT to the most recent saved_diagnostic.  */
+
+void
+diagnostic_manager::add_event (std::unique_ptr<checker_event> event)
+{
+  LOG_FUNC (get_logger ());
+  gcc_assert (event);
+
+  /* Get most recent saved_diagnostic.  */
+  gcc_assert (m_saved_diagnostics.length () > 0);
+  saved_diagnostic *sd = m_saved_diagnostics[m_saved_diagnostics.length () - 1];
+  sd->add_event (std::move (event));
+}
+
 /* Return a new json::object of the form
    {"diagnostics"  : [obj for saved_diagnostic]}.  */
 
@@ -1308,7 +1344,7 @@  public:
       {
 	saved_diagnostic **slot = m_map.get (key);
 	gcc_assert (*slot);
-	const saved_diagnostic *sd = *slot;
+	saved_diagnostic *sd = *slot;
 	dm->emit_saved_diagnostic (eg, *sd);
       }
   }
@@ -1370,7 +1406,7 @@  diagnostic_manager::emit_saved_diagnostics (const exploded_graph &eg)
 
 void
 diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg,
-					   const saved_diagnostic &sd)
+					   saved_diagnostic &sd)
 {
   LOG_SCOPE (get_logger ());
   log ("sd[%i]: %qs at SN: %i",
@@ -1395,6 +1431,11 @@  diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg,
   /* Now prune it to just cover the most pertinent events.  */
   prune_path (&emission_path, sd.m_sm, sd.m_sval, sd.m_state);
 
+  /* Add any saved events to the path, giving contextual information
+     about what the analyzer was simulating as the diagnostic was
+     generated.  These don't get pruned, as they are probably pertinent.  */
+  sd.add_any_saved_events (emission_path);
+
   /* Add a final event to the path, covering the diagnostic itself.
      We use the final enode from the epath, which might be different from
      the sd.m_enode, as the dedupe code doesn't care about enodes, just
diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h
index d3022b888dd5..413ab0c90b14 100644
--- a/gcc/analyzer/diagnostic-manager.h
+++ b/gcc/analyzer/diagnostic-manager.h
@@ -42,6 +42,7 @@  public:
   bool operator== (const saved_diagnostic &other) const;
 
   void add_note (std::unique_ptr<pending_note> pn);
+  void add_event (std::unique_ptr<checker_event> event);
 
   json::object *to_json () const;
 
@@ -64,6 +65,8 @@  public:
 
   bool supercedes_p (const saved_diagnostic &other) const;
 
+  void add_any_saved_events (checker_path &dst_path);
+
   void emit_any_notes () const;
 
   //private:
@@ -87,6 +90,12 @@  private:
 
   auto_vec<const saved_diagnostic *> m_duplicates;
   auto_delete_vec <pending_note> m_notes;
+
+  /* Optionally: additional context-dependent events to be emitted
+     immediately before the warning_event, giving more details of what
+     operation was being simulated when a diagnostic was saved
+     e.g. "looking for null terminator in param 2 of 'foo'".  */
+  auto_delete_vec <checker_event> m_saved_events;
 };
 
 class path_builder;
@@ -124,11 +133,12 @@  public:
 		       std::unique_ptr<pending_diagnostic> d);
 
   void add_note (std::unique_ptr<pending_note> pn);
+  void add_event (std::unique_ptr<checker_event> event);
 
   void emit_saved_diagnostics (const exploded_graph &eg);
 
   void emit_saved_diagnostic (const exploded_graph &eg,
-			      const saved_diagnostic &sd);
+			      saved_diagnostic &sd);
 
   unsigned get_num_diagnostics () const
   {
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 61685f43fba0..3700154eec2c 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -149,6 +149,14 @@  impl_region_model_context::add_note (std::unique_ptr<pending_note> pn)
     m_eg->get_diagnostic_manager ().add_note (std::move (pn));
 }
 
+void
+impl_region_model_context::add_event (std::unique_ptr<checker_event> event)
+{
+  LOG_FUNC (get_logger ());
+  if (m_eg)
+    m_eg->get_diagnostic_manager ().add_event (std::move (event));
+}
+
 void
 impl_region_model_context::on_svalue_leak (const svalue *sval)
 
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index 4a4ef9d12b48..5a7ab645bfee 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -58,6 +58,7 @@  class impl_region_model_context : public region_model_context
 
   bool warn (std::unique_ptr<pending_diagnostic> d) final override;
   void add_note (std::unique_ptr<pending_note> pn) final override;
+  void add_event (std::unique_ptr<checker_event> event) final override;
   void on_svalue_leak (const svalue *) override;
   void on_liveness_change (const svalue_set &live_svalues,
 			   const region_model *model) final override;
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 5c165ff127f8..fa30193943d2 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -5855,6 +5855,11 @@  noop_region_model_context::add_note (std::unique_ptr<pending_note>)
 {
 }
 
+void
+noop_region_model_context::add_event (std::unique_ptr<checker_event>)
+{
+}
+
 void
 noop_region_model_context::bifurcate (std::unique_ptr<custom_edge_info>)
 {
@@ -5865,6 +5870,14 @@  noop_region_model_context::terminate_path ()
 {
 }
 
+/* class region_model_context_decorator : public region_model_context.  */
+
+void
+region_model_context_decorator::add_event (std::unique_ptr<checker_event> event)
+{
+  m_inner->add_event (std::move (event));
+}
+
 /* struct model_merger.  */
 
 /* Dump a multiline representation of this merger to PP.  */
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 88772655bc5b..cdfce0727cf7 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -627,6 +627,10 @@  class region_model_context
      pending diagnostic.  */
   virtual void add_note (std::unique_ptr<pending_note> pn) = 0;
 
+  /* Hook for clients to add an event to the last previously stored
+     pending diagnostic.  */
+  virtual void add_event (std::unique_ptr<checker_event> event) = 0;
+
   /* Hook for clients to be notified when an SVAL that was reachable
      in a previous state is no longer live, so that clients can emit warnings
      about leaks.  */
@@ -733,6 +737,7 @@  class noop_region_model_context : public region_model_context
 public:
   bool warn (std::unique_ptr<pending_diagnostic>) override { return false; }
   void add_note (std::unique_ptr<pending_note>) override;
+  void add_event (std::unique_ptr<checker_event>) override;
   void on_svalue_leak (const svalue *) override {}
   void on_liveness_change (const svalue_set &,
 			   const region_model *) override {}
@@ -815,6 +820,7 @@  class region_model_context_decorator : public region_model_context
   {
     m_inner->add_note (std::move (pn));
   }
+  void add_event (std::unique_ptr<checker_event> event) override;
 
   void on_svalue_leak (const svalue *sval) override
   {