[cfe-dev] Implementation of the CGRecordLayoutBuilder for Microsoft ABI.

[r4start]

On 26/09/2011 11:48, Eli Friedman wrote:
> On Mon, Sep 26, 2011 at 12:34 AM, r4start<r4start at gmail.com>  wrote:
>> On 26/09/2011 11:18, Eli Friedman wrote:
>>> On Sun, Sep 25, 2011 at 11:55 PM, r4start<r4start at gmail.com>    wrote:
>>>> On 23/09/2011 22:44, John McCall wrote:
>>>>> On Sep 23, 2011, at 5:30 AM, r4start wrote:
>>>>>> I am writing CGRecordLayoutBuilder for Microsoft ABI and I have a
>>>>>> problem.
>>>>>> I have the following code for testing http://pastebin.com/27U2hcDC .
>>>>>>
>>>>>> When generated layout for a class C, I have break assert in
>>>>>> CGRecordLayoutBuilder.cpp line 968
>>>>>> assert(TypeSizeInBits == getTargetData().getTypeAllocSizeInBits(Ty)&&
>>>>>>            "Type size mismatch!");.
>>>>>> During debugging, I discovered that TypeSizeInBits smaller
>>>>>> getTargetData().getTypeAllocSizeInBits(Ty)  4 bytes.
>>>>>> After investigation, I discovered that the alignment is added in
>>>>>> StructLayout constructor (TargetData.cpp line 73)
>>>>>> // Add padding to the end of the struct so that it could be put in an
>>>>>> array
>>>>>>    // and all array elements would be aligned correctly.
>>>>>>     if ((StructSize&      (StructAlignment-1)) != 0)
>>>>>>       StructSize = TargetData::RoundUpAlignment(StructSize,
>>>>>> StructAlignment);
>>>>>>
>>>>>> But this alignment is not necessary for class C.
>>>>>> MSVC doesn't add to the end of the class alignment, if it has a virtual
>>>>>> base classes.
>>>>>> I see two solutions to this problem:
>>>>>>    1. Rewrite assert like this
>>>>>>        (pseudocode)
>>>>>>        if ABI == Microsoft&&      Class has virtual basses
>>>>>>          assert(TypeSizeInBits ==
>>>>>> (getTargetData().getTypeAllocSizeInBits(Ty) - 32)&&
>>>>>>                       "Type size mismatch!");
>>>>>>         else
>>>>>>           old version;
>>>>>>     2. Provide to StructLyout information about ABI.
>>>>>>
>>>>>> I think the first way is more simple.
>>>>> No.  A *lot* of downstream code will be broken if LLVM doesn't lay
>>>>> out the IR type the way we think it's laid out, and that includes the
>>>>> presence or absence of tail padding.
>>>>>
>>>>> I don't see how this is possible, though.  Can you run through an
>>>>> example?  What's the size and layout of class B here?
>>>>>
>>>>> class A {
>>>>>     char c;
>>>>> };
>>>>>
>>>>> class B : public virtual A {
>>>>>     void *p;
>>>>> };
>>>>>
>>>>> I would expect that sizeof(B) is 12, and that it's laid out like this:
>>>>>     [0-3] virtual base pointer
>>>>>     [4-7] B.p
>>>>>       [8] A.c
>>>>>    [9-11] tail padding
>>>> MSVS with pragma pack 8 generates such layout:
>>>> 1>      class A    size(1):
>>>> 1>          +---
>>>> 1>      0  | c
>>>> 1>          +---
>>>>
>>>> 1>      class B    size(9):
>>>> 1>          +---
>>>> 1>      0  | {vbptr}
>>>> 1>      4  | p
>>>> 1>          +---
>>>> 1>          +--- (virtual base A)
>>>> 1>      8  | c
>>>> 1>          +---
>>>>
>>>> My code generates same layout (i32* in B is vbtable pointer):
>>>> *** Dumping AST Record Layout
>>>>     0 | class A
>>>>     0 |   char c
>>>>    sizeof=1, dsize=1, align=1
>>>>    nvsize=1, nvalign=1
>>>>
>>>> LLVMType:%class.A = type { i8 }
>>>> NonVirtualBaseLLVMType:%class.A = type { i8 }
>>>> IsZeroInitializable:1
>>>>
>>>> *** Dumping AST Record Layout
>>>>     0 | class B
>>>>     0 |   (B vtable pointer)
>>>>     0 |   (B vbtable pointer)
>>>>     4 |   void * p
>>>>     8 |   class A (virtual base)
>>>>     8 |     char c
>>>>    sizeof=9, dsize=9, align=4
>>>>    nvsize=8, nvalign=4
>>> That's can't be what MSVC actually does: the size of B must be a
>>> multiple of its alignment.
>> I check this layout twice. I get this layout from MSVC compiler output. And
>> I check it in a memory while debugging.
>>>   If you're representation of what MSVC
>>> computes is accurate, you must be miscomputing the alignment.
>> Can you clarify?
> What is the value of (long)(((B*)0)+1)?  From what you are saying, it
> should be 9; in that case, the "align=4" from your dump is not
> correct.
>
> -Eli
I suppose that align must be 8, am I right?

- Dmitry.