From 513ee7d2cd9a60339a50dc9c4cba39a8f1c707f0 Mon Sep 17 00:00:00 2001 From: Marek Polacek Date: Thu, 28 Jan 2021 16:21:50 -0500 Subject: [PATCH] tree: Don't reuse types if TYPE_USER_ALIGN differ [PR94775] A year ago I submitted this patch: ~~ 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. ~~ However, the patch broke a few tests on STRICT_ALIGNMENT platforms and had to be reverted. This is another try. The original patch is kept unchanged, but I added the finalize_type_size hunk that ought to fix the STRICT_ALIGNMENT issues. The problem is that finalize_type_size can clear TYPE_USER_ALIGN on the main variant of a type, but doesn't clear it on any of the variants. Then we end up with types which share the same TYPE_MAIN_VARIANT, but their TYPE_CANONICAL differs and then the usual "canonical types differ for identical types" follows. I've created alignas19.C to exercise this scenario. What happens is: - when parsing the class S we create a type S in xref_tag, - we see alignas(8) so common_handle_aligned_attribute sets T_U_A in S, - we parse the member function fn and build_memfn_type creates a copy of S to add const; this variant has T_U_A set, - we finish_struct S which calls layout_class_type -> finish_record_type -> finalize_size_type where we reset T_U_A in S (but const S keeps it), - finish_non_static_data_member for arr calls maybe_dummy_object with type = S, - maybe_dummy_object calls same_type_ignoring_top_level_qualifiers_p to check if S and TREE_TYPE (current_class_ref), which is const S, are the same, - same_type_ignoring_top_level_qualifiers_p creates cv-unqualified versions of the passed types. Previously we'd use our main variant S when stripping "const S" of const, but since the T_U_A flags don't match (check_base_type), we create a new variant S'. Then we crash in comptypes because S and S' have the same TYPE_MAIN_VARIANT but different TYPE_CANONICALs. With my patch we'll clear T_U_A for S's variants too, and then instead of S' we'll just use S. gcc/ChangeLog: PR c++/94775 * stor-layout.c (finalize_type_size): If we reset TYPE_USER_ALIGN in the main variant, maybe reset it in its variants too. * tree.c (check_base_type): Return true only if TYPE_USER_ALIGN match. (check_aligned_type): Check if TYPE_USER_ALIGN match. gcc/testsuite/ChangeLog: PR c++/94775 * g++.dg/cpp0x/alignas19.C: New test. * g++.dg/warn/Warray-bounds15.C: New test. --- gcc/stor-layout.c | 16 +++++++++--- gcc/testsuite/g++.dg/cpp0x/alignas19.C | 8 ++++++ gcc/testsuite/g++.dg/warn/Warray-bounds15.C | 40 +++++++++++++++++++++++++++++ gcc/tree.c | 4 ++- 4 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/alignas19.C create mode 100644 gcc/testsuite/g++.dg/warn/Warray-bounds15.C diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c index 7d6b1b5..784f131 100644 --- a/gcc/stor-layout.c +++ b/gcc/stor-layout.c @@ -1926,6 +1926,7 @@ finalize_type_size (tree type) However, where strict alignment is not required, avoid over-aligning structures, since most compilers do not do this alignment. */ + bool tua_cleared_p = false; if (TYPE_MODE (type) != BLKmode && TYPE_MODE (type) != VOIDmode && (STRICT_ALIGNMENT || !AGGREGATE_TYPE_P (type))) @@ -1937,7 +1938,9 @@ finalize_type_size (tree type) if (mode_align >= TYPE_ALIGN (type)) { SET_TYPE_ALIGN (type, mode_align); - TYPE_USER_ALIGN (type) = 0; + /* Remember that we're about to reset this flag. */ + tua_cleared_p = TYPE_USER_ALIGN (type); + TYPE_USER_ALIGN (type) = false; } } @@ -1991,14 +1994,21 @@ finalize_type_size (tree type) /* Copy it into all variants. */ for (variant = TYPE_MAIN_VARIANT (type); - variant != 0; + variant != NULL_TREE; variant = TYPE_NEXT_VARIANT (variant)) { TYPE_SIZE (variant) = size; TYPE_SIZE_UNIT (variant) = size_unit; unsigned valign = align; if (TYPE_USER_ALIGN (variant)) - valign = MAX (valign, TYPE_ALIGN (variant)); + { + valign = MAX (valign, TYPE_ALIGN (variant)); + /* If we reset TYPE_USER_ALIGN on the main variant, we might + need to reset it on the variants too. TYPE_MODE will be set + to MODE in this variant, so we can use that. */ + if (tua_cleared_p && GET_MODE_ALIGNMENT (mode) >= valign) + TYPE_USER_ALIGN (variant) = false; + } else TYPE_USER_ALIGN (variant) = user_align; SET_TYPE_ALIGN (variant, valign); diff --git a/gcc/testsuite/g++.dg/cpp0x/alignas19.C b/gcc/testsuite/g++.dg/cpp0x/alignas19.C new file mode 100644 index 0000000..892125b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/alignas19.C @@ -0,0 +1,8 @@ +// PR c++/94775 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-mstrict-align" { target { aarch64*-*-* powerpc*-*-linux* powerpc*-*-elf* } } } + +struct alignas(8) S { + S *arr[1]; + void fn () const { (void) arr[0]; } +}; diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds15.C b/gcc/testsuite/g++.dg/warn/Warray-bounds15.C new file mode 100644 index 0000000..0a18f63 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds15.C @@ -0,0 +1,40 @@ +// PR c++/94775 +// { dg-do compile { target c++14 } } +// { dg-options "-O2 -Warray-bounds" } + +template using a = int; +template using b = int; +typedef char d; +template using e = int; +template struct h { using i = b>, e>; }; +template using j = typename h::i; +long ab, k, aj; +const d l[]{}; +class m { +public: + m(int); +}; +class n { + void ad() const; + template void o(long) const { + using c __attribute__((aligned(1))) = const ae; + } + long p; + template + auto s(unsigned long, unsigned long, unsigned long, unsigned long) const; + template auto q(unsigned long, unsigned long) const; +}; +template +auto n::s(unsigned long, unsigned long, unsigned long, unsigned long t) const { + o(p); + return t; +} +template auto n::q(unsigned long p1, unsigned long p2) const { + using r = j<4, false>; + using ai = j<4, g>; + return s(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 f9d57e6..430b761 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -6573,7 +6573,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. */ @@ -6608,6 +6609,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)); -- 2.7.4