r206004 - [MS-ABI] Update virtual base padding rules to match MSVC 10+

Warren Hunt whunt at google.com
Thu Apr 10 18:07:58 PDT 2014


That's a good question Nico and one that we've asked.  It would probably be
helpful.  It's a bit of a corner case and one that, to my knowledge,
Microsoft provides no tools to help diagnose.  I just committed a document
known incompatibilities.  The better solution is probably just to
re-implement that code path but check fmsc-version first.  Presently we
don't support or claim to support VS2010 compatibility.

-Warren


On Thu, Apr 10, 2014 at 5:36 PM, Nico Weber <thakis at chromium.org> wrote:

> Should clang warn if this codepath is taken and -fmsc-version is VS2010?
>
>
> On Thu, Apr 10, 2014 at 5:14 PM, Warren Hunt <whunt at google.com> wrote:
>
>> Author: whunt
>> Date: Thu Apr 10 19:14:09 2014
>> New Revision: 206004
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=206004&view=rev
>> Log:
>> [MS-ABI] Update virtual base padding rules to match MSVC 10+
>> In version 9 (VS2010) (and prior)? versions of msvc, if the last field
>> in a record was a bitfield padding equal to the size of the storage
>> class of that bitfield was added before each vbase and vtordisp.  This
>> patch removes that feature from clang and updates the lit tests to
>> reflect it.
>>
>>
>> Modified:
>>     cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
>>     cfe/trunk/test/Layout/ms-x86-bitfields-vbases.cpp
>>     cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
>>
>> Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=206004&r1=206003&r2=206004&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
>> +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Apr 10 19:14:09 2014
>> @@ -2091,9 +2091,6 @@ static bool isMsLayout(const RecordDecl*
>>  //   one.
>>  // * The last zero size virtual base may be placed at the end of the
>> struct.
>>  //   and can potentially alias a zero sized type in the next struct.
>> -// * If the last field is a non-zero length bitfield, all virtual bases
>> will
>> -//   have extra padding added before them for no obvious reason.  The
>> padding
>> -//   has the same number of bits as the type of the bitfield.
>>  // * When laying out empty non-virtual bases, an extra byte of padding
>> is added
>>  //   if the non-virtual base before the empty non-virtual base has a
>> vbptr.
>>  // * The ABI attempts to avoid aliasing of zero sized bases by adding
>> padding
>> @@ -2595,10 +2592,6 @@ void MicrosoftRecordLayoutBuilder::layou
>>      const CXXRecordDecl *BaseDecl = I.getType()->getAsCXXRecordDecl();
>>      const ASTRecordLayout &BaseLayout =
>> Context.getASTRecordLayout(BaseDecl);
>>      bool HasVtordisp = HasVtordispSet.count(BaseDecl);
>> -    // If the last field we laid out was a non-zero length bitfield then
>> add
>> -    // some extra padding for no obvious reason.
>> -    if (LastFieldIsNonZeroWidthBitfield)
>> -      Size += CurrentBitfieldSize;
>>      // Insert padding between two bases if the left first one is zero
>> sized or
>>      // contains a zero sized subobject and the right is zero sized or
>> one leads
>>      // with a zero sized base.  The padding between virtual bases is 4
>>
>> Modified: cfe/trunk/test/Layout/ms-x86-bitfields-vbases.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-bitfields-vbases.cpp?rev=206004&r1=206003&r2=206004&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Layout/ms-x86-bitfields-vbases.cpp (original)
>> +++ cfe/trunk/test/Layout/ms-x86-bitfields-vbases.cpp Thu Apr 10 19:14:09
>> 2014
>> @@ -13,17 +13,17 @@ struct A : virtual B0 { char a : 1; };
>>  // CHECK-NEXT:    0 | struct A
>>  // CHECK-NEXT:    0 |   (A vbtable pointer)
>>  // CHECK-NEXT:    4 |   char a
>> -// CHECK-NEXT:   12 |   struct B0 (virtual base)
>> -// CHECK-NEXT:   12 |     int a
>> -// CHECK-NEXT:      | [sizeof=16, align=4
>> +// CHECK-NEXT:    8 |   struct B0 (virtual base)
>> +// CHECK-NEXT:    8 |     int a
>> +// CHECK-NEXT:      | [sizeof=12, align=4
>>  // CHECK-NEXT:      |  nvsize=8, nvalign=4]
>>  // CHECK-X64: *** Dumping AST Record Layout
>>  // CHECK-X64: *** Dumping AST Record Layout
>>  // CHECK-X64-NEXT:    0 | struct A
>>  // CHECK-X64-NEXT:    0 |   (A vbtable pointer)
>>  // CHECK-X64-NEXT:    8 |   char a
>> -// CHECK-X64-NEXT:   20 |   struct B0 (virtual base)
>> -// CHECK-X64-NEXT:   20 |     int a
>> +// CHECK-X64-NEXT:   16 |   struct B0 (virtual base)
>> +// CHECK-X64-NEXT:   16 |     int a
>>  // CHECK-X64-NEXT:      | [sizeof=24, align=8
>>  // CHECK-X64-NEXT:      |  nvsize=16, nvalign=8]
>>
>> @@ -33,16 +33,16 @@ struct B : virtual B0 { short a : 1; };
>>  // CHECK-NEXT:    0 | struct B
>>  // CHECK-NEXT:    0 |   (B vbtable pointer)
>>  // CHECK-NEXT:    4 |   short a
>> -// CHECK-NEXT:   12 |   struct B0 (virtual base)
>> -// CHECK-NEXT:   12 |     int a
>> -// CHECK-NEXT:      | [sizeof=16, align=4
>> +// CHECK-NEXT:    8 |   struct B0 (virtual base)
>> +// CHECK-NEXT:    8 |     int a
>> +// CHECK-NEXT:      | [sizeof=12, align=4
>>  // CHECK-NEXT:      |  nvsize=8, nvalign=4]
>>  // CHECK-X64: *** Dumping AST Record Layout
>>  // CHECK-X64-NEXT:    0 | struct B
>>  // CHECK-X64-NEXT:    0 |   (B vbtable pointer)
>>  // CHECK-X64-NEXT:    8 |   short a
>> -// CHECK-X64-NEXT:   20 |   struct B0 (virtual base)
>> -// CHECK-X64-NEXT:   20 |     int a
>> +// CHECK-X64-NEXT:   16 |   struct B0 (virtual base)
>> +// CHECK-X64-NEXT:   16 |     int a
>>  // CHECK-X64-NEXT:      | [sizeof=24, align=8
>>  // CHECK-X64-NEXT:      |  nvsize=16, nvalign=8]
>>
>> @@ -94,22 +94,22 @@ struct E : virtual B0, virtual B1 { long
>>  // CHECK-NEXT:    0 | struct E
>>  // CHECK-NEXT:    0 |   (E vbtable pointer)
>>  // CHECK-NEXT:    8 |   long long
>> -// CHECK-NEXT:   24 |   struct B0 (virtual base)
>> -// CHECK-NEXT:   24 |     int a
>> -// CHECK-NEXT:   36 |   struct B1 (virtual base)
>> -// CHECK-NEXT:   36 |     int a
>> -// CHECK-NEXT:      | [sizeof=40, align=8
>> +// CHECK-NEXT:   16 |   struct B0 (virtual base)
>> +// CHECK-NEXT:   16 |     int a
>> +// CHECK-NEXT:   20 |   struct B1 (virtual base)
>> +// CHECK-NEXT:   20 |     int a
>> +// CHECK-NEXT:      | [sizeof=24, align=8
>>  // CHECK-NEXT:      |  nvsize=16, nvalign=8]
>>  // CHECK-X64: *** Dumping AST Record Layout
>>  // CHECK-X64: *** Dumping AST Record Layout
>>  // CHECK-X64-NEXT:    0 | struct E
>>  // CHECK-X64-NEXT:    0 |   (E vbtable pointer)
>>  // CHECK-X64-NEXT:    8 |   long long
>> -// CHECK-X64-NEXT:   24 |   struct B0 (virtual base)
>> -// CHECK-X64-NEXT:   24 |     int a
>> -// CHECK-X64-NEXT:   36 |   struct B1 (virtual base)
>> -// CHECK-X64-NEXT:   36 |     int a
>> -// CHECK-X64-NEXT:      | [sizeof=40, align=8
>> +// CHECK-X64-NEXT:   16 |   struct B0 (virtual base)
>> +// CHECK-X64-NEXT:   16 |     int a
>> +// CHECK-X64-NEXT:   20 |   struct B1 (virtual base)
>> +// CHECK-X64-NEXT:   20 |     int a
>> +// CHECK-X64-NEXT:      | [sizeof=24, align=8
>>  // CHECK-X64-NEXT:      |  nvsize=16, nvalign=8]
>>
>>  int a[
>>
>> Modified: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=206004&r1=206003&r2=206004&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
>> +++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Thu Apr 10 19:14:09
>> 2014
>> @@ -473,7 +473,7 @@ struct RE {
>>  // CHECK-NEXT:    0 | struct RB2
>>  // CHECK-NEXT:    0 |   (RB2 vbtable pointer)
>>  // CHECK-NEXT: 1024 |   int b
>> -// CHECK-NEXT: 1032 |   struct RA (virtual base) (empty)
>> +// CHECK-NEXT: 1028 |   struct RA (virtual base) (empty)
>>  // CHECK-NEXT:      | [sizeof=1032, align=1024
>>  // CHECK-NEXT:      |  nvsize=1028, nvalign=1024]
>>  // CHECK: *** Dumping AST Record Layout
>> @@ -481,7 +481,7 @@ struct RE {
>>  // CHECK-NEXT:    0 |   (RB3 vftable pointer)
>>  // CHECK-NEXT: 1024 |   (RB3 vbtable pointer)
>>  // CHECK-NEXT: 2048 |   int b
>> -// CHECK-NEXT: 2056 |   struct RA (virtual base) (empty)
>> +// CHECK-NEXT: 2052 |   struct RA (virtual base) (empty)
>>  // CHECK-NEXT:      | [sizeof=2056, align=1024
>>  // CHECK-NEXT:      |  nvsize=2052, nvalign=1024]
>>  // CHECK: *** Dumping AST Record Layout
>> @@ -516,7 +516,7 @@ struct RE {
>>  // CHECK-X64-NEXT:    0 | struct RB2
>>  // CHECK-X64-NEXT:    0 |   (RB2 vbtable pointer)
>>  // CHECK-X64-NEXT: 1024 |   int b
>> -// CHECK-X64-NEXT: 1032 |   struct RA (virtual base) (empty)
>> +// CHECK-X64-NEXT: 1028 |   struct RA (virtual base) (empty)
>>  // CHECK-X64-NEXT:      | [sizeof=1032, align=1024
>>  // CHECK-X64-NEXT:      |  nvsize=1028, nvalign=1024]
>>  // CHECK-X64: *** Dumping AST Record Layout
>> @@ -524,7 +524,7 @@ struct RE {
>>  // CHECK-X64-NEXT:    0 |   (RB3 vftable pointer)
>>  // CHECK-X64-NEXT: 1024 |   (RB3 vbtable pointer)
>>  // CHECK-X64-NEXT: 2048 |   int b
>> -// CHECK-X64-NEXT: 2056 |   struct RA (virtual base) (empty)
>> +// CHECK-X64-NEXT: 2052 |   struct RA (virtual base) (empty)
>>  // CHECK-X64-NEXT:      | [sizeof=2056, align=1024
>>  // CHECK-X64-NEXT:      |  nvsize=2052, nvalign=1024]
>>  // CHECK-X64: *** Dumping AST Record Layout
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140410/cc66d53c/attachment.html>


More information about the cfe-commits mailing list