[clang] caa1ebd - Don't assume that a new cleanup was added to InnermostEHScope.

[James Y Knight via cfe-commits]

Author: James Y Knight
Date: 2022-02-04T23:39:42-05:00
New Revision: caa1ebde70673ddb7124a0599ba846362a1f8b1e

URL: https://github.com/llvm/llvm-project/commit/caa1ebde70673ddb7124a0599ba846362a1f8b1e
DIFF: https://github.com/llvm/llvm-project/commit/caa1ebde70673ddb7124a0599ba846362a1f8b1e.diff

LOG: Don't assume that a new cleanup was added to InnermostEHScope.

After fa87fa97fb79, this was no longer guaranteed to be the cleanup
just added by this code, if IsEHCleanup got disabled. Instead, use
stable_begin(), which _is_ guaranteed to be the cleanup just added.

This caused a crash when a object that is callee destroyed (e.g. with the MS ABI) was passed in a call from a noexcept function.

Added a test to verify.

Fixes: fa87fa97fb79

Added: 
    

Modified: 
    clang/lib/CodeGen/CGCall.cpp
    clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index a37ff8844e885..67a3f73525e35 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4331,7 +4331,7 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
                                               type);
       // This unreachable is a temporary marker which will be removed later.
       llvm::Instruction *IsActive = Builder.CreateUnreachable();
-      args.addArgCleanupDeactivation(EHStack.getInnermostEHScope(), IsActive);
+      args.addArgCleanupDeactivation(EHStack.stable_begin(), IsActive);
     }
     return;
   }

diff  --git a/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp b/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
index 92fa17b145001..e6d602464d9b3 100644
--- a/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
+++ b/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
@@ -32,6 +32,28 @@ void HasEHCleanup() {
 // WIN32-NOT: @"??1A@@QAE at XZ"
 // WIN32: }
 
+
+// This test verifies the fix for a crash that occurred after
+// fa87fa97fb79.
+void HasEHCleanupNoexcept() noexcept {
+  TakesTwo(getA(), getA());
+}
+
+// With exceptions, we need to clean up at least one of these temporaries.
+// WIN32-LABEL: define dso_local void @"?HasEHCleanupNoexcept@@YAXXZ"() {{.*}} {
+// WIN32:   %[[base:.*]] = call i8* @llvm.stacksave()
+// WIN32:   invoke void @"?getA@@YA?AUA@@XZ"(%struct.A* sret(%struct.A) align 4 %{{.*}})
+// WIN32:   invoke void @"?getA@@YA?AUA@@XZ"(%struct.A* sret(%struct.A) align 4 %{{.*}})
+// WIN32:   invoke noundef i32 @"?TakesTwo@@YAHUA@@0 at Z"
+// WIN32:   call void @llvm.stackrestore
+// WIN32:   ret void
+//
+//    Since all the calls terminate, there should be no dtors on the unwind
+// WIN32:   cleanuppad
+// WIN32-NOT: @"??1A@@QAE at XZ"
+// WIN32: }
+
+
 void TakeRef(const A &a);
 int HasDeactivatedCleanups() {
   return TakesTwo((TakeRef(A()), A()), (TakeRef(A()), A()));