From 8069928d5c2a99973ac16a49d6c25bc4dc886e14 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 11 Nov 2020 21:18:59 -0500 Subject: [PATCH] analyzer: precision-of-wording for -Wanalyzer-stale-setjmp-buffer This patch adds a custom event to paths emitted by -Wanalyzer-stale-setjmp-buffer highlighting the place where the pertinent stack frame is popped, and updates the final event in the path to reference this. gcc/analyzer/ChangeLog: * checker-path.h (checker_event::get_id_ptr): New. * diagnostic-manager.cc (path_builder::path_builder): Add "sd" param and use it to initialize new field "m_sd". (path_builder::get_pending_diagnostic): New. (path_builder::m_sd): New field. (diagnostic_manager::emit_saved_diagnostic): Pass sd to path_builder ctor. (diagnostic_manager::add_events_for_superedge): Call new maybe_add_custom_events_for_superedge vfunc. * engine.cc (stale_jmp_buf::stale_jmp_buf): Add "setjmp_point" param and use it to initialize new field "m_setjmp_point". Initialize new field "m_stack_pop_event". (stale_jmp_buf::maybe_add_custom_events_for_superedge): New vfunc implementation. (stale_jmp_buf::describe_final_event): New vfunc implementation. (stale_jmp_buf::m_setjmp_point): New field. (stale_jmp_buf::m_stack_pop_event): New field. (exploded_node::on_longjmp): Pass setjmp_point to stale_jmp_buf ctor. * pending-diagnostic.h (pending_diagnostic::maybe_add_custom_events_for_superedge): New vfunc. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/setjmp-5.c: Update expected path output to show an event where the pertinent stack frame is popped. Update expected message from final event to reference this event. --- gcc/analyzer/checker-path.h | 6 +++ gcc/analyzer/diagnostic-manager.cc | 18 +++++++- gcc/analyzer/engine.cc | 55 ++++++++++++++++++++++-- gcc/analyzer/pending-diagnostic.h | 15 +++++++ gcc/testsuite/gcc.dg/analyzer/setjmp-5.c | 13 ++++-- 5 files changed, 99 insertions(+), 8 deletions(-) diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h index 7b86d48e983..a00b3cf09ea 100644 --- a/gcc/analyzer/checker-path.h +++ b/gcc/analyzer/checker-path.h @@ -97,6 +97,12 @@ public: virtual bool is_function_entry_p () const { return false; } virtual bool is_return_p () const { return false; } + /* For use with %@. */ + const diagnostic_event_id_t *get_id_ptr () const + { + return &m_emission_id; + } + void dump (pretty_printer *pp) const; public: diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 93f270f7c2c..13f6dd2f341 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -161,15 +161,22 @@ class path_builder public: path_builder (const exploded_graph &eg, const exploded_path &epath, - const feasibility_problem *problem) + const feasibility_problem *problem, + const saved_diagnostic &sd) : m_eg (eg), m_diag_enode (epath.get_final_enode ()), + m_sd (sd), m_reachability (eg, m_diag_enode), m_feasibility_problem (problem) {} const exploded_node *get_diag_node () const { return m_diag_enode; } + pending_diagnostic *get_pending_diagnostic () const + { + return m_sd.m_d; + } + bool reachable_from_p (const exploded_node *src_enode) const { return m_reachability.reachable_from_p (src_enode); @@ -190,6 +197,8 @@ private: /* The enode where the diagnostic occurs. */ const exploded_node *m_diag_enode; + const saved_diagnostic &m_sd; + /* Precompute all enodes from which the diagnostic is reachable. */ enode_reachability m_reachability; @@ -629,7 +638,7 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg, pretty_printer *pp = global_dc->printer->clone (); /* Precompute all enodes from which the diagnostic is reachable. */ - path_builder pb (eg, epath, sd.get_feasibility_problem ()); + path_builder pb (eg, epath, sd.get_feasibility_problem (), sd); /* This is the diagnostic_path subclass that will be built for the diagnostic. */ @@ -1175,6 +1184,11 @@ diagnostic_manager::add_events_for_superedge (const path_builder &pb, { gcc_assert (eedge.m_sedge); + /* Give diagnostics an opportunity to override this function. */ + pending_diagnostic *pd = pb.get_pending_diagnostic (); + if (pd->maybe_add_custom_events_for_superedge (eedge, emission_path)) + return; + /* Don't add events for insignificant edges at verbosity levels below 3. */ if (m_verbosity < 3) if (!significant_edge_p (pb, eedge)) diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index d247ebbc20e..0a8e5b87e01 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -1277,8 +1277,10 @@ valid_longjmp_stack_p (const program_point &longjmp_point, class stale_jmp_buf : public pending_diagnostic_subclass { public: - stale_jmp_buf (const gcall *setjmp_call, const gcall *longjmp_call) - : m_setjmp_call (setjmp_call), m_longjmp_call (longjmp_call) + stale_jmp_buf (const gcall *setjmp_call, const gcall *longjmp_call, + const program_point &setjmp_point) + : m_setjmp_call (setjmp_call), m_longjmp_call (longjmp_call), + m_setjmp_point (setjmp_point), m_stack_pop_event (NULL) {} bool emit (rich_location *richloc) FINAL OVERRIDE @@ -1299,9 +1301,56 @@ public: && m_longjmp_call == other.m_longjmp_call); } + bool + maybe_add_custom_events_for_superedge (const exploded_edge &eedge, + checker_path *emission_path) + FINAL OVERRIDE + { + /* Detect exactly when the stack first becomes invalid, + and issue an event then. */ + if (m_stack_pop_event) + return false; + const exploded_node *src_node = eedge.m_src; + const program_point &src_point = src_node->get_point (); + const exploded_node *dst_node = eedge.m_dest; + const program_point &dst_point = dst_node->get_point (); + if (valid_longjmp_stack_p (src_point, m_setjmp_point) + && !valid_longjmp_stack_p (dst_point, m_setjmp_point)) + { + /* Compare with diagnostic_manager::add_events_for_superedge. */ + const int src_stack_depth = src_point.get_stack_depth (); + m_stack_pop_event = new custom_event + (src_point.get_location (), + src_point.get_fndecl (), + src_stack_depth, + "stack frame is popped here, invalidating saved environment"); + emission_path->add_event (m_stack_pop_event); + return false; + } + return false; + } + + label_text describe_final_event (const evdesc::final_event &ev) + { + if (m_stack_pop_event) + return ev.formatted_print + ("%qs called after enclosing function of %qs returned at %@", + get_user_facing_name (m_longjmp_call), + get_user_facing_name (m_setjmp_call), + m_stack_pop_event->get_id_ptr ()); + else + return ev.formatted_print + ("%qs called after enclosing function of %qs has returned", + get_user_facing_name (m_longjmp_call), + get_user_facing_name (m_setjmp_call));; + } + + private: const gcall *m_setjmp_call; const gcall *m_longjmp_call; + program_point m_setjmp_point; + custom_event *m_stack_pop_event; }; /* Handle LONGJMP_CALL, a call to longjmp or siglongjmp. @@ -1344,7 +1393,7 @@ exploded_node::on_longjmp (exploded_graph &eg, /* Verify that the setjmp's call_stack hasn't been popped. */ if (!valid_longjmp_stack_p (longjmp_point, setjmp_point)) { - ctxt->warn (new stale_jmp_buf (setjmp_call, longjmp_call)); + ctxt->warn (new stale_jmp_buf (setjmp_call, longjmp_call, setjmp_point)); return; } diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h index b1ff2688bcc..ce626f67871 100644 --- a/gcc/analyzer/pending-diagnostic.h +++ b/gcc/analyzer/pending-diagnostic.h @@ -246,6 +246,21 @@ class pending_diagnostic } /* End of precision-of-wording vfuncs. */ + + /* Vfunc for extending/overriding creation of the events for an + exploded_edge that corresponds to a superedge, allowing for custom + events to be created that are pertinent to a particular + pending_diagnostic subclass. + + For example, the -Wanalyzer-stale-setjmp-buffer diagnostic adds a + custom event showing when the pertinent stack frame is popped + (and thus the point at which the jmp_buf becomes invalid). */ + + virtual bool maybe_add_custom_events_for_superedge (const exploded_edge &, + checker_path *) + { + return false; + } }; /* A template to make it easier to make subclasses of pending_diagnostic. diff --git a/gcc/testsuite/gcc.dg/analyzer/setjmp-5.c b/gcc/testsuite/gcc.dg/analyzer/setjmp-5.c index bf5b9bfda81..4787fa38032 100644 --- a/gcc/testsuite/gcc.dg/analyzer/setjmp-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/setjmp-5.c @@ -51,18 +51,25 @@ void outer (void) | | | | | (4) 'setjmp' called here | + 'inner': event 5 + | + | NN | } + | | ^ + | | | + | | (5) stack frame is popped here, invalidating saved environment + | <------+ | - 'outer': events 5-6 + 'outer': events 6-7 | | NN | inner (); | | ^~~~~~~~ | | | - | | (5) returning to 'outer' from 'inner' + | | (6) returning to 'outer' from 'inner' | NN | | NN | longjmp (env, 42); | | ~~~~~~~~~~~~~~~~~ | | | - | | (6) here + | | (7) 'longjmp' called after enclosing function of 'setjmp' returned at (5) | { dg-end-multiline-output "" } */ -- 2.30.2