Different class layouts between clang and MSVC

Hi,

I am synced to 134398 as of 2011-07-05, 00:09:02+0100.

I'm using the clang frontend to calculate class layouts which I'm hoping are identical to MSVC. Before I go any further, I suppose the first questions are:

* Is the goal of MSRecordLayoutBuilder to be as identical to MSVC layout as is knowable?
* Has that goal been achieved or are their known defects?

My test case is the following code:

#pragma pack(push, 8)
struct BaseStruct
{
// BaseStruct() { }
double v0;
float v1;
};

struct DerivedStruct : public BaseStruct
{
int x;
};
#pragma pack(pop)

With the constructor above commented out, I get the following:

sizeof(BaseStruct): 16
clang size: 16
sizeof(DerivedStruct): 24
clang size: 24
offsetof(DerivedStruct::x): 16
clang offset: 16

Which all matches up. However, with the constructor restored, I get:

sizeof(BaseStruct): 16
clang size: 16
sizeof(DerivedStruct): 24
clang size: 16
offsetof(DerivedStruct::x): 16
clang offset: 12

clang seems to be changing its behaviour based on the addition of the constructor. I'm currently tracing through the code trying to figure out how it all works in the hope of diagnosing the issue. Does anybody have any clues as to what's going on that could help me?

Thanks,
- Don

Some further points:

* LangOptions::RTTI = 0
* LangOptions::Microsoft = 1
* TargetOptions::CXXABI = "microsoft"

After stepping it appears the problem is related to the NonVirtualSize field of CXXRecordLayoutInfo in ASTRecordLayout:

Without constructor: 16
With constructor: 12

This is calculated in ASTContext::getASTRecordLayout with the following lines:

// FIXME: This should be done in FinalizeLayout\.
CharUnits DataSize =
  IsPODForThePurposeOfLayout ? Builder\->getSize\(\) : Builder\->getDataSize\(\);
CharUnits NonVirtualSize = 
  IsPODForThePurposeOfLayout ? DataSize : Builder\->NonVirtualSize;

The inputs are:

Without constructor, DataSize: 16, Builder->NonVirtualSize: 12, IsPODForThePurposeOfLayout: true
With constructor, DataSize: 12, Builder->NonVirtualSize: 12, IsPODForThePurposeOfLayout: false

In both cases, Builder Size and DataSize are 128 and 96, respectively, so it looks like the difference is triggered by IsPODForThePurposeOfLayout.

The change that modified the code is: http://llvm.org/viewvc/llvm-project?view=rev&revision=77352

From my simple understanding of this, Size is different to DataSize (in this limited test) due to being aligned upwards to the alignment of the double field in FinishLayout. What's the reasoning behind making POD types use an aligned value vs. an unaligned value for NonVirtualSize? Can this behaviour be changed based on ABI?

Thanks,
- Don

One other failure has turned up in my testing:

struct G
{
virtual ~G() { }
int a;
// double b;
};

As it stands, the offset of a is 4 with both compilers. When any double-parameters are introduced (uncomment b above), MSVC places a at 8 and clang keeps it at 4 - MSVC is padding up from the vtable ptr to a. It obviously then has to shift b up and results in an object that's 8 bytes larger.

Is there any use/interest in having clang and MSVC match up? Obviously it's of interest to me, but it'd be nice to know the bigger picture :slight_smile:

Thanks,
- Don

One other failure has turned up in my testing:

struct G
{
virtual ~G() { }
int a;
// double b;
};

As it stands, the offset of a is 4 with both compilers. When any double-parameters are introduced (uncomment b above), MSVC places a at 8 and clang keeps it at 4 - MSVC is padding up from the vtable ptr to a. It obviously then has to shift b up and results in an object that's 8 bytes larger.

Bizarre.

Is there any use/interest in having clang and MSVC match up? Obviously it's of interest to me, but it'd be nice to know the bigger picture :slight_smile:

Our intent when compiling for the MS ABI is to exactly match MSVC's layout. Obviously, that's still a work in progress. :slight_smile:

If you're interested in working on this, that would be great; I don't know of anyone else currently doing it. You seem to have done a good job debugging them so far.

At the very least, please file bugs as you find these problems; it's easy for things to get lost when they're just on the mailing list.

John.

Sorry, my reply-all skills are lacking!

The problem with this is that the record builder doesn’t know the value of Alignment until its done a complete pass on the layout - the double field might be right at the end of the struct.

Here are the modifications I’ve made that have allowed me to keep working - it passes all my tests, at least. I suppose we need to work on what’s required to get this to patch-quality level :slight_smile:

It will take me a while to get this up to quality as I’m self-employed currently and really need to spend time on my little business. Does this discussion need to be take to the commit list?

Index: RecordLayoutBuilder.cpp