From 8bc9e4ee874ea3618780413b79b51412dcc40363 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 30 Nov 2022 21:26:43 -0500 Subject: 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 --- gcc/analyzer/engine.cc | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) (limited to 'gcc/analyzer/engine.cc') diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 0c49bb2..991b592 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 ()) -- cgit v1.1