VS 2013 Miscompiles

[Reid Kleckner]

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


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131101/908d549d/attachment.html>