From 2b0a7fe3abfbd47081f714a0a1263afe00c5cfd9 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Sat, 1 Jun 2024 13:50:32 -0400 Subject: analyzer: detect -Wanalyzer-allocation-size at call stmts [PR106203] gcc/analyzer/ChangeLog: PR analyzer/106203 * checker-event.h: Include "analyzer/event-loc-info.h". (struct event_loc_info): Move to its own header file. * diagnostic-manager.cc (diagnostic_manager::emit_saved_diagnostic): Move creation of event_loc_info here from add_final_event, and if we have a stmt_finder, call its update_event_loc_info method. * engine.cc (leak_stmt_finder::update_event_loc_info): New. (exploded_node::detect_leaks): Likewise. (exploded_node::detect_leaks): Pass nullptr as call_stmt arg to region_model::pop_frame. * event-loc-info.h: New file, with content taken from checker-event.h. * exploded-graph.h (stmt_finder::update_event_loc_info): New pure virtual function. * infinite-loop.cc (infinite_loop_diagnostic::add_final_event): Update for change to vfunc signature. * infinite-recursion.cc (infinite_recursion_diagnostic::add_final_event): Likewise. * pending-diagnostic.cc (pending_diagnostic::add_final_event): Pass in the event_loc_info from the caller, rather than generating it from a gimple stmt and enode. * pending-diagnostic.h (pending_diagnostic::add_final_event): Likewise. * region-model.cc (region_model::on_longjmp): Pass nullptr as call_stmt arg to region_model::pop_frame. (region_model::update_for_return_gcall): Likewise, but pass call_stmt. (class caller_context): New. (region_model::pop_frame): Add "call_stmt" argument. Use it and the frame_region with a caller_context when setting result_dst_reg's value so that any diagnostic is reported at the call stmt in the caller. (selftest::test_stack_frames): Pass nullptr as call_stmt arg to region_model::pop_frame. (selftest::test_alloca): Likewise. * region-model.h (region_model::pop_frame): Add "call_stmt" argument. gcc/testsuite/ChangeLog: PR analyzer/106203 * c-c++-common/analyzer/allocation-size-1.c (test_9): Remove xfail. * c-c++-common/analyzer/allocation-size-2.c (test_8): Likewise. * gcc.dg/analyzer/allocation-size-multiline-4.c: New test. * gcc.dg/plugin/analyzer_cpython_plugin.c (refcnt_stmt_finder::update_event_loc_info): New. Signed-off-by: David Malcolm --- gcc/analyzer/checker-event.h | 14 +------ gcc/analyzer/diagnostic-manager.cc | 13 +++++- gcc/analyzer/engine.cc | 7 +++- gcc/analyzer/event-loc-info.h | 41 +++++++++++++++++++ gcc/analyzer/exploded-graph.h | 1 + gcc/analyzer/infinite-loop.cc | 2 +- gcc/analyzer/infinite-recursion.cc | 2 +- gcc/analyzer/pending-diagnostic.cc | 6 +-- gcc/analyzer/pending-diagnostic.h | 2 +- gcc/analyzer/region-model.cc | 84 +++++++++++++++++++++++++++++++++++--- gcc/analyzer/region-model.h | 1 + 11 files changed, 145 insertions(+), 28 deletions(-) create mode 100644 gcc/analyzer/event-loc-info.h (limited to 'gcc/analyzer') diff --git a/gcc/analyzer/checker-event.h b/gcc/analyzer/checker-event.h index 7a4510e..d0935ac 100644 --- a/gcc/analyzer/checker-event.h +++ b/gcc/analyzer/checker-event.h @@ -23,22 +23,10 @@ along with GCC; see the file COPYING3. If not see #include "tree-logical-location.h" #include "analyzer/program-state.h" +#include "analyzer/event-loc-info.h" namespace ana { -/* A bundle of location information for a checker_event. */ - -struct event_loc_info -{ - event_loc_info (location_t loc, tree fndecl, int depth) - : m_loc (loc), m_fndecl (fndecl), m_depth (depth) - {} - - location_t m_loc; - tree m_fndecl; - int m_depth; -}; - /* An enum for discriminating between the concrete subclasses of checker_event. */ diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index da98b96..20e793d 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -1588,8 +1588,17 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg, 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 snodes. */ - sd.m_d->add_final_event (sd.m_sm, epath->get_final_enode (), sd.m_stmt, - sd.m_var, sd.m_state, &emission_path); + { + const exploded_node *const enode = epath->get_final_enode (); + const gimple *stmt = sd.m_stmt; + event_loc_info loc_info (get_stmt_location (stmt, enode->get_function ()), + enode->get_function ()->decl, + enode->get_stack_depth ()); + if (sd.m_stmt_finder) + sd.m_stmt_finder->update_event_loc_info (loc_info); + sd.m_d->add_final_event (sd.m_sm, enode, loc_info, + sd.m_var, sd.m_state, &emission_path); + } /* 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 diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 46d9b80..8b3706c 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -681,6 +681,11 @@ public: return NULL; } + void update_event_loc_info (event_loc_info &) final override + { + /* No-op. */ + } + private: const exploded_graph &m_eg; tree m_var; @@ -2056,7 +2061,7 @@ exploded_node::detect_leaks (exploded_graph &eg) &old_state, &new_state, &uncertainty, NULL, get_stmt ()); const svalue *result = NULL; - new_state.m_region_model->pop_frame (NULL, &result, &ctxt); + new_state.m_region_model->pop_frame (NULL, &result, &ctxt, nullptr); program_state::detect_leaks (old_state, new_state, result, eg.get_ext_state (), &ctxt); } diff --git a/gcc/analyzer/event-loc-info.h b/gcc/analyzer/event-loc-info.h new file mode 100644 index 0000000..60b2035 --- /dev/null +++ b/gcc/analyzer/event-loc-info.h @@ -0,0 +1,41 @@ +/* A bundle of location information for a checker_event. + Copyright (C) 2019-2024 Free Software Foundation, Inc. + Contributed by David Malcolm . + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it +under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +. */ + +#ifndef GCC_ANALYZER_EVENT_LOC_INFO_H +#define GCC_ANALYZER_EVENT_LOC_INFO_H + +namespace ana { + +/* A bundle of location information for a checker_event. */ + +struct event_loc_info +{ + event_loc_info (location_t loc, tree fndecl, int depth) + : m_loc (loc), m_fndecl (fndecl), m_depth (depth) + {} + + location_t m_loc; + tree m_fndecl; + int m_depth; +}; + +} // namespace ana + +#endif /* GCC_ANALYZER_EVENT_LOC_INFO_H */ diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h index 642d69b..f847c01 100644 --- a/gcc/analyzer/exploded-graph.h +++ b/gcc/analyzer/exploded-graph.h @@ -1035,6 +1035,7 @@ public: virtual ~stmt_finder () {} virtual std::unique_ptr clone () const = 0; virtual const gimple *find_stmt (const exploded_path &epath) = 0; + virtual void update_event_loc_info (event_loc_info &) = 0; }; // TODO: split the above up? diff --git a/gcc/analyzer/infinite-loop.cc b/gcc/analyzer/infinite-loop.cc index a83b813..8ba8e70 100644 --- a/gcc/analyzer/infinite-loop.cc +++ b/gcc/analyzer/infinite-loop.cc @@ -229,7 +229,7 @@ public: /* Customize the location where the warning_event appears. */ void add_final_event (const state_machine *, const exploded_node *enode, - const gimple *, + const event_loc_info &, tree, state_machine::state_t, checker_path *emission_path) final override diff --git a/gcc/analyzer/infinite-recursion.cc b/gcc/analyzer/infinite-recursion.cc index 6103c2b..ef8ae90 100644 --- a/gcc/analyzer/infinite-recursion.cc +++ b/gcc/analyzer/infinite-recursion.cc @@ -183,7 +183,7 @@ public: it at the topmost entrypoint to the function. */ void add_final_event (const state_machine *, const exploded_node *enode, - const gimple *, + const event_loc_info &, tree, state_machine::state_t, checker_path *emission_path) final override diff --git a/gcc/analyzer/pending-diagnostic.cc b/gcc/analyzer/pending-diagnostic.cc index ff37ae6..b0637be 100644 --- a/gcc/analyzer/pending-diagnostic.cc +++ b/gcc/analyzer/pending-diagnostic.cc @@ -270,15 +270,13 @@ pending_diagnostic::add_region_creation_events (const region *reg, void pending_diagnostic::add_final_event (const state_machine *sm, const exploded_node *enode, - const gimple *stmt, + const event_loc_info &loc_info, tree var, state_machine::state_t state, checker_path *emission_path) { emission_path->add_event (make_unique - (event_loc_info (get_stmt_location (stmt, enode->get_function ()), - enode->get_function ()->decl, - enode->get_stack_depth ()), + (loc_info, enode, sm, var, state)); } diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h index 288410a..ee1f620 100644 --- a/gcc/analyzer/pending-diagnostic.h +++ b/gcc/analyzer/pending-diagnostic.h @@ -371,7 +371,7 @@ class pending_diagnostic of the called function. */ virtual void add_final_event (const state_machine *sm, const exploded_node *enode, - const gimple *stmt, + const event_loc_info &loc_info, tree var, state_machine::state_t state, checker_path *emission_path); diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index e71c6ec..d142d85 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -2331,7 +2331,7 @@ region_model::on_longjmp (const gcall *longjmp_call, const gcall *setjmp_call, setjmp was called. */ gcc_assert (get_stack_depth () >= setjmp_stack_depth); while (get_stack_depth () > setjmp_stack_depth) - pop_frame (NULL, NULL, ctxt, false); + pop_frame (NULL, NULL, ctxt, nullptr, false); gcc_assert (get_stack_depth () == setjmp_stack_depth); @@ -5673,7 +5673,7 @@ region_model::update_for_return_gcall (const gcall *call_stmt, so that pop_frame can determine the region with respect to the *caller* frame. */ tree lhs = gimple_call_lhs (call_stmt); - pop_frame (lhs, NULL, ctxt); + pop_frame (lhs, NULL, ctxt, call_stmt); } /* Extract calling information from the superedge and update the model for the @@ -6083,6 +6083,70 @@ region_model::get_current_function () const return &frame->get_function (); } +/* Custom region_model_context for the assignment to the result + at a call statement when popping a frame (PR analyzer/106203). */ + +class caller_context : public region_model_context_decorator +{ +public: + caller_context (region_model_context *inner, + const gcall *call_stmt, + const frame_region &caller_frame) + : region_model_context_decorator (inner), + m_call_stmt (call_stmt), + m_caller_frame (caller_frame) + {} + bool warn (std::unique_ptr d, + const stmt_finder *custom_finder) override + { + if (m_inner && custom_finder == nullptr) + { + /* Custom stmt_finder to use m_call_stmt for the + diagnostic. */ + class my_finder : public stmt_finder + { + public: + my_finder (const gcall *call_stmt, + const frame_region &caller_frame) + : m_call_stmt (call_stmt), + m_caller_frame (caller_frame) + {} + std::unique_ptr clone () const override + { + return ::make_unique (m_call_stmt, m_caller_frame); + } + const gimple *find_stmt (const exploded_path &) override + { + return m_call_stmt; + } + void update_event_loc_info (event_loc_info &loc_info) final override + { + loc_info.m_fndecl = m_caller_frame.get_fndecl (); + loc_info.m_depth = m_caller_frame.get_stack_depth (); + } + + private: + const gcall *m_call_stmt; + const frame_region &m_caller_frame; + }; + my_finder finder (m_call_stmt, m_caller_frame); + return m_inner->warn (std::move (d), &finder); + } + else + return region_model_context_decorator::warn (std::move (d), + custom_finder); + } + const gimple *get_stmt () const override + { + return m_call_stmt; + }; + +private: + const gcall *m_call_stmt; + const frame_region &m_caller_frame; +}; + + /* Pop the topmost frame_region from this region_model's stack; If RESULT_LVALUE is non-null, copy any return value from the frame @@ -6091,6 +6155,9 @@ region_model::get_current_function () const If OUT_RESULT is non-null, copy any return value from the frame into *OUT_RESULT. + If non-null, use CALL_STMT as the location when complaining about + assignment of the return value to RESULT_LVALUE. + If EVAL_RETURN_SVALUE is false, then don't evaluate the return value. This is for use when unwinding frames e.g. due to longjmp, to suppress erroneously reporting uninitialized return values. @@ -6103,6 +6170,7 @@ void region_model::pop_frame (tree result_lvalue, const svalue **out_result, region_model_context *ctxt, + const gcall *call_stmt, bool eval_return_svalue) { gcc_assert (m_current_frame); @@ -6137,7 +6205,13 @@ region_model::pop_frame (tree result_lvalue, /* Compute result_dst_reg using RESULT_LVALUE *after* popping the frame, but before poisoning pointers into the old frame. */ const region *result_dst_reg = get_lvalue (result_lvalue, ctxt); - set_value (result_dst_reg, retval, ctxt); + + /* Assign retval to result_dst_reg, using caller_context + to set the call_stmt and the popped_frame for any diagnostics + due to the assignment. */ + gcc_assert (m_current_frame); + caller_context caller_ctxt (ctxt, call_stmt, *m_current_frame); + set_value (result_dst_reg, retval, call_stmt ? &caller_ctxt : ctxt); } unbind_region_and_descendents (frame_reg,POISON_KIND_POPPED_STACK); @@ -8130,7 +8204,7 @@ test_stack_frames () ASSERT_FALSE (a_in_parent_reg->descendent_of_p (child_frame_reg)); /* Pop the "child_fn" frame from the stack. */ - model.pop_frame (NULL, NULL, &ctxt); + model.pop_frame (NULL, NULL, &ctxt, nullptr); ASSERT_FALSE (model.region_exists_p (child_frame_reg)); ASSERT_TRUE (model.region_exists_p (parent_frame_reg)); @@ -9243,7 +9317,7 @@ test_alloca () /* Verify that the pointers to the alloca region are replaced by poisoned values when the frame is popped. */ - model.pop_frame (NULL, NULL, &ctxt); + model.pop_frame (NULL, NULL, &ctxt, nullptr); ASSERT_EQ (model.get_rvalue (p, NULL)->get_kind (), SK_POISONED); } diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 400d80b..1a0233f 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -363,6 +363,7 @@ class region_model void pop_frame (tree result_lvalue, const svalue **out_result, region_model_context *ctxt, + const gcall *call_stmt, bool eval_return_svalue = true); int get_stack_depth () const; const frame_region *get_frame_at_index (int index) const; -- cgit v1.1