VS 2013 Miscompiles

[Aaron Ballman]

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