[llvm] r303275 - Revert "[CrashRecovery] Use SEH __try instead of VEH when available"
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Wed May 17 10:15:00 PDT 2017
Author: rnk
Date: Wed May 17 12:15:00 2017
New Revision: 303275
URL: http://llvm.org/viewvc/llvm-project?rev=303275&view=rev
Log:
Revert "[CrashRecovery] Use SEH __try instead of VEH when available"
This reverts commit r303274, it appears to break some clang tests.
Removed:
llvm/trunk/unittests/Support/CrashRecoveryTest.cpp
Modified:
llvm/trunk/lib/Support/CrashRecoveryContext.cpp
llvm/trunk/unittests/Support/CMakeLists.txt
Modified: llvm/trunk/lib/Support/CrashRecoveryContext.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CrashRecoveryContext.cpp?rev=303275&r1=303274&r2=303275&view=diff
==============================================================================
--- llvm/trunk/lib/Support/CrashRecoveryContext.cpp (original)
+++ llvm/trunk/lib/Support/CrashRecoveryContext.cpp Wed May 17 12:15:00 2017
@@ -78,9 +78,6 @@ static bool gCrashRecoveryEnabled = fals
static ManagedStatic<sys::ThreadLocal<const CrashRecoveryContext>>
tlIsRecoveringFromCrash;
-static void installExceptionOrSignalHandlers();
-static void uninstallExceptionOrSignalHandlers();
-
CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
CrashRecoveryContext::~CrashRecoveryContext() {
@@ -116,23 +113,6 @@ CrashRecoveryContext *CrashRecoveryConte
return CRCI->CRC;
}
-void CrashRecoveryContext::Enable() {
- sys::ScopedLock L(*gCrashRecoveryContextMutex);
- // FIXME: Shouldn't this be a refcount or something?
- if (gCrashRecoveryEnabled)
- return;
- gCrashRecoveryEnabled = true;
- installExceptionOrSignalHandlers();
-}
-
-void CrashRecoveryContext::Disable() {
- sys::ScopedLock L(*gCrashRecoveryContextMutex);
- if (!gCrashRecoveryEnabled)
- return;
- gCrashRecoveryEnabled = false;
- uninstallExceptionOrSignalHandlers();
-}
-
void CrashRecoveryContext::registerCleanup(CrashRecoveryContextCleanup *cleanup)
{
if (!cleanup)
@@ -160,51 +140,27 @@ CrashRecoveryContext::unregisterCleanup(
delete cleanup;
}
-#if defined(_MSC_VER)
-// If _MSC_VER is defined, we must have SEH. Use it if it's available. It's way
-// better than VEH. Vectored exception handling catches all exceptions happening
-// on the thread with installed exception handlers, so it can interfere with
-// internal exception handling of other libraries on that thread. SEH works
-// exactly as you would expect normal exception handling to work: it only
-// catches exceptions if they would bubble out from the stack frame with __try /
-// __except.
+#ifdef LLVM_ON_WIN32
-static void installExceptionOrSignalHandlers() {}
-static void uninstallExceptionOrSignalHandlers() {}
-
-bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
- bool Result = true;
- __try {
- Fn();
- } __except (1) { // Catch any exception.
- Result = false;
- }
- return Result;
-}
-
-#else // !_MSC_VER
+#include "Windows/WindowsSupport.h"
-#if defined(LLVM_ON_WIN32)
-// This is a non-MSVC compiler, probably mingw gcc or clang without
-// -fms-extensions. Use vectored exception handling (VEH).
-//
-// On Windows, we can make use of vectored exception handling to catch most
-// crashing situations. Note that this does mean we will be alerted of
-// exceptions *before* structured exception handling has the opportunity to
-// catch it. Unfortunately, this causes problems in practice with other code
-// running on threads with LLVM crash recovery contexts, so we would like to
-// eventually move away from VEH.
+// On Windows, we can make use of vectored exception handling to
+// catch most crashing situations. Note that this does mean
+// we will be alerted of exceptions *before* structured exception
+// handling has the opportunity to catch it. But that isn't likely
+// to cause problems because nowhere in the project is SEH being
+// used.
//
-// Vectored works on a per-thread basis, which is an advantage over
-// SetUnhandledExceptionFilter. SetUnhandledExceptionFilter also doesn't have
-// any native support for chaining exception handlers, but VEH allows more than
-// one.
+// Vectored exception handling is built on top of SEH, and so it
+// works on a per-thread basis.
//
// The vectored exception handler functionality was added in Windows
// XP, so if support for older versions of Windows is required,
// it will have to be added.
-
-#include "Windows/WindowsSupport.h"
+//
+// If we want to support as far back as Win2k, we could use the
+// SetUnhandledExceptionFilter API, but there's a risk of that
+// being entirely overwritten (it's not a chain).
static LONG CALLBACK ExceptionHandler(PEXCEPTION_POINTERS ExceptionInfo)
{
@@ -247,7 +203,14 @@ static LONG CALLBACK ExceptionHandler(PE
// non-NULL, valid VEH handles, or NULL.
static sys::ThreadLocal<const void> sCurrentExceptionHandle;
-static void installExceptionOrSignalHandlers() {
+void CrashRecoveryContext::Enable() {
+ sys::ScopedLock L(*gCrashRecoveryContextMutex);
+
+ if (gCrashRecoveryEnabled)
+ return;
+
+ gCrashRecoveryEnabled = true;
+
// We can set up vectored exception handling now. We will install our
// handler as the front of the list, though there's no assurances that
// it will remain at the front (another call could install itself before
@@ -256,7 +219,14 @@ static void installExceptionOrSignalHand
sCurrentExceptionHandle.set(handle);
}
-static void uninstallExceptionOrSignalHandlers() {
+void CrashRecoveryContext::Disable() {
+ sys::ScopedLock L(*gCrashRecoveryContextMutex);
+
+ if (!gCrashRecoveryEnabled)
+ return;
+
+ gCrashRecoveryEnabled = false;
+
PVOID currentHandle = const_cast<PVOID>(sCurrentExceptionHandle.get());
if (currentHandle) {
// Now we can remove the vectored exception handler from the chain
@@ -267,7 +237,7 @@ static void uninstallExceptionOrSignalHa
}
}
-#else // !LLVM_ON_WIN32
+#else
// Generic POSIX implementation.
//
@@ -319,7 +289,14 @@ static void CrashRecoverySignalHandler(i
const_cast<CrashRecoveryContextImpl*>(CRCI)->HandleCrash();
}
-static void installExceptionOrSignalHandlers() {
+void CrashRecoveryContext::Enable() {
+ sys::ScopedLock L(*gCrashRecoveryContextMutex);
+
+ if (gCrashRecoveryEnabled)
+ return;
+
+ gCrashRecoveryEnabled = true;
+
// Setup the signal handler.
struct sigaction Handler;
Handler.sa_handler = CrashRecoverySignalHandler;
@@ -331,13 +308,20 @@ static void installExceptionOrSignalHand
}
}
-static void uninstallExceptionOrSignalHandlers() {
+void CrashRecoveryContext::Disable() {
+ sys::ScopedLock L(*gCrashRecoveryContextMutex);
+
+ if (!gCrashRecoveryEnabled)
+ return;
+
+ gCrashRecoveryEnabled = false;
+
// Restore the previous signal handlers.
for (unsigned i = 0; i != NumSignals; ++i)
sigaction(Signals[i], &PrevActions[i], nullptr);
}
-#endif // !LLVM_ON_WIN32
+#endif
bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
// If crash recovery is disabled, do nothing.
@@ -355,8 +339,6 @@ bool CrashRecoveryContext::RunSafely(fun
return true;
}
-#endif // !_MSC_VER
-
void CrashRecoveryContext::HandleCrash() {
CrashRecoveryContextImpl *CRCI = (CrashRecoveryContextImpl *) Impl;
assert(CRCI && "Crash recovery context never initialized!");
Modified: llvm/trunk/unittests/Support/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CMakeLists.txt?rev=303275&r1=303274&r2=303275&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CMakeLists.txt (original)
+++ llvm/trunk/unittests/Support/CMakeLists.txt Wed May 17 12:15:00 2017
@@ -11,7 +11,6 @@ add_llvm_unittest(SupportTests
BlockFrequencyTest.cpp
BranchProbabilityTest.cpp
CachePruningTest.cpp
- CrashRecoveryTest.cpp
Casting.cpp
Chrono.cpp
CommandLineTest.cpp
Removed: llvm/trunk/unittests/Support/CrashRecoveryTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CrashRecoveryTest.cpp?rev=303274&view=auto
==============================================================================
--- llvm/trunk/unittests/Support/CrashRecoveryTest.cpp (original)
+++ llvm/trunk/unittests/Support/CrashRecoveryTest.cpp (removed)
@@ -1,83 +0,0 @@
-//===- llvm/unittest/Support/CrashRecoveryTest.cpp ------------------------===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "llvm/Support/CrashRecoveryContext.h"
-#include "llvm/Support/Compiler.h"
-#include "gtest/gtest.h"
-
-#ifdef LLVM_ON_WIN32
-#define WIN32_LEAN_AND_MEAN
-#define NOGDI
-#include <windows.h>
-#endif
-
-using namespace llvm;
-using namespace llvm::sys;
-
-static int GlobalInt = 0;
-static void nullDeref() { *(volatile int *)nullptr = 0; }
-static void incrementGlobal() { ++GlobalInt; }
-static void llvmTrap() { LLVM_BUILTIN_TRAP; }
-
-TEST(CrashRecoveryTest, Basic) {
- llvm::CrashRecoveryContext::Enable();
- GlobalInt = 0;
- EXPECT_TRUE(CrashRecoveryContext().RunSafely(incrementGlobal));
- EXPECT_EQ(1, GlobalInt);
- EXPECT_FALSE(CrashRecoveryContext().RunSafely(nullDeref));
- EXPECT_FALSE(CrashRecoveryContext().RunSafely(llvmTrap));
-}
-
-struct IncrementGlobalCleanup : CrashRecoveryContextCleanup {
- IncrementGlobalCleanup(CrashRecoveryContext *CRC)
- : CrashRecoveryContextCleanup(CRC) {}
- virtual void recoverResources() { ++GlobalInt; }
-};
-
-static void noop() {}
-
-TEST(CrashRecoveryTest, Cleanup) {
- llvm::CrashRecoveryContext::Enable();
- GlobalInt = 0;
- {
- CrashRecoveryContext CRC;
- CRC.registerCleanup(new IncrementGlobalCleanup(&CRC));
- EXPECT_TRUE(CRC.RunSafely(noop));
- } // run cleanups
- EXPECT_EQ(1, GlobalInt);
-
- GlobalInt = 0;
- {
- CrashRecoveryContext CRC;
- CRC.registerCleanup(new IncrementGlobalCleanup(&CRC));
- EXPECT_FALSE(CRC.RunSafely(nullDeref));
- } // run cleanups
- EXPECT_EQ(1, GlobalInt);
-}
-
-#ifdef LLVM_ON_WIN32
-static void raiseIt() {
- RaiseException(123, EXCEPTION_NONCONTINUABLE, 0, NULL);
-}
-
-TEST(CrashRecoveryTest, RaiseException) {
- llvm::CrashRecoveryContext::Enable();
- EXPECT_FALSE(CrashRecoveryContext().RunSafely(raiseIt));
-}
-
-static void outputString() {
- OutputDebugStringA("output for debugger\n");
-}
-
-TEST(CrashRecoveryTest, CallOutputDebugString) {
- llvm::CrashRecoveryContext::Enable();
- EXPECT_TRUE(CrashRecoveryContext().RunSafely(outputString));
-}
-
-#endif
More information about the llvm-commits
mailing list