On Sat, 15 Aug 2020 at 00:52, David Malcolm <dmalc...@redhat.com> wrote:
>
> PR testsuite/96609 and PR analyzer/96616 report various testsuite
> failures seen on powerpc64, aarch64, and arm in new tests added by
> r11-2694-g808f4dfeb3a95f50f15e71148e5c1067f90a126d.
>
> Some of these failures (in gcc.dg/analyzer/init.c, and on arm
> in gcc.dg/analyzer/casts-1.c) relate to initializations from var_decls
> in the constant pool. I wrote the tests assuming that the gimplified
> stmts would initialize the locals via a gassign of code CONSTRUCTOR,
> whereas on these targets some of the initializations are gassign from
> a VAR_DECL e.g.:
> c = *.LC0;
> where "*.LC0" is a var_decl with DECL_IN_CONSTANT_POOL set.
>
> For example, in test_7:
> struct coord c[2] = {{3, 4}, {5, 6}};
> __analyzer_eval (c[0].x == 3); /* { dg-warning "TRUE" } */
> after the initialization, the store was simply recording:
> cluster for: c: INIT_VAL(*.LC0)
> when I was expecting the cluster for c to have:
> cluster for: c
> key: {kind: direct, start: 0, size: 32, next: 32}
> value: 'int' {(int)3}
> key: {kind: direct, start: 32, size: 32, next: 64}
> value: 'int' {(int)4}
> key: {kind: direct, start: 64, size: 32, next: 96}
> value: 'int' {(int)5}
> key: {kind: direct, start: 96, size: 32, next: 128}
> value: 'int' {(int)6}
> The test for c[0].x == 3 would then generate:
> cluster for: _2: (SUB(SUB(INIT_VAL(*.LC0), c[(int)0]), c[(int)0].x)==(int)3)
> which is UNKNOWN, leading to the test failing.
>
> This patch fixes the init.c and casts-1.c failures by special-casing
> reads from a var_decl with DECL_IN_CONSTANT_POOL set, so that they build
> a compound_svalue containing the bindings implied by the CONSTRUCTOR
> node for DECL_INITIAL.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> Manually verified the fixes to init.c and casts-1.c on
> aarch64-unknown-linux-gnu, arm-unknown-eabi, and powerpc64-linux-gnu
> (-m32 and -m64).
>
> Pushed to master as r11-2708-g2867118ddda9b56d991c16022f7d3d634ed08313.
>
Hi David,
Thanks for fixing this.
However, this patch is causing 2 ICEs on arm:
gcc.dg/analyzer/data-model-1.c (internal compiler error)
gcc.dg/analyzer/pr94639.c (internal compiler error)
Christophe
> This doesn't address the bogus -Wanalyzer-too-complex messages
> for pr93032-mztools.c reported in the bugs, which seem to be a
> separate issue that I'm now investigating.
>
> gcc/analyzer/ChangeLog:
> PR testsuite/96609
> PR analyzer/96616
> * region-model.cc (region_model::get_store_value): Call
> maybe_get_constant_value on decl_regions first.
> * region-model.h (decl_region::maybe_get_constant_value): New decl.
> * region.cc (decl_region::get_stack_depth): Likewise.
> (decl_region::maybe_get_constant_value): New.
> * store.cc (get_subregion_within_ctor): New.
> (binding_map::apply_ctor_to_region): New.
> * store.h (binding_map::apply_ctor_to_region): New decl.
> ---
> gcc/analyzer/region-model.cc | 5 +++
> gcc/analyzer/region-model.h | 2 ++
> gcc/analyzer/region.cc | 27 +++++++++++++++++
> gcc/analyzer/store.cc | 59 ++++++++++++++++++++++++++++++++++++
> gcc/analyzer/store.h | 3 ++
> 5 files changed, 96 insertions(+)
>
> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
> index 649e20438e4..3c7ea40e8d8 100644
> --- a/gcc/analyzer/region-model.cc
> +++ b/gcc/analyzer/region-model.cc
> @@ -1192,6 +1192,11 @@ region_model::get_rvalue (tree expr,
> region_model_context *ctxt)
> const svalue *
> region_model::get_store_value (const region *reg) const
> {
> + /* Special-case: handle var_decls in the constant pool. */
> + if (const decl_region *decl_reg = reg->dyn_cast_decl_region ())
> + if (const svalue *sval = decl_reg->maybe_get_constant_value (m_mgr))
> + return sval;
> +
> const svalue *sval
> = m_store.get_any_binding (m_mgr->get_store_manager (), reg);
> if (sval)
> diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
> index 33aa3461611..3d044bf8d6c 100644
> --- a/gcc/analyzer/region-model.h
> +++ b/gcc/analyzer/region-model.h
> @@ -1869,6 +1869,8 @@ public:
> tree get_decl () const { return m_decl; }
> int get_stack_depth () const;
>
> + const svalue *maybe_get_constant_value (region_model_manager *mgr) const;
> +
> private:
> tree m_decl;
> };
> diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
> index f3f577c43de..afe416b001b 100644
> --- a/gcc/analyzer/region.cc
> +++ b/gcc/analyzer/region.cc
> @@ -874,6 +874,33 @@ decl_region::get_stack_depth () const
> return 0;
> }
>
> +/* If the underlying decl is in the global constant pool,
> + return an svalue representing the constant value.
> + Otherwise return NULL. */
> +
> +const svalue *
> +decl_region::maybe_get_constant_value (region_model_manager *mgr) const
> +{
> + if (TREE_CODE (m_decl) == VAR_DECL
> + && DECL_IN_CONSTANT_POOL (m_decl)
> + && DECL_INITIAL (m_decl)
> + && TREE_CODE (DECL_INITIAL (m_decl)) == CONSTRUCTOR)
> + {
> + tree ctor = DECL_INITIAL (m_decl);
> + gcc_assert (!TREE_CLOBBER_P (ctor));
> +
> + /* Create a binding map, applying ctor to it, using this
> + decl_region as the base region when building child regions
> + for offset calculations. */
> + binding_map map;
> + map.apply_ctor_to_region (this, ctor, mgr);
> +
> + /* Return a compound svalue for the map we built. */
> + return mgr->get_or_create_compound_svalue (get_type (), map);
> + }
> + return NULL;
> +}
> +
> /* class field_region : public region. */
>
> /* Implementation of region::dump_to_pp vfunc for field_region. */
> diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
> index 950a7784542..232920019e0 100644
> --- a/gcc/analyzer/store.cc
> +++ b/gcc/analyzer/store.cc
> @@ -366,6 +366,65 @@ binding_map::dump (bool simple) const
> pp_flush (&pp);
> }
>
> +/* Get the child region of PARENT_REG based upon INDEX within a
> + CONSTRUCTOR. */
> +
> +static const region *
> +get_subregion_within_ctor (const region *parent_reg, tree index,
> + region_model_manager *mgr)
> +{
> + switch (TREE_CODE (index))
> + {
> + default:
> + gcc_unreachable ();
> + case INTEGER_CST:
> + {
> + const svalue *index_sval
> + = mgr->get_or_create_constant_svalue (index);
> + return mgr->get_element_region (parent_reg,
> + TREE_TYPE (parent_reg->get_type ()),
> + index_sval);
> + }
> + break;
> + case FIELD_DECL:
> + return mgr->get_field_region (parent_reg, index);
> + }
> +}
> +
> +/* Bind values from CONSTRUCTOR to this map, relative to
> + PARENT_REG's relationship to its base region. */
> +
> +void
> +binding_map::apply_ctor_to_region (const region *parent_reg, tree ctor,
> + region_model_manager *mgr)
> +{
> + gcc_assert (parent_reg);
> + gcc_assert (TREE_CODE (ctor) == CONSTRUCTOR);
> + gcc_assert (!CONSTRUCTOR_NO_CLEARING (ctor));
> +
> + unsigned ix;
> + tree index;
> + tree val;
> + FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (ctor), ix, index, val)
> + {
> + if (!index)
> + index = build_int_cst (integer_type_node, ix);
> + const region *child_reg
> + = get_subregion_within_ctor (parent_reg, index, mgr);
> + if (TREE_CODE (val) == CONSTRUCTOR)
> + apply_ctor_to_region (child_reg, val, mgr);
> + else
> + {
> + gcc_assert (CONSTANT_CLASS_P (val));
> + const svalue *cst_sval = mgr->get_or_create_constant_svalue (val);
> + const binding_key *k
> + = binding_key::make (mgr->get_store_manager (), child_reg,
> + BK_direct);
> + put (k, cst_sval);
> + }
> + }
> +}
> +
> /* class binding_cluster. */
>
> /* binding_cluster's copy ctor. */
> diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h
> index 4f251d6420f..16bad030b36 100644
> --- a/gcc/analyzer/store.h
> +++ b/gcc/analyzer/store.h
> @@ -340,6 +340,9 @@ public:
> void dump_to_pp (pretty_printer *pp, bool simple, bool multiline) const;
> void dump (bool simple) const;
>
> + void apply_ctor_to_region (const region *parent_reg, tree ctor,
> + region_model_manager *mgr);
> +
> private:
> map_t m_map;
> };
> --
> 2.26.2
>
