Hi rjmccall,
Based on Peter Collingbourne's destructor patches.
Prior to this change, clang was considering ?1 to be the complete
destructor and the base destructor, which was wrong. This lead to
crashes in TUs with non-inline dtors where both complete and base dtors
would be emitted and mangle to the same symbol.
Furthermore, TUs with non-inline dtors don't emit a complete destructor.
They are emitted as inline thunks in TUs that need them, and they always
delegate to the base dtors of the derived class and its virtual bases.
This change uses the DeferredDecls machinery to emit them. There are
comments in EmitDestructorBody() that suggest this won't work when the
body of the dtor is a try block, but this is how cl.exe does things.
Finally, for classes that don't use virtual inheritance, MSVC always
calls the base destructor (?1) directly. This is a useful code size
optimization that avoids emitting lots of extra thunks or aliases.
Implementing it also means our existing tests continue to pass, and is
consistent with MSVC's output.
We could probably do the same for Itanium without breaking anything.
There is an existing -mconstructor-aliases that makes the complete dtor
a linker alias, but it is off by default.
http://llvm-reviews.chandlerc.com/D1066
Files:
include/clang/Basic/TargetCXXABI.h
lib/AST/MicrosoftMangle.cpp
lib/CodeGen/CGCXX.cpp
lib/CodeGen/CGClass.cpp
lib/CodeGen/CGExprCXX.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGenCXX/microsoft-abi-structors.cpp
Index: include/clang/Basic/TargetCXXABI.h
===================================================================
--- include/clang/Basic/TargetCXXABI.h
+++ include/clang/Basic/TargetCXXABI.h
@@ -152,12 +152,6 @@
return isItaniumFamily();
}
- /// \brief Does this ABI have different entrypoints for complete-object
- /// and base-subobject destructors?
- bool hasDestructorVariants() const {
- return isItaniumFamily();
- }
-
/// \brief Does this ABI allow virtual bases to be primary base classes?
bool hasPrimaryVBases() const {
return isItaniumFamily();
Index: lib/AST/MicrosoftMangle.cpp
===================================================================
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -512,9 +512,9 @@
// use the type we were given.
mangleCXXDtorType(static_cast<CXXDtorType>(StructorType));
else
- // Otherwise, use the complete destructor name. This is relevant if a
+ // Otherwise, use the base destructor name. This is relevant if a
// class with a destructor is declared within a destructor.
- mangleCXXDtorType(Dtor_Complete);
+ mangleCXXDtorType(Dtor_Base);
break;
case DeclarationName::CXXConversionFunctionName:
@@ -577,18 +577,19 @@
}
void MicrosoftCXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
+ // Microsoft uses the names on the case labels for these dtor variants. Clang
+ // uses the Itanium terminology internally. Everything in this ABI delegates
+ // towards the base dtor.
switch (T) {
- case Dtor_Deleting:
- Out << "?_G";
- return;
- case Dtor_Base:
- // FIXME: We should be asked to mangle base dtors.
- // However, fixing this would require larger changes to the CodeGenModule.
- // Please put llvm_unreachable here when CGM is changed.
- // For now, just mangle a base dtor the same way as a complete dtor...
- case Dtor_Complete:
- Out << "?1";
- return;
+ // <operator-name> ::= ?1 # destructor
+ case Dtor_Base: Out << "?1"; return;
+ // <operator-name> ::= ?_D # vbase destructor
+ case Dtor_Complete: Out << "?_D"; return;
+ // <operator-name> ::= ?_G # scalar deleting destructor
+ case Dtor_Deleting: Out << "?_G"; return;
+ // <operator-name> ::= ?_E # vector deleting destructor
+ // FIXME: Add a vector deleting dtor type. It goes in the vtable, so we need
+ // it.
}
llvm_unreachable("Unsupported dtor type?");
}
Index: lib/CodeGen/CGCXX.cpp
===================================================================
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -277,16 +277,26 @@
llvm::GlobalValue *
CodeGenModule::GetAddrOfCXXDestructor(const CXXDestructorDecl *dtor,
CXXDtorType dtorType,
- const CGFunctionInfo *fnInfo) {
+ const CGFunctionInfo *fnInfo,
+ llvm::FunctionType *fnType) {
+ // If the class has no virtual bases, then the complete and base destructors
+ // are equivalent, for all C++ ABIs supported by clang. We can save on code
+ // size by calling the base dtor directly.
+ // XXX: We could do this for Itanium, but we should consult with John first.
+ if (getTarget().getCXXABI().isMicrosoft() && dtorType == Dtor_Complete &&
+ dtor->getParent()->getNumVBases() == 0)
+ dtorType = Dtor_Base;
+
GlobalDecl GD(dtor, dtorType);
StringRef name = getMangledName(GD);
if (llvm::GlobalValue *existing = GetGlobalValue(name))
return existing;
- if (!fnInfo) fnInfo = &getTypes().arrangeCXXDestructor(dtor, dtorType);
-
- llvm::FunctionType *fnType = getTypes().GetFunctionType(*fnInfo);
+ if (!fnType) {
+ if (!fnInfo) fnInfo = &getTypes().arrangeCXXDestructor(dtor, dtorType);
+ fnType = getTypes().GetFunctionType(*fnInfo);
+ }
return cast<llvm::Function>(GetOrCreateLLVMFunction(name, fnType, GD,
/*ForVTable=*/false));
}
Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1266,22 +1266,30 @@
// the epilogue will destruct the virtual bases. But we can't do
// this optimization if the body is a function-try-block, because
// we'd introduce *two* handler blocks.
+ // XXX: In the Microsoft ABI, we want to emit a delegating complete dtor
+ // without a definition, which means we won't be able to tell if the
+ // definition is a try body. In this case, MSVC simply delegates, so we do
+ // the same.
switch (DtorType) {
case Dtor_Deleting: llvm_unreachable("already handled deleting case");
case Dtor_Complete:
+ assert((Body || getTarget().getCXXABI().isMicrosoft()) &&
+ "can't emit a dtor without a body for non-Microsoft ABIs");
+
// Enter the cleanup scopes for virtual bases.
EnterDtorCleanups(Dtor, Dtor_Complete);
- if (!isTryBody &&
- CGM.getTarget().getCXXABI().hasDestructorVariants()) {
+ if (!isTryBody) {
EmitCXXDestructorCall(Dtor, Dtor_Base, /*ForVirtualBase=*/false,
/*Delegating=*/false, LoadCXXThis());
break;
}
// Fallthrough: act like we're in the base variant.
case Dtor_Base:
+ assert(Body);
+
// Enter the cleanup scopes for fields and non-virtual bases.
EnterDtorCleanups(Dtor, Dtor_Base);
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -271,7 +271,7 @@
else
FInfo = &CGM.getTypes().arrangeCXXMethodDeclaration(CalleeDecl);
- llvm::Type *Ty = CGM.getTypes().GetFunctionType(*FInfo);
+ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
// C++ [class.virtual]p12:
// Explicit qualification with the scope operator (5.1) suppresses the
@@ -296,7 +296,7 @@
ME->hasQualifier())
Callee = BuildAppleKextVirtualCall(MD, ME->getQualifier(), Ty);
else if (!DevirtualizedMethod)
- Callee = CGM.GetAddrOfFunction(GlobalDecl(Dtor, Dtor_Complete), Ty);
+ Callee = CGM.GetAddrOfCXXDestructor(Dtor, Dtor_Complete, FInfo, Ty);
else {
const CXXDestructorDecl *DDtor =
cast<CXXDestructorDecl>(DevirtualizedMethod);
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -515,7 +515,15 @@
llvm::GlobalValue::LinkageTypes
CodeGenModule::getFunctionLinkage(GlobalDecl GD) {
- return getFunctionLinkage(cast<FunctionDecl>(GD.getDecl()));
+ const FunctionDecl *D = cast<FunctionDecl>(GD.getDecl());
+
+ // In the MS ABI, non-base dtors are always linkonce_odr thunks that delegate
+ // to each other, bottoming out with the base dtor, which might be external.
+ if (getTarget().getCXXABI().isMicrosoft() && isa<CXXDestructorDecl>(D) &&
+ GD.getDtorType() != Dtor_Base)
+ return llvm::Function::LinkOnceODRLinkage;
+
+ return getFunctionLinkage(D);
}
llvm::GlobalValue::LinkageTypes
@@ -1023,12 +1031,20 @@
Annotations.push_back(EmitAnnotateAttr(GV, *ai, D->getLocation()));
}
-bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
+bool CodeGenModule::MayDeferGeneration(GlobalDecl GD) {
// Never defer when EmitAllDecls is specified.
if (LangOpts.EmitAllDecls)
return false;
- return !getContext().DeclMustBeEmitted(Global);
+ const Decl *D = GD.getDecl();
+
+ // In the Microsoft ABI, all delegating dtor variants are emitted on an
+ // as-needed basis.
+ if (getTarget().getCXXABI().isMicrosoft() && isa<CXXDestructorDecl>(D) &&
+ GD.getDtorType() != Dtor_Base)
+ return true;
+
+ return !getContext().DeclMustBeEmitted(D);
}
llvm::Constant *CodeGenModule::GetAddrOfUuidDescriptor(
@@ -1157,7 +1173,7 @@
// Defer code generation when possible if this is a static definition, inline
// function etc. These we only want to emit if they are used.
- if (!MayDeferGeneration(Global)) {
+ if (!MayDeferGeneration(GD)) {
// Emit the definition if it can't be deferred.
EmitGlobalDefinition(GD);
return;
@@ -1321,13 +1337,15 @@
llvm::Constant *
CodeGenModule::GetOrCreateLLVMFunction(StringRef MangledName,
llvm::Type *Ty,
- GlobalDecl D, bool ForVTable,
+ GlobalDecl GD, bool ForVTable,
llvm::AttributeSet ExtraAttrs) {
+ const Decl *D = GD.getDecl();
+
// Lookup the entry, lazily creating it if necessary.
llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
if (Entry) {
if (WeakRefReferences.erase(Entry)) {
- const FunctionDecl *FD = cast_or_null<FunctionDecl>(D.getDecl());
+ const FunctionDecl *FD = cast_or_null<FunctionDecl>(D);
if (FD && !FD->hasAttr<WeakAttr>())
Entry->setLinkage(llvm::Function::ExternalLinkage);
}
@@ -1339,6 +1357,14 @@
return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
}
+ // All MSVC dtors other than the base dtor are linkonce_odr and delegate to
+ // each other bottoming out with the base dtor. Therefore we emit non-base
+ // dtors on usage, even if there is no dtor definition in the TU.
+ if (getLangOpts().CPlusPlus && getTarget().getCXXABI().isMicrosoft() && D &&
+ isa<CXXDestructorDecl>(D) && GD.getDtorType() != Dtor_Base) {
+ DeferredDeclsToEmit.push_back(GD);
+ }
+
// This function doesn't have a complete type (for example, the return
// type is an incomplete struct). Use a fake type instead, and make
// sure not to try to set attributes.
@@ -1356,8 +1382,8 @@
llvm::Function::ExternalLinkage,
MangledName, &getModule());
assert(F->getName() == MangledName && "name was uniqued!");
- if (D.getDecl())
- SetFunctionAttributes(D, F, IsIncompleteFunction);
+ if (D)
+ SetFunctionAttributes(GD, F, IsIncompleteFunction);
if (ExtraAttrs.hasAttributes(llvm::AttributeSet::FunctionIndex)) {
llvm::AttrBuilder B(ExtraAttrs, llvm::AttributeSet::FunctionIndex);
F->addAttributes(llvm::AttributeSet::FunctionIndex,
@@ -1387,18 +1413,18 @@
//
// We also don't emit a definition for a function if it's going to be an entry
// in a vtable, unless it's already marked as used.
- } else if (getLangOpts().CPlusPlus && D.getDecl()) {
+ } else if (getLangOpts().CPlusPlus && D) {
// Look for a declaration that's lexically in a record.
- const FunctionDecl *FD = cast<FunctionDecl>(D.getDecl());
+ const FunctionDecl *FD = cast<FunctionDecl>(D);
FD = FD->getMostRecentDecl();
do {
if (isa<CXXRecordDecl>(FD->getLexicalDeclContext())) {
if (FD->isImplicit() && !ForVTable) {
assert(FD->isUsed() && "Sema didn't mark implicit function as used!");
- DeferredDeclsToEmit.push_back(D.getWithDecl(FD));
+ DeferredDeclsToEmit.push_back(GD.getWithDecl(FD));
break;
} else if (FD->doesThisDeclarationHaveABody()) {
- DeferredDeclsToEmit.push_back(D.getWithDecl(FD));
+ DeferredDeclsToEmit.push_back(GD.getWithDecl(FD));
break;
}
}
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -752,7 +752,8 @@
/// given type.
llvm::GlobalValue *GetAddrOfCXXDestructor(const CXXDestructorDecl *dtor,
CXXDtorType dtorType,
- const CGFunctionInfo *fnInfo = 0);
+ const CGFunctionInfo *fnInfo = 0,
+ llvm::FunctionType *fnType = 0);
/// getBuiltinLibFunction - Given a builtin id for a function like
/// "__builtin_fabsf", return a Function* for "fabsf".
@@ -1113,7 +1114,7 @@
/// MayDeferGeneration - Determine if the given decl can be emitted
/// lazily; this is only relevant for definitions. The given decl
/// must be either a function or var decl.
- bool MayDeferGeneration(const ValueDecl *D);
+ bool MayDeferGeneration(GlobalDecl GD);
/// SimplifyPersonality - Check whether we can use a "simpler", more
/// core exceptions personality function.
Index: test/CodeGenCXX/microsoft-abi-structors.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-structors.cpp
+++ test/CodeGenCXX/microsoft-abi-structors.cpp
@@ -1,15 +1,11 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - -cxx-abi microsoft -triple=i386-pc-win32 -fno-rtti > %t 2>&1
-// RUN: FileCheck %s < %t
-// Using a different check prefix as the inline destructors might be placed
-// anywhere in the output.
-// RUN: FileCheck --check-prefix=DTORS %s < %t
+// RUN: %clang_cc1 -emit-llvm %s -o - -cxx-abi microsoft -triple=i386-pc-win32 -fno-rtti | FileCheck %s
namespace basic {
class A {
public:
A() { }
- ~A() { }
+ ~A();
};
void no_constructor_destructor_infinite_recursion() {
@@ -21,7 +17,9 @@
// CHECK-NEXT: [[T1:%[.0-9A-Z_a-z]+]] = load %"class.basic::A"** [[THIS_ADDR]]
// CHECK-NEXT: ret %"class.basic::A"* [[T1]]
// CHECK-NEXT: }
+}
+A::~A() {
// Make sure that the destructor doesn't call itself:
// CHECK: define {{.*}} @"\01??1A@basic@@QAE@XZ"
// CHECK-NOT: call void @"\01??1A@basic@@QAE@XZ"
@@ -40,27 +38,8 @@
struct C {
virtual ~C() {
-// Complete destructor first:
-// DTORS: define {{.*}} x86_thiscallcc void @"\01??1C@basic@@UAE@XZ"(%"struct.basic::C"* %this)
-
-// Then, the scalar deleting destructor (used in the vtable):
-// FIXME: add a test that verifies that the out-of-line scalar deleting
-// destructor is linkonce_odr too.
-// DTORS: define linkonce_odr x86_thiscallcc void @"\01??_GC@basic@@UAEPAXI@Z"(%"struct.basic::C"* %this, i1 zeroext %should_call_delete)
-// DTORS: %[[FROMBOOL:[0-9a-z]+]] = zext i1 %should_call_delete to i8
-// DTORS-NEXT: store i8 %[[FROMBOOL]], i8* %[[SHOULD_DELETE_VAR:[0-9a-z._]+]], align 1
-// DTORS: %[[SHOULD_DELETE_VALUE:[0-9a-z._]+]] = load i8* %[[SHOULD_DELETE_VAR]]
-// DTORS: call x86_thiscallcc void @"\01??1C@basic@@UAE@XZ"(%"struct.basic::C"* %[[THIS:[0-9a-z]+]])
-// DTORS-NEXT: %[[CONDITION:[0-9]+]] = icmp eq i8 %[[SHOULD_DELETE_VALUE]], 0
-// DTORS-NEXT: br i1 %[[CONDITION]], label %[[CONTINUE_LABEL:[0-9a-z._]+]], label %[[CALL_DELETE_LABEL:[0-9a-z._]+]]
-//
-// DTORS: [[CALL_DELETE_LABEL]]
-// DTORS-NEXT: %[[THIS_AS_VOID:[0-9a-z]+]] = bitcast %"struct.basic::C"* %[[THIS]] to i8*
-// DTORS-NEXT: call void @"\01??3@YAXPAX@Z"(i8* %[[THIS_AS_VOID]]) [[NUW:#[0-9]+]]
-// DTORS-NEXT: br label %[[CONTINUE_LABEL]]
-//
-// DTORS: [[CONTINUE_LABEL]]
-// DTORS-NEXT: ret void
+ // The deleting dtor is installed in the vtable and deferred to the end of
+ // the TU.
}
virtual void foo();
};
@@ -102,6 +81,7 @@
// CHECK: ret void
}
+
struct D {
static int foo();
@@ -119,8 +99,6 @@
void use_D() { D c; }
-// DTORS: attributes [[NUW]] = { nounwind{{.*}} }
-
} // end namespace basic
@@ -228,3 +206,109 @@
}
} // end namespace constructors
+
+namespace dtors {
+
+struct A {
+ ~A();
+};
+
+void call_nv_complete(A *a) {
+ a->~A();
+// CHECK: define void @"\01?call_nv_complete@dtors@@YAXPAUA@1@@Z"
+// CHECK: call x86_thiscallcc void @"\01??1A@dtors@@QAE@XZ"
+// CHECK: ret
+}
+
+// CHECK: declare x86_thiscallcc void @"\01??1A@dtors@@QAE@XZ"
+
+// Now try some virtual bases, where we need the complete dtor.
+
+struct B : virtual A { ~B(); };
+struct C : virtual A { ~C(); };
+struct D : B, C { ~D(); };
+
+void call_vbase_complete(D *d) {
+ d->~D();
+// CHECK: define void @"\01?call_vbase_complete@dtors@@YAXPAUD@1@@Z"
+// CHECK: call x86_thiscallcc void @"\01??_DD@dtors@@QAE@XZ"
+// CHECK: ret
+}
+
+// The complete dtor should call the base dtors for D and the vbase A (once).
+// CHECK: define linkonce_odr x86_thiscallcc void @"\01??_DD@dtors@@QAE@XZ"
+// CHECK-NOT: call
+// CHECK: call x86_thiscallcc void @"\01??1D@dtors@@QAE@XZ"
+// CHECK-NOT: call
+// CHECK: call x86_thiscallcc void @"\01??1A@dtors@@QAE@XZ"
+// CHECK-NOT: call
+// CHECK: ret
+
+} // end namespace dtors
+
+namespace dtors2 {
+
+// Test virtual base destruction again, this time from the stack.
+
+struct A { ~A(); };
+struct B : virtual A { ~B(); };
+struct C : virtual A { ~C(); };
+struct D : B, C { ~D(); };
+
+void destroy_d_complete() { D d; }
+// CHECK: define void @"\01?destroy_d_complete@dtors2@@YAXXZ"
+// CHECK: call x86_thiscallcc void @"\01??_DD@dtors2@@QAE@XZ"
+// CHECK: ret
+
+// The complete dtor should call the base dtors for D and the vbase A (once).
+// CHECK: define linkonce_odr x86_thiscallcc void @"\01??_DD@dtors2@@QAE@XZ"
+// CHECK-NOT: call
+// CHECK: call x86_thiscallcc void @"\01??1D@dtors2@@QAE@XZ"
+// CHECK-NOT: call
+// CHECK: call x86_thiscallcc void @"\01??1A@dtors2@@QAE@XZ"
+// CHECK-NOT: call
+// CHECK: ret
+
+}
+
+namespace del_vbase {
+
+// Test that the scalar deleting dtor delegates to the complete dtor.
+// FIXME: We could call the scalar deleting dtor here instead of manually
+// inlining the deletion.
+
+struct A { ~A(); };
+struct B : virtual A { ~B(); };
+
+void call_deleting_dtor(B *b) {
+ delete b;
+// CHECK: define void @"\01?call_deleting_dtor@del_vbase@@YAXPAUB@1@@Z"
+// CHECK: call x86_thiscallcc void @"\01??_DB@del_vbase@@QAE@XZ"
+// CHECK: ret
+}
+
+// Defers the complete dtor.
+
+// CHECK: define linkonce_odr x86_thiscallcc void @"\01??_DB@del_vbase@@QAE@XZ"
+// CHECK: call x86_thiscallcc void @"\01??1B@del_vbase@@QAE@XZ"
+// CHECK: ret
+
+}
+
+// Deferred data used by vtables:
+
+// CHECK: define linkonce_odr x86_thiscallcc void @"\01??_GC@basic@@UAEPAXI@Z"(%"struct.basic::C"* %this, i1 zeroext %should_call_delete)
+// CHECK: %[[FROMBOOL:[0-9a-z]+]] = zext i1 %should_call_delete to i8
+// CHECK-NEXT: store i8 %[[FROMBOOL]], i8* %[[SHOULD_DELETE_VAR:[0-9a-z._]+]], align 1
+// CHECK: %[[SHOULD_DELETE_VALUE:[0-9a-z._]+]] = load i8* %[[SHOULD_DELETE_VAR]]
+// CHECK: call x86_thiscallcc void @"\01??1C@basic@@UAE@XZ"(%"struct.basic::C"* %[[THIS:[0-9a-z]+]])
+// CHECK-NEXT: %[[CONDITION:[0-9]+]] = icmp eq i8 %[[SHOULD_DELETE_VALUE]], 0
+// CHECK-NEXT: br i1 %[[CONDITION]], label %[[CONTINUE_LABEL:[0-9a-z._]+]], label %[[CALL_DELETE_LABEL:[0-9a-z._]+]]
+//
+// CHECK: [[CALL_DELETE_LABEL]]
+// CHECK-NEXT: %[[THIS_AS_VOID:[0-9a-z]+]] = bitcast %"struct.basic::C"* %[[THIS]] to i8*
+// CHECK-NEXT: call void @"\01??3@YAXPAX@Z"(i8* %[[THIS_AS_VOID]])
+// CHECK-NEXT: br label %[[CONTINUE_LABEL]]
+//
+// CHECK: [[CONTINUE_LABEL]]
+// CHECK-NEXT: ret void
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
