Ping?
On Fri, Nov 1, 2013 at 5:41 PM, Aaron Ballman <[email protected]> wrote: > On Fri, Nov 1, 2013 at 4:15 PM, Reid Kleckner <[email protected]> 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 <[email protected]> >> 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 <[email protected]> >>> 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 >> >> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
