From 8e71efd87951f0ba72d81252bd7e93980c4c8652 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Tue, 21 Jul 2020 09:30:46 -0400 Subject: [PATCH 313/315] FIXME: implement store canonicalization and visitor class --- gcc/analyzer/analyzer.h | 4 +- gcc/analyzer/engine.cc | 8 + gcc/analyzer/region-model2.cc | 167 ++++++++++++++++++ gcc/analyzer/region-model2.h | 53 +++++- gcc/analyzer/store2.cc | 58 ++++++ gcc/analyzer/store2.h | 5 + .../gcc.dg/analyzer/malloc-in-loop.c | 19 ++ 7 files changed, 310 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h index 183a11cf0fe..b82241e1b0f 100644 --- a/gcc/analyzer/analyzer.h +++ b/gcc/analyzer/analyzer.h @@ -59,13 +59,15 @@ struct canonicalization; class svalue2; class region_svalue2; class constant_svalue2; - class poisoned_svalue2; class unknown_svalue2; + class poisoned_svalue2; class setjmp_svalue2; class initial_svalue2; class unaryop_svalue2; class binop_svalue2; + class sub_svalue2; class unmergeable_svalue2; + class placeholder_svalue2; class widening_svalue2; class compound_svalue2; class region2; diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index bc7821bd161..6725f6e323d 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -266,6 +266,14 @@ setjmp_svalue::print_details (const region_model &model ATTRIBUTE_UNUSED, /* FIXME. */ +void +setjmp_svalue2::accept (visitor *v) const +{ + v->visit_setjmp_svalue2 (this); +} + +/* FIXME. */ + void setjmp_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const { diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc index 5e6badd9fad..0d0a46ae93a 100644 --- a/gcc/analyzer/region-model2.cc +++ b/gcc/analyzer/region-model2.cc @@ -377,6 +377,15 @@ region_svalue2::compare_fields (const region_svalue2 &other) const } #endif +/* FIXME. */ + +void +region_svalue2::accept (visitor *v) const +{ + v->visit_region_svalue2 (this); + m_reg->accept (v); +} + /* Implementation of svalue2::dump_dot_to_pp for region_svalue2. */ #if 0 @@ -570,6 +579,14 @@ constant_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const } } +/* FIXME. */ + +void +constant_svalue2::accept (visitor *v) const +{ + v->visit_constant_svalue2 (this); +} + /* Compare the fields of this constant_svalue2 with OTHER, returning true if they are equal. For use by svalue2::operator==. */ @@ -681,6 +698,14 @@ unknown_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const } } +/* FIXME. */ + +void +unknown_svalue2::accept (visitor *v) const +{ + v->visit_unknown_svalue2 (this); +} + /* class poisoned_svalue2 : public svalue2. */ /* Compare the fields of this poisoned_svalue2 with OTHER, returning true @@ -696,6 +721,14 @@ poisoned_svalue2::compare_fields (const poisoned_svalue2 &other) const /* FIXME. */ +void +poisoned_svalue2::accept (visitor *v) const +{ + v->visit_poisoned_svalue2 (this); +} + +/* FIXME. */ + void poisoned_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -731,6 +764,15 @@ initial_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const } } +/* FIXME. */ + +void +initial_svalue2::accept (visitor *v) const +{ + v->visit_initial_svalue2 (this); + m_reg->accept (v); +} + /* Compare the fields of this initial_svalue2 with OTHER, returning true if they are equal. For use by svalue2::operator==. */ @@ -777,6 +819,15 @@ unaryop_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const } } +/* FIXME. */ + +void +unaryop_svalue2::accept (visitor *v) const +{ + v->visit_unaryop_svalue2 (this); + m_arg->accept (v); +} + /* class binop_svalue2 : public svalue2. */ void @@ -803,6 +854,16 @@ binop_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const } } +/* FIXME. */ + +void +binop_svalue2::accept (visitor *v) const +{ + v->visit_binop_svalue2 (this); + m_arg0->accept (v); + m_arg1->accept (v); +} + /* class sub_svalue2 : public svalue2. */ sub_svalue2::sub_svalue2 (tree type, const svalue2 *parent_svalue, @@ -814,6 +875,16 @@ sub_svalue2::sub_svalue2 (tree type, const svalue2 *parent_svalue, { } +/* FIXME. */ + +void +sub_svalue2::accept (visitor *v) const +{ + v->visit_sub_svalue2 (this); + m_parent_svalue->accept (v); + m_subregion->accept (v); +} + void sub_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -839,6 +910,16 @@ sub_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const /* class widening_svalue2 : public svalue2. */ +/* FIXME. */ + +void +widening_svalue2::accept (visitor *v) const +{ + v->visit_widening_svalue2 (this); + m_base_sval->accept (v); + m_iter_sval->accept (v); +} + void widening_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -985,6 +1066,14 @@ widening_svalue2::eval_condition_without_cm (enum tree_code op, /* class placeholder_svalue2 : public svalue2. */ +/* FIXME. */ + +void +placeholder_svalue2::accept (visitor *v) const +{ + v->visit_placeholder_svalue2 (this); +} + void placeholder_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -1001,6 +1090,15 @@ placeholder_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const /* class unmergeable_svalue2 : public svalue2. */ +/* FIXME. */ + +void +unmergeable_svalue2::accept (visitor *v) const +{ + v->visit_unmergeable_svalue2 (this); + m_arg->accept (v); +} + void unmergeable_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -1020,6 +1118,20 @@ unmergeable_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const /* class compound_svalue2 : public svalue2. */ +/* FIXME. */ + +void +compound_svalue2::accept (visitor *v) const +{ + v->visit_compound_svalue2 (this); + for (binding_map::iterator_t iter = m_map.begin (); + iter != m_map.end (); ++iter) + { + //(*iter).first.accept (v); + (*iter).second->accept (v); + } +} + void compound_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -2082,6 +2194,16 @@ region2::get_desc (bool simple) const return label_text::take (xstrdup (pp_formatted_text (&pp))); } +/* FIXME. */ + +void +region2::accept (visitor *v) const +{ + v->visit_region (this); + if (m_parent) + m_parent->accept (v); +} + /* Base implementation of region2::validate vfunc. Assert that the fields of "region2" are valid; subclasses should chain up their implementation to this one. */ @@ -2334,6 +2456,14 @@ frame_region2::~frame_region2 () delete (*iter).second; } +void +frame_region2::accept (visitor *v) const +{ + region2::accept (v); + if (m_calling_frame) + m_calling_frame->accept (v); +} + void frame_region2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -2944,6 +3074,15 @@ symbolic_region2::compare_fields (const symbolic_region2 &other) const /* FIXME. */ +void +symbolic_region2::accept (visitor *v) const +{ + region2::accept (v); + m_sval_ptr->accept (v); +} + +/* FIXME. */ + void symbolic_region2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -3050,6 +3189,15 @@ element_region2::compare_fields (const element_region2 &other) const return m_index == other.m_index; } +/* FIXME. */ + +void +element_region2::accept (visitor *v) const +{ + region2::accept (v); + m_index->accept (v); +} + void element_region2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -3089,6 +3237,15 @@ offset_region2::compare_fields (const offset_region2 &other) const return m_byte_offset == other.m_byte_offset; } +/* FIXME. */ + +void +offset_region2::accept (visitor *v) const +{ + region2::accept (v); + m_byte_offset->accept (v); +} + void offset_region2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -3127,6 +3284,15 @@ cast_region2::compare_fields (const cast_region2 &other) const return m_original_region == other.m_original_region; } +/* FIXME. */ + +void +cast_region2::accept (visitor *v) const +{ + region2::accept (v); + m_original_region->accept (v); +} + void cast_region2::dump_to_pp (pretty_printer *pp, bool simple) const { @@ -4634,6 +4800,7 @@ region_model2::validate () const void region_model2::canonicalize () { + m_store.canonicalize (); m_constraints2->canonicalize (); } diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h index 45986012ed0..a1ecf9c472e 100644 --- a/gcc/analyzer/region-model2.h +++ b/gcc/analyzer/region-model2.h @@ -145,6 +145,26 @@ struct complexity unsigned m_max_depth; }; +class visitor +{ +public: + virtual void visit_region_svalue2 (const region_svalue2 *) {} + virtual void visit_constant_svalue2 (const constant_svalue2 *) {} + virtual void visit_unknown_svalue2 (const unknown_svalue2 *) {} + virtual void visit_poisoned_svalue2 (const poisoned_svalue2 *) {} + virtual void visit_setjmp_svalue2 (const setjmp_svalue2 *) {} + virtual void visit_initial_svalue2 (const initial_svalue2 *) {} + virtual void visit_unaryop_svalue2 (const unaryop_svalue2 *) {} + virtual void visit_binop_svalue2 (const binop_svalue2 *) {} + virtual void visit_sub_svalue2 (const sub_svalue2 *) {} + virtual void visit_unmergeable_svalue2 (const unmergeable_svalue2 *) {} + virtual void visit_placeholder_svalue2 (const placeholder_svalue2 *) {} + virtual void visit_widening_svalue2 (const widening_svalue2 *) {} + virtual void visit_compound_svalue2 (const compound_svalue2 *) {} + + virtual void visit_region (const region2 *) {} +}; + /* svalue and its subclasses. The class hierarchy looks like this (using indentation to show @@ -245,6 +265,8 @@ public: const complexity &get_complexity () const { return m_complexity; } + virtual void accept (visitor *v) const = 0; + protected: svalue2 (complexity c, tree type) : m_complexity (c), m_type (type) @@ -281,6 +303,8 @@ public: const region_svalue2 * dyn_cast_region_svalue2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + const region2 * get_pointee () const { return m_reg; } #if 0 @@ -329,6 +353,8 @@ public: const constant_svalue2 * dyn_cast_constant_svalue2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + tree get_constant () const { return m_cst_expr; } #if 0 @@ -372,6 +398,7 @@ public: {} enum kind get_kind () const FINAL OVERRIDE { return SK_UNKNOWN; } + void accept (visitor *v) const FINAL OVERRIDE; void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; }; @@ -437,6 +464,8 @@ public: const poisoned_svalue2 * dyn_cast_poisoned_svalue2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + enum poison_kind get_poison_kind () const { return m_kind; } private: @@ -508,6 +537,8 @@ public: const setjmp_svalue2 * dyn_cast_setjmp_svalue2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + int get_enode_index () const; const setjmp_record &get_setjmp_record () const { return m_setjmp_record; } @@ -565,6 +596,8 @@ public: const initial_svalue2 * dyn_cast_initial_svalue2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + const region2 * get_region () const { return m_reg; } #if 0 @@ -639,6 +672,7 @@ public: const unaryop_svalue2 * dyn_cast_unaryop_svalue2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; #if 0 void dump_dot_to_pp (const region_model2 &model, @@ -730,6 +764,8 @@ public: enum kind get_kind () const FINAL OVERRIDE { return SK_BINOP; } virtual const binop_svalue2 *dyn_cast_binop_svalue2 () const { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + enum tree_code get_op () const { return m_op; } const svalue2 *get_arg0 () const { return m_arg0; } const svalue2 *get_arg1 () const { return m_arg1; } @@ -815,6 +851,7 @@ public: const region2 *subregion); enum kind get_kind () const FINAL OVERRIDE { return SK_SUB; } + void accept (visitor *v) const FINAL OVERRIDE; #if 0 void dump_dot_to_pp (const region_model2 &model, @@ -877,7 +914,7 @@ public: enum kind get_kind () const FINAL OVERRIDE { return SK_UNMERGEABLE; } const unmergeable_svalue2 * dyn_cast_unmergeable_svalue2 () const FINAL OVERRIDE { return this; } - + void accept (visitor *v) const FINAL OVERRIDE; #if 0 void dump_dot_to_pp (const region_model2 &model, @@ -919,6 +956,7 @@ public: } enum kind get_kind () const FINAL OVERRIDE { return SK_PLACEHOLDER; } + void accept (visitor *v) const FINAL OVERRIDE; #if 0 void dump_dot_to_pp (const region_model2 &model, @@ -1074,6 +1112,7 @@ public: enum kind get_kind () const FINAL OVERRIDE { return SK_WIDENING; } const widening_svalue2 *dyn_cast_widening_svalue2 () const { return this; } + void accept (visitor *v) const FINAL OVERRIDE; #if 0 void dump_dot_to_pp (const region_model2 &model, @@ -1177,6 +1216,7 @@ public: enum kind get_kind () const FINAL OVERRIDE { return SK_COMPOUND; } const compound_svalue2 *dyn_cast_compound_svalue2 () const { return this; } + void accept (visitor *v) const FINAL OVERRIDE; iterator_t begin () const { return m_map.begin (); } iterator_t end () const { return m_map.end (); } @@ -1316,6 +1356,8 @@ public: virtual const string_region2 * dyn_cast_string_region2 () const { return NULL; } + virtual void accept (visitor *v) const; + const region2 *get_parent_region () const { return m_parent; } const region2 *get_base_region () const; bool base_region_p () const; @@ -1458,6 +1500,7 @@ public: { return this; } + void accept (visitor *v) const FINAL OVERRIDE; void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; #if 0 void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE; @@ -1784,7 +1827,7 @@ public: bool compare_fields (const symbolic_region2 &other) const; enum kind get_kind () const FINAL OVERRIDE { return region2::RK_SYMBOLIC; } - + void accept (visitor *v) const FINAL OVERRIDE; void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; const svalue2 *get_pointer () const { return m_sval_ptr; } @@ -1900,6 +1943,8 @@ public: const element_region2 * dyn_cast_element_region2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; const svalue2 *get_index () const { return m_index; } @@ -1941,6 +1986,8 @@ public: const offset_region2 * dyn_cast_offset_region2 () const FINAL OVERRIDE { return this; } + void accept (visitor *v) const FINAL OVERRIDE; + void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; const svalue2 *get_byte_offset () const { return m_byte_offset; } @@ -1980,7 +2027,7 @@ public: enum region2::kind get_kind () const FINAL OVERRIDE { return RK_CAST; } const cast_region2 * dyn_cast_cast_region2 () const FINAL OVERRIDE { return this; } - + void accept (visitor *v) const FINAL OVERRIDE; void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; /* FIXME: should this just be the parent region? */ diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc index 7772b477d0c..591aae47c40 100644 --- a/gcc/analyzer/store2.cc +++ b/gcc/analyzer/store2.cc @@ -1735,6 +1735,64 @@ store2::remove_overlapping_bindings (store2_manager *mgr, const region2 *reg) (*cluster_slot)->remove_overlapping_bindings (mgr, reg); } +/* FIXME. */ + +struct region2_finder : public visitor +{ + void visit_region (const region2 *reg) FINAL OVERRIDE + { + m_regs.add (reg); + } + + hash_set m_regs; +}; + +/* FIXME. */ + +void +store2::canonicalize () +{ + /* If we have e.g.: + cluster for: HEAP_ALLOCATED_REGION(543) + ESCAPED + TOUCHED + where the heap region is empty and unreferenced, then purge that + cluster, to avoid unbounded state chains involving these. */ + + /* Find regions that are referenced by bound values in the store. */ + region2_finder s; + for (cluster_map_t::iterator iter = m_cluster_map.begin (); + iter != m_cluster_map.end (); ++iter) + { + binding_cluster2 *cluster = (*iter).second; + for (binding_cluster2::iterator_t bind_iter = cluster->m_map.begin (); + bind_iter != cluster->m_map.end (); ++bind_iter) + (*bind_iter).second->accept (&s); + } + + /* Locate heap-allocated regions that have empty bindings that weren't + found above. */ + hash_set purgeable_regions; + for (cluster_map_t::iterator iter = m_cluster_map.begin (); + iter != m_cluster_map.end (); ++iter) + { + const region2 *base_reg = (*iter).first; + binding_cluster2 *cluster = (*iter).second; + if (base_reg->get_kind () == region2::RK_HEAP_ALLOCATED) + if (cluster->empty_p ()) + if (!s.m_regs.contains (base_reg)) + purgeable_regions.add (base_reg); + } + + /* Purge them. */ + for (hash_set::iterator iter = purgeable_regions.begin (); + iter != purgeable_regions.end (); ++iter) + { + const region2 *base_reg = *iter; + purge_cluster (base_reg); + } +} + #if CHECKING_P namespace selftest { diff --git a/gcc/analyzer/store2.h b/gcc/analyzer/store2.h index 4a1b29df870..7981dd665b2 100644 --- a/gcc/analyzer/store2.h +++ b/gcc/analyzer/store2.h @@ -327,6 +327,8 @@ private: class binding_cluster2 { public: + friend class store2; // FIXME + typedef hash_map map_t; typedef map_t::iterator iterator_t; @@ -392,6 +394,7 @@ public: bool touched_p () const { return m_touched; } bool redundant_p () const; + bool empty_p () const { return m_map.elements () == 0; } void get_representative_path_vars (const region_model2 *model, hash_set *visited, @@ -527,6 +530,8 @@ public: (*iter).second->for_each_binding (v); } + void canonicalize (); + private: void remove_overlapping_bindings (store2_manager *mgr, const region2 *reg); diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c b/gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c new file mode 100644 index 00000000000..a8c85a9c618 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/malloc-in-loop.c @@ -0,0 +1,19 @@ +#include +#include "analyzer-decls.h" + +extern void foo (int *); + +void test (int n) +{ + __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */ + + for (int i = 0; i < n; i++) + { + int *ptr = (int *)malloc (sizeof (int) * i); + foo (ptr); + free (ptr); + __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 processed enodes" } */ + } + + __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */ +} -- 2.26.2