From 03169edb4d36e1e9de7a1a8ea58d0e21d835c583 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Tue, 12 May 2020 12:26:23 -0400 Subject: [PATCH 115/179] FIXME: get extracting a zero field from a zero-filled struct working --- gcc/analyzer/analyzer.h | 1 + gcc/analyzer/region-model2.cc | 65 +++++++++++++++++++- gcc/analyzer/region-model2.h | 109 ++++++++++++++++++++++++++++++++-- gcc/analyzer/store2.cc | 27 +++++++-- gcc/analyzer/store2.h | 4 +- 5 files changed, 193 insertions(+), 13 deletions(-) diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h index 91c2369ed11..c15cd687d55 100644 --- a/gcc/analyzer/analyzer.h +++ b/gcc/analyzer/analyzer.h @@ -61,6 +61,7 @@ class svalue2; class unknown_svalue2; class setjmp_svalue2; class initial_svalue2; + class unaryop_svalue2; class region2; class function_region2; class label_region2; diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc index f2a4b560605..dcd0a4ab38d 100644 --- a/gcc/analyzer/region-model2.cc +++ b/gcc/analyzer/region-model2.cc @@ -788,6 +788,31 @@ binop_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const } } +/* class sub_svalue2 : public svalue2. */ + +void +sub_svalue2::dump_to_pp (pretty_printer *pp, bool simple) const +{ + if (simple) + { + pp_string (pp, "SUB("); + m_parent_svalue->dump_to_pp (pp, simple); + pp_string (pp, ", "); + m_subregion->dump_to_pp (pp, simple); + pp_character (pp, ')'); + } + else + { + pp_string (pp, "sub_svalue2 ("); + // TODO: type? + pp_string (pp, ", "); + m_parent_svalue->dump_to_pp (pp, simple); + pp_string (pp, ", "); + m_subregion->dump_to_pp (pp, simple); + pp_character (pp, ')'); + } +} + /* class region2 and its various subclasses. */ /* Get a string for KIND for use in debug dumps. */ @@ -1009,11 +1034,12 @@ region2::get_bit_size (bit_size_t *out) const tree type = get_type (); gcc_assert (type); - // FIXME: use TYPE_SIZE ?? HOST_WIDE_INT bytes = int_size_in_bytes (type); if (bytes == -1) { - // FIXME: this will fail for VLAs + /* FIXME: this will fail for VLAs + TODO: how should we handle those? + TODO: also, what happens for the "empty array final field" idiom? */ return false; } *out = bytes * BITS_PER_UNIT; @@ -2812,6 +2838,9 @@ region_model2_manager::~region_model2_manager () for (binop_values_map_t::iterator iter = m_binop_values_map.begin (); iter != m_binop_values_map.end (); ++iter) delete (*iter).second; + for (sub_values_map_t::iterator iter = m_sub_values_map.begin (); + iter != m_sub_values_map.end (); ++iter) + delete (*iter).second; /* Delete consolidated regions. */ for (fndecls_map_t::iterator iter = m_fndecls_map.begin (); @@ -2972,6 +3001,38 @@ region_model2_manager::get_or_create_binop (tree type, enum tree_code op, return binop_sval; } +/* FIXME. */ + +const svalue2 * +region_model2_manager::get_or_create_sub_svalue2 (tree type, + const svalue2 *parent_svalue2, + const region2 *subregion) +{ + /* Specialcase: if we have a subregion of a zero-fill, it's zero. */ + if (const unaryop_svalue2 *unary + = parent_svalue2->dyn_cast_unaryop_svalue2 ()) + { + if (unary->get_op () == NOP_EXPR) + if (tree cst = unary->get_arg ()->maybe_get_constant ()) + if (zerop (cst)) + { + const svalue2 *cst_sval + = get_or_create_constant_svalue2 (cst); + const svalue2 *cast_sval + = get_or_create_unaryop (type, NOP_EXPR, cst_sval); + return cast_sval; + } + } + + sub_svalue2::key_t key (type, parent_svalue2, subregion); + if (sub_svalue2 **slot = m_sub_values_map.get (key)) + return *slot; + sub_svalue2 *sub_sval + = new sub_svalue2 (type, parent_svalue2, subregion); + m_sub_values_map.put (key, sub_sval); + return sub_sval; +} + /* region2 consolidation. */ /* FIXME. */ diff --git a/gcc/analyzer/region-model2.h b/gcc/analyzer/region-model2.h index c19f3d50fa8..518faf98937 100644 --- a/gcc/analyzer/region-model2.h +++ b/gcc/analyzer/region-model2.h @@ -148,7 +148,8 @@ public: SK_SETJMP, SK_INITIAL, SK_UNARYOP, - SK_BINOP + SK_BINOP, + SK_SUB, }; virtual ~svalue2 () {} @@ -189,6 +190,8 @@ public: dyn_cast_setjmp_svalue2 () const { return NULL; } virtual const initial_svalue2 * dyn_cast_initial_svalue2 () const { return NULL; } + virtual const unaryop_svalue2 * + dyn_cast_unaryop_svalue2 () const { return NULL; } tree maybe_get_constant () const; @@ -633,6 +636,9 @@ public: } enum kind get_kind () const FINAL OVERRIDE { return SK_UNARYOP; } + const unaryop_svalue2 * + dyn_cast_unaryop_svalue2 () const FINAL OVERRIDE { return this; } + #if 0 void dump_dot_to_pp (const region_model2 &model, @@ -642,15 +648,14 @@ public: void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; -#if 0 - const unaryop_svalue2 * - dyn_cast_unaryop_svalue2 () const FINAL OVERRIDE { return this; } -#endif #if 0 void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE; #endif + enum tree_code get_op () const { return m_op; } + const svalue2 *get_arg () const { return m_arg; } + private: #if 0 void print_details (const region_model2 &model, pretty_printer *pp) const @@ -775,6 +780,94 @@ template <> struct default_hash_traits namespace ana { +/* Concrete subclass of svalue2 representing the result of accessing a subregion + of another svalue2 (the value of a component/field of a struct, or an element + from an array). + FIXME: do we want to split this into two different classes for that? */ + +class sub_svalue2 : public svalue2 +{ +public: + /* A support class for uniquifying instances of sub_svalue2. */ + struct key_t + { + key_t (tree type, const svalue2 *parent_svalue, const region2 *subregion) + : m_type (type), m_parent_svalue (parent_svalue), m_subregion (subregion) + {} + + hashval_t hash () const + { + return 42; // FIXME + } + + bool operator== (const key_t &other) const + { + return (m_type == other.m_type + && m_parent_svalue == other.m_parent_svalue + && m_subregion == other.m_subregion); + } + + void mark_deleted () { m_type = reinterpret_cast (1); } + void mark_empty () { m_type = NULL_TREE; } + bool is_deleted () const { return m_type == reinterpret_cast (1); } + bool is_empty () const { return m_type == NULL_TREE; } + + tree m_type; + const svalue2 *m_parent_svalue; + const region2 *m_subregion; + }; + sub_svalue2 (tree type, const svalue2 *parent_svalue, + const region2 *subregion) + : svalue2 (type), m_parent_svalue (parent_svalue), m_subregion (subregion) + { + } + + enum kind get_kind () const FINAL OVERRIDE { return SK_SUB; } + +#if 0 + void dump_dot_to_pp (const region_model2 &model, + pretty_printer *pp) const + FINAL OVERRIDE; +#endif + + void dump_to_pp (pretty_printer *pp, bool simple) const FINAL OVERRIDE; + +#if 0 + binop_svalue2 *dyn_cast_binop_svalue2 () FINAL OVERRIDE { return this; } +#endif + +#if 0 + void add_to_hash (inchash::hash &hstate) const FINAL OVERRIDE; +#endif + + private: +#if 0 + void print_details (const region_model2 &model, pretty_printer *pp) const + FINAL OVERRIDE; +#endif + + const svalue2 *m_parent_svalue; + const region2 *m_subregion; +}; + +} // namespace ana + +template <> +template <> +inline bool +is_a_helper ::test (svalue2 *sval) +{ + return sval->get_kind () == svalue2::SK_SUB; +} + +template <> struct default_hash_traits +: public member_function_hash_traits +{ + static const bool empty_zero_p = true; +}; + +namespace ana { + /* Region and its subclasses. The class hierarchy looks like this (using indentation to show @@ -1626,6 +1719,9 @@ public: const svalue2 *get_or_create_binop (tree type, enum tree_code op, const svalue2 *arg0, const svalue2 *arg1); + const svalue2 *get_or_create_sub_svalue2 (tree type, + const svalue2 *parent_svalue, + const region2 *subregion); /* region2 consolidation. */ const function_region2 *get_region_for_fndecl (tree fndecl, @@ -1696,6 +1792,9 @@ private: typedef hash_map binop_values_map_t; binop_values_map_t m_binop_values_map; + typedef hash_map sub_values_map_t; + sub_values_map_t m_sub_values_map; + /* region2 consolidation. */ code_region2 m_code_region; diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc index 07b7f2365ee..76a68d86ff6 100644 --- a/gcc/analyzer/store2.cc +++ b/gcc/analyzer/store2.cc @@ -367,7 +367,12 @@ binding_cluster2::get_default_binding_recursive (store2_manager *mgr, if (const region2 *parent_reg = reg->get_parent_region ()) if (const svalue2 *parent_default_sval = get_default_binding_recursive (mgr, parent_reg)) - gcc_unreachable (); // TODO: extract value for child + { + /* Extract child svalue from parent svalue. */ + region_model2_manager *rmm_mgr = mgr->get_svalue2_manager (); + return rmm_mgr->get_or_create_sub_svalue2 (reg->get_type (), + parent_default_sval, reg); + } return NULL; } @@ -432,9 +437,23 @@ binding_cluster2::remove_overlapping_bindings (store2_manager *mgr, get_overlapping_bindings (mgr, reg, &bindings); unsigned i; - const binding_key2 *binding; - FOR_EACH_VEC_ELT (bindings, i, binding) - m_map.remove (binding); + const binding_key2 *iter_binding; + FOR_EACH_VEC_ELT (bindings, i, iter_binding) + { + /* FIXME: don't remove default bindings, unless the default binding + is fully covered by REG. */ + if (iter_binding->get_kind () == BK_default) + { + const binding_key2 *reg_binding + = binding_key2::make (mgr, reg, BK_default); + /* FIXME: is this condition correct? + I *think* we want to skip it if there's a part of + iter_binding that's not covered by reg_binding. */ + if (reg_binding != iter_binding) + continue; + } + m_map.remove (iter_binding); + } } /* FIXME. */ diff --git a/gcc/analyzer/store2.h b/gcc/analyzer/store2.h index cd1f224490a..6f05bafe111 100644 --- a/gcc/analyzer/store2.h +++ b/gcc/analyzer/store2.h @@ -69,11 +69,11 @@ public: virtual const concrete_binding2 *dyn_cast_concrete_binding2 () const { return NULL; } + enum binding_kind get_kind () const { return m_kind; } + protected: binding_key2 (enum binding_kind kind) : m_kind (kind) {} - enum binding_kind get_kind () const { return m_kind; } - hashval_t impl_hash () const { return m_kind; -- 2.21.0