Backport fix for PR 52085 to gcc-5-branch?

Submitted by Senthil Kumar Selvaraj on Oct. 17, 2016, 10:21 a.m.

Details

Message ID 87y41ne0n1.fsf@atmel.com
State New
Headers show

Commit Message

Senthil Kumar Selvaraj Oct. 17, 2016, 10:21 a.m.
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?

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.

Comments

Richard Guenther Oct. 17, 2016, 11:23 a.m.
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" } */
Senthil Kumar Selvaraj Oct. 17, 2016, 4:57 p.m.
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" } */
Richard Guenther Oct. 18, 2016, 8:12 a.m.
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" } */
>
Marek Polacek Oct. 18, 2016, 8:18 a.m.
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
Jakub Jelinek Oct. 18, 2016, 8:19 a.m.
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
Senthil Kumar Selvaraj Oct. 18, 2016, 9:16 a.m.
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
Jakub Jelinek Oct. 18, 2016, 9:32 a.m.
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
Senthil Kumar Selvaraj Oct. 18, 2016, 9:43 a.m.
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

Patch hide | download patch | download mbox

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" } */