[cfe-dev] CGRecordLayoutBuilder for MS ABI

[John McCall]

On Oct 28, 2011, at 3:18 AM, Дмитрий Соколов wrote:
> 2011/10/28 John McCall <rjmccall at apple.com>
> On Oct 28, 2011, at 1:13 AM, r4start wrote:
> > On 27/10/2011 23:31, John McCall wrote:
> >> +  } else if (
> >> +    Types.getContext().getTargetInfo().getCXXABI() == CXXABI_Microsoft) {
> >> +    // In MS mode this code helps insert padding bytes before fields,
> >> +    // like MSVC does.
> >> +    CharUnits padding = fieldOffset - NextFieldOffset;
> >> +
> >> +    AppendBytes(padding);
> >>
> >> This still shouldn't be necessary.  This routine just adds padding bytes
> >> to get us up to fieldOffset, which is determined by the ASTRecordLayout.
> >> If the alignment of the next IR type is high enough, we don't need explicit
> >> padding bytes, which is what the 'if' condition above this is testing.  This
> >> isn't any different in the MS ABI.
> >>
> >> That said, it does look like there's a bug here, because we *always* need
> >> to add padding bytes if we're building a packed layout, and that can
> >> happen in MSVC;  see above.  So I think the condition should be:
> >>
> >>   if (alignedNextFieldOffset<  fieldOffset ||
> >>       (Packed&&  fieldOffset != NextFieldOffset)) {
> > This condition doesn't work for class I from test.
> > We have such layout
> > struct I
> >  0 |   (I vftable pointer)
> >  8 |   (I vbtable pointer)
> > 16 |   double q
> > 24 |   class D (virtual base)
> > 24 |     (D vftable pointer)
> > 32 |     double a
> > and layout must be %struct.I = type { i32 (...)**, i32*, [4 x i8], double, %class.D }
> > But we will have %struct.I = type { i32 (...)**, i32*, double, %class.D } and Packed = false.
> 
> Neither of these types is correct;  both incorrectly place the
> vbtbl at offset 4.  It's just that the first type happens to create
> some extra padding in the wrong place, and that makes the
> total struct size right.
> 
> The type should be this:
>  type { i32 (...)**, [4 x i8], i32*, double, %class.D }
> 
> If you place the vbtbl at the right offset, you should find that
> everything else falls out correctly.
> 
> As you can see in my patch I place vbptr on offset from RecordLayout.
> So I expect that LLVM add padding between them.

Well, somehow your patch gives us the wrong type, so I imagine that
some invariant is being broken.  LLVM definitely will not add padding
between the first two elements in that type.

> If we want show layout like MSVC then type must be
> type { i32 (...)**, [4 x i8], i32*, [4 x i8], double, %class.D } .
> But MSVC never shows padding between vfptr and vbptr.

I don't really care about how we print out layouts for debugging;  I just
want all the data to have the right offset in the IR struct type we make.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20111028/fcc4fea6/attachment.html>