On Wed, Oct 19, 2016 at 3:38 PM, David Majnemer <david.majne...@gmail.com> wrote:
> > > On Wed, Oct 19, 2016 at 2:04 PM, Hans Wennborg via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: hans >> Date: Wed Oct 19 13:04:27 2016 >> New Revision: 284624 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=284624&view=rev >> Log: >> MS ABI: Fix assert when generating virtual function call with virtual >> bases and -flto (PR30731) >> >> getClassAtVTableLocation() was calling >> ASTRecordLayout::getBaseClassOffset() on a virtual base, causing an >> assert. >> >> Differential Revision: https://reviews.llvm.org/D25779 >> >> Added: >> cfe/trunk/test/CodeGenCXX/pr30731.cpp >> Modified: >> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp >> >> Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/Mi >> crosoftCXXABI.cpp?rev=284624&r1=284623&r2=284624&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original) >> +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed Oct 19 13:04:27 2016 >> @@ -1773,15 +1773,8 @@ static const CXXRecordDecl *getClassAtVT >> CharUnits MaxBaseOffset; >> for (auto &&B : RD->bases()) { >> const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl(); >> - CharUnits BaseOffset = Layout.getBaseClassOffset(Base); >> - if (BaseOffset <= Offset && BaseOffset >= MaxBaseOffset) { >> - MaxBase = Base; >> - MaxBaseOffset = BaseOffset; >> - } >> - } >> - for (auto &&B : RD->vbases()) { >> - const CXXRecordDecl *Base = B.getType()->getAsCXXRecordDecl(); >> - CharUnits BaseOffset = Layout.getVBaseClassOffset(Base); >> + CharUnits BaseOffset = B.isVirtual() ? Layout.getVBaseClassOffset(Bas >> e) >> + : Layout.getBaseClassOffset(Base >> ); >> if (BaseOffset <= Offset && BaseOffset >= MaxBaseOffset) { >> MaxBase = Base; >> MaxBaseOffset = BaseOffset; >> > > I don't think this code is correct. > It uses a base's layout to find virtual base offsets, that seems wrong > because the virtual base offsets can be different in a base and the most > derived class. > I think we need to do getVBaseOffset with the MDC's layout similar to how > VTableBuilder.cpp's findPathsToSubobject operates. > Thanks, I will put it on my todo list to try and understand how we need to handle vbases here. Peter > > >> >> Added: cfe/trunk/test/CodeGenCXX/pr30731.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCX >> X/pr30731.cpp?rev=284624&view=auto >> ============================================================ >> ================== >> --- cfe/trunk/test/CodeGenCXX/pr30731.cpp (added) >> +++ cfe/trunk/test/CodeGenCXX/pr30731.cpp Wed Oct 19 13:04:27 2016 >> @@ -0,0 +1,21 @@ >> +// RUN: %clang_cc1 -triple i386-pc-win32 -emit-llvm -flto -std=c++11 -o >> - %s | FileCheck %s >> + >> +struct A { >> + virtual ~A(); >> +}; >> + >> +struct B {}; >> + >> +struct C { >> + virtual void f(); >> +}; >> + >> +struct S : A, virtual B, C { >> + void f() override; >> +}; >> + >> +void f(S* s) { s->f(); } >> + >> +// CHECK-LABEL: define void @"\01?f@@YAXPAUS@@@Z" >> +// CHECK: call >> +// CHECK: ret void >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > -- -- Peter
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
