[PR c++/87137] GCC-8 Fix

[PR c++/87137] GCC-8 Fix

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1391 bytes --]

This defect concerns bitfield layout in the Microsoft ABI.  This is a 
fix for gcc-8.

As well as MINGW targets, MS-ABI can be enabled on PowerPC & SuperH by 
suitable use of attributes or options.

When I folded TYPE_METHODS into TYPE_FIELDS, the 'am I the last field' 
check of place_field could give a false positive in more cases. 
Specifically if two bitfields were separated by a member function 
declaration, they'd now be placed in separate allocation units.

The place_field code was working under the assumption that the only 
things on the TYPE_FIELDS list could be FIELD_DECLs possibly followed by 
TYPE_DECLs.  That stopped being true some time ago, and as such we 
already had a layout bug.

But, it would be bad to make that particular ABI fix in a point release, 
so this patch just reverts the regression I caused.  Sadly, because it 
requires understanding TEMPLATE_DECL, we can't simply update 
place_field.  Instead I temporarily stitch out undesired DECLs around 
the call to place_field.  This seems the least intrusive, and happens 
only when ms_bitfield_layout_p is in effect.

I have manually checked the new testcase behaves the same in gcc-7 and 
patched gcc-8 for an x86_64-mingw32 target.  Liu Hao has verified that 
the microsoft compiler gives the same results.

I also attach a change to wwwdocs describing this change.

Thoughts?

nathan

-- 
Nathan Sidwell

[-- Attachment #2: pr87137-8.diff --]
[-- Type: text/x-patch, Size: 4234 bytes --]

2018-08-29  Nathan Sidwell  <nathan@acm.org>

	PR c++/87137
	gcc/
	* stor-layout.c (place_field): Comment about
	layout_nonempty_base_or_field behaviour.
	gcc/cp/
	* class.c (layout_nonempty_base_or_field): Stitch out member
	functions during place_field call when ms_bitfield_layout_p is in
	effect.
	gcc/testsuite/
	* g++.dg/abi/pr87137.C: New.

Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 263959)
+++ gcc/cp/class.c	(working copy)
@@ -4041,6 +4041,32 @@ layout_nonempty_base_or_field (record_la
       field_p = true;
     }
 
+  /* PR c++/87137 When ms_bitfield_layout_p is in effect, place_field
+     used to just look at the DECL's TREE_CHAIN to see if it ended the
+     struct.  That was ok when only FIELD_DECLs and TYPE_DECLs were on
+     the chain, AND the TYPE_DECLs were known to be last.  That
+     stopped working when other things, such as static members were
+     also placed there.  However, in GCC 8 onwards all members are on
+     the chain (adding member functions).  We want to restore the
+     pre-GCC-8 behaviour, so the ABI doesn't change in a point
+     release.  Because the middle-end doesn't know about
+     TEMPLATE_DECL, we have to do nastiness here, to make DECL_CHAIN
+     look like it used to before calling place_field.  THIS IS STILL
+     WRONG, but it's the same wrongness ass gcc-7 and earier.  */
+  tree chain = NULL_TREE;
+  if (targetm.ms_bitfield_layout_p (rli->t))
+    {
+      tree probe = decl;
+      while ((probe = TREE_CHAIN (probe)))
+	if (TREE_CODE (STRIP_TEMPLATE (probe)) != FUNCTION_DECL)
+	  break;
+      if (probe != TREE_CHAIN (decl))
+	{
+	  chain = TREE_CHAIN (decl);
+	  TREE_CHAIN (decl) = probe;
+	}
+    }
+
   /* Try to place the field.  It may take more than one try if we have
      a hard time placing the field without putting two objects of the
      same type at the same address.  */
@@ -4111,6 +4137,11 @@ layout_nonempty_base_or_field (record_la
 	break;
     }
 
+  if (chain)
+    /* Restore the original chain our ms-bifield-offset shenanigans
+       above overwrote.  */
+    TREE_CHAIN (decl) = chain;
+  
   /* Now that we know where it will be placed, update its
      BINFO_OFFSET.  */
   if (binfo && CLASS_TYPE_P (BINFO_TYPE (binfo)))
Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c	(revision 263959)
+++ gcc/stor-layout.c	(working copy)
@@ -1685,6 +1685,9 @@ place_field (record_layout_info rli, tre
     {
       rli->bitpos = size_binop (PLUS_EXPR, rli->bitpos, DECL_SIZE (field));
 
+      /* PR c++/87137, see note in cp/class.c
+	 layout_nonempty_base_or_field about why this is wrong and
+	 why we can't fix it in this release.  */
       /* If we ended a bitfield before the full length of the type then
 	 pad the struct out to the full length of the last type.  */
       if ((DECL_CHAIN (field) == NULL
Index: gcc/testsuite/g++.dg/abi/pr87137.C
===================================================================
--- gcc/testsuite/g++.dg/abi/pr87137.C	(nonexistent)
+++ gcc/testsuite/g++.dg/abi/pr87137.C	(working copy)
@@ -0,0 +1,39 @@
+// PR c++/87137
+
+// Empty macro args are undefined in C++ 98
+// { dg-do compule { target c++11 } }
+
+// We got confused by non-field decls separating bitfields when
+// ms_bitfield_layout was in effect.
+
+#define DEFINE(NAME,BASE,THING) \
+  struct NAME BASE {		\
+    unsigned one : 12;		\
+    THING			\
+      unsigned two : 4;		\
+  }
+
+#define BOTH(NAME,BASE,THING)			\
+  DEFINE(NAME##_WITH,BASE,THING);		\
+  DEFINE(NAME##_WITHOUT,BASE,)
+
+template<unsigned I, unsigned J> struct Test;
+template<unsigned I> struct Test<I,I> {};
+
+#define TEST(NAME,BASE,THING)			\
+  BOTH(NAME,BASE,THING);			\
+  int NAME = sizeof (Test<sizeof(NAME##_WITH),sizeof (NAME##_WITHOUT)>)
+
+TEST(NSFun,,int fun (););
+TEST(SFun,,static int fun (););
+TEST(Tdef,,typedef int tdef;);
+TEST(MType,,struct X{int bob;};);
+TEST(TmplFN,,template<unsigned> void fu (););
+
+// Following already failed in GCC-7
+#if 0
+TEST(Var,,static int var;);
+struct base { int f; };
+TEST(Use,:base,using base::f;);
+TEST(Tmpl,,template<unsigned> class X {};);
+#endif

[-- Attachment #3: changes-8.diff --]
[-- Type: text/x-patch, Size: 1289 bytes --]

Index: htdocs/gcc-8/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-8/changes.html,v
retrieving revision 1.93
diff -r1.93 changes.html
1350a1351,1374
> <!-- .................................................................. -->
> <h2 id="GCC8.3">GCC 8.3</h2>
> 
> GCC 8.3 is <em>not</em> yet released.
> 
> <h3>Structure Layout for Windows, PowerPC &amp; SuperH targets</h4> A
>   C++ Microsoft ABI bitfield layout
>   regression, <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87137">PR87137</a>
>   introduced in GCC 8.1, has been fixed.  A declaration of a member
>   function or member function templare could cause the current
>   bitfield allocation unit to be completed, incorrectly placing a
>   following bitfield into a new allocation unit.  Microsoft ABI is
>   selected for:
>   <ul>
>     <li>Mingw targets
>     <li>PowerPC targets when <code>__attribute__((ms_struct))</code>
>       is used
>     <li>SuperH targets when the <code>-mhitachi</code> option is
>       specified, or the <code>__attribute__((renesas))</code>
>       attribute is used
>   </ul>
>   The layout bug could also be triggered by other intermediate
>   declarations.  This pre-existing issue will be fixed in GCC 9.1.
> 

Re: [PR c++/87137] GCC-8 Fix

[Richard Biener]

On August 29, 2018 7:36:15 PM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote:
>This defect concerns bitfield layout in the Microsoft ABI.  This is a 
>fix for gcc-8.
>
>As well as MINGW targets, MS-ABI can be enabled on PowerPC & SuperH by 
>suitable use of attributes or options.
>
>When I folded TYPE_METHODS into TYPE_FIELDS, the 'am I the last field' 
>check of place_field could give a false positive in more cases. 
>Specifically if two bitfields were separated by a member function 
>declaration, they'd now be placed in separate allocation units.
>
>The place_field code was working under the assumption that the only 
>things on the TYPE_FIELDS list could be FIELD_DECLs possibly followed
>by 
>TYPE_DECLs.  That stopped being true some time ago, and as such we 
>already had a layout bug.
>
>But, it would be bad to make that particular ABI fix in a point
>release, 
>so this patch just reverts the regression I caused.  Sadly, because it 
>requires understanding TEMPLATE_DECL, we can't simply update 
>place_field.  Instead I temporarily stitch out undesired DECLs around 
>the call to place_field.  This seems the least intrusive, and happens 
>only when ms_bitfield_layout_p is in effect.
>
>I have manually checked the new testcase behaves the same in gcc-7 and 
>patched gcc-8 for an x86_64-mingw32 target.  Liu Hao has verified that 
>the microsoft compiler gives the same results.
>
>I also attach a change to wwwdocs describing this change.
>
>Thoughts?

If it is that cumbersome to fix just the regression part we might also consider to fix the latent problem as well for GCC 8?  We're already breaking the GCC 8 ABI and will be breaking the ABI again for GCC 9.

Richard. 

>
>nathan

Re: [PR c++/87137] GCC-8 Fix

[Jakub Jelinek]

On Wed, Aug 29, 2018 at 01:36:15PM -0400, Nathan Sidwell wrote:
> --- gcc/cp/class.c	(revision 263959)
> +++ gcc/cp/class.c	(working copy)
> @@ -4041,6 +4041,32 @@ layout_nonempty_base_or_field (record_la
>        field_p = true;
>      }
>  
> +  /* PR c++/87137 When ms_bitfield_layout_p is in effect, place_field
> +     used to just look at the DECL's TREE_CHAIN to see if it ended the
> +     struct.  That was ok when only FIELD_DECLs and TYPE_DECLs were on
> +     the chain, AND the TYPE_DECLs were known to be last.  That
> +     stopped working when other things, such as static members were
> +     also placed there.  However, in GCC 8 onwards all members are on
> +     the chain (adding member functions).  We want to restore the
> +     pre-GCC-8 behaviour, so the ABI doesn't change in a point
> +     release.  Because the middle-end doesn't know about
> +     TEMPLATE_DECL, we have to do nastiness here, to make DECL_CHAIN
> +     look like it used to before calling place_field.  THIS IS STILL
> +     WRONG, but it's the same wrongness ass gcc-7 and earier.  */

s/ass gcc-7 and earier/as gcc-7 and earlier/ ?

> +  tree chain = NULL_TREE;
> +  if (targetm.ms_bitfield_layout_p (rli->t))
> +    {
> +      tree probe = decl;
> +      while ((probe = TREE_CHAIN (probe)))

Any reason you are using TREE_CHAIN rather than DECL_CHAIN everywhere (in
comments as well as here and below?
Shouldn't all chain elts be decls?

> +	if (TREE_CODE (STRIP_TEMPLATE (probe)) != FUNCTION_DECL)
> +	  break;
> +      if (probe != TREE_CHAIN (decl))
> +	{
> +	  chain = TREE_CHAIN (decl);
> +	  TREE_CHAIN (decl) = probe;
> +	}
> +    }
> +
>    /* Try to place the field.  It may take more than one try if we have
>       a hard time placing the field without putting two objects of the
>       same type at the same address.  */
> @@ -4111,6 +4137,11 @@ layout_nonempty_base_or_field (record_la
>  	break;
>      }
>  
> +  if (chain)
> +    /* Restore the original chain our ms-bifield-offset shenanigans
> +       above overwrote.  */
> +    TREE_CHAIN (decl) = chain;
> +  
>    /* Now that we know where it will be placed, update its
>       BINFO_OFFSET.  */
>    if (binfo && CLASS_TYPE_P (BINFO_TYPE (binfo)))

	Jakub

Re: [PR c++/87137] GCC-8 Fix

[Liu Hao]

在 2018-08-30 01:36, Nathan Sidwell 写道:
> But, it would be bad to make that particular ABI fix in a point release, 
> so this patch just reverts the regression I caused.  Sadly, because it 
> requires understanding TEMPLATE_DECL, we can't simply update 
> place_field.  Instead I temporarily stitch out undesired DECLs around 
> the call to place_field.  This seems the least intrusive, and happens 
> only when ms_bitfield_layout_p is in effect.
> 
>

It is strictly an ABI break but I doubt whether code in real world that 
got broken by this bug ever exists. Usually when people expect 
consecutive bitfields to be packed into a single word they wouldn't put 
irrelevant declarations between them.

An important reason why such code could be rare is that the following 
code compiles differently as C and C++:

```
E:\Desktop>cat test.cc
#include <stdio.h>

struct foo
   {
     unsigned a : 21;
     union x1 { char x; };
     unsigned b : 11;
     union x1 u;
   };

struct bar
   {
     union x2 { char x; };
     unsigned a : 21;
     unsigned b : 11;
     union x2 u;
   };

int main(void)
   {
     printf("%d\n", (int)sizeof(struct foo));
     printf("%d\n", (int)sizeof(struct bar));
   }
```

When compiled as C the output is:
```
E:\Desktop>cl /nologo /Tctest.cc /Fe:a.exe && a.exe
test.cc
16
12
```

while as C++ the output is:
```
E:\Desktop>cl /nologo /Tptest.cc /Fe:a.exe && a.exe
test.cc
8
8
```

Basing on the fact that such code is rare I think it should be safe to 
trade tolerance (or 'compatibility for this bug') for the correct behavior.


-- 
Best regards,
LH_Mouse

Re: [PR c++/87137] GCC-8 Fix

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

On 08/29/2018 11:06 PM, Liu Hao wrote:

> It is strictly an ABI break but I doubt whether code in real world that 
> got broken by this bug ever exists. Usually when people expect 
> consecutive bitfields to be packed into a single word they wouldn't put 
> irrelevant declarations between them.

You're probably right.  I'm guessing this bug was found because:

    int bob : 1;
    int get_bob () const {return bob;}
    void set_bob (bool v) {bob=v;}
    int bill : 1;
    ...

might be a useful style.

> An important reason why such code could be rare is that the following 
> code compiles differently as C and C++:

> struct foo
>    {
>      unsigned a : 21;
>      union x1 { char x; };
>      unsigned b : 11;
>      union x1 u;
>    };

(for C++) this happens to be a case we already get right.  <aside> I 
think that'd be a C vendor extension, I don't think unnamed fields are 
permitted there?

Here's a version of the patch to completely resolve the issue, tested on 
trunk.  I noticed regular x86 targets support the ms_struct attribute, 
so we can give this wider testing.  I did consider trying to reorder the 
field decls before struct layour, but that seemed a more invasive change 
-- besides it might be nice to be able to remove the template-specific 
CLASSTYPE_DECL_LIST which almost but not quite mirrors TYPE_FIELDS.

Ok for trunk, ok for 8?

nathan
-- 
Nathan Sidwell

[-- Attachment #2: pr87137-trunk.diff --]
[-- Type: text/x-patch, Size: 2840 bytes --]

2018-08-30  Nathan Sidwell  <nathan@acm.org>

	PR c++/87137
	* stor-layout.c (place_field): Scan forwards to check last
	bitfield when ms_bitfield_placement is in effect.
	gcc/testsuite/
	* g++.dg/abi/pr87137.C: New.

Index: stor-layout.c
===================================================================
--- stor-layout.c	(revision 263956)
+++ stor-layout.c	(working copy)
@@ -1686,14 +1686,21 @@ place_field (record_layout_info rli, tre
     {
       rli->bitpos = size_binop (PLUS_EXPR, rli->bitpos, DECL_SIZE (field));
 
-      /* If we ended a bitfield before the full length of the type then
-	 pad the struct out to the full length of the last type.  */
-      if ((DECL_CHAIN (field) == NULL
-	   || TREE_CODE (DECL_CHAIN (field)) != FIELD_DECL)
-	  && DECL_BIT_FIELD_TYPE (field)
+      /* If FIELD is the last field and doesn't end at the full length
+	 of the type then pad the struct out to the full length of the
+	 last type.  */
+      if (DECL_BIT_FIELD_TYPE (field)
 	  && !integer_zerop (DECL_SIZE (field)))
-	rli->bitpos = size_binop (PLUS_EXPR, rli->bitpos,
-				  bitsize_int (rli->remaining_in_alignment));
+	{
+	  /* We have to scan, because non-field DECLS are also here.  */
+	  tree probe = field;
+	  while ((probe = DECL_CHAIN (probe)))
+	    if (TREE_CODE (probe) == FIELD_DECL)
+	      break;
+	  if (!probe)
+	    rli->bitpos = size_binop (PLUS_EXPR, rli->bitpos,
+				      bitsize_int (rli->remaining_in_alignment));
+	}
 
       normalize_rli (rli);
     }
Index: testsuite/g++.dg/abi/pr87137.C
===================================================================
--- testsuite/g++.dg/abi/pr87137.C	(nonexistent)
+++ testsuite/g++.dg/abi/pr87137.C	(working copy)
@@ -0,0 +1,40 @@
+// PR c++/87137
+
+// Empty macro args are undefined in C++ 98
+// { dg-do compule { target c++11 } }
+
+// We got confused by non-field decls separating bitfields when
+// ms_bitfield_layout was in effect.
+
+#if defined (__x86_64__) || defined (__i686__) || defined (__powerpc__)
+#define ATTRIB  __attribute__((ms_struct))
+#elif defined (__superh__)
+#define ATTRIB __attribute__((renesas))
+#else
+#define ATTRIB
+#endif
+
+#define DEFINE(NAME,BASE,THING)		\
+  struct ATTRIB NAME BASE {		\
+    unsigned one : 12;			\
+    THING				\
+    unsigned two : 4;			\
+  }
+
+template<unsigned I, unsigned J> struct Test;
+template<unsigned I> struct Test<I,I> {};
+
+#define TEST(NAME,BASE,THING)			\
+  DEFINE(NAME##_WITH,BASE,THING);		\
+  DEFINE(NAME##_WITHOUT,BASE,);			\
+  int NAME = sizeof (Test<sizeof(NAME##_WITH),sizeof (NAME##_WITHOUT)>)
+
+TEST(NSFun,,int fun (););
+TEST(SFun,,static int fun (););
+TEST(Tdef,,typedef int tdef;);
+
+TEST(Var,,static int var;);
+struct base { int f; };
+TEST(Use,:base,using base::f;);
+TEST(Tmpl,,template<unsigned> class X {};);
+TEST(TmplFN,,template<unsigned> void fu (););

[-- Attachment #3: changes-trunk.diff --]
[-- Type: text/x-patch, Size: 1001 bytes --]

Index: htdocs/gcc-9/changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/changes.html,v
retrieving revision 1.18
diff -r1.18 changes.html
211c211
< <!-- <h3 id="windows">Windows</h3> -->
---
> <h3 id="windows">Windows</h3>
212a213,227
> <ul>
>   <li>A C++ Microsoft ABI bitfield layout
>   bug, <a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87137">PR87137</a>
>   has been fixed.  A non-field declaration could cause the current
>   bitfield allocation unit to be completed, incorrectly placing a
>   following bitfield into a new allocation unit.  Microsoft ABI is
>   selected for:
>   <ul>
>     <li>Mingw targets
>     <li>PowerPC, IA-32 or x86-64 targets
>       when <code>-mms-bitfields</code> option is specified
>       or <code>__attribute__((ms_struct))</code> is used
>     <li>SuperH targets when the <code>-mhitachi</code> option is
>       specified, or <code>__attribute__((renesas))</code> is used
>   </ul>

Re: [PR c++/87137] GCC-8 Fix

[JonY]

[-- Attachment #1.1: Type: text/plain, Size: 1587 bytes --]

On 08/30/2018 11:59 AM, Nathan Sidwell wrote:
> On 08/29/2018 11:06 PM, Liu Hao wrote:
> 
>> It is strictly an ABI break but I doubt whether code in real world
>> that got broken by this bug ever exists. Usually when people expect
>> consecutive bitfields to be packed into a single word they wouldn't
>> put irrelevant declarations between them.
> 
> You're probably right.  I'm guessing this bug was found because:
> 
>    int bob : 1;
>    int get_bob () const {return bob;}
>    void set_bob (bool v) {bob=v;}
>    int bill : 1;
>    ...
> 
> might be a useful style.
> 
>> An important reason why such code could be rare is that the following
>> code compiles differently as C and C++:
> 
>> struct foo
>>    {
>>      unsigned a : 21;
>>      union x1 { char x; };
>>      unsigned b : 11;
>>      union x1 u;
>>    };
> 
> (for C++) this happens to be a case we already get right.  <aside> I
> think that'd be a C vendor extension, I don't think unnamed fields are
> permitted there?
> 
> Here's a version of the patch to completely resolve the issue, tested on
> trunk.  I noticed regular x86 targets support the ms_struct attribute,
> so we can give this wider testing.  I did consider trying to reorder the
> field decls before struct layour, but that seemed a more invasive change
> -- besides it might be nice to be able to remove the template-specific
> CLASSTYPE_DECL_LIST which almost but not quite mirrors TYPE_FIELDS.
> 
> Ok for trunk, ok for 8?
> 
> nathan

I don't have any objections.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PR c++/87137] GCC-8 Fix

[Richard Biener]

On Thu, Aug 30, 2018 at 5:09 PM JonY <10walls@gmail.com> wrote:
>
> On 08/30/2018 11:59 AM, Nathan Sidwell wrote:
> > On 08/29/2018 11:06 PM, Liu Hao wrote:
> >
> >> It is strictly an ABI break but I doubt whether code in real world
> >> that got broken by this bug ever exists. Usually when people expect
> >> consecutive bitfields to be packed into a single word they wouldn't
> >> put irrelevant declarations between them.
> >
> > You're probably right.  I'm guessing this bug was found because:
> >
> >    int bob : 1;
> >    int get_bob () const {return bob;}
> >    void set_bob (bool v) {bob=v;}
> >    int bill : 1;
> >    ...
> >
> > might be a useful style.
> >
> >> An important reason why such code could be rare is that the following
> >> code compiles differently as C and C++:
> >
> >> struct foo
> >>    {
> >>      unsigned a : 21;
> >>      union x1 { char x; };
> >>      unsigned b : 11;
> >>      union x1 u;
> >>    };
> >
> > (for C++) this happens to be a case we already get right.  <aside> I
> > think that'd be a C vendor extension, I don't think unnamed fields are
> > permitted there?
> >
> > Here's a version of the patch to completely resolve the issue, tested on
> > trunk.  I noticed regular x86 targets support the ms_struct attribute,
> > so we can give this wider testing.  I did consider trying to reorder the
> > field decls before struct layour, but that seemed a more invasive change
> > -- besides it might be nice to be able to remove the template-specific
> > CLASSTYPE_DECL_LIST which almost but not quite mirrors TYPE_FIELDS.
> >
> > Ok for trunk, ok for 8?
> >
> > nathan
>
> I don't have any objections.

Me neither if appropriately documented in changes.html.

Richard.

>

Re: [PR c++/87137] GCC-8 Fix

[Michael Matz]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

Hi,

On Thu, 30 Aug 2018, Nathan Sidwell wrote:

> > struct foo
> >    {
> >      unsigned a : 21;
> >      union x1 { char x; };
> >      unsigned b : 11;
> >      union x1 u;
> >    };
> 
> (for C++) this happens to be a case we already get right.  <aside> I think
> that'd be a C vendor extension, I don't think unnamed fields are permitted
> there?

Anonymous structures and unions are in C11 (and before that a widely 
accepted extension).


Ciao,
Michael.

Re: [PR c++/87137] GCC-8 Fix

[Jason Merrill]

On Fri, Aug 31, 2018 at 10:35 AM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Thu, 30 Aug 2018, Nathan Sidwell wrote:
>
>> > struct foo
>> >    {
>> >      unsigned a : 21;
>> >      union x1 { char x; };
>> >      unsigned b : 11;
>> >      union x1 u;
>> >    };
>>
>> (for C++) this happens to be a case we already get right.  <aside> I think
>> that'd be a C vendor extension, I don't think unnamed fields are permitted
>> there?
>
> Anonymous structures and unions are in C11 (and before that a widely
> accepted extension).

This isn't an anonymous union, it's named "x1".  I don't think that
line declares a field in C, either.

Jason

Re: [PR c++/87137] GCC-8 Fix

[Joseph Myers]

On Fri, 31 Aug 2018, Jason Merrill wrote:

> > Anonymous structures and unions are in C11 (and before that a widely
> > accepted extension).
> 
> This isn't an anonymous union, it's named "x1".  I don't think that
> line declares a field in C, either.

Indeed, the case where the field has a tag is one of the extensions from 
-fms-extensions / -fplan9-extensions, not part of the C11 anonymous struct 
/ union feature which requires a struct or union without a tag as the type 
of the unnamed field.

-- 
Joseph S. Myers
joseph@codesourcery.com