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
2018-08-29 Nathan Sidwell <nat...@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
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 & 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.
>
