[clang] 1c2e7d2 - [MS] Fix crash involving gnu stmt exprs and inalloca

[Reid Kleckner via cfe-commits]

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 at Z"(<{ %struct.Foo, %struct.Foo }>* inalloca %{{.*}})
+// CHECK: call void @llvm.stackrestore(i8* %inalloca.save)
+// CHECK: out: