Author: Reid Kleckner Date: 2021-03-04T13:57:46-08:00 New Revision: 1c2e7d200df27e91631ba300965245518bfe252c
URL: https://github.com/llvm/llvm-project/commit/1c2e7d200df27e91631ba300965245518bfe252c DIFF: https://github.com/llvm/llvm-project/commit/1c2e7d200df27e91631ba300965245518bfe252c.diff LOG: [MS] Fix crash involving gnu stmt exprs and inalloca Use a WeakTrackingVH to cope with the stmt emission logic that cleans up unreachable blocks. This invalidates the reference to the deferred replacement placeholder. Cope with it. Fixes PR25102 (from 2015!) Added: clang/test/CodeGenCXX/inalloca-stmtexpr.cpp Modified: clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenFunction.h Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index f5411daaa677..edcaa528437b 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -4436,7 +4436,8 @@ llvm::CallBase *CodeGenFunction::EmitCallOrInvoke(llvm::FunctionCallee Callee, void CodeGenFunction::deferPlaceholderReplacement(llvm::Instruction *Old, llvm::Value *New) { - DeferredReplacements.push_back(std::make_pair(Old, New)); + DeferredReplacements.push_back( + std::make_pair(llvm::WeakTrackingVH(Old), New)); } namespace { diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 6d95adcb6ef0..53bf69f8f86d 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -452,13 +452,13 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) { if (CGM.getCodeGenOpts().EmitDeclMetadata) EmitDeclMetadata(); - for (SmallVectorImpl<std::pair<llvm::Instruction *, llvm::Value *> >::iterator - I = DeferredReplacements.begin(), - E = DeferredReplacements.end(); - I != E; ++I) { - I->first->replaceAllUsesWith(I->second); - I->first->eraseFromParent(); + for (const auto &R : DeferredReplacements) { + if (llvm::Value *Old = R.first) { + Old->replaceAllUsesWith(R.second); + cast<llvm::Instruction>(Old)->eraseFromParent(); + } } + DeferredReplacements.clear(); // Eliminate CleanupDestSlot alloca by replacing it with SSA values and // PHIs if the current function is a coroutine. We don't do it for all diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 8ef0de018a98..2ce87ac7c8e3 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -4539,8 +4539,8 @@ class CodeGenFunction : public CodeGenTypeCache { void deferPlaceholderReplacement(llvm::Instruction *Old, llvm::Value *New); - llvm::SmallVector<std::pair<llvm::Instruction *, llvm::Value *>, 4> - DeferredReplacements; + llvm::SmallVector<std::pair<llvm::WeakTrackingVH, llvm::Value *>, 4> + DeferredReplacements; /// Set the address of a local variable. void setAddrOfLocalVar(const VarDecl *VD, Address Addr) { diff --git a/clang/test/CodeGenCXX/inalloca-stmtexpr.cpp b/clang/test/CodeGenCXX/inalloca-stmtexpr.cpp new file mode 100644 index 000000000000..e7ae2cb4e703 --- /dev/null +++ b/clang/test/CodeGenCXX/inalloca-stmtexpr.cpp @@ -0,0 +1,51 @@ +// RUN: %clang_cc1 %s -emit-llvm -triple i686-windows-msvc -o - | FileCheck %s + +// Statement allow the user to exit the evaluation scope of a CallExpr without +// executing the call. Check that clang generates reasonable IR for that case. + +// Not trivially copyable, subject to inalloca. +struct Foo { + int x; + Foo(); + ~Foo(); +}; + +void inalloca(Foo x, Foo y); + +// PR25102: In this case, clang attempts to clean up unreachable blocks *during* +// IR generation. inalloca defers some RAUW operations to the end of codegen, +// and those references would become stale when the unreachable call to +// 'inalloca' got deleted. +extern "C" void pr25102() { + inalloca(Foo(), ({ + goto out; + Foo(); + })); +out:; +} + +// CHECK-LABEL: define dso_local void @pr25102() +// CHECK: br label %out +// CHECK: out: +// CHECK: ret void + +bool cond(); +extern "C" void seqAbort() { + inalloca(Foo(), ({ + if (cond()) + goto out; + Foo(); + })); +out:; +} + +// FIXME: This can cause a stack leak. We should really have a "normal" cleanup +// that goto branches through. +// CHECK-LABEL: define dso_local void @seqAbort() +// CHECK: alloca inalloca <{ %struct.Foo, %struct.Foo }> +// CHECK: call zeroext i1 @"?cond@@YA_NXZ"() +// CHECK: br i1 +// CHECK: br label %out +// CHECK: call void @"?inalloca@@YAXUFoo@@0@Z"(<{ %struct.Foo, %struct.Foo }>* inalloca %{{.*}}) +// CHECK: call void @llvm.stackrestore(i8* %inalloca.save) +// CHECK: out: _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
