[cfe-dev] double/virtual offsets for MSVC broken in 3.1

[Nikola Smiljanic]

You should have Python installed and GnuWin32 in your path. After that
you just build clang-test project inside Visual Studio and it runs the
test suite.

On Thu, Aug 30, 2012 at 6:04 PM, Don Williamson
<don.williamson at yahoo.com> wrote:
>
> I've got to figure out how to write/run tests before I can do that but I'll
> give it a shot.
>
> Cheers,
> - Don
>
> ________________________________
> From: João Matos <ripzonetriton at gmail.com>
> To: Don Williamson <don.williamson at yahoo.com>
> Cc: cfe-dev <cfe-dev at cs.uiuc.edu>
> Sent: Thursday, August 30, 2012 2:53 PM
>
> Subject: Re: [cfe-dev] double/virtual offsets for MSVC broken in 3.1
>
> Can you add some new test cases testing this particular behaviour?
>
> On Thu, Aug 30, 2012 at 2:46 PM, Don Williamson <don.williamson at yahoo.com>
> wrote:
>
> So, the problem is that the same approach needs to be applied to the vftable
> pointer, as well as the vbtable pointer.
>
> I have a small diff of changes that pass all my tests... is this good enough
> to submit? (I'm working against 3.1 but the same is true of the trunk and I
> can sync to that to submit a patch)
>
> Index: lib/AST/RecordLayoutBuilder.cpp
> ===================================================================
> --- lib/AST/RecordLayoutBuilder.cpp     (revision 162839)
> +++ lib/AST/RecordLayoutBuilder.cpp     (working copy)
> @@ -1009,7 +1009,7 @@
>
>    // Compute base subobject info.
>    ComputeBaseSubobjectInfo(RD);
> -
> +
>    // If we have a primary base class, lay it out.
>    if (PrimaryBase) {
>      if (PrimaryBaseIsVirtual) {
> @@ -1044,8 +1044,13 @@
>      CharUnits PtrAlign =
>
> Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerAlign(0));
>      EnsureVTablePointerAlignment(PtrAlign);
> -    if (isMicrosoftCXXABI())
> +    if (isMicrosoftCXXABI()) {
>        VFPtrOffset = getSize();
> +
> +      // MSVC will extend the width of the vftable pointer to the max width
> +         // of all fields in the record
> +      PtrWidth = std::max(Alignment, PtrWidth);
> +    }
>      setSize(getSize() + PtrWidth);
>      setDataSize(getSize());
>    }
> @@ -2222,6 +2227,16 @@
>        Builder.Layout(RD);
>      }
>
> +       // The same is true in MSVC when there is a vftable pointer
> +    CharUnits PtrWidth =
> +      toCharUnitsFromBits(getTargetInfo().getPointerWidth(0));
> +    if (Builder.isMicrosoftCXXABI() &&
> +        Builder.VFPtrOffset != CharUnits::fromQuantity(-1) &&
> +        Builder.NonVirtualAlignment.getQuantity() > PtrWidth.getQuantity())
> {
> +      Builder.resetWithTargetAlignment(Builder.NonVirtualAlignment);
> +      Builder.Layout(RD);
> +    }
> +
>      // FIXME: This is not always correct. See the part about bitfields at
>      // http://www.codesourcery.com/public/cxx-abi/abi.html#POD for more
> info.
>      // FIXME: IsPODForThePurposeOfLayout should be stored in the record
> layout.
>
>
>
> ________________________________
> From: Don Williamson <don.williamson at yahoo.com>
> To: cfe-dev <cfe-dev at cs.uiuc.edu>
> Sent: Thursday, August 30, 2012 12:07 PM
> Subject: Re: [cfe-dev] double/virtual offsets for MSVC broken in 3.1
>
> The change appears to stem from:
>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?r1=143800&r2=144072&diff_format=h
> Revision 144072, Modified Mon Nov 7 22:01:03 2011 CST (9 months, 3 weeks
> ago) by rjmccall
> Fix the layout of vb-tables and vf-tables in the MS C++ ABI. Based on work
> by Dmitry Sokolov!
>
> Trying to figure out what's different...
>
>
> ________________________________
> From: Don Williamson <don.williamson at yahoo.com>
> To: cfe-dev <cfe-dev at cs.uiuc.edu>
> Sent: Thursday, August 30, 2012 11:26 AM
> Subject: [cfe-dev] double/virtual offsets for MSVC broken in 3.1
>
> Hi,
>
> last year I submitted a patch (commit by somebody else) to solve cases like
> this:
>
> // The addition of a double anywhere in this struct forces the vtable ptr to
> occupy 4 bytes + 4 bytes padding in MSVC
> struct DoubleInPolymorphicStruct
> {
> virtual void Empty() { }
> int a;
> double b;
> };
>
> The offset of a in MSVC is 8 bytes, but clang in MSVC offset calculation
> mode puts it at 4. The initial posts are here:
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016944.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016970.html
>
> These changes did not make it into 3.0 but as far as I could tell, did make
> it into the trunk. However, I've just sync'ed up to the 3.1 release and this
> is still a problem.
>
> I'm about to go back and take a look and what's broken but it'll take me a
> couple of hours to settle back into the code again.
>
> Meanwhile, could anybody help with any ideas as to what may have changed?
>
> Thanks!
> - Don
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
>
> --
> João Matos
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>