| Message ID | 20160323135200.GF3401@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/23/2016 07:52 AM, Marek Polacek wrote: > 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 <polacek@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 <polacek@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. OK. jeff
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");