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

[Don Williamson]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120830/9ab7174a/attachment.html>