| Message ID | 20150423204616.GG2813@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/23/2015 02:46 PM, Marek Polacek wrote: > This PR points out a problem with enum forward declarations (so C++ is out as > these are forbidden in C++). If we forward declare an enum, and later on > declare the enum with __attribute__ ((packed)), the attribute is ignored. The > reason is that when we first see the forward declaration, parser_xref_tag sets > up a forward-reference node, which has a default layout like unsigned int, so > among other things it does > > TYPE_PRECISION (ref) = TYPE_PRECISION (unsigned_type_node); > > But then in finish_enum we were confused: the code thinks that the precision > was specified with the mode attribute so it uses wrong TYPE_PRECISION. After > playing with this a bit, I think easiest fix is to check for a mode attribute. > If we can't find it, we know that TYPE_PRECISION was set when processing the > forward declaration. > We also need to copy TYPE_ALIGN, otherwise subsequent layout_type won't DTRT. > > While at it, I noticed that we don't test the "specified mode too small" error > at all, so I've added enum-mode-1.c test. > > It's unclear what to do with a combination of "packed" and "mode" attributes; I > suppose that should be invalid, but I'm not changing it now. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2015-04-23 Marek Polacek <polacek@redhat.com> > > PR c/52085 > * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for "mode" > attribute. > > * gcc.dg/enum-incomplete-2.c: New test. > * gcc.dg/enum-mode-1.c: New test. What happens if we have used the enum inside an aggregate? Can we just blindly change the alignment/precision like that? My inclination would to to issue an error if we had a forward enum decl followed by a later packed enum. jeff
On Thu, Apr 23, 2015 at 08:25:51PM -0600, Jeff Law wrote: > What happens if we have used the enum inside an aggregate? Can we just > blindly change the alignment/precision like that? If you just forward declare an enum/struct, it has an incomplete type, so you cannot use it inside an aggregate. In fact, you can't do much with that until it's completed, except creating a pointer to it. E.g. applying sizeof to an incomplete type is forbidden. With my patch, we behave the same as clang. Marek
On 04/24/2015 05:36 AM, Marek Polacek wrote: > On Thu, Apr 23, 2015 at 08:25:51PM -0600, Jeff Law wrote: >> What happens if we have used the enum inside an aggregate? Can we just >> blindly change the alignment/precision like that? > > If you just forward declare an enum/struct, it has an incomplete type, so you > cannot use it inside an aggregate. In fact, you can't do much with that until > it's completed, except creating a pointer to it. E.g. applying sizeof to an > incomplete type is forbidden. With my patch, we behave the same as clang. Clearly a "duh" moment for me. OK for the trunk. jeff
diff --git gcc/c/c-decl.c gcc/c/c-decl.c index 27ecd8b..4f6761d 100644 --- gcc/c/c-decl.c +++ gcc/c/c-decl.c @@ -8010,11 +8010,13 @@ finish_enum (tree enumtype, tree values, tree attributes) TYPE_MIN_VALUE (enumtype) = TYPE_MIN_VALUE (tem); TYPE_MAX_VALUE (enumtype) = TYPE_MAX_VALUE (tem); TYPE_UNSIGNED (enumtype) = TYPE_UNSIGNED (tem); + TYPE_ALIGN (enumtype) = TYPE_ALIGN (tem); TYPE_SIZE (enumtype) = 0; - /* If the precision of the type was specific with an attribute and it + /* If the precision of the type was specified with an attribute and it was too small, give an error. Otherwise, use it. */ - if (TYPE_PRECISION (enumtype)) + if (TYPE_PRECISION (enumtype) + && lookup_attribute ("mode", attributes)) { if (precision > TYPE_PRECISION (enumtype)) error ("specified mode too small for enumeral values"); diff --git gcc/testsuite/gcc.dg/enum-incomplete-2.c gcc/testsuite/gcc.dg/enum-incomplete-2.c index e69de29..5970551 100644 --- gcc/testsuite/gcc.dg/enum-incomplete-2.c +++ gcc/testsuite/gcc.dg/enum-incomplete-2.c @@ -0,0 +1,41 @@ +/* PR c/52085 */ +/* { dg-do compile } */ +/* { dg-options "" } */ + +#define SA(X) _Static_assert((X),#X) + +enum e1; +enum e1 { A } __attribute__ ((__packed__)); +enum e2 { B } __attribute__ ((__packed__)); +SA (sizeof (enum e1) == sizeof (enum e2)); +SA (_Alignof (enum e1) == _Alignof (enum e2)); + +enum e3; +enum e3 { C = 256 } __attribute__ ((__packed__)); +enum e4 { D = 256 } __attribute__ ((__packed__)); +SA (sizeof (enum e3) == sizeof (enum e4)); +SA (_Alignof (enum e3) == _Alignof (enum e4)); + +enum e5; +enum e5 { E = __INT_MAX__ } __attribute__ ((__packed__)); +enum e6 { F = __INT_MAX__ } __attribute__ ((__packed__)); +SA (sizeof (enum e5) == sizeof (enum e6)); +SA (_Alignof (enum e5) == _Alignof (enum e6)); + +enum e7; +enum e7 { G } __attribute__ ((__mode__(__byte__))); +enum e8 { H } __attribute__ ((__mode__(__byte__))); +SA (sizeof (enum e7) == sizeof (enum e8)); +SA (_Alignof (enum e7) == _Alignof (enum e8)); + +enum e9; +enum e9 { I } __attribute__ ((__packed__, __mode__(__byte__))); +enum e10 { J } __attribute__ ((__packed__, __mode__(__byte__))); +SA (sizeof (enum e9) == sizeof (enum e10)); +SA (_Alignof (enum e9) == _Alignof (enum e10)); + +enum e11; +enum e11 { K } __attribute__ ((__mode__(__word__))); +enum e12 { L } __attribute__ ((__mode__(__word__))); +SA (sizeof (enum e11) == sizeof (enum e12)); +SA (_Alignof (enum e11) == _Alignof (enum e12)); diff --git gcc/testsuite/gcc.dg/enum-mode-1.c gcc/testsuite/gcc.dg/enum-mode-1.c index e69de29..a701123 100644 --- gcc/testsuite/gcc.dg/enum-mode-1.c +++ gcc/testsuite/gcc.dg/enum-mode-1.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ + +enum e1 { A = 256 } __attribute__((__mode__(__byte__))); /* { dg-error "specified mode too small for enumeral values" } */ +enum e2 { B = 256 } __attribute__((__packed__, __mode__(__byte__))); /* { dg-error "specified mode too small for enumeral values" } */ + +enum e3 { C = __INT_MAX__ } __attribute__((__mode__(__QI__))); /* { dg-error "specified mode too small for enumeral values" } */ +enum e4 { D = __INT_MAX__ } __attribute__((__packed__, __mode__(__QI__))); /* { dg-error "specified mode too small for enumeral values" } */ + +enum e5 { E = __INT_MAX__ } __attribute__((__mode__(__HI__))); /* { dg-error "specified mode too small for enumeral values" } */ +enum e6 { F = __INT_MAX__ } __attribute__((__packed__, __mode__(__HI__))); /* { dg-error "specified mode too small for enumeral values" } */