Can you add some new test cases testing this particular behaviour?<br><br><div class="gmail_quote">On Thu, Aug 30, 2012 at 2:46 PM, Don Williamson <span dir="ltr"><<a href="mailto:don.williamson@yahoo.com" target="_blank">don.williamson@yahoo.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div style="font-size:10pt;font-family:tahoma,new york,times,serif"><div style="font-family:tahoma,'new york',times,serif;font-size:10pt">
<span>So, the problem is that the same approach needs to be applied to the vftable pointer, as well as the vbtable pointer.</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif">
<span><br></span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif">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)</div>
<div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif"><br></div><div style="background-color:transparent"><div style="background-color:transparent"><font>Index: lib/AST/RecordLayoutBuilder.cpp</font></div>
<div style="background-color:transparent"><font>===================================================================</font></div><div style="background-color:transparent"><font>--- lib/AST/RecordLayoutBuilder.cpp (revision 162839)</font></div>
<div style="background-color:transparent"><font>+++ lib/AST/RecordLayoutBuilder.cpp (working copy)</font></div><div style="background-color:transparent"><font>@@ -1009,7 +1009,7 @@</font></div><div style="background-color:transparent">
<font><br></font></div><div style="background-color:transparent"><font> // Compute base subobject info.</font></div><div style="background-color:transparent"><font> ComputeBaseSubobjectInfo(RD);</font></div><div style="background-color:transparent">
<font>-</font></div><div style="background-color:transparent"><font>+</font></div><div style="background-color:transparent"><font> // If we have a primary base class, lay it out.</font></div><div style="background-color:transparent">
<font> if (PrimaryBase) {</font></div><div style="background-color:transparent"><font> if (PrimaryBaseIsVirtual) {</font></div><div style="background-color:transparent"><font>@@ -1044,8 +1044,13 @@</font></div><div style="background-color:transparent">
<font> CharUnits PtrAlign =</font></div><div style="background-color:transparent"><font> Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerAlign(0));</font></div><div style="background-color:transparent">
<font> EnsureVTablePointerAlignment(PtrAlign);</font></div><div style="background-color:transparent"><font>- if (isMicrosoftCXXABI())</font></div><div style="background-color:transparent"><font>+ if (isMicrosoftCXXABI()) {</font></div>
<div style="background-color:transparent"><font> VFPtrOffset = getSize();</font></div><div style="background-color:transparent"><font>+</font></div><div style="background-color:transparent"><font>+ // MSVC will extend the width of the vftable pointer to the max width</font></div>
<div style="background-color:transparent"><font>+ // of all fields in the record</font></div><div style="background-color:transparent"><font>+ PtrWidth = std::max(Alignment,
PtrWidth);</font></div><div style="background-color:transparent"><font>+ }</font></div><div style="background-color:transparent"><font> setSize(getSize() + PtrWidth);</font></div><div style="background-color:transparent">
<font> setDataSize(getSize());</font></div><div style="background-color:transparent"><font> }</font></div><div style="background-color:transparent"><font>@@ -2222,6 +2227,16 @@</font></div><div style="background-color:transparent">
<font> Builder.Layout(RD);</font></div><div style="background-color:transparent"><font> }</font></div><div style="background-color:transparent"><font><br></font></div><div style="background-color:transparent"><font>+ // The same is true in MSVC when there is a vftable
pointer</font></div><div style="background-color:transparent"><font>+ CharUnits PtrWidth =</font></div><div style="background-color:transparent"><font>+ toCharUnitsFromBits(getTargetInfo().getPointerWidth(0));</font></div>
<div style="background-color:transparent"><font>+ if (Builder.isMicrosoftCXXABI() &&</font></div><div style="background-color:transparent"><font>+ Builder.VFPtrOffset != CharUnits::fromQuantity(-1) &&</font></div>
<div style="background-color:transparent"><font>+ Builder.NonVirtualAlignment.getQuantity() > PtrWidth.getQuantity()) {</font></div><div style="background-color:transparent"><font>+ Builder.resetWithTargetAlignment(Builder.NonVirtualAlignment);</font></div>
<div style="background-color:transparent"><font>+
Builder.Layout(RD);</font></div><div style="background-color:transparent"><font>+ }</font></div><div style="background-color:transparent"><font>+</font></div><div style="background-color:transparent"><font> // FIXME: This is not always correct. See the part about bitfields at</font></div>
<div style="background-color:transparent"><font> // <a href="http://www.codesourcery.com/public/cxx-abi/abi.html#POD" target="_blank">http://www.codesourcery.com/public/cxx-abi/abi.html#POD</a> for more info.</font></div>
<div style="background-color:transparent"><font> // FIXME: IsPODForThePurposeOfLayout should be stored in the record layout.</font></div></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif">
<br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif"><br></div><div style="font-family:tahoma,'new york',times,serif;font-size:10pt">
<br></div> <div style="font-family:tahoma,'new york',times,serif;font-size:10pt"> <div style="font-family:'times new roman','new york',times,serif;font-size:12pt"> <div dir="ltr"> <font face="Arial"><div class="im">
<hr size="1"> <b><span style="font-weight:bold">From:</span></b> Don Williamson <<a href="mailto:don.williamson@yahoo.com" target="_blank">don.williamson@yahoo.com</a>><br> <b><span style="font-weight:bold">To:</span></b> cfe-dev <<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a>> <br>
</div><b><span style="font-weight:bold">Sent:</span></b> Thursday, August 30, 2012 12:07 PM<br> <b><span style="font-weight:bold">Subject:</span></b> Re: [cfe-dev] double/virtual offsets for MSVC broken in 3.1<br> </font> </div>
<div><div class="h5"> <br>
<div><div><div style="font-size:10pt;font-family:arial,helvetica,sans-serif"><div style="font-size:10pt;font-family:arial,helvetica,sans-serif"><span>The change appears to stem from:</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<span><br></span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><span><span style="white-space:pre-wrap"> </span></span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?r1=143800&r2=144072&diff_format=h" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?r1=143800&r2=144072&diff_format=h</a></div>
<div style="background-color:transparent"><span style="font-family:tahoma,'new york',times,serif;font-size:13px"><span style="white-space:pre-wrap"> </span><span>Revision </span><span style="color:rgb(136,0,136)"><a rel="nofollow" href="http://llvm.org/viewvc/llvm-project?view=rev&revision=144072" style="color:rgb(136,0,136)" target="_blank">144072</a>, </span>Modified Mon Nov 7 22:01:03 2011 CST (9 months, 3 weeks ago) by rjmccall<span> </span></span></div>
<div style="background-color:transparent"><span style="font-family:tahoma,'new york',times,serif;font-size:13px"><span><span style="white-space:pre-wrap"> </span></span><span style="white-space:pre-wrap">Fix the layout of vb-tables and vf-tables in the MS C++ ABI. </span></span><span style="font-family:tahoma,'new york',times,serif;font-size:13px;white-space:pre-wrap;background-color:transparent">Based on work by Dmitry
Sokolov!</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif"><span style="font-family:tahoma,'new york',times,serif;font-size:13px;white-space:pre-wrap;background-color:transparent"><br>
</span></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif"><span style="white-space:pre-wrap">Trying to figure out what's different...</span></div>
<div style="font-style:normal;font-size:13px;background-color:transparent;font-family:tahoma,'new york',times,serif"><span style="font-family:tahoma,'new york',times,serif;font-size:13px;white-space:pre-wrap;background-color:transparent"><br>
</span></div><div style="font-size:10pt;font-family:arial,helvetica,sans-serif"><br></div> <div style="font-size:10pt;font-family:arial,helvetica,sans-serif"> <div style="font-family:'times new roman','new york',times,serif;font-size:12pt">
<div dir="ltr"> <font face="Arial"> <hr size="1"> <b><span style="font-weight:bold">From:</span></b> Don Williamson <<a href="mailto:don.williamson@yahoo.com" target="_blank">don.williamson@yahoo.com</a>><br> <b><span style="font-weight:bold">To:</span></b> cfe-dev <<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a>> <br>
<b><span style="font-weight:bold">Sent:</span></b> Thursday, August 30, 2012 11:26 AM<br> <b><span style="font-weight:bold">Subject:</span></b> [cfe-dev] double/virtual offsets for MSVC broken in 3.1<br> </font> </div> <br>
<div><div><div style="font-size:10pt;font-family:arial,helvetica,sans-serif"><div style="font-family:arial,helvetica,sans-serif;font-size:10pt">Hi,</div><div style="font-family:arial,helvetica,sans-serif;font-size:10pt">
<br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">last year I submitted a patch (commit by somebody else) to solve cases like this:</div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<br></div><div style="background-color:transparent"><div style="background-color:transparent"><font face="arial, helvetica, sans-serif"><span style="white-space:pre-wrap"> </span>// The addition of a double anywhere in
this struct forces the vtable ptr to occupy 4 bytes
+ 4 bytes padding in MSVC</font></div><div style="background-color:transparent"><font face="arial, helvetica, sans-serif"><span style="white-space:pre-wrap"> </span>struct DoubleInPolymorphicStruct</font></div><div style="background-color:transparent">
<font face="arial, helvetica, sans-serif"><span style="white-space:pre-wrap"> </span>{</font></div><div style="background-color:transparent"><font face="arial, helvetica, sans-serif"><span style="white-space:pre-wrap"> </span>virtual void Empty() { }</font></div>
<div style="background-color:transparent"><font face="arial, helvetica, sans-serif"><span style="white-space:pre-wrap"> </span>int a;</font></div><div style="background-color:transparent"><font face="arial, helvetica, sans-serif"><span style="white-space:pre-wrap"> </span>double b;</font></div>
<div style="background-color:transparent"><span style="font-family:arial,helvetica,sans-serif;font-size:13px;background-color:transparent;white-space:pre-wrap"> </span><span style="font-family:arial,helvetica,sans-serif;font-size:small;background-color:transparent">};</span><br>
</div><div style="font-style:normal;font-size:13px;font-family:arial,helvetica,sans-serif"><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">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:</div>
<div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<span style="white-space:pre-wrap"> </span><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016944.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016944.html</a></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<span style="white-space:pre-wrap"> </span><a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016970.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016970.html</a></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
<br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">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.</div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><br></div>
<div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">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.</div>
<div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
Meanwhile, could anybody help with any ideas as to what may have changed?</div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif"><br></div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">
Thanks!</div><div style="font-style:normal;font-size:13px;background-color:transparent;font-family:arial,helvetica,sans-serif">- Don</div></div></div></div></div><br>_______________________________________________<br>cfe-dev mailing list<br>
<a rel="nofollow" href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br><br> </div> </div> </div></div></div><br>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br><br> </div></div></div> </div> </div></div><br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>João Matos<br>