public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-4430] analyzer: unify bounds-checking class hierarchies
@ 2022-12-01 2:31 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2022-12-01 2:31 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:8bc9e4ee874ea3618780413b79b51412dcc40363
commit r13-4430-g8bc9e4ee874ea3618780413b79b51412dcc40363
Author: David Malcolm <dmalcolm@redhat.com>
Date: Wed Nov 30 21:26:43 2022 -0500
analyzer: unify bounds-checking class hierarchies
Convert out-of-bounds class hierarchy from:
pending_diagnostic
out_of_bounds
past_the_end
buffer_overflow (*)
buffer_over_read (*)
buffer_underwrite (*)
buffer_under_read (*)
symbolic_past_the_end
symbolic_buffer_overflow (*)
symbolic_buffer_over_read (*)
to:
pending_diagnostic
out_of_bounds
concrete_out_of_bounds
concrete_past_the_end
concrete_buffer_overflow (*)
concrete_buffer_over_read (*)
concrete_buffer_underwrite (*)
concrete_buffer_under_read (*)
symbolic_past_the_end
symbolic_buffer_overflow (*)
symbolic_buffer_over_read (*)
where the concrete classes (i.e. the instantiable ones) are marked
with a (*).
Doing so undercovered a bug where, for CWE-131-examples.c, we were
emitting an extra:
warning: heap-based buffer over-read [CWE-122] [-Wanalyzer-out-of-bounds]
at the:
WidgetList[numWidgets] = NULL;
The issue was that within set_next_state we get the rvalue for the LHS,
which looks like a read to the bounds-checker. The patch fixes this by
passing NULL as the region_model_context * for such accesses.
gcc/analyzer/ChangeLog:
* bounds-checking.cc (class out_of_bounds): Split out from...
(class concrete_out_of_bounds): New abstract subclass.
(class past_the_end): Rename to...
(class concrete_past_the_end): ...this, and make a subclass of
concrete_out_of_bounds.
(class buffer_overflow): Rename to...
(class concrete_buffer_overflow): ...this, and make a subclass of
concrete_past_the_end.
(class buffer_over_read): Rename to...
(class concrete_buffer_over_read): ...this, and make a subclass of
concrete_past_the_end.
(class buffer_underwrite): Rename to...
(class concrete_buffer_underwrite): ...this, and make a subclass
of concrete_out_of_bounds.
(class buffer_under_read): Rename to...
(class concrete_buffer_under_read): ...this, and make a subclass
of concrete_out_of_bounds.
(class symbolic_past_the_end): Convert to a subclass of
out_of_bounds.
(symbolic_buffer_overflow::get_kind): New.
(symbolic_buffer_over_read::get_kind): New.
(region_model::check_region_bounds): Update for renamings.
* engine.cc (impl_sm_context::set_next_state): Eliminate
"new_ctxt", passing NULL to get_rvalue instead.
(impl_sm_context::warn): Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Diff:
---
gcc/analyzer/bounds-checking.cc | 185 ++++++++++++++++++++++++----------------
gcc/analyzer/engine.cc | 24 ++----
2 files changed, 115 insertions(+), 94 deletions(-)
diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc
index bc7d2dd17ae..aaf3f22109b 100644
--- a/gcc/analyzer/bounds-checking.cc
+++ b/gcc/analyzer/bounds-checking.cc
@@ -37,27 +37,21 @@ along with GCC; see the file COPYING3. If not see
namespace ana {
-/* Abstract base class for all out-of-bounds warnings with concrete values. */
+/* Abstract base class for all out-of-bounds warnings. */
-class out_of_bounds : public pending_diagnostic_subclass<out_of_bounds>
+class out_of_bounds : public pending_diagnostic
{
public:
- out_of_bounds (const region *reg, tree diag_arg,
- byte_range out_of_bounds_range)
- : m_reg (reg), m_diag_arg (diag_arg),
- m_out_of_bounds_range (out_of_bounds_range)
+ out_of_bounds (const region *reg, tree diag_arg)
+ : m_reg (reg), m_diag_arg (diag_arg)
{}
- const char *get_kind () const final override
- {
- return "out_of_bounds_diagnostic";
- }
-
- bool operator== (const out_of_bounds &other) const
+ bool subclass_equal_p (const pending_diagnostic &base_other) const override
{
- return m_reg == other.m_reg
- && m_out_of_bounds_range == other.m_out_of_bounds_range
- && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg);
+ const out_of_bounds &other
+ (static_cast <const out_of_bounds &>(base_other));
+ return (m_reg == other.m_reg
+ && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg));
}
int get_controlling_option () const final override
@@ -106,25 +100,51 @@ protected:
const region *m_reg;
tree m_diag_arg;
+};
+
+/* Abstract base class for all out-of-bounds warnings where the
+ out-of-bounds range is concrete. */
+
+class concrete_out_of_bounds : public out_of_bounds
+{
+public:
+ concrete_out_of_bounds (const region *reg, tree diag_arg,
+ byte_range out_of_bounds_range)
+ : out_of_bounds (reg, diag_arg),
+ m_out_of_bounds_range (out_of_bounds_range)
+ {}
+
+ bool subclass_equal_p (const pending_diagnostic &base_other) const override
+ {
+ const concrete_out_of_bounds &other
+ (static_cast <const concrete_out_of_bounds &>(base_other));
+ return (out_of_bounds::subclass_equal_p (other)
+ && m_out_of_bounds_range == other.m_out_of_bounds_range);
+ }
+
+protected:
byte_range m_out_of_bounds_range;
};
-/* Abstract subclass to complaing about out-of-bounds
+/* Abstract subclass to complaing about concrete out-of-bounds
past the end of the buffer. */
-class past_the_end : public out_of_bounds
+class concrete_past_the_end : public concrete_out_of_bounds
{
public:
- past_the_end (const region *reg, tree diag_arg, byte_range range,
- tree byte_bound)
- : out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound)
+ concrete_past_the_end (const region *reg, tree diag_arg, byte_range range,
+ tree byte_bound)
+ : concrete_out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound)
{}
- bool operator== (const past_the_end &other) const
+ bool
+ subclass_equal_p (const pending_diagnostic &base_other) const final override
{
- return out_of_bounds::operator== (other)
- && pending_diagnostic::same_tree_p (m_byte_bound,
- other.m_byte_bound);
+ const concrete_past_the_end &other
+ (static_cast <const concrete_past_the_end &>(base_other));
+ return (concrete_out_of_bounds::subclass_equal_p (other)
+ && pending_diagnostic::same_tree_p (m_byte_bound,
+ other.m_byte_bound));
}
label_text
@@ -143,14 +163,19 @@ protected:
/* Concrete subclass to complain about buffer overflows. */
-class buffer_overflow : public past_the_end
+class concrete_buffer_overflow : public concrete_past_the_end
{
public:
- buffer_overflow (const region *reg, tree diag_arg,
+ concrete_buffer_overflow (const region *reg, tree diag_arg,
byte_range range, tree byte_bound)
- : past_the_end (reg, diag_arg, range, byte_bound)
+ : concrete_past_the_end (reg, diag_arg, range, byte_bound)
{}
+ const char *get_kind () const final override
+ {
+ return "concrete_buffer_overflow";
+ }
+
bool emit (rich_location *rich_loc) final override
{
diagnostic_metadata m;
@@ -241,14 +266,19 @@ public:
/* Concrete subclass to complain about buffer over-reads. */
-class buffer_over_read : public past_the_end
+class concrete_buffer_over_read : public concrete_past_the_end
{
public:
- buffer_over_read (const region *reg, tree diag_arg,
- byte_range range, tree byte_bound)
- : past_the_end (reg, diag_arg, range, byte_bound)
+ concrete_buffer_over_read (const region *reg, tree diag_arg,
+ byte_range range, tree byte_bound)
+ : concrete_past_the_end (reg, diag_arg, range, byte_bound)
{}
+ const char *get_kind () const final override
+ {
+ return "concrete_buffer_over_read";
+ }
+
bool emit (rich_location *rich_loc) final override
{
diagnostic_metadata m;
@@ -337,13 +367,19 @@ public:
/* Concrete subclass to complain about buffer underwrites. */
-class buffer_underwrite : public out_of_bounds
+class concrete_buffer_underwrite : public concrete_out_of_bounds
{
public:
- buffer_underwrite (const region *reg, tree diag_arg, byte_range range)
- : out_of_bounds (reg, diag_arg, range)
+ concrete_buffer_underwrite (const region *reg, tree diag_arg,
+ byte_range range)
+ : concrete_out_of_bounds (reg, diag_arg, range)
{}
+ const char *get_kind () const final override
+ {
+ return "concrete_buffer_underwrite";
+ }
+
bool emit (rich_location *rich_loc) final override
{
diagnostic_metadata m;
@@ -403,13 +439,19 @@ public:
/* Concrete subclass to complain about buffer under-reads. */
-class buffer_under_read : public out_of_bounds
+class concrete_buffer_under_read : public concrete_out_of_bounds
{
public:
- buffer_under_read (const region *reg, tree diag_arg, byte_range range)
- : out_of_bounds (reg, diag_arg, range)
+ concrete_buffer_under_read (const region *reg, tree diag_arg,
+ byte_range range)
+ : concrete_out_of_bounds (reg, diag_arg, range)
{}
+ const char *get_kind () const final override
+ {
+ return "concrete_buffer_under_read";
+ }
+
bool emit (rich_location *rich_loc) final override
{
diagnostic_metadata m;
@@ -470,38 +512,26 @@ public:
/* Abstract class to complain about out-of-bounds read/writes where
the values are symbolic. */
-class symbolic_past_the_end
- : public pending_diagnostic_subclass<symbolic_past_the_end>
+class symbolic_past_the_end : public out_of_bounds
{
public:
symbolic_past_the_end (const region *reg, tree diag_arg, tree offset,
tree num_bytes, tree capacity)
- : m_reg (reg), m_diag_arg (diag_arg), m_offset (offset),
- m_num_bytes (num_bytes), m_capacity (capacity)
+ : out_of_bounds (reg, diag_arg),
+ m_offset (offset),
+ m_num_bytes (num_bytes),
+ m_capacity (capacity)
{}
- const char *get_kind () const final override
- {
- return "symbolic_past_the_end";
- }
-
- bool operator== (const symbolic_past_the_end &other) const
- {
- return m_reg == other.m_reg
- && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg)
- && pending_diagnostic::same_tree_p (m_offset, other.m_offset)
- && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes)
- && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity);
- }
-
- int get_controlling_option () const final override
- {
- return OPT_Wanalyzer_out_of_bounds;
- }
-
- void mark_interesting_stuff (interesting_t *interest) final override
+ bool
+ subclass_equal_p (const pending_diagnostic &base_other) const final override
{
- interest->add_region_creation (m_reg);
+ const symbolic_past_the_end &other
+ (static_cast <const symbolic_past_the_end &>(base_other));
+ return (out_of_bounds::subclass_equal_p (other)
+ && pending_diagnostic::same_tree_p (m_offset, other.m_offset)
+ && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes)
+ && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity));
}
label_text
@@ -566,13 +596,6 @@ public:
}
protected:
- enum memory_space get_memory_space () const
- {
- return m_reg->get_memory_space ();
- }
-
- const region *m_reg;
- tree m_diag_arg;
tree m_offset;
tree m_num_bytes;
tree m_capacity;
@@ -591,6 +614,11 @@ public:
m_dir_str = "write";
}
+ const char *get_kind () const final override
+ {
+ return "symbolic_buffer_overflow";
+ }
+
bool emit (rich_location *rich_loc) final override
{
diagnostic_metadata m;
@@ -624,6 +652,11 @@ public:
m_dir_str = "read";
}
+ const char *get_kind () const final override
+ {
+ return "symbolic_buffer_over_read";
+ }
+
bool emit (rich_location *rich_loc) final override
{
diagnostic_metadata m;
@@ -776,10 +809,12 @@ region_model::check_region_bounds (const region *reg,
gcc_unreachable ();
break;
case DIR_READ:
- ctxt->warn (make_unique<buffer_under_read> (reg, diag_arg, out));
+ ctxt->warn (make_unique<concrete_buffer_under_read> (reg, diag_arg,
+ out));
break;
case DIR_WRITE:
- ctxt->warn (make_unique<buffer_underwrite> (reg, diag_arg, out));
+ ctxt->warn (make_unique<concrete_buffer_underwrite> (reg, diag_arg,
+ out));
break;
}
}
@@ -804,12 +839,12 @@ region_model::check_region_bounds (const region *reg,
gcc_unreachable ();
break;
case DIR_READ:
- ctxt->warn (make_unique<buffer_over_read> (reg, diag_arg,
- out, byte_bound));
+ ctxt->warn (make_unique<concrete_buffer_over_read> (reg, diag_arg,
+ out, byte_bound));
break;
case DIR_WRITE:
- ctxt->warn (make_unique<buffer_overflow> (reg, diag_arg,
- out, byte_bound));
+ ctxt->warn (make_unique<concrete_buffer_overflow> (reg, diag_arg,
+ out, byte_bound));
break;
}
}
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 0c49bb26872..991b592b828 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -310,21 +310,17 @@ public:
}
- void set_next_state (const gimple *stmt,
+ void set_next_state (const gimple *,
tree var,
state_machine::state_t to,
tree origin) final override
{
logger * const logger = get_logger ();
LOG_FUNC (logger);
- impl_region_model_context new_ctxt (m_eg, m_enode_for_diag,
- m_old_state, m_new_state,
- NULL, NULL,
- stmt);
const svalue *var_new_sval
- = m_new_state->m_region_model->get_rvalue (var, &new_ctxt);
+ = m_new_state->m_region_model->get_rvalue (var, NULL);
const svalue *origin_new_sval
- = m_new_state->m_region_model->get_rvalue (origin, &new_ctxt);
+ = m_new_state->m_region_model->get_rvalue (origin, NULL);
/* We use the new sval here to avoid issues with uninitialized values. */
state_machine::state_t current
@@ -350,12 +346,8 @@ public:
(m_eg, m_enode_for_diag, NULL, NULL, NULL/*m_enode->get_state ()*/,
NULL, stmt);
- impl_region_model_context new_ctxt (m_eg, m_enode_for_diag,
- m_old_state, m_new_state,
- NULL, NULL,
- stmt);
const svalue *origin_new_sval
- = m_new_state->m_region_model->get_rvalue (origin, &new_ctxt);
+ = m_new_state->m_region_model->get_rvalue (origin, NULL);
state_machine::state_t current
= m_old_smap->get_state (sval, m_eg.get_ext_state ());
@@ -380,11 +372,8 @@ public:
{
LOG_FUNC (get_logger ());
gcc_assert (d);
- impl_region_model_context old_ctxt
- (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL);
-
const svalue *var_old_sval
- = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
+ = m_old_state->m_region_model->get_rvalue (var, NULL);
state_machine::state_t current
= (var
? m_old_smap->get_state (var_old_sval, m_eg.get_ext_state ())
@@ -400,9 +389,6 @@ public:
{
LOG_FUNC (get_logger ());
gcc_assert (d);
- impl_region_model_context old_ctxt
- (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL);
-
state_machine::state_t current
= (sval
? m_old_smap->get_state (sval, m_eg.get_ext_state ())
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2022-12-01 2:31 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 2:31 [gcc r13-4430] analyzer: unify bounds-checking class hierarchies 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).