[llvm] 24f5105 - [Support] On Windows, ensure abort() can be catched several times in a row with CrashRecoveryContext

[Alexandre Ganea via llvm-commits]

Author: Alexandre Ganea
Date: 2020-09-24T08:21:42-04:00
New Revision: 24f510570fedf2ac6ea421478c7500d777c8c3c6

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

LOG: [Support] On Windows, ensure abort() can be catched several times in a row with CrashRecoveryContext

Before this patch, the CrashRecoveryContext would only catch the first abort(). Any further calls to abort() inside subsquent CrashRecoveryContexts would not be catched. This is because the Windows CRT removes the abort() handler before calling it.

This is part of https://reviews.llvm.org/D70378

Added: 
    

Modified: 
    llvm/include/llvm/Support/CrashRecoveryContext.h
    llvm/lib/Support/CrashRecoveryContext.cpp
    llvm/unittests/Support/CrashRecoveryTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/CrashRecoveryContext.h b/llvm/include/llvm/Support/CrashRecoveryContext.h
index 1529c197a301..0fd1d4675c7c 100644
--- a/llvm/include/llvm/Support/CrashRecoveryContext.h
+++ b/llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -44,11 +44,11 @@ class CrashRecoveryContextCleanup;
 /// executed in any case, whether crash occurs or not. These actions may be used
 /// to reclaim resources in the case of crash.
 class CrashRecoveryContext {
-  void *Impl;
-  CrashRecoveryContextCleanup *head;
+  void *Impl = nullptr;
+  CrashRecoveryContextCleanup *head = nullptr;
 
 public:
-  CrashRecoveryContext() : Impl(nullptr), head(nullptr) {}
+  CrashRecoveryContext();
   ~CrashRecoveryContext();
 
   /// Register cleanup handler, which is used when the recovery context is

diff  --git a/llvm/lib/Support/CrashRecoveryContext.cpp b/llvm/lib/Support/CrashRecoveryContext.cpp
index d4fd8216123f..2a41754c7786 100644
--- a/llvm/lib/Support/CrashRecoveryContext.cpp
+++ b/llvm/lib/Support/CrashRecoveryContext.cpp
@@ -95,6 +95,13 @@ static void uninstallExceptionOrSignalHandlers();
 
 CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
 
+CrashRecoveryContext::CrashRecoveryContext() {
+  // On Windows, if abort() was previously triggered (and caught by a previous
+  // CrashRecoveryContext) the Windows CRT removes our installed signal handler,
+  // so we need to install it again.
+  sys::DisableSystemDialogsOnCrash();
+}
+
 CrashRecoveryContext::~CrashRecoveryContext() {
   // Reclaim registered resources.
   CrashRecoveryContextCleanup *i = head;

diff  --git a/llvm/unittests/Support/CrashRecoveryTest.cpp b/llvm/unittests/Support/CrashRecoveryTest.cpp
index 8af66461f005..a8040dc0110e 100644
--- a/llvm/unittests/Support/CrashRecoveryTest.cpp
+++ b/llvm/unittests/Support/CrashRecoveryTest.cpp
@@ -123,4 +123,11 @@ TEST(CrashRecoveryTest, CallOutputDebugString) {
   EXPECT_TRUE(CrashRecoveryContext().RunSafely(outputString));
 }
 
+TEST(CrashRecoveryTest, Abort) {
+  llvm::CrashRecoveryContext::Enable();
+  auto A = []() { abort(); };
+  EXPECT_FALSE(CrashRecoveryContext().RunSafely(A));
+  // Test a second time to ensure we reinstall the abort signal handler.
+  EXPECT_FALSE(CrashRecoveryContext().RunSafely(A));
+}
 #endif