[llvm] r303279 - Re-land r303274: "[CrashRecovery] Use SEH __try instead of VEH when available"
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed May 17 13:13:51 PDT 2017
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5002/steps/check-llvm%20asan/logs/stdio
SAN:DEADLYSIGNAL
=================================================================
==24802==ERROR: AddressSanitizer: SEGV on unknown address
0x000000000000 (pc 0x000000660cef bp 0x7fffa59551f0 sp 0x7fffa59551f0
T0)
==24802==The signal is caused by a WRITE memory access.
==24802==Hint: address points to the zero page.
#0 0x660cee in nullDeref()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/unittests/Support/CrashRecoveryTest.cpp:24:52
#1 0xc7e51b in operator()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/STLExtras.h:111:12
#2 0xc7e51b in
llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Support/CrashRecoveryContext.cpp:359
#3 0x6614a1 in CrashRecoveryTest_Cleanup_Test::TestBody()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/unittests/Support/CrashRecoveryTest.cpp:59:5
#4 0xdc9a3e in HandleExceptionsInMethodIfSupported<testing::Test,
void> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2458:12
#5 0xdc9a3e in testing::Test::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2474
#6 0xdce174 in testing::TestInfo::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
#7 0xdcf4a6 in testing::TestCase::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
#8 0xdef336 in testing::internal::UnitTestImpl::RunAllTests()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
#9 0xdee692 in
HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:2458:12
#10 0xdee692 in testing::UnitTest::Run()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/src/gtest.cc:4257
#11 0xdad59b in RUN_ALL_TESTS
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
#12 0xdad59b in main
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/utils/unittest/UnitTestMain/TestMain.cpp:51
#13 0x7ff81ab8982f in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#14 0x485d98 in _start
(/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/unittests/Support/SupportTests+0x485d98)
On Wed, May 17, 2017 at 11:29 AM Reid Kleckner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: rnk
> Date: Wed May 17 13:16:17 2017
> New Revision: 303279
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303279&view=rev
> Log:
> Re-land r303274: "[CrashRecovery] Use SEH __try instead of VEH when
> available"
>
> We have to check gCrashRecoveryEnabled before using __try.
>
> In other words, SEH works too well and we ended up recovering from
> crashes in implicit module builds that we weren't supposed to. Only
> libclang is supposed to enable CrashRecoveryContext to allow implicit
> module builds to crash.
>
> Added:
> 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=303279&r1=303278&r2=303279&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/CrashRecoveryContext.cpp (original)
> +++ llvm/trunk/lib/Support/CrashRecoveryContext.cpp Wed May 17 13:16:17
> 2017
> @@ -78,6 +78,9 @@ static bool gCrashRecoveryEnabled = fals
> static ManagedStatic<sys::ThreadLocal<const CrashRecoveryContext>>
> tlIsRecoveringFromCrash;
>
> +static void installExceptionOrSignalHandlers();
> +static void uninstallExceptionOrSignalHandlers();
> +
> CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
>
> CrashRecoveryContext::~CrashRecoveryContext() {
> @@ -113,6 +116,23 @@ 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)
> @@ -140,27 +160,56 @@ CrashRecoveryContext::unregisterCleanup(
> delete cleanup;
> }
>
> -#ifdef LLVM_ON_WIN32
> +#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.
>
> -#include "Windows/WindowsSupport.h"
> +static void installExceptionOrSignalHandlers() {}
> +static void uninstallExceptionOrSignalHandlers() {}
> +
> +bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
> + if (!gCrashRecoveryEnabled) {
> + Fn();
> + return true;
> + }
> +
> + bool Result = true;
> + __try {
> + Fn();
> + } __except (1) { // Catch any exception.
> + Result = false;
> + }
> + return Result;
> +}
> +
> +#else // !_MSC_VER
>
> -// 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.
> +#if defined(LLVM_ON_WIN32)
> +// This is a non-MSVC compiler, probably mingw gcc or clang without
> +// -fms-extensions. Use vectored exception handling (VEH).
> //
> -// Vectored exception handling is built on top of SEH, and so it
> -// works on a per-thread basis.
> +// 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.
> +//
> +// 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.
> //
> // 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.
> -//
> -// 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).
> +
> +#include "Windows/WindowsSupport.h"
>
> static LONG CALLBACK ExceptionHandler(PEXCEPTION_POINTERS ExceptionInfo)
> {
> @@ -203,14 +252,7 @@ static LONG CALLBACK ExceptionHandler(PE
> // non-NULL, valid VEH handles, or NULL.
> static sys::ThreadLocal<const void> sCurrentExceptionHandle;
>
> -void CrashRecoveryContext::Enable() {
> - sys::ScopedLock L(*gCrashRecoveryContextMutex);
> -
> - if (gCrashRecoveryEnabled)
> - return;
> -
> - gCrashRecoveryEnabled = true;
> -
> +static void installExceptionOrSignalHandlers() {
> // 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
> @@ -219,14 +261,7 @@ void CrashRecoveryContext::Enable() {
> sCurrentExceptionHandle.set(handle);
> }
>
> -void CrashRecoveryContext::Disable() {
> - sys::ScopedLock L(*gCrashRecoveryContextMutex);
> -
> - if (!gCrashRecoveryEnabled)
> - return;
> -
> - gCrashRecoveryEnabled = false;
> -
> +static void uninstallExceptionOrSignalHandlers() {
> PVOID currentHandle = const_cast<PVOID>(sCurrentExceptionHandle.get());
> if (currentHandle) {
> // Now we can remove the vectored exception handler from the chain
> @@ -237,7 +272,7 @@ void CrashRecoveryContext::Disable() {
> }
> }
>
> -#else
> +#else // !LLVM_ON_WIN32
>
> // Generic POSIX implementation.
> //
> @@ -289,14 +324,7 @@ static void CrashRecoverySignalHandler(i
> const_cast<CrashRecoveryContextImpl*>(CRCI)->HandleCrash();
> }
>
> -void CrashRecoveryContext::Enable() {
> - sys::ScopedLock L(*gCrashRecoveryContextMutex);
> -
> - if (gCrashRecoveryEnabled)
> - return;
> -
> - gCrashRecoveryEnabled = true;
> -
> +static void installExceptionOrSignalHandlers() {
> // Setup the signal handler.
> struct sigaction Handler;
> Handler.sa_handler = CrashRecoverySignalHandler;
> @@ -308,20 +336,13 @@ void CrashRecoveryContext::Enable() {
> }
> }
>
> -void CrashRecoveryContext::Disable() {
> - sys::ScopedLock L(*gCrashRecoveryContextMutex);
> -
> - if (!gCrashRecoveryEnabled)
> - return;
> -
> - gCrashRecoveryEnabled = false;
> -
> +static void uninstallExceptionOrSignalHandlers() {
> // Restore the previous signal handlers.
> for (unsigned i = 0; i != NumSignals; ++i)
> sigaction(Signals[i], &PrevActions[i], nullptr);
> }
>
> -#endif
> +#endif // !LLVM_ON_WIN32
>
> bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
> // If crash recovery is disabled, do nothing.
> @@ -339,6 +360,8 @@ 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=303279&r1=303278&r2=303279&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/CMakeLists.txt (original)
> +++ llvm/trunk/unittests/Support/CMakeLists.txt Wed May 17 13:16:17 2017
> @@ -11,6 +11,7 @@ add_llvm_unittest(SupportTests
> BlockFrequencyTest.cpp
> BranchProbabilityTest.cpp
> CachePruningTest.cpp
> + CrashRecoveryTest.cpp
> Casting.cpp
> Chrono.cpp
> CommandLineTest.cpp
>
> Added: llvm/trunk/unittests/Support/CrashRecoveryTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CrashRecoveryTest.cpp?rev=303279&view=auto
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/CrashRecoveryTest.cpp (added)
> +++ llvm/trunk/unittests/Support/CrashRecoveryTest.cpp Wed May 17 13:16:17
> 2017
> @@ -0,0 +1,83 @@
> +//===- 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170517/274ea6cc/attachment.html>
More information about the llvm-commits
mailing list