On Mon, Mar 21, 2016 at 09:57:54PM +0100, Richard Biener wrote:
> On March 21, 2016 6:55:28 PM GMT+01:00, Marek Polacek <pola...@redhat.com>
> wrote:
> >This PR points out to a GC problem: when we freed a duplicate typedef,
> >we were
> >leaving its type in the variants list, with its TYPE_NAME still
> >pointing to the
> >now-freed TYPE_DECL, leading to a crash. I was lucky to discover that
> >the
> >exact same problem was fixed in November for the C++ FE, so I could
> >just follow
> >suit here. And because that change didn't add a new testcase, I'm
> >putting the
> >new test into c-c++-common/.
> >
> >Bootstrapped/regtested on x86_64-linux, ok for trunk/5?
>
> But IIRC this will drop the aligned attribute effect as that applied to the
> new type?
Yes. So this metamorphosed into another issue: what to do with
typedef int T;
typedef int T __attribute__((aligned (16)));
typedef int T __attribute__((aligned (32)));
? Either we can reject this, or do what clang (and other compilers) do, that
is pick the strictest alignment - 32 in this case. The following patch does
that.
I've also checked what the C FE does for a few other attributes:
packed
visibility
- ignored on TYPE_DECLs
used
unused
deprecated
scalar_storage_order
- we seem to do the right thing
transparent_union
- typedef int T;
typedef int T __attribute__((transparent_union));
gives an error
vector_size
mode
- typedef int T;
typedef int T __attribute__((mode (SI)));
works while
typedef int T;
typedef int T __attribute__((mode (DI)));
results in "conflicts" error
similarly for vector_size
Well, here's the patch. Thoughts?
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-03-23 Marek Polacek <pola...@redhat.com>
PR c/70297
* c-decl.c (merge_decls): Also set TYPE_ALIGN and TYPE_USER_ALIGN.
* decl.c (duplicate_decls): Also set TYPE_ALIGN and TYPE_USER_ALIGN.
* c-c++-common/pr70297.c: New test.
* g++.dg/cpp0x/typedef-redecl.C: New test.
* gcc.dg/typedef-redecl2.c: New test.
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index bab4715..0dd2121 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -2358,6 +2358,35 @@ merge_decls (tree newdecl, tree olddecl, tree newtype,
tree oldtype)
DECL_ATTRIBUTES (newdecl)
= targetm.merge_decl_attributes (olddecl, newdecl);
+ /* For typedefs use the old type, as the new type's DECL_NAME points
+ at newdecl, which will be ggc_freed. */
+ if (TREE_CODE (newdecl) == TYPE_DECL)
+ {
+ /* But NEWTYPE might have an attribute, honor that. */
+ tree tem = newtype;
+ newtype = oldtype;
+
+ if (TYPE_USER_ALIGN (tem))
+ {
+ if (TYPE_ALIGN (tem) > TYPE_ALIGN (newtype))
+ TYPE_ALIGN (newtype) = TYPE_ALIGN (tem);
+ TYPE_USER_ALIGN (newtype) = true;
+ }
+
+ /* And remove the new type from the variants list. */
+ if (TYPE_NAME (TREE_TYPE (newdecl)) == newdecl)
+ {
+ tree remove = TREE_TYPE (newdecl);
+ for (tree t = TYPE_MAIN_VARIANT (remove); ;
+ t = TYPE_NEXT_VARIANT (t))
+ if (TYPE_NEXT_VARIANT (t) == remove)
+ {
+ TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (remove);
+ break;
+ }
+ }
+ }
+
/* Merge the data types specified in the two decls. */
TREE_TYPE (newdecl)
= TREE_TYPE (olddecl)
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 47a53cb..c21d321 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -2033,8 +2033,17 @@ duplicate_decls (tree newdecl, tree olddecl, bool
newdecl_is_friend)
at newdecl, which will be ggc_freed. */
if (TREE_CODE (newdecl) == TYPE_DECL)
{
+ /* But NEWTYPE might have an attribute, honor that. */
+ tree tem = TREE_TYPE (newdecl);
newtype = oldtype;
+ if (TYPE_USER_ALIGN (tem))
+ {
+ if (TYPE_ALIGN (tem) > TYPE_ALIGN (newtype))
+ TYPE_ALIGN (newtype) = TYPE_ALIGN (tem);
+ TYPE_USER_ALIGN (newtype) = true;
+ }
+
/* And remove the new type from the variants list. */
if (TYPE_NAME (TREE_TYPE (newdecl)) == newdecl)
{
diff --git gcc/testsuite/c-c++-common/pr70297.c
gcc/testsuite/c-c++-common/pr70297.c
index e69de29..70a4f15 100644
--- gcc/testsuite/c-c++-common/pr70297.c
+++ gcc/testsuite/c-c++-common/pr70297.c
@@ -0,0 +1,9 @@
+/* PR c/70297 */
+/* { dg-do compile } */
+/* { dg-options "-g" } */
+
+typedef int T;
+typedef int T __attribute__((aligned (4)));
+struct S {
+ T *t;
+};
diff --git gcc/testsuite/g++.dg/cpp0x/typedef-redecl.C
gcc/testsuite/g++.dg/cpp0x/typedef-redecl.C
index e69de29..576c7ce 100644
--- gcc/testsuite/g++.dg/cpp0x/typedef-redecl.C
+++ gcc/testsuite/g++.dg/cpp0x/typedef-redecl.C
@@ -0,0 +1,12 @@
+// PR c/70297
+// { dg-do compile { target c++11 } }
+
+#define N 64
+
+typedef int T;
+typedef int T __attribute__((aligned (N)));
+typedef int T __attribute__((aligned (N * 2)));
+typedef int T __attribute__((aligned (N)));
+typedef int T;
+
+static_assert (alignof (T) == N * 2, "N * 2");
diff --git gcc/testsuite/gcc.dg/typedef-redecl2.c
gcc/testsuite/gcc.dg/typedef-redecl2.c
index e69de29..c2314c7 100644
--- gcc/testsuite/gcc.dg/typedef-redecl2.c
+++ gcc/testsuite/gcc.dg/typedef-redecl2.c
@@ -0,0 +1,13 @@
+/* PR c/70297 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+#define N 64
+
+typedef int T;
+typedef int T __attribute__((aligned (N)));
+typedef int T __attribute__((aligned (N * 2)));
+typedef int T __attribute__((aligned (N)));
+typedef int T;
+
+_Static_assert (_Alignof (T) == N * 2, "N * 2");
Marek
