On 1/28/21 10:34 AM, Marek Polacek wrote:
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. Does this seem sane?
Bootstrapped/regtested on
* x86_64-pc-linux-gnu
* powerpc64le-unknown-linux-gnu
* aarch64-linux-gnu
ok for trunk?
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 7d6b1b5ce52..784f131ebb8 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)
I'm not sure why you want to check both of these conditions. Maybe drop
the local variable?
Also this existing code seems more complex than necessary to handle
variants with different alignment than the primary class, which I think
is now ill-formed because you can't add attributes to a class after the
definition.
+ 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 00000000000..892125b54df
--- /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 00000000000..0a18f637e0e
--- /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 <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 f9d57e6d409..430b76168b2 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));
base-commit: 1cdca4261e88f4dc9c3293c6b3c2fff3071ca32b
