https://gcc.gnu.org/g:70f26314b62e2d636b1f2d3db43e75abb026e535
commit r15-1108-g70f26314b62e2d636b1f2d3db43e75abb026e535 Author: David Malcolm <dmalc...@redhat.com> Date: Fri Jun 7 16:14:29 2024 -0400 analyzer: eliminate cast_region::m_original_region cast_region had its own field m_original_region, rather than simply using region::m_parent, leading to lots of pointless special-casing of RK_CAST. Remove the field and simply use the parent region. Doing so revealed a bug (seen in gcc.dg/analyzer/taint-alloc-4.c) where region_model::get_representative_path_var_1's RK_CAST case was always failing, due to using the "parent region" (actually that of the original region's parent), rather than the original region; the patch fixes the bug by removing the distinction. gcc/analyzer/ChangeLog: * call-summary.cc (call_summary_replay::convert_region_from_summary_1): Update for removal of cast_region::m_original_region. * region-model-manager.cc (region_model_manager::get_or_create_initial_value): Likewise. * region-model.cc (region_model::get_store_value): Likewise. * region.cc (region::get_base_region): Likewise. (region::descendent_of_p): Likewise. (region::maybe_get_frame_region): Likewise. (region::get_memory_space): Likewise. (region::calc_offset): Likewise. (cast_region::accept): Delete. (cast_region::dump_to_pp): Update for removal of cast_region::m_original_region. (cast_region::add_dump_widget_children): Delete. * region.h (struct cast_region::key_t): Rename "original_region" to "parent". (cast_region::cast_region): Likewise. Update for removal of cast_region::m_original_region. (cast_region::accept): Delete. (cast_region::add_dump_widget_children): Delete. (cast_region::get_original_region): Delete. (cast_region::m_original_region): Delete. * sm-taint.cc (region_model::check_region_for_taint): Remove special-casing for RK_CAST. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/taint-alloc-4.c: Update expected result to reflect change in message due to region_model::get_representative_path_var_1 now handling RK_CAST. Signed-off-by: David Malcolm <dmalc...@redhat.com> Diff: --- gcc/analyzer/call-summary.cc | 11 +++--- gcc/analyzer/region-model-manager.cc | 2 +- gcc/analyzer/region-model.cc | 2 +- gcc/analyzer/region.cc | 50 ++++----------------------- gcc/analyzer/region.h | 37 +++++++------------- gcc/analyzer/sm-taint.cc | 8 ----- gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c | 4 +-- 7 files changed, 29 insertions(+), 85 deletions(-) diff --git a/gcc/analyzer/call-summary.cc b/gcc/analyzer/call-summary.cc index 60ca78a334d..46b4e2a3bbd 100644 --- a/gcc/analyzer/call-summary.cc +++ b/gcc/analyzer/call-summary.cc @@ -726,13 +726,12 @@ call_summary_replay::convert_region_from_summary_1 (const region *summary_reg) { const cast_region *summary_cast_reg = as_a <const cast_region *> (summary_reg); - const region *summary_original_reg - = summary_cast_reg->get_original_region (); - const region *caller_original_reg - = convert_region_from_summary (summary_original_reg); - if (!caller_original_reg) + const region *summary_parent_reg = summary_reg->get_parent_region (); + const region *caller_parent_reg + = convert_region_from_summary (summary_parent_reg); + if (!caller_parent_reg) return NULL; - return mgr->get_cast_region (caller_original_reg, + return mgr->get_cast_region (caller_parent_reg, summary_reg->get_type ()); } break; diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc index b094b2f7e43..8154d914e81 100644 --- a/gcc/analyzer/region-model-manager.cc +++ b/gcc/analyzer/region-model-manager.cc @@ -327,7 +327,7 @@ region_model_manager::get_or_create_initial_value (const region *reg, /* The initial value of a cast is a cast of the initial value. */ if (const cast_region *cast_reg = reg->dyn_cast_cast_region ()) { - const region *original_reg = cast_reg->get_original_region (); + const region *original_reg = cast_reg->get_parent_region (); return get_or_create_cast (cast_reg->get_type (), get_or_create_initial_value (original_reg)); } diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index d6bcb8630cd..9f24011c17b 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -2933,7 +2933,7 @@ region_model::get_store_value (const region *reg, /* Special-case: read the initial char of a STRING_CST. */ if (const cast_region *cast_reg = reg->dyn_cast_cast_region ()) if (const string_region *str_reg - = cast_reg->get_original_region ()->dyn_cast_string_region ()) + = cast_reg->get_parent_region ()->dyn_cast_string_region ()) { tree string_cst = str_reg->get_string_cst (); tree byte_offset_cst = integer_zero_node; diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc index 71bae97b6f1..1fc42f2cd97 100644 --- a/gcc/analyzer/region.cc +++ b/gcc/analyzer/region.cc @@ -425,10 +425,8 @@ region::get_base_region () const case RK_OFFSET: case RK_SIZED: case RK_BIT_RANGE: - iter = iter->get_parent_region (); - continue; case RK_CAST: - iter = iter->dyn_cast_cast_region ()->get_original_region (); + iter = iter->get_parent_region (); continue; default: return iter; @@ -468,10 +466,7 @@ region::descendent_of_p (const region *elder) const { if (iter == elder) return true; - if (iter->get_kind () == RK_CAST) - iter = iter->dyn_cast_cast_region ()->get_original_region (); - else - iter = iter->get_parent_region (); + iter = iter->get_parent_region (); } return false; } @@ -487,10 +482,7 @@ region::maybe_get_frame_region () const { if (const frame_region *frame_reg = iter->dyn_cast_frame_region ()) return frame_reg; - if (iter->get_kind () == RK_CAST) - iter = iter->dyn_cast_cast_region ()->get_original_region (); - else - iter = iter->get_parent_region (); + iter = iter->get_parent_region (); } return NULL; } @@ -525,10 +517,7 @@ region::get_memory_space () const case RK_PRIVATE: return MEMSPACE_PRIVATE; } - if (iter->get_kind () == RK_CAST) - iter = iter->dyn_cast_cast_region ()->get_original_region (); - else - iter = iter->get_parent_region (); + iter = iter->get_parent_region (); } return MEMSPACE_UNKNOWN; } @@ -958,15 +947,8 @@ region::calc_offset (region_model_manager *mgr) const } continue; case RK_SIZED: - iter_region = iter_region->get_parent_region (); - continue; - case RK_CAST: - { - const cast_region *cast_reg - = as_a <const cast_region *> (iter_region); - iter_region = cast_reg->get_original_region (); - } + iter_region = iter_region->get_parent_region (); continue; default: @@ -2276,15 +2258,6 @@ sized_region::get_bit_size_sval (region_model_manager *mgr) const /* class cast_region : public region. */ -/* Implementation of region::accept vfunc for cast_region. */ - -void -cast_region::accept (visitor *v) const -{ - region::accept (v); - m_original_region->accept (v); -} - /* Implementation of region::dump_to_pp vfunc for cast_region. */ void @@ -2295,13 +2268,13 @@ cast_region::dump_to_pp (pretty_printer *pp, bool simple) const pp_string (pp, "CAST_REG("); print_quoted_type (pp, get_type ()); pp_string (pp, ", "); - m_original_region->dump_to_pp (pp, simple); + get_parent_region ()->dump_to_pp (pp, simple); pp_string (pp, ")"); } else { pp_string (pp, "cast_region("); - m_original_region->dump_to_pp (pp, simple); + get_parent_region ()->dump_to_pp (pp, simple); pp_string (pp, ", "); print_quoted_type (pp, get_type ()); pp_printf (pp, ")"); @@ -2314,15 +2287,6 @@ cast_region::print_dump_widget_label (pretty_printer *pp) const pp_printf (pp, "cast_region"); } -void -cast_region:: -add_dump_widget_children (text_art::tree_widget &w, - const text_art::dump_widget_info &dwi) const -{ - w.add_child - (m_original_region->make_dump_widget (dwi, "m_original_region")); -} - /* Implementation of region::get_relative_concrete_offset vfunc for cast_region. */ diff --git a/gcc/analyzer/region.h b/gcc/analyzer/region.h index 5d58abc2693..211dd3458c0 100644 --- a/gcc/analyzer/region.h +++ b/gcc/analyzer/region.h @@ -1170,65 +1170,54 @@ public: /* A support class for uniquifying instances of cast_region. */ struct key_t { - key_t (const region *original_region, tree type) - : m_original_region (original_region), m_type (type) + key_t (const region *parent, tree type) + : m_parent (parent), m_type (type) { - gcc_assert (original_region); + gcc_assert (parent); } hashval_t hash () const { inchash::hash hstate; - hstate.add_ptr (m_original_region); + hstate.add_ptr (m_parent); hstate.add_ptr (m_type); return hstate.end (); } bool operator== (const key_t &other) const { - return (m_original_region == other.m_original_region + return (m_parent == other.m_parent && m_type == other.m_type); } void mark_deleted () { - m_original_region = reinterpret_cast<const region *> (1); + m_parent = reinterpret_cast<const region *> (1); } - void mark_empty () { m_original_region = nullptr; } + void mark_empty () { m_parent = nullptr; } bool is_deleted () const { - return m_original_region == reinterpret_cast<const region *> (1); + return m_parent == reinterpret_cast<const region *> (1); } - bool is_empty () const { return m_original_region == nullptr; } + bool is_empty () const { return m_parent == nullptr; } - const region *m_original_region; + const region *m_parent; tree m_type; }; - cast_region (symbol::id_t id, const region *original_region, tree type) - : region (complexity (original_region), id, - original_region->get_parent_region (), type), - m_original_region (original_region) + cast_region (symbol::id_t id, const region *parent, tree type) + : region (complexity (parent), id, + parent, type) {} enum region_kind get_kind () const final override { return RK_CAST; } const cast_region * dyn_cast_cast_region () const final override { return this; } - void accept (visitor *v) const final override; void dump_to_pp (pretty_printer *pp, bool simple) const final override; void print_dump_widget_label (pretty_printer *pp) const final override; - void - add_dump_widget_children (text_art::tree_widget &, - const text_art::dump_widget_info &dwi) - const final override; bool get_relative_concrete_offset (bit_offset_t *out) const final override; - - const region *get_original_region () const { return m_original_region; } - -private: - const region *m_original_region; }; } // namespace ana diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc index bd85f0d5611..3bd050f9635 100644 --- a/gcc/analyzer/sm-taint.cc +++ b/gcc/analyzer/sm-taint.cc @@ -1625,14 +1625,6 @@ region_model::check_region_for_taint (const region *reg, } break; - case RK_CAST: - { - const cast_region *cast_reg - = as_a <const cast_region *> (iter_region); - iter_region = cast_reg->get_original_region (); - continue; - } - case RK_SIZED: { const sized_region *sized_reg diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c index 9df9422491c..7fe813d4e9e 100644 --- a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c +++ b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c @@ -23,6 +23,6 @@ test_1 (void *data) /* { dg-message "\\(1\\) function 'test_1' marked with '__at __analyzer_dump_state ("taint", args); /* { dg-warning "state: 'tainted'" } */ __analyzer_dump_state ("taint", args->sz); /* { dg-warning "state: 'tainted'" } */ - p = malloc (args->sz); /* { dg-warning "use of attacker-controlled value '\\*args.sz' as allocation size without upper-bounds checking" "warning" } */ - /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value '\\*args.sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-1 } */ + p = malloc (args->sz); /* { dg-warning "use of attacker-controlled value '\[^'\]*.sz' as allocation size without upper-bounds checking" "warning" } */ + /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value '\[^'\]*.sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-1 } */ }