On Thu, Apr 30, 2020 at 08:36:32AM +0200, Richard Biener via Gcc-patches wrote:
> On Wed, Apr 29, 2020 at 11:43 PM Marek Polacek via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Here we trip on the TYPE_USER_ALIGN (t) assert in strip_typedefs: it
> > gets "const d[0]" with TYPE_USER_ALIGN=0 but the result built by
> > build_cplus_array_type is "const char[0]" with TYPE_USER_ALIGN=1.
> >
> > When we strip_typedefs the element of the array "const d", we see it's
> > a typedef_variant_p, so we look at its DECL_ORIGINAL_TYPE, which is
> > char, but we need to add the const qualifier, so we call
> > cp_build_qualified_type -> build_qualified_type
> > where get_qualified_type checks to see if we already have such a type
> > by walking the variants list, which in this case is:
> >
> >   char -> c -> const char -> const char -> d -> const d
> >
> > Because check_base_type only checks TYPE_ALIGN and not TYPE_USER_ALIGN,
> > we choose the first const char, which has TYPE_USER_ALIGN set.  If the
> > element type of an array has TYPE_USER_ALIGN, the array type gets it too.
> >
> > So we can make check_base_type stricter.  I was afraid that it might make
> > us reuse types less often, but measuring showed that we build the same
> > amount of types with and without the patch, while bootstrapping.
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/9/8?
> 
> Hmm, so while I wonder whether a mismatch in TYPE_USER_ALIGN would
> affect program semantics we should make sure to handle this flag consistently.
> 
> Thus the patch is OK for trunk and branches after a while.

Thanks.

> Note I question that we build new types during diagnostic printing.

This is in the context of printing an 'aka' for a typedef, so we need
strip_typedefs.

> Thanks,
> Richard.
> 
> >         PR c++/94775
> >         * tree.c (check_base_type): Return true only if TYPE_USER_ALIGN 
> > match.
> >         (check_aligned_type): Check if TYPE_USER_ALIGN match.
> >
> >         * g++.dg/warn/Warray-bounds-10.C: New test.
> > ---
> >  gcc/testsuite/g++.dg/warn/Warray-bounds-10.C | 40 ++++++++++++++++++++
> >  gcc/tree.c                                   |  4 +-
> >  2 files changed, 43 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> >
> > diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C 
> > b/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> > new file mode 100644
> > index 00000000000..0a18f637e0e
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-10.C
> > @@ -0,0 +1,40 @@
> > +// PR c++/94775
> > +// { dg-do compile { target c++14 } }
> > +// { dg-options "-O2 -Warray-bounds" }
> > +
> > +template <typename> using a = int;
> > +template <bool, typename, typename> using b = int;
> > +typedef char d;
> > +template <long> using e = int;
> > +template <int f, int q> struct h { using i = b<q, a<e<f>>, e<f>>; };
> > +template <long f, bool g> using j = typename h<f, g>::i;
> > +long ab, k, aj;
> > +const d l[]{};
> > +class m {
> > +public:
> > +  m(int);
> > +};
> > +class n {
> > +  void ad() const;
> > +  template <class ae> void o(long) const {
> > +    using c __attribute__((aligned(1))) = const ae;
> > +  }
> > +  long p;
> > +  template <class, class>
> > +  auto s(unsigned long, unsigned long, unsigned long, unsigned long) const;
> > +  template <bool = false> auto q(unsigned long, unsigned long) const;
> > +};
> > +template <class, class>
> > +auto n::s(unsigned long, unsigned long, unsigned long, unsigned long t) 
> > const {
> > +  o<d>(p);
> > +  return t;
> > +}
> > +template <bool g> auto n::q(unsigned long p1, unsigned long p2) const {
> > +  using r = j<4, false>;
> > +  using ai = j<4, g>;
> > +  return s<ai, r>(ab, k, p1, p2);
> > +}
> > +void n::ad() const {
> > +  long f(l[aj]); // { dg-warning "outside array bounds" }
> > +  m(q(8, f));
> > +}
> > diff --git a/gcc/tree.c b/gcc/tree.c
> > index e451401822c..341766c51e5 100644
> > --- a/gcc/tree.c
> > +++ b/gcc/tree.c
> > @@ -6493,7 +6493,8 @@ check_base_type (const_tree cand, const_tree base)
> >                                 TYPE_ATTRIBUTES (base)))
> >      return false;
> >    /* Check alignment.  */
> > -  if (TYPE_ALIGN (cand) == TYPE_ALIGN (base))
> > +  if (TYPE_ALIGN (cand) == TYPE_ALIGN (base)
> > +      && TYPE_USER_ALIGN (cand) == TYPE_USER_ALIGN (base))
> >      return true;
> >    /* Atomic types increase minimal alignment.  We must to do so as well
> >       or we get duplicated canonical types. See PR88686.  */
> > @@ -6528,6 +6529,7 @@ check_aligned_type (const_tree cand, const_tree base, 
> > unsigned int align)
> >           && TYPE_CONTEXT (cand) == TYPE_CONTEXT (base)
> >           /* Check alignment.  */
> >           && TYPE_ALIGN (cand) == align
> > +         && TYPE_USER_ALIGN (cand) == TYPE_USER_ALIGN (base)
> >           && attribute_list_equal (TYPE_ATTRIBUTES (cand),
> >                                    TYPE_ATTRIBUTES (base))
> >           && check_lang_type (cand, base));
> >
> > base-commit: 8f1591763fd50b143af0dc1770741f326a97583a
> > --
> > Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
> >
> 

Reply via email to