tree: Don't reuse types if TYPE_USER_ALIGN differ [PR94775]
authorMarek Polacek <polacek@redhat.com>
Thu, 28 Jan 2021 21:21:50 +0000 (16:21 -0500)
committerMarek Polacek <polacek@redhat.com>
Thu, 28 Jan 2021 21:21:50 +0000 (16:21 -0500)
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
gcc/testsuite/g++.dg/cpp0x/alignas19.C [new file with mode: 0644]
gcc/testsuite/g++.dg/warn/Warray-bounds15.C [new file with mode: 0644]
gcc/tree.c

index 7d6b1b5ce529eb67486dab32c72b7a310d0ecd9d..784f131ebb89364024ae0000fe5549c63cd33006 100644 (file)
@@ -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 (file)
index 0000000..892125b
--- /dev/null
@@ -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 (file)
index 0000000..0a18f63
--- /dev/null
@@ -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));
+}
index f9d57e6d4094ef1755ebf7d75e543f572e53e1df..430b76168b2b0203fe0bae98a9e380bb0a9b1252 100644 (file)
@@ -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));