| Message ID | 87y41ne0n1.fsf@atmel.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > Hi, > > The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the > same issue on a gcc 5.x and found that the fix didn't get backported. > > Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to > backport to gcc-5-branch? Ok with me but please double-check there was no fallout. Richard. > Regards > Senthil > > gcc/c/ChangeLog > > 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > Backport from mainline > 2015-04-25 Marek Polacek <polacek@redhat.com> > PR c/52085 > * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for "mode" > attribute. > > gcc/testsuite/ChangeLog > 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > Backport from mainline > 2015-04-25 Marek Polacek <polacek@redhat.com> > PR c/52085 > * gcc.dg/enum-incomplete-2.c: New test. > * gcc.dg/enum-mode-1.c: New test. > > > diff --git gcc/c/c-decl.c gcc/c/c-decl.c > index d1e7444..c508e7f 100644 > --- gcc/c/c-decl.c > +++ gcc/c/c-decl.c > @@ -8050,7 +8050,7 @@ finish_enum (tree enumtype, tree values, tree attributes) > > /* 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)) > { > @@ -8078,6 +8078,7 @@ 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; > TYPE_PRECISION (enumtype) = TYPE_PRECISION (tem); > > diff --git gcc/testsuite/gcc.dg/enum-incomplete-2.c gcc/testsuite/gcc.dg/enum-incomplete-2.c > new file mode 100644 > index 0000000..5970551 > --- /dev/null > +++ 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 > new file mode 100644 > index 0000000..a701123 > --- /dev/null > +++ 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" } */
Richard Biener writes: > On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj > <senthil_kumar.selvaraj@atmel.com> wrote: >> Hi, >> >> The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the >> same issue on a gcc 5.x and found that the fix didn't get backported. >> >> Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to >> backport to gcc-5-branch? > > Ok with me but please double-check there was no fallout. I boostrapped and ran against x86_64-pc-linux again, just to be sure. No regressions. I'll run the reg tests against arm-none-eabi. Can I commit it if that passes? Regards Senthil > > Richard. > >> Regards >> Senthil >> >> gcc/c/ChangeLog >> >> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >> Backport from mainline >> 2015-04-25 Marek Polacek <polacek@redhat.com> >> PR c/52085 >> * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for "mode" >> attribute. >> >> gcc/testsuite/ChangeLog >> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >> Backport from mainline >> 2015-04-25 Marek Polacek <polacek@redhat.com> >> PR c/52085 >> * gcc.dg/enum-incomplete-2.c: New test. >> * gcc.dg/enum-mode-1.c: New test. >> >> >> diff --git gcc/c/c-decl.c gcc/c/c-decl.c >> index d1e7444..c508e7f 100644 >> --- gcc/c/c-decl.c >> +++ gcc/c/c-decl.c >> @@ -8050,7 +8050,7 @@ finish_enum (tree enumtype, tree values, tree attributes) >> >> /* 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)) >> { >> @@ -8078,6 +8078,7 @@ 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; >> TYPE_PRECISION (enumtype) = TYPE_PRECISION (tem); >> >> diff --git gcc/testsuite/gcc.dg/enum-incomplete-2.c gcc/testsuite/gcc.dg/enum-incomplete-2.c >> new file mode 100644 >> index 0000000..5970551 >> --- /dev/null >> +++ 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 >> new file mode 100644 >> index 0000000..a701123 >> --- /dev/null >> +++ 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" } */
On Mon, Oct 17, 2016 at 6:57 PM, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > > Richard Biener writes: > >> On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj >> <senthil_kumar.selvaraj@atmel.com> wrote: >>> Hi, >>> >>> The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the >>> same issue on a gcc 5.x and found that the fix didn't get backported. >>> >>> Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to >>> backport to gcc-5-branch? >> >> Ok with me but please double-check there was no fallout. > > I boostrapped and ran against x86_64-pc-linux again, just to be sure. > No regressions. I meant fallout only fixed with followup patches. ISTR some in that area but I might confuse it with another patch. Marek might remember. Richard. > I'll run the reg tests against arm-none-eabi. Can I commit it if that > passes? > > Regards > Senthil >> >> Richard. >> >>> Regards >>> Senthil >>> >>> gcc/c/ChangeLog >>> >>> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >>> >>> Backport from mainline >>> 2015-04-25 Marek Polacek <polacek@redhat.com> >>> PR c/52085 >>> * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for "mode" >>> attribute. >>> >>> gcc/testsuite/ChangeLog >>> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >>> >>> Backport from mainline >>> 2015-04-25 Marek Polacek <polacek@redhat.com> >>> PR c/52085 >>> * gcc.dg/enum-incomplete-2.c: New test. >>> * gcc.dg/enum-mode-1.c: New test. >>> >>> >>> diff --git gcc/c/c-decl.c gcc/c/c-decl.c >>> index d1e7444..c508e7f 100644 >>> --- gcc/c/c-decl.c >>> +++ gcc/c/c-decl.c >>> @@ -8050,7 +8050,7 @@ finish_enum (tree enumtype, tree values, tree attributes) >>> >>> /* 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)) >>> { >>> @@ -8078,6 +8078,7 @@ 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; >>> TYPE_PRECISION (enumtype) = TYPE_PRECISION (tem); >>> >>> diff --git gcc/testsuite/gcc.dg/enum-incomplete-2.c gcc/testsuite/gcc.dg/enum-incomplete-2.c >>> new file mode 100644 >>> index 0000000..5970551 >>> --- /dev/null >>> +++ 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 >>> new file mode 100644 >>> index 0000000..a701123 >>> --- /dev/null >>> +++ 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" } */ >
On Tue, Oct 18, 2016 at 10:12:24AM +0200, Richard Biener wrote: > On Mon, Oct 17, 2016 at 6:57 PM, Senthil Kumar Selvaraj > <senthil_kumar.selvaraj@atmel.com> wrote: > > > > Richard Biener writes: > > > >> On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj > >> <senthil_kumar.selvaraj@atmel.com> wrote: > >>> Hi, > >>> > >>> The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the > >>> same issue on a gcc 5.x and found that the fix didn't get backported. > >>> > >>> Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to > >>> backport to gcc-5-branch? > >> > >> Ok with me but please double-check there was no fallout. > > > > I boostrapped and ran against x86_64-pc-linux again, just to be sure. > > No regressions. > > I meant fallout only fixed with followup patches. ISTR some in that area > but I might confuse it with another patch. Marek might remember. I don't remember any fallout here (and a quick look at the ML around that time doesn't reveal any). Marek
On Tue, Oct 18, 2016 at 10:12:24AM +0200, Richard Biener wrote: > On Mon, Oct 17, 2016 at 6:57 PM, Senthil Kumar Selvaraj > <senthil_kumar.selvaraj@atmel.com> wrote: > > > > Richard Biener writes: > > > >> On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj > >> <senthil_kumar.selvaraj@atmel.com> wrote: > >>> Hi, > >>> > >>> The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the > >>> same issue on a gcc 5.x and found that the fix didn't get backported. > >>> > >>> Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to > >>> backport to gcc-5-branch? > >> > >> Ok with me but please double-check there was no fallout. > > > > I boostrapped and ran against x86_64-pc-linux again, just to be sure. > > No regressions. > > I meant fallout only fixed with followup patches. ISTR some in that area > but I might confuse it with another patch. Marek might remember. I'm not convinced it is desirable to backport such changes, it affects ABI, people are used to deal with minor ABI changes in between major GCC releases, but we'd need a strong reason to change it between minor releases. > >>> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > >>> > >>> Backport from mainline > >>> 2015-04-25 Marek Polacek <polacek@redhat.com> > >>> PR c/52085 > >>> * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for "mode" > >>> attribute. > >>> > >>> gcc/testsuite/ChangeLog > >>> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > >>> > >>> Backport from mainline > >>> 2015-04-25 Marek Polacek <polacek@redhat.com> > >>> PR c/52085 > >>> * gcc.dg/enum-incomplete-2.c: New test. > >>> * gcc.dg/enum-mode-1.c: New test. Jakub
Jakub Jelinek writes: > On Tue, Oct 18, 2016 at 10:12:24AM +0200, Richard Biener wrote: >> On Mon, Oct 17, 2016 at 6:57 PM, Senthil Kumar Selvaraj >> <senthil_kumar.selvaraj@atmel.com> wrote: >> > >> > Richard Biener writes: >> > >> >> On Mon, Oct 17, 2016 at 12:21 PM, Senthil Kumar Selvaraj >> >> <senthil_kumar.selvaraj@atmel.com> wrote: >> >>> Hi, >> >>> >> >>> The fix for PR 52085 went into trunk when trunk was 6.0. I ran into the >> >>> same issue on a gcc 5.x and found that the fix didn't get backported. >> >>> >> >>> Bootstrapped and reg tested below patch with x86-64-pc-linux. Ok to >> >>> backport to gcc-5-branch? >> >> >> >> Ok with me but please double-check there was no fallout. >> > >> > I boostrapped and ran against x86_64-pc-linux again, just to be sure. >> > No regressions. >> >> I meant fallout only fixed with followup patches. ISTR some in that area >> but I might confuse it with another patch. Marek might remember. > > I'm not convinced it is desirable to backport such changes, it affects ABI, > people are used to deal with minor ABI changes in between major GCC > releases, but we'd need a strong reason to change it between minor releases. Hmm, I tracked this down from a (internal) bug reported on arm-none-eabi, where the inconsistent enum size (used in sizeof to malloc) was eventually causing heap corruption. When debugging the issue, I noticed you'd already backported https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69669, so I thought this should be good. Regards Senthil > >> >>> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >>> >> >>> Backport from mainline >> >>> 2015-04-25 Marek Polacek <polacek@redhat.com> >> >>> PR c/52085 >> >>> * c-decl.c (finish_enum): Copy over TYPE_ALIGN. Also check for "mode" >> >>> attribute. >> >>> >> >>> gcc/testsuite/ChangeLog >> >>> 2016-10-17 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >>> >> >>> Backport from mainline >> >>> 2015-04-25 Marek Polacek <polacek@redhat.com> >> >>> PR c/52085 >> >>> * gcc.dg/enum-incomplete-2.c: New test. >> >>> * gcc.dg/enum-mode-1.c: New test. > > Jakub
On Tue, Oct 18, 2016 at 02:46:29PM +0530, Senthil Kumar Selvaraj wrote: > > I'm not convinced it is desirable to backport such changes, it affects ABI, > > people are used to deal with minor ABI changes in between major GCC > > releases, but we'd need a strong reason to change it between minor releases. > > Hmm, I tracked this down from a (internal) bug reported on arm-none-eabi, where the > inconsistent enum size (used in sizeof to malloc) was eventually causing > heap corruption. > > When debugging the issue, I noticed you'd already backported > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69669, so I thought this > should be good. That one has been a regression, older GCCs handled it the same as does the 5 branch now. Is that the case here? Jakub
Jakub Jelinek writes: > On Tue, Oct 18, 2016 at 02:46:29PM +0530, Senthil Kumar Selvaraj wrote: >> > I'm not convinced it is desirable to backport such changes, it affects ABI, >> > people are used to deal with minor ABI changes in between major GCC >> > releases, but we'd need a strong reason to change it between minor releases. >> >> Hmm, I tracked this down from a (internal) bug reported on arm-none-eabi, where the >> inconsistent enum size (used in sizeof to malloc) was eventually causing >> heap corruption. >> >> When debugging the issue, I noticed you'd already backported >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69669, so I thought this >> should be good. > > That one has been a regression, older GCCs handled it the same as does the 5 > branch now. Is that the case here? No, I can reproduce the bug on 4.9 as well. So not ok to backport then, I guess? Regards Senthil
diff --git gcc/c/c-decl.c gcc/c/c-decl.c index d1e7444..c508e7f 100644 --- gcc/c/c-decl.c +++ gcc/c/c-decl.c @@ -8050,7 +8050,7 @@ finish_enum (tree enumtype, tree values, tree attributes) /* 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)) { @@ -8078,6 +8078,7 @@ 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; TYPE_PRECISION (enumtype) = TYPE_PRECISION (tem); diff --git gcc/testsuite/gcc.dg/enum-incomplete-2.c gcc/testsuite/gcc.dg/enum-incomplete-2.c new file mode 100644 index 0000000..5970551 --- /dev/null +++ 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 new file mode 100644 index 0000000..a701123 --- /dev/null +++ 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" } */