This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH 08/11] [analyzer] Show rewind destination for leaks due to longjmp
- From: David Malcolm <dmalcolm at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Wed, 20 Nov 2019 16:13:47 -0500
- Subject: [PATCH 08/11] [analyzer] Show rewind destination for leaks due to longjmp
- References: <1574284430-8776-1-git-send-email-dmalcolm@redhat.com>
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(-)
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
--
1.8.5.3