diff --git a/clang/include/clang/Basic/TargetCXXABI.h b/clang/include/clang/Basic/TargetCXXABI.h index 0a9aeff9f6cc..b04fab866f24 100644 --- a/clang/include/clang/Basic/TargetCXXABI.h +++ b/clang/include/clang/Basic/TargetCXXABI.h @@ -131,10 +131,21 @@ public: /// \brief Is the default C++ member function calling convention /// the same as the default calling convention? bool isMemberFunctionCCDefault() const { - // Right now, this is always true for Microsoft. + // Right now, this is always false for Microsoft. return !isMicrosoft(); } + /// Are temporary objects passed by value to a call destroyed by the callee? + /// This is a fundamental language change, since it implies that objects + /// passed by value do *not* live to the end of the full expression. + /// Temporaries passed to a function taking a const reference live to the end + /// of the full expression as usual. Both the caller and the callee must + /// have access to the destructor, while only the caller needs the + /// destructor if this is false. + bool isArgumentDestroyedByCallee() const { + return isMicrosoft(); + } + /// \brief Does this ABI have different entrypoints for complete-object /// and base-subobject constructors? bool hasConstructorVariants() const { diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 80446393d5b8..cb5c78a6d598 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -1865,6 +1865,19 @@ static void emitWritebacks(CodeGenFunction &CGF, emitWriteback(CGF, *i); } +static void deactivateArgCleanupsBeforeCall(CodeGenFunction &CGF, + const CallArgList &CallArgs) { + assert(CGF.getTarget().getCXXABI().isArgumentDestroyedByCallee()); + ArrayRef Cleanups = + CallArgs.getCleanupsToDeactivate(); + // Iterate in reverse to increase the likelihood of popping the cleanup. + for (ArrayRef::reverse_iterator + I = Cleanups.rbegin(), E = Cleanups.rend(); I != E; ++I) { + CGF.DeactivateCleanupBlock(I->Cleanup, I->IsActiveIP); + I->IsActiveIP->eraseFromParent(); + } +} + static const Expr *maybeGetUnaryAddrOfOperand(const Expr *E) { if (const UnaryOperator *uop = dyn_cast(E->IgnoreParens())) if (uop->getOpcode() == UO_AddrOf) @@ -2016,8 +2029,31 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E, return args.add(EmitReferenceBindingToExpr(E), type); } - if (hasAggregateEvaluationKind(type) && - isa(E) && + bool HasAggregateEvalKind = hasAggregateEvaluationKind(type); + + // In the Microsoft C++ ABI, aggregate arguments are destructed by the callee. + // However, we still have to push an EH-only cleanup in case we unwind before + // we make it to the call. + if (HasAggregateEvalKind && + CGM.getTarget().getCXXABI().isArgumentDestroyedByCallee()) { + const CXXRecordDecl *RD = type->getAsCXXRecordDecl(); + if (RD && RD->hasNonTrivialDestructor()) { + AggValueSlot Slot = CreateAggTemp(type, "agg.arg.tmp"); + Slot.setExternallyDestructed(); + EmitAggExpr(E, Slot); + RValue RV = Slot.asRValue(); + args.add(RV, type); + + pushDestroy(EHCleanup, RV.getAggregateAddr(), type, destroyCXXObject, + /*useEHCleanupForArray*/ true); + // This unreachable is a temporary marker which will be removed later. + llvm::Instruction *IsActive = Builder.CreateUnreachable(); + args.addArgCleanupDeactivation(EHStack.getInnermostEHScope(), IsActive); + return; + } + } + + if (HasAggregateEvalKind && isa(E) && cast(E)->getCastKind() == CK_LValueToRValue) { LValue L = EmitLValue(cast(E)->getSubExpr()); assert(L.isSimple()); @@ -2428,6 +2464,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, } } + if (!CallArgs.getCleanupsToDeactivate().empty()) + deactivateArgCleanupsBeforeCall(*this, CallArgs); + // If the callee is a bitcast of a function to a varargs pointer to function // type, check to see if we can remove the bitcast. This handles some cases // with unprototyped functions. diff --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h index 85c3320ec0ee..04e600536e57 100644 --- a/clang/lib/CodeGen/CGCall.h +++ b/clang/lib/CodeGen/CGCall.h @@ -16,6 +16,7 @@ #define CLANG_CODEGEN_CGCALL_H #include "CGValue.h" +#include "EHScopeStack.h" #include "clang/AST/CanonicalType.h" #include "clang/AST/Type.h" #include "llvm/ADT/FoldingSet.h" @@ -67,6 +68,14 @@ namespace CodeGen { llvm::Value *ToUse; }; + struct CallArgCleanup { + EHScopeStack::stable_iterator Cleanup; + + /// The "is active" insertion point. This instruction is temporary and + /// will be removed after insertion. + llvm::Instruction *IsActiveIP; + }; + void add(RValue rvalue, QualType type, bool needscopy = false) { push_back(CallArg(rvalue, type, needscopy)); } @@ -92,8 +101,25 @@ namespace CodeGen { writeback_iterator writeback_begin() const { return Writebacks.begin(); } writeback_iterator writeback_end() const { return Writebacks.end(); } + void addArgCleanupDeactivation(EHScopeStack::stable_iterator Cleanup, + llvm::Instruction *IsActiveIP) { + CallArgCleanup ArgCleanup; + ArgCleanup.Cleanup = Cleanup; + ArgCleanup.IsActiveIP = IsActiveIP; + CleanupsToDeactivate.push_back(ArgCleanup); + } + + ArrayRef getCleanupsToDeactivate() const { + return CleanupsToDeactivate; + } + private: SmallVector Writebacks; + + /// Deactivate these cleanups immediately before making the call. This + /// is used to cleanup objects that are owned by the callee once the call + /// occurs. + SmallVector CleanupsToDeactivate; }; /// A class for recording the number of arguments that a function diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 429d2c8ccac1..519f03aba16d 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -1651,10 +1651,18 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg, } llvm::Value *DeclPtr; + bool HasNonScalarEvalKind = !CodeGenFunction::hasScalarEvaluationKind(Ty); // If this is an aggregate or variable sized value, reuse the input pointer. - if (!Ty->isConstantSizeType() || - !CodeGenFunction::hasScalarEvaluationKind(Ty)) { + if (HasNonScalarEvalKind || !Ty->isConstantSizeType()) { DeclPtr = Arg; + // Push a destructor cleanup for this parameter if the ABI requires it. + if (HasNonScalarEvalKind && + getTarget().getCXXABI().isArgumentDestroyedByCallee()) { + if (const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl()) { + if (RD->hasNonTrivialDestructor()) + pushDestroy(QualType::DK_cxx_destructor, DeclPtr, Ty); + } + } } else { // Otherwise, create a temporary to hold the value. llvm::AllocaInst *Alloc = CreateTempAlloca(ConvertTypeForMem(Ty), diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 1ff794695cbd..929130bc4766 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -36,7 +36,7 @@ public: } RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const { - if (RD->hasNonTrivialCopyConstructor()) + if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialDestructor()) return RAA_DirectInMemory; return RAA_Default; } diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 16b12ea91e10..87fe76358914 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -5777,6 +5777,15 @@ bool Sema::CheckParmsForFunctionDef(ParmVarDecl **P, ParmVarDecl **PEnd, } PType= AT->getElementType(); } + + // MSVC destroys objects passed by value in the callee. Therefore a + // function definition which takes such a parameter must be able to call the + // object's destructor. + if (getLangOpts().CPlusPlus && + Context.getTargetInfo().getCXXABI().isArgumentDestroyedByCallee()) { + if (const RecordType *RT = Param->getType()->getAs()) + FinalizeVarWithDestructor(Param, RT); + } } return HasInvalidParm; diff --git a/clang/test/CodeGenCXX/microsoft-abi-exceptions.cpp b/clang/test/CodeGenCXX/microsoft-abi-exceptions.cpp new file mode 100644 index 000000000000..383270a1f2eb --- /dev/null +++ b/clang/test/CodeGenCXX/microsoft-abi-exceptions.cpp @@ -0,0 +1,118 @@ +// RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-pc-win32 -cxx-abi microsoft -fexceptions | FileCheck -check-prefix WIN32 %s + +struct A { + A(); + ~A(); + int a; +}; + +A getA(); + +int TakesTwo(A a, A b); +void HasEHCleanup() { + TakesTwo(getA(), getA()); +} + +// With exceptions, we need to clean up at least one of these temporaries. +// WIN32: define void @"\01?HasEHCleanup@@YAXXZ"() {{.*}} { +// First one doesn't have any cleanups, no need for invoke. +// WIN32: call void @"\01?getA@@YA?AUA@@XZ"(%struct.A* sret %{{.*}}) +// If this call throws, we have to cleanup the first temporary. +// WIN32: invoke void @"\01?getA@@YA?AUA@@XZ"(%struct.A* sret %{{.*}}) +// If this call throws, we already popped our cleanups +// WIN32: call i32 @"\01?TakesTwo@@YAHUA@@0@Z" +// WIN32: ret void +// +// There should be one dtor call for unwinding from the second getA. +// WIN32: invoke x86_thiscallcc void @"\01??1A@@QAE@XZ" +// WIN32: } + +void TakeRef(const A &a); +int HasDeactivatedCleanups() { + return TakesTwo((TakeRef(A()), A()), (TakeRef(A()), A())); +} + +// WIN32: define i32 @"\01?HasDeactivatedCleanups@@YAHXZ"() {{.*}} { +// WIN32: %[[isactive:.*]] = alloca i1 +// WIN32: call x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ" +// WIN32: invoke void @"\01?TakeRef@@YAXABUA@@@Z" +// WIN32: invoke x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ"(%struct.A* %[[arg1:.*]]) +// WIN32: store i1 true, i1* %[[isactive]] +// WIN32: invoke x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ" +// WIN32: invoke void @"\01?TakeRef@@YAXABUA@@@Z" +// WIN32: invoke x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ" +// WIN32: store i1 false, i1* %[[isactive]] +// WIN32: invoke i32 @"\01?TakesTwo@@YAHUA@@0@Z" +// Destroy the two const ref temporaries. +// WIN32: invoke x86_thiscallcc void @"\01??1A@@QAE@XZ" +// WIN32: call x86_thiscallcc void @"\01??1A@@QAE@XZ" +// WIN32: ret i32 +// +// Conditionally destroy arg1. +// WIN32: %[[cond:.*]] = load i1* %[[isactive]] +// WIN32: br i1 %[[cond]] +// WIN32: invoke x86_thiscallcc void @"\01??1A@@QAE@XZ"(%struct.A* %[[arg1]]) +// WIN32: } + +// Test putting the cleanups inside a conditional. +int CouldThrow(); +int HasConditionalCleanup(bool cond) { + return (cond ? TakesTwo(A(), A()) : CouldThrow()); +} + +// WIN32: define i32 @"\01?HasConditionalCleanup@@YAH_N@Z"(i1 zeroext %{{.*}}) {{.*}} { +// WIN32: store i1 false +// WIN32: br i1 +// No cleanups, so we call and then activate a cleanup if it succeeds. +// WIN32: call x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ"(%struct.A* %[[arg1:.*]]) +// WIN32: store i1 true +// Now we have a cleanup for the first aggregate, so we invoke. +// WIN32: invoke x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ"(%struct.A* %{{.*}}) +// Now we have no cleanups because TakeTwo will destruct both args. +// WIN32: call i32 @"\01?TakesTwo@@YAHUA@@0@Z" +// Still no cleanups, so call. +// WIN32: call i32 @"\01?CouldThrow@@YAHXZ"() +// Somewhere in the landing pad for our single invoke, call the dtor. +// WIN32: invoke x86_thiscallcc void @"\01??1A@@QAE@XZ"(%struct.A* %[[arg1]]) +// WIN32: } + +// Now test both. +int HasConditionalDeactivatedCleanups(bool cond) { + return (cond ? TakesTwo((TakeRef(A()), A()), (TakeRef(A()), A())) : CouldThrow()); +} + +// WIN32: define i32 @"\01?HasConditionalDeactivatedCleanups@@YAH_N@Z"{{.*}} { +// WIN32: %[[arg1:.*]] = alloca %struct.A, align 4 +// WIN32: alloca i1 +// WIN32: %[[arg1_cond:.*]] = alloca i1 +// Start all four cleanups as deactivated. +// WIN32: store i1 false +// WIN32: store i1 false +// WIN32: store i1 false +// WIN32: store i1 false +// WIN32: br i1 +// True condition. +// WIN32: call x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ" +// WIN32: store i1 true +// WIN32: invoke void @"\01?TakeRef@@YAXABUA@@@Z" +// WIN32: invoke x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ"(%struct.A* %[[arg1]]) +// WIN32: store i1 true, i1* %[[arg1_cond]] +// WIN32: invoke x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ" +// WIN32: store i1 true +// WIN32: invoke void @"\01?TakeRef@@YAXABUA@@@Z" +// WIN32: invoke x86_thiscallcc %struct.A* @"\01??0A@@QAE@XZ" +// WIN32: store i1 true +// WIN32: store i1 false, i1* %[[arg1_cond]] +// WIN32: invoke i32 @"\01?TakesTwo@@YAHUA@@0@Z" +// False condition. +// WIN32: invoke i32 @"\01?CouldThrow@@YAHXZ"() +// Two normal cleanups for TakeRef args. +// WIN32: invoke x86_thiscallcc void @"\01??1A@@QAE@XZ" +// WIN32: call x86_thiscallcc void @"\01??1A@@QAE@XZ" +// WIN32: ret i32 +// +// Somewhere in the landing pad soup, we conditionally destroy arg1. +// WIN32: %[[isactive:.*]] = load i1* %[[arg1_cond]] +// WIN32: br i1 %[[isactive]] +// WIN32: invoke x86_thiscallcc void @"\01??1A@@QAE@XZ"(%struct.A* %[[arg1]]) +// WIN32: } diff --git a/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp b/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp index 060c1728586c..fa57dd124f6e 100644 --- a/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp +++ b/clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp @@ -22,6 +22,12 @@ struct SmallWithCtor { int x; }; +struct SmallWithDtor { + SmallWithDtor(); + ~SmallWithDtor(); + int x; +}; + struct SmallWithVftable { int x; virtual void foo(); @@ -95,6 +101,44 @@ void small_arg_with_ctor(SmallWithCtor s) {} // WIN32: define void @"\01?small_arg_with_ctor@@YAXUSmallWithCtor@@@Z"(%struct.SmallWithCtor* byval align 4 %s) // WIN64: define void @"\01?small_arg_with_ctor@@YAXUSmallWithCtor@@@Z"(i32 %s.coerce) +// Test that dtors are invoked in the callee. +void small_arg_with_dtor(SmallWithDtor s) {} +// WIN32: define void @"\01?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(%struct.SmallWithDtor* byval align 4 %s) {{.*}} { +// WIN32: call x86_thiscallcc void @"\01??1SmallWithDtor@@QAE@XZ"(%struct.SmallWithDtor* %s) +// WIN32: } +// WIN64: define void @"\01?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(%struct.SmallWithDtor* byval %s) {{.*}} { +// WIN64: call void @"\01??1SmallWithDtor@@QEAA@XZ"(%struct.SmallWithDtor* %s) +// WIN64: } + +// Test that references aren't destroyed in the callee. +void ref_small_arg_with_dtor(const SmallWithDtor &s) { } +// WIN32: define void @"\01?ref_small_arg_with_dtor@@YAXABUSmallWithDtor@@@Z"(%struct.SmallWithDtor* %s) {{.*}} { +// WIN32-NOT: call x86_thiscallcc void @"\01??1SmallWithDtor@@QAE@XZ" +// WIN32: } + +// Test that temporaries passed by reference are destroyed in the caller. +void temporary_ref_with_dtor() { + ref_small_arg_with_dtor(SmallWithDtor()); +} +// WIN32: define void @"\01?temporary_ref_with_dtor@@YAXXZ"() {{.*}} { +// WIN32: call x86_thiscallcc %struct.SmallWithDtor* @"\01??0SmallWithDtor@@QAE@XZ" +// WIN32: call void @"\01?ref_small_arg_with_dtor@@YAXABUSmallWithDtor@@@Z" +// WIN32: call x86_thiscallcc void @"\01??1SmallWithDtor@@QAE@XZ" +// WIN32: } + +void takes_two_by_val_with_dtor(SmallWithDtor a, SmallWithDtor b); +void eh_cleanup_arg_with_dtor() { + takes_two_by_val_with_dtor(SmallWithDtor(), SmallWithDtor()); +} +// When exceptions are off, we don't have any cleanups. See +// microsoft-abi-exceptions.cpp for these cleanups. +// WIN32: define void @"\01?eh_cleanup_arg_with_dtor@@YAXXZ"() {{.*}} { +// WIN32: call x86_thiscallcc %struct.SmallWithDtor* @"\01??0SmallWithDtor@@QAE@XZ" +// WIN32: call x86_thiscallcc %struct.SmallWithDtor* @"\01??0SmallWithDtor@@QAE@XZ" +// WIN32: call void @"\01?takes_two_by_val_with_dtor@@YAXUSmallWithDtor@@0@Z" +// WIN32-NOT: call x86_thiscallcc void @"\01??1SmallWithDtor@@QAE@XZ" +// WIN32: } + void small_arg_with_vftable(SmallWithVftable s) {} // LINUX: define void @_Z22small_arg_with_vftable16SmallWithVftable(%struct.SmallWithVftable* %s) // WIN32: define void @"\01?small_arg_with_vftable@@YAXUSmallWithVftable@@@Z"(%struct.SmallWithVftable* byval align 4 %s) @@ -167,3 +211,19 @@ void use_class() { c.thiscall_method_arg(SmallWithCtor()); c.thiscall_method_arg(Big()); } + +struct X { + X(); + ~X(); +}; +void g(X) { +} +// WIN32: define void @"\01?g@@YAXUX@@@Z"(%struct.X* byval align 4) {{.*}} { +// WIN32: call x86_thiscallcc void @"\01??1X@@QAE@XZ"(%struct.X* %0) +// WIN32: } +void f() { + g(X()); +} +// WIN32: define void @"\01?f@@YAXXZ"() {{.*}} { +// WIN32-NOT: call {{.*}} @"\01??1X@@QAE@XZ" +// WIN32: } diff --git a/clang/test/SemaCXX/microsoft-dtor-lookup.cpp b/clang/test/SemaCXX/microsoft-dtor-lookup.cpp index e7a60970e1e2..d264bab09bf7 100644 --- a/clang/test/SemaCXX/microsoft-dtor-lookup.cpp +++ b/clang/test/SemaCXX/microsoft-dtor-lookup.cpp @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -triple i686-pc-win32 -cxx-abi itanium -fsyntax-only %s -// RUN: %clang_cc1 -triple i686-pc-win32 -cxx-abi microsoft -verify %s +// RUN: %clang_cc1 -triple i686-pc-win32 -cxx-abi microsoft -verify -DMSVC_ABI %s + +namespace Test1 { // Should be accepted under the Itanium ABI (first RUN line) but rejected // under the Microsoft ABI (second RUN line), as Microsoft ABI requires @@ -21,3 +23,65 @@ struct C : A, B { struct VC : A, B { virtual ~VC(); // expected-error {{member 'operator delete' found in multiple base classes of different types}} }; + +} + +namespace Test2 { + +// In the MSVC ABI, functions must destroy their aggregate arguments. foo +// requires a dtor for B, but we can't implicitly define it because ~A is +// private. bar should be able to call A's private dtor without error, even +// though MSVC rejects bar. + +class A { +private: + ~A(); // expected-note 2{{declared private here}} + int a; +}; + +struct B : public A { // expected-error {{base class 'Test2::A' has private destructor}} + int b; +}; + +struct C { + ~C(); + int c; +}; + +struct D { + // D has a non-trivial implicit dtor that destroys C. + C o; +}; + +void foo(B b) { } // expected-note {{implicit destructor for 'Test2::B' first required here}} +void bar(A a) { } // expected-error {{variable of type 'Test2::A' has private destructor}} +void baz(D d) { } // no error + +} + +#ifdef MSVC_ABI +namespace Test3 { + +class A { + A(); + ~A(); // expected-note 2{{implicitly declared private here}} + friend void bar(A); + int a; +}; + +void bar(A a) { } +void baz(A a) { } // expected-error {{variable of type 'Test3::A' has private destructor}} + +// MSVC accepts foo() but we reject it for consistency with Itanium. MSVC also +// rejects this if A has a copy ctor or if we call A's ctor. +void foo(A *a) { + bar(*a); // expected-error {{temporary of type 'Test3::A' has private destructor}} +} +} +#endif + +namespace Test4 { +// Don't try to access the dtor of an incomplete on a function declaration. +class A; +void foo(A a); +}