* [pushed] analyzer: detect -Wanalyzer-allocation-size at call stmts [PR106203]
@ 2024-06-01 18:24 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2024-06-01 18:24 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Successful run of analyzer integration tests on x86_64-pc-linux-gnu.
Pushed to trunk as r15-969-g2b0a7fe3abfbd4.
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 <dmalcolm@redhat.com>
---
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 +
.../c-c++-common/analyzer/allocation-size-1.c | 8 +-
.../c-c++-common/analyzer/allocation-size-2.c | 8 +-
.../analyzer/allocation-size-multiline-4.c | 64 ++++++++++++++
.../gcc.dg/plugin/analyzer_cpython_plugin.c | 5 ++
15 files changed, 216 insertions(+), 42 deletions(-)
create mode 100644 gcc/analyzer/event-loc-info.h
create mode 100644 gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-4.c
diff --git a/gcc/analyzer/checker-event.h b/gcc/analyzer/checker-event.h
index 7a4510ee81d0..d0935aca9853 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 da98b9679cbb..20e793d72c19 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 46d9b80f4f7e..8b3706cdfa87 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 000000000000..60b2035bd2c1
--- /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 <dmalcolm@redhat.com>.
+
+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
+<http://www.gnu.org/licenses/>. */
+
+#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 642d69bbcc0a..f847c01132a2 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<stmt_finder> 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 a83b8130e43c..8ba8e70acffc 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 6103c2b564fb..ef8ae90ab08e 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 ff37ae6f7b43..b0637be179fe 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<warning_event>
- (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 288410af71a3..ee1f6204f2d2 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 e71c6ec693b4..d142d851a26f 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<pending_diagnostic> 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<stmt_finder> clone () const override
+ {
+ return ::make_unique<my_finder> (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 400d80b25683..1a0233f2e8ab 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;
diff --git a/gcc/testsuite/c-c++-common/analyzer/allocation-size-1.c b/gcc/testsuite/c-c++-common/analyzer/allocation-size-1.c
index 05efc4f8028a..5b4e73927825 100644
--- a/gcc/testsuite/c-c++-common/analyzer/allocation-size-1.c
+++ b/gcc/testsuite/c-c++-common/analyzer/allocation-size-1.c
@@ -103,13 +103,7 @@ void test_8 (void)
void test_9 (void)
{
- /* FIXME: At the moment, region_model::set_value (lhs, <return_value>)
- is called at the src_node of the return edge. This edge has no stmts
- associated with it, leading to a rejection of the warning inside
- impl_region_model_context::warn. To ensure that the indentation
- in the diagnostic is right, the warning has to be emitted on an EN
- that is after the return edge. */
- int32_t *buf = (int32_t *) create_buffer(42); /* { dg-warning "" "" { xfail *-*-* } } */
+ int32_t *buf = (int32_t *) create_buffer(42); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
free (buf);
}
diff --git a/gcc/testsuite/c-c++-common/analyzer/allocation-size-2.c b/gcc/testsuite/c-c++-common/analyzer/allocation-size-2.c
index ff4cb5634695..a9f92cf2d02c 100644
--- a/gcc/testsuite/c-c++-common/analyzer/allocation-size-2.c
+++ b/gcc/testsuite/c-c++-common/analyzer/allocation-size-2.c
@@ -86,13 +86,7 @@ void test_7(int32_t n)
void test_8(int32_t n)
{
- /* FIXME: At the moment, region_model::set_value (lhs, <return_value>)
- is called at the src_node of the return edge. This edge has no stmts
- associated with it, leading to a rejection of the warning inside
- impl_region_model_context::warn. To ensure that the indentation
- in the diagnostic is right, the warning has to be emitted on an EN
- that is after the return edge. */
- int32_t *buf = (int32_t *)create_buffer(n * sizeof(int16_t)); /* { dg-warning "" "" { xfail *-*-* } } */
+ int32_t *buf = (int32_t *)create_buffer(n * sizeof(int16_t)); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
free (buf);
}
diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-4.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-4.c
new file mode 100644
index 000000000000..6408ef800e17
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-multiline-4.c
@@ -0,0 +1,64 @@
+/* Verify that we report events that occur at returns from functions
+ with valid-looking interprocedural paths.
+
+ C only: there are only minor C/C++ differences in e.g. how the
+ function names are printed. Trying to handle both would overcomplicate
+ the test. */
+
+/* { dg-additional-options "-fdiagnostics-show-line-numbers" } */
+/* { dg-enable-nn-line-numbers "" } */
+/* { dg-additional-options "-fdiagnostics-path-format=inline-events" } */
+/* { dg-additional-options "-fdiagnostics-show-caret" } */
+/* { dg-additional-options "-fdiagnostics-show-path-depths" } */
+
+#include <stdlib.h>
+#include <stdint.h>
+
+void *create_buffer (int32_t n)
+{
+ return malloc(n);
+}
+
+void test_9 (void)
+{
+ int32_t *buf = (int32_t *) create_buffer(42); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */
+ free (buf);
+}
+
+/* { dg-begin-multiline-output "" }
+ NN | int32_t *buf = (int32_t *) create_buffer(42);
+ | ^~~~~~~~~~~~~~~~~
+ 'test_9': events 1-2 (depth 1)
+ |
+ | NN | void test_9 (void)
+ | | ^~~~~~
+ | | |
+ | | (1) entry to 'test_9'
+ | NN | {
+ | NN | int32_t *buf = (int32_t *) create_buffer(42);
+ | | ~~~~~~~~~~~~~~~~~
+ | | |
+ | | (2) calling 'create_buffer' from 'test_9'
+ |
+ +--> 'create_buffer': events 3-4 (depth 2)
+ |
+ | NN | void *create_buffer (int32_t n)
+ | | ^~~~~~~~~~~~~
+ | | |
+ | | (3) entry to 'create_buffer'
+ | NN | {
+ | NN | return malloc(n);
+ | | ~~~~~~~~~
+ | | |
+ | | (4) allocated 42 bytes here
+ |
+ <------+
+ |
+ 'test_9': event 5 (depth 1)
+ |
+ | NN | int32_t *buf = (int32_t *) create_buffer(42);
+ | | ^~~~~~~~~~~~~~~~~
+ | | |
+ | | (5) assigned to 'int32_t *'
+ |
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
index 1fc43007cf1f..b53b347bb4fa 100644
--- a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
@@ -274,6 +274,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;
--
2.26.3
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2024-06-01 18:24 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-01 18:24 [pushed] analyzer: detect -Wanalyzer-allocation-size at call stmts [PR106203] David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).