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
