| Message ID | 20200429205943.2277840-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tree: Don't reuse types if TYPE_USER_ALIGN differ [PR94775] | expand |
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. Note I question that we build new types during diagnostic printing. 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 >
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 > > >
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));