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

[Eli Friedman]

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