Author: rsmith Date: Tue Nov 27 11:33:49 2018 New Revision: 347692 URL: http://llvm.org/viewvc/llvm-project?rev=347692&view=rev Log: Don't speculatively emit VTTs for classes unless we are able to correctly emit references to all the functions they will (directly or indirectly) reference.
Summary: This fixes a miscompile where we'd emit a VTT for a class that ends up referencing an inline virtual member function that we can't actually emit a body for (because we never instantiated it in the current TU), which in a corner case of a corner case can lead to link errors. Reviewers: rjmccall Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D54768 Added: cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=347692&r1=347691&r2=347692&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original) +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Tue Nov 27 11:33:49 2018 @@ -287,6 +287,7 @@ public: void emitVirtualInheritanceTables(const CXXRecordDecl *RD) override; bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const override; + bool canSpeculativelyEmitVTableAsBaseClass(const CXXRecordDecl *RD) const; void setThunkLinkage(llvm::Function *Thunk, bool ForVTable, GlobalDecl GD, bool ReturnAdjustment) override { @@ -1777,7 +1778,8 @@ void ItaniumCXXABI::emitVirtualInheritan VTables.EmitVTTDefinition(VTT, CGM.getVTableLinkage(RD), RD); } -bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const { +bool ItaniumCXXABI::canSpeculativelyEmitVTableAsBaseClass( + const CXXRecordDecl *RD) const { // We don't emit available_externally vtables if we are in -fapple-kext mode // because kext mode does not permit devirtualization. if (CGM.getLangOpts().AppleKext) @@ -1795,7 +1797,43 @@ bool ItaniumCXXABI::canSpeculativelyEmit // to emit an available_externally copy of vtable. // FIXME we can still emit a copy of the vtable if we // can emit definition of the inline functions. - return !hasAnyUnusedVirtualInlineFunction(RD); + if (hasAnyUnusedVirtualInlineFunction(RD)) + return false; + + // For a class with virtual bases, we must also be able to speculatively + // emit the VTT, because CodeGen doesn't have separate notions of "can emit + // the vtable" and "can emit the VTT". For a base subobject, this means we + // need to be able to emit non-virtual base vtables. + if (RD->getNumVBases()) { + for (const auto &B : RD->bases()) { + auto *BRD = B.getType()->getAsCXXRecordDecl(); + assert(BRD && "no class for base specifier"); + if (B.isVirtual() || !BRD->isDynamicClass()) + continue; + if (!canSpeculativelyEmitVTableAsBaseClass(BRD)) + return false; + } + } + + return true; +} + +bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const { + if (!canSpeculativelyEmitVTableAsBaseClass(RD)) + return false; + + // For a complete-object vtable (or more specifically, for the VTT), we need + // to be able to speculatively emit the vtables of all dynamic virtual bases. + for (const auto &B : RD->vbases()) { + auto *BRD = B.getType()->getAsCXXRecordDecl(); + assert(BRD && "no class for base specifier"); + if (!BRD->isDynamicClass()) + continue; + if (!canSpeculativelyEmitVTableAsBaseClass(BRD)) + return false; + } + + return true; } static llvm::Value *performTypeAdjustment(CodeGenFunction &CGF, Address InitialPtr, Added: cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp?rev=347692&view=auto ============================================================================== --- cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp (added) +++ cfe/trunk/test/CodeGenCXX/speculative-vtt.cpp Tue Nov 27 11:33:49 2018 @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -O2 -disable-llvm-passes -emit-llvm -o - | FileCheck %s +struct A { virtual ~A(); }; +template<typename T> struct B : virtual A { + ~B() override {} +}; +struct C : B<int>, B<float> { C(); ~C() override; }; +struct D : C { ~D() override; }; + +// We must not create a reference to B<int>::~B() here, because we're not going to emit it. +// CHECK-NOT: @_ZN1BIiED1Ev +// CHECK-NOT: @_ZTC1D0_1BIiE = +// CHECK-NOT: @_ZTT1D = available_externally +D *p = new D; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
