[08/11,analyzer] Show rewind destination for leaks due to longjmp
diff mbox series

Message ID 1574284430-8776-9-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • Static analysis v2
Related show

Commit Message

David Malcolm Nov. 20, 2019, 9:13 p.m. UTC
This patch adds some special-case logic to how paths are generated
so that leaks due to longjmp rewinding past a frame now show where the
longjmp rewinds to (longjmp and setjmp are already something of a
special-case so this seems reasonable).

A colorized LTO example of the output can be seen at:
https://dmalcolm.fedorapeople.org/gcc/2019-11-18/lto-longjmp-leak-demo.html

gcc/ChangeLog:
	* analyzer/diagnostic-manager.cc
	(saved_diagnostic::saved_diagnostic): Initialize new field
	m_trailing_eedge.
	(diagnostic_manager::emit_saved_diagnostic): If m_trailing_eedge
	is set, use it to add extra events after the "final" event.
	* analyzer/diagnostic-manager.h (saved_diagnostic::operator==):
	Compare m_trailing_eedge fields.
	(saved_diagnostic::m_trailing_eedge): New field.
	(diagnostic_manager::get_num_diagnostics): New accessor.
	(diagnostic_manager::get_saved_diagnostic): New accessor.
	* analyzer/engine.cc (exploded_node::on_longjmp): Set
	m_trailing_edge on any diagnostics saved when handling
	region_model::on_longjmp.
	(exploded_graph::add_edge): Return the newly-created eedge.
	* analyzer/exploded-graph.h (exploded_graph::add_edge): Convert
	return type from void to exploded_edge *.

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/setjmp-7a.c: Update expected multiline output to
	include a final pair of events showing the longjmp and the setjmp
	that it rewinds to.
---
 gcc/analyzer/diagnostic-manager.cc        |  9 ++++-
 gcc/analyzer/diagnostic-manager.h         | 13 ++++++-
 gcc/analyzer/engine.cc                    | 56 ++++++++++++++++++++++++++++---
 gcc/analyzer/exploded-graph.h             |  8 ++---
 gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c | 13 +++++--
 5 files changed, 86 insertions(+), 13 deletions(-)

Patch
diff mbox series

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index fae38c7..926900b 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -47,7 +47,7 @@  saved_diagnostic::saved_diagnostic (const state_machine *sm,
     outlive that.  */
   m_stmt_finder (stmt_finder ? stmt_finder->clone () : NULL),
   m_var (var), m_state (state),
-  m_d (d)
+  m_d (d), m_trailing_eedge (NULL)
 {
   gcc_assert (m_stmt || m_stmt_finder);
 
@@ -427,6 +427,13 @@  diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg,
   emission_path.add_final_event (sd.m_sm, epath.get_final_enode (), stmt,
 				 sd.m_var, sd.m_state);
 
+  /* The "final" event might not be final; if the saved_diagnostic has a
+     trailing eedge stashed, add any events for it.  This is for use
+     in handling longjmp, to show where a longjmp is rewinding to.  */
+  if (sd.m_trailing_eedge)
+    add_events_for_eedge (*sd.m_trailing_eedge, eg.get_ext_state (),
+			  &emission_path);
+
   emission_path.prepare_for_emission (sd.m_d);
 
   gcc_rich_location rich_loc (stmt->location);
diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h
index 0f4444c..8591d2e 100644
--- a/gcc/analyzer/diagnostic-manager.h
+++ b/gcc/analyzer/diagnostic-manager.h
@@ -45,7 +45,8 @@  public:
 	    /* We don't compare m_stmt_finder.  */
 	    && m_var == other.m_var
 	    && m_state == other.m_state
-	    && m_d->equal_p (*other.m_d));
+	    && m_d->equal_p (*other.m_d)
+	    && m_trailing_eedge == other.m_trailing_eedge);
   }
 
   //private:
@@ -57,6 +58,7 @@  public:
   tree m_var;
   state_machine::state_t m_state;
   pending_diagnostic *m_d;
+  exploded_edge *m_trailing_eedge;
 };
 
 /* A class with responsibility for saving pending diagnostics, so that
@@ -93,6 +95,15 @@  public:
 			      const gimple *stmt,
 			      int num_dupes);
 
+  unsigned get_num_diagnostics () const
+  {
+    return m_saved_diagnostics.length ();
+  }
+  saved_diagnostic *get_saved_diagnostic (unsigned idx)
+  {
+    return m_saved_diagnostics[idx];
+  }
+
 private:
   void build_emission_path (const exploded_graph &eg,
 			    const exploded_path &epath,
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 9936e1e..aa4a359 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -1083,6 +1083,13 @@  exploded_node::on_longjmp (exploded_graph &eg,
 	      >= setjmp_point.get_stack_depth ());
 
   /* Update the state for use by the destination node.  */
+
+  /* Stash the current number of diagnostics so that we can update
+     any that this adds to show where the longjmp is rewinding to.  */
+
+  diagnostic_manager *dm = &eg.get_diagnostic_manager ();
+  unsigned prev_num_diagnostics = dm->get_num_diagnostics ();
+
   new_region_model->on_longjmp (longjmp_call, setjmp_call,
 				setjmp_point.get_stack_depth (), ctxt);
 
@@ -1095,9 +1102,46 @@  exploded_node::on_longjmp (exploded_graph &eg,
 
   /* Create custom exploded_edge for a longjmp.  */
   if (next)
-    eg.add_edge (const_cast<exploded_node *> (this), next, NULL,
-		 change,
-		 new rewind_info_t (enode_origin));
+    {
+      exploded_edge *eedge
+	= eg.add_edge (const_cast<exploded_node *> (this), next, NULL,
+		       change,
+		       new rewind_info_t (enode_origin));
+
+      /* For any diagnostics that were queued here (such as leaks) we want
+	 the checker_path to show the rewinding events after the "final event"
+	 so that the user sees where the longjmp is rewinding to (otherwise the
+	 path is meaningless).
+
+	 For example, we want to emit something like:
+                        |   NN | {
+                        |   NN |   longjmp (env, 1);
+                        |      |   ~~~~~~~~~~~~~~~~
+                        |      |   |
+                        |      |   (10) 'ptr' leaks here; was allocated at (7)
+                        |      |   (11) rewinding from 'longjmp' in 'inner'...
+                        |
+          <-------------+
+          |
+        'outer': event 12
+          |
+          |   NN |   i = setjmp(env);
+          |      |       ^~~~~~
+          |      |       |
+          |      |       (12) ...to 'setjmp' in 'outer' (saved at (2))
+
+	 where the "final" event above is event (10), but we want to append
+	 events (11) and (12) afterwards.
+
+	 Do this by setting m_trailing_eedge on any diagnostics that were
+	 just saved.  */
+      unsigned num_diagnostics = dm->get_num_diagnostics ();
+      for (unsigned i = prev_num_diagnostics; i < num_diagnostics; i++)
+	{
+	  saved_diagnostic *sd = dm->get_saved_diagnostic (i);
+	  sd->m_trailing_eedge = eedge;
+	}
+    }
 }
 
 /* Subroutine of exploded_graph::process_node for finding the successors
@@ -1768,9 +1812,10 @@  exploded_graph::get_or_create_node (const program_point &point,
 
 /* Add an exploded_edge from SRC to DEST, recording its association
    with SEDGE (which may be NULL), and, if non-NULL, taking ownership
-   of REWIND_INFO.  */
+   of REWIND_INFO.
+   Return the newly-created eedge.  */
 
-void
+exploded_edge *
 exploded_graph::add_edge (exploded_node *src, exploded_node *dest,
 			  const superedge *sedge,
 			  const state_change &change,
@@ -1778,6 +1823,7 @@  exploded_graph::add_edge (exploded_node *src, exploded_node *dest,
 {
   exploded_edge *e = new exploded_edge (src, dest, sedge, change, rewind_info);
   digraph::add_edge (e);
+  return e;
 }
 
 /* Ensure that this graph has per-program_point-data for POINT;
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index f97d2b6..97e1005 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -627,10 +627,10 @@  public:
   exploded_node *get_or_create_node (const program_point &point,
 				     const program_state &state,
 				     state_change *change);
-  void add_edge (exploded_node *src, exploded_node *dest,
-		 const superedge *sedge,
-		 const state_change &change,
-		 rewind_info_t *rewind_info = NULL);
+  exploded_edge *add_edge (exploded_node *src, exploded_node *dest,
+			   const superedge *sedge,
+			   const state_change &change,
+			   rewind_info_t *rewind_info = NULL);
 
   per_program_point_data *
   get_or_create_per_program_point_data (const program_point &);
diff --git a/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c b/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c
index 1d5fc2e..6f99df5 100644
--- a/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c
+++ b/gcc/testsuite/gcc.dg/analyzer/setjmp-7a.c
@@ -85,7 +85,7 @@  void outer (void)
            |      |   |
            |      |   (8) calling 'inner' from 'middle'
            |
-           +--> 'inner': events 9-10
+           +--> 'inner': events 9-11
                   |
                   |   NN | static void inner (void)
                   |      |             ^~~~~
@@ -96,6 +96,15 @@  void outer (void)
                   |      |   ~~~~~~~~~~~~~~~~
                   |      |   |
                   |      |   (10) 'ptr' leaks here; was allocated at (7)
+                  |      |   (11) rewinding from 'longjmp' in 'inner'...
                   |
+    <-------------+
+    |
+  'outer': event 12
+    |
+    |   NN |   i = setjmp(env);
+    |      |       ^~~~~~
+    |      |       |
+    |      |       (12) ...to 'setjmp' in 'outer' (saved at (2))
+    |
     { dg-end-multiline-output "" } */
-// TODO: show the rewind to the setjmp