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

[Eli Friedman]

On Mon, Sep 26, 2011 at 12:59 AM, r4start <r4start at gmail.com> wrote:
> 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;
>
> Yes, that right.
>>
>>  in that case, the "align=4" from your dump is not
>> correct.
>>
> How I can modify align from command line options?

Err, what?  The align=4 is getting computed by the RecordLayoutBuilder.

-Eli