* [pushed] analyzer: eliminate cast_region::m_original_region
@ 2024-06-07 20:41 David Malcolm
0 siblings, 0 replies; only message in thread
From: David Malcolm @ 2024-06-07 20:41 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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.
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-1108-g70f26314b62e2d.
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 <dmalcolm@redhat.com>
---
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 60ca78a334da..46b4e2a3bbd7 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 b094b2f7e434..8154d914e81c 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 d6bcb8630cd6..9f24011c17bf 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 71bae97b6f11..1fc42f2cd979 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 5d58abc26938..211dd3458c03 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 bd85f0d56113..3bd050f96352 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 9df9422491c8..7fe813d4e9e6 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 } */
}
--
2.26.3
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2024-06-07 20:41 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-07 20:41 [pushed] analyzer: eliminate cast_region::m_original_region 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).