From aea8269867f7610cc2d632215dafc3a3d0c8643c Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Fri, 25 Jul 2014 21:39:46 +0000 Subject: [PATCH] MS ABI: Don't push destructor cleanups for aggregate parameters in thunks The target method of the thunk will perform the cleanup. This can't be tested in 32-bit x86 yet because passing something by value would create an inalloca, and we refuse to generate broken code for that. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@213976 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGDecl.cpp | 4 ++- lib/CodeGen/CGVTables.cpp | 1 + lib/CodeGen/CodeGenFunction.cpp | 20 +++++++-------- lib/CodeGen/CodeGenFunction.h | 4 +++ .../CodeGenCXX/microsoft-abi-byval-thunks.cpp | 25 +++++++++++++++++++ 5 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 test/CodeGenCXX/microsoft-abi-byval-thunks.cpp diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index 91f8041930..b42a44342c 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/lib/CodeGen/CGDecl.cpp @@ -1656,7 +1656,9 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg, DeclPtr = Arg->getType() == IRTy ? Arg : Builder.CreateBitCast(Arg, IRTy, D.getName()); // Push a destructor cleanup for this parameter if the ABI requires it. - if (!IsScalar && + // Don't push a cleanup in a thunk for a method that will also emit a + // cleanup. + if (!IsScalar && !CurFuncIsThunk && getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) { const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); if (RD && RD->hasNonTrivialDestructor()) diff --git a/lib/CodeGen/CGVTables.cpp b/lib/CodeGen/CGVTables.cpp index 0df2c43d11..c53e7ed33d 100644 --- a/lib/CodeGen/CGVTables.cpp +++ b/lib/CodeGen/CGVTables.cpp @@ -194,6 +194,7 @@ void CodeGenFunction::StartThunk(llvm::Function *Fn, GlobalDecl GD, const CGFunctionInfo &FnInfo) { assert(!CurGD.getDecl() && "CurGD was already set!"); CurGD = GD; + CurFuncIsThunk = true; // Build FunctionArgs. const CXXMethodDecl *MD = cast(GD.getDecl()); diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp index 5ca3a78bb4..67eae450e8 100644 --- a/lib/CodeGen/CodeGenFunction.cpp +++ b/lib/CodeGen/CodeGenFunction.cpp @@ -38,16 +38,16 @@ CodeGenFunction::CodeGenFunction(CodeGenModule &cgm, bool suppressNewContext) Builder(cgm.getModule().getContext(), llvm::ConstantFolder(), CGBuilderInserterTy(this)), CapturedStmtInfo(nullptr), SanOpts(&CGM.getLangOpts().Sanitize), - IsSanitizerScope(false), AutoreleaseResult(false), BlockInfo(nullptr), - BlockPointer(nullptr), LambdaThisCaptureField(nullptr), - NormalCleanupDest(nullptr), NextCleanupDestIndex(1), - FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr), - EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()), - DisableDebugInfo(false), DidCallStackSave(false), IndirectBranch(nullptr), - PGO(cgm), SwitchInsn(nullptr), SwitchWeights(nullptr), - CaseRangeBlock(nullptr), UnreachableBlock(nullptr), NumReturnExprs(0), - NumSimpleReturnExprs(0), CXXABIThisDecl(nullptr), - CXXABIThisValue(nullptr), CXXThisValue(nullptr), + IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false), + BlockInfo(nullptr), BlockPointer(nullptr), + LambdaThisCaptureField(nullptr), NormalCleanupDest(nullptr), + NextCleanupDestIndex(1), FirstBlockInfo(nullptr), EHResumeBlock(nullptr), + ExceptionSlot(nullptr), EHSelectorSlot(nullptr), + DebugInfo(CGM.getModuleDebugInfo()), DisableDebugInfo(false), + DidCallStackSave(false), IndirectBranch(nullptr), PGO(cgm), + SwitchInsn(nullptr), SwitchWeights(nullptr), CaseRangeBlock(nullptr), + UnreachableBlock(nullptr), NumReturnExprs(0), NumSimpleReturnExprs(0), + CXXABIThisDecl(nullptr), CXXABIThisValue(nullptr), CXXThisValue(nullptr), CXXDefaultInitExprThis(nullptr), CXXStructorImplicitParamDecl(nullptr), CXXStructorImplicitParamValue(nullptr), OutermostConditional(nullptr), CurLexicalScope(nullptr), TerminateLandingPad(nullptr), diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 335a8910e8..ccf7651fdc 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -258,6 +258,10 @@ public: ~SanitizerScope(); }; + /// In C++, whether we are code generating a thunk. This controls whether we + /// should emit cleanups. + bool CurFuncIsThunk; + /// In ARC, whether we should autorelease the return value. bool AutoreleaseResult; diff --git a/test/CodeGenCXX/microsoft-abi-byval-thunks.cpp b/test/CodeGenCXX/microsoft-abi-byval-thunks.cpp new file mode 100644 index 0000000000..a9ffc12014 --- /dev/null +++ b/test/CodeGenCXX/microsoft-abi-byval-thunks.cpp @@ -0,0 +1,25 @@ +// RUN: not %clang_cc1 %s -fno-rtti -triple=i686-pc-win32 -emit-llvm -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK32 +// RUN: %clang_cc1 %s -fno-rtti -triple=x86_64-pc-win32 -emit-llvm -o - | FileCheck --check-prefix=CHECK64 + +namespace byval_thunk { +struct Agg { + Agg(); + Agg(const Agg &); + ~Agg(); + int x; +}; + +struct A { virtual void foo(Agg x); }; +struct B { virtual void foo(Agg x); }; +struct C : A, B { virtual void foo(Agg x); }; +C c; + +// CHECK32: cannot compile this non-trivial argument copy for thunk yet + +// CHECK64-LABEL: define linkonce_odr void @"\01?foo@C@byval_thunk@@W7EAAXUAgg@2@@Z" +// CHECK64: (%"struct.byval_thunk::C"* %this, %"struct.byval_thunk::Agg"* %x) +// CHECK64: getelementptr i8* %{{.*}}, i32 -8 +// CHECK64: call void @"\01?foo@C@byval_thunk@@UEAAXUAgg@2@@Z"(%"struct.byval_thunk::C"* %2, %"struct.byval_thunk::Agg"* %x) +// CHECK64-NOT: call +// CHECK64: ret void +} -- 2.40.0