VS 2013 Miscompiles
Aaron Ballman
aaron at aaronballman.com
Wed Nov 6 09:09:30 PST 2013
Ping?
On Fri, Nov 1, 2013 at 5:41 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Fri, Nov 1, 2013 at 4:15 PM, Reid Kleckner <rnk at google.com> wrote:
>> Thanks for tackling and reducing this! This was reported in July, and it's
>> a shame we didn't report it to them before they went from preview to
>> release: http://llvm.org/bugs/show_bug.cgi?id=16606
>
> Agreed!
>
> And from what I can tell, the patch I provided is probably the best
> way for us to work around the issue for now; the alternative is to
> remove the const version of the method from the base class. The good
> news is, this only seems to affect situations involving this
> particular pattern, which doesn't appear to be very common in the
> source base, so we likely are not hitting other miscompiles.
>
> ~Aaron
>
>>
>>
>> On Fri, Nov 1, 2013 at 12:16 PM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>>>
>>> I have this as narrowed down as I think I can get it, and I've
>>> reported it to Microsoft Connect.
>>>
>>>
>>> https://connect.microsoft.com/VisualStudio/feedback/details/807479/miscompile-involving-using-declarations-and-multiple-inheritance
>>>
>>> The miscompile seems to happen as a result of multiple inheritance
>>> (the thunk is missing in 2013 but there in 2012), and seems to have
>>> something to do with the using declaration importing two methods. At
>>> least, from what I can tell, when I remove the const version of the
>>> method, the thunk comes back in 2013.
>>>
>>> ~Aaron
>>>
>>> On Thu, Oct 31, 2013 at 5:16 PM, Aaron Ballman <aaron at aaronballman.com>
>>> wrote:
>>> > While updating clang to VS 2013, I've seen numerous miscompiles on x86
>>> > that did not appear in VS 2012 related to the Redeclarable class. It
>>> > seems that VS 2012 would properly calculate the offset to the base
>>> > class, while VS 2013 would not. Specifically, VS 2012 would generate
>>> > the following code:
>>> >
>>> > RecordDecl *getPreviousDecl() {
>>> > 025FF040 push ebp
>>> > 025FF041 mov ebp,esp
>>> > 025FF043 push ecx
>>> > 025FF044 mov dword ptr [this],0CCCCCCCCh
>>> > 025FF04B mov dword ptr [this],ecx
>>> > return cast_or_null<RecordDecl>(TagDecl::getPreviousDecl());
>>> > 025FF04E mov ecx,dword ptr [this]
>>> > 025FF051 add ecx,34h
>>> > 025FF054 call
>>> > clang::Redeclarable<clang::TagDecl>::getPreviousDecl (025FEF40h)
>>> > 025FF059 push eax
>>> > 025FF05A call
>>> > llvm::cast_or_null<clang::RecordDecl,clang::TagDecl> (025A9850h)
>>> > 025FF05F add esp,4
>>> > }
>>> >
>>> > VS 2013, however, is missing the add ecx, 34h instruction:
>>> >
>>> > RecordDecl *getPreviousDecl() {
>>> > 02500010 push ebp
>>> > 02500011 mov ebp,esp
>>> > 02500013 push ecx
>>> > 02500014 mov dword ptr [this],0CCCCCCCCh
>>> > 0250001B mov dword ptr [this],ecx
>>> > return cast_or_null<RecordDecl>(TagDecl::getPreviousDecl());
>>> > 0250001E mov ecx,dword ptr [this]
>>> > 02500021 call
>>> > clang::Redeclarable<clang::TagDecl>::getPreviousDecl (024FFF10h)
>>> > 02500026 push eax
>>> > 02500027 call
>>> > llvm::cast_or_null<clang::RecordDecl,clang::TagDecl> (024AA9B0h)
>>> > 0250002C add esp,4
>>> >
>>> > This would cause crashes in getPreviousDecl. Unfortunately, this also
>>> > affects other calls on Redeclarable such as getMostRecentDecl.
>>> >
>>> > This patch "addresses" the miscompile such that it now compiles
>>> > properly for VS 2013 as well as VS 2012. Instead of calling through a
>>> > scoped lookup on a method coming from Redeclarable, it statically
>>> > casts the this pointer and calls the method. This seems to force
>>> > Visual Studio to generate the proper code.
>>> >
>>> > ~Aaron
>>
>>
More information about the cfe-commits
mailing list