From 7038cfdeb327d9f4ff74b350740a1184befa1312 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Wed, 10 Jun 2020 20:28:45 -0700 Subject: [PATCH 1/7] handle C++ exceptions, unpacking the internal structures to get the variable type (and any superclasses) being thrown --- interface/src/CrashHandler_Crashpad.cpp | 103 +++++++++++++++++++++++- 1 file changed, 100 insertions(+), 3 deletions(-) diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index 900a296955..6ce8d170e2 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -16,11 +16,13 @@ #include #include +#include #include -#include -#include -#include +#include +#include +#include +#include #if defined(__clang__) @@ -59,6 +61,10 @@ static const QString CRASHPAD_HANDLER_NAME { "crashpad_handler" }; #ifdef Q_OS_WIN #include +#include + +void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo); + LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { if (!client) { @@ -70,8 +76,99 @@ LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { client->DumpAndCrash(pExceptionInfo); } + if (pExceptionInfo->ExceptionRecord->ExceptionCode == 0xE06D7363) { + fatalCxxException(pExceptionInfo); + client->DumpAndCrash(pExceptionInfo); + } + return EXCEPTION_CONTINUE_SEARCH; } + +#pragma pack(push, ehdata, 4) + +struct PMD_internal { + int mdisp; + int pdisp; + int vdisp; +}; + +struct ThrowInfo_internal { + __int32 attributes; + __int32 pmfnUnwind; // 32-bit RVA + __int32 pForwardCompat; // 32-bit RVA + __int32 pCatchableTypeArray; // 32-bit RVA +}; + +struct CatchableType_internal { + __int32 properties; + __int32 pType; // 32-bit RVA + PMD_internal thisDisplacement; + __int32 sizeOrOffset; + __int32 copyFunction; // 32-bit RVA +}; + +#pragma warning(disable : 4200) +struct CatchableTypeArray_internal { + int nCatchableTypes; + __int32 arrayOfCatchableTypes[0]; // 32-bit RVA +}; +#pragma warning(default : 4200) + +#pragma pack(pop, ehdata) + +// everything inside this function is extremely undocumented, attempting to extract +// the underlying C++ exception type (or at least its name) before throwing the whole +// mess at crashpad +void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { + PEXCEPTION_RECORD ExceptionRecord = pExceptionInfo->ExceptionRecord; + + if(ExceptionRecord->NumberParameters != 4 || ExceptionRecord->ExceptionInformation[0] != 0x19930520) { + // doesn't match expected parameter counts or magic numbers + return; + } + + //ULONG_PTR signature = ExceptionRecord->ExceptionInformation[0]; + void* pExceptionObject = reinterpret_cast(ExceptionRecord->ExceptionInformation[1]); // the object that generated the exception + ThrowInfo_internal* pThrowInfo = reinterpret_cast(ExceptionRecord->ExceptionInformation[2]); + ULONG_PTR moduleBase = ExceptionRecord->ExceptionInformation[3]; + if(moduleBase == 0 || pThrowInfo == NULL) { + return; // broken assumption + } + + // now we start breaking the pThrowInfo internal structure apart + if(pThrowInfo->pCatchableTypeArray == 0) { + return; // broken assumption + } + CatchableTypeArray_internal* pCatchableTypeArray = reinterpret_cast(moduleBase + pThrowInfo->pCatchableTypeArray); + if(pCatchableTypeArray->nCatchableTypes == 0 || pCatchableTypeArray->arrayOfCatchableTypes[0] == 0) { + return; // broken assumption + } + CatchableType_internal* pCatchableType = reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[0]); + if(pCatchableType->pType == 0) { + return; // broken assumption + } + const std::type_info* type = reinterpret_cast(moduleBase + pCatchableType->pType); + + // we're crashing, not really sure it matters who's currently holding the lock (although this could in theory really mess things up + annotationMutex.try_lock(); + crashpadAnnotations->SetKeyValue("thrownObject", type->name()); + + QString compatibleObjects; + for (int catchTypeIdx = 1; catchTypeIdx < pCatchableTypeArray->nCatchableTypes; catchTypeIdx++) { + CatchableType_internal* pCatchableSuperclassType = reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[catchTypeIdx]); + if (pCatchableSuperclassType->pType == 0) { + return; // broken assumption + } + const std::type_info* superclassType = reinterpret_cast(moduleBase + pCatchableSuperclassType->pType); + + if (!compatibleObjects.isEmpty()) { + compatibleObjects += ", "; + } + compatibleObjects += superclassType->name(); + } + crashpadAnnotations->SetKeyValue("thrownObjectLike", compatibleObjects.toStdString()); +} + #endif bool startCrashHandler(std::string appPath) { From 9af8b6b452e3e03f5e9801bcd0a5ff1dae68a122 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Sat, 13 Jun 2020 16:32:17 -0700 Subject: [PATCH 2/7] documenting the magic numbers a bit better (or just giving them names) --- interface/src/CrashHandler_Crashpad.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index 6ce8d170e2..2ddcd3f398 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -49,6 +49,9 @@ using namespace crashpad; static const std::string BACKTRACE_URL { CMAKE_BACKTRACE_URL }; static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN }; +static constexpr DWORD STATUS_MSVC_CPP_EXCEPTION = 0xE06D7363; +static constexpr ULONG_PTR MSVC_CPP_EXCEPTION_SIGNATURE = 0x19930520; + CrashpadClient* client { nullptr }; std::mutex annotationMutex; crashpad::SimpleStringDictionary* crashpadAnnotations { nullptr }; @@ -76,7 +79,7 @@ LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { client->DumpAndCrash(pExceptionInfo); } - if (pExceptionInfo->ExceptionRecord->ExceptionCode == 0xE06D7363) { + if (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_MSVC_CPP_EXCEPTION) { fatalCxxException(pExceptionInfo); client->DumpAndCrash(pExceptionInfo); } @@ -121,21 +124,27 @@ struct CatchableTypeArray_internal { // mess at crashpad void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { PEXCEPTION_RECORD ExceptionRecord = pExceptionInfo->ExceptionRecord; + /* + Exception arguments for Microsoft C++ exceptions: + [0] signature - magic number + [1] void* - variable that is being thrown + [2] ThrowInfo* - description of the variable that was thrown + [3] HMODULE - (64-bit only) base address that all 32bit pointers are added to + */ - if(ExceptionRecord->NumberParameters != 4 || ExceptionRecord->ExceptionInformation[0] != 0x19930520) { + if (ExceptionRecord->NumberParameters != 4 || ExceptionRecord->ExceptionInformation[0] != MSVC_CPP_EXCEPTION_SIGNATURE) { // doesn't match expected parameter counts or magic numbers return; } - //ULONG_PTR signature = ExceptionRecord->ExceptionInformation[0]; - void* pExceptionObject = reinterpret_cast(ExceptionRecord->ExceptionInformation[1]); // the object that generated the exception + // get the ThrowInfo struct from the exception arguments ThrowInfo_internal* pThrowInfo = reinterpret_cast(ExceptionRecord->ExceptionInformation[2]); ULONG_PTR moduleBase = ExceptionRecord->ExceptionInformation[3]; if(moduleBase == 0 || pThrowInfo == NULL) { return; // broken assumption } - // now we start breaking the pThrowInfo internal structure apart + // get the CatchableTypeArray* struct from ThrowInfo if(pThrowInfo->pCatchableTypeArray == 0) { return; // broken assumption } @@ -143,6 +152,8 @@ void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { if(pCatchableTypeArray->nCatchableTypes == 0 || pCatchableTypeArray->arrayOfCatchableTypes[0] == 0) { return; // broken assumption } + + // get the CatchableType struct for the actual exception type from CatchableTypeArray CatchableType_internal* pCatchableType = reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[0]); if(pCatchableType->pType == 0) { return; // broken assumption @@ -153,6 +164,8 @@ void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { annotationMutex.try_lock(); crashpadAnnotations->SetKeyValue("thrownObject", type->name()); + // After annotating the name of the actual object type, go through the other entries in CatcahleTypeArray and itemize the list of possible + // catch() commands that could have caught this so we can find the list of its superclasses QString compatibleObjects; for (int catchTypeIdx = 1; catchTypeIdx < pCatchableTypeArray->nCatchableTypes; catchTypeIdx++) { CatchableType_internal* pCatchableSuperclassType = reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[catchTypeIdx]); From d92bb16d0c3ea10d659b9dbce668696e8a283599 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Wed, 17 Jun 2020 00:58:57 -0700 Subject: [PATCH 3/7] replacing the use of std::mutex with a spinlock, which we can try to lock but exit if the attempt times out. --- interface/src/CrashHandler_Crashpad.cpp | 119 +++++++++++++++++++++--- 1 file changed, 104 insertions(+), 15 deletions(-) diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index 2ddcd3f398..b11a3e119e 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -15,16 +15,15 @@ #include -#include #include #include -#include +#include +#include #include #include #include - #if defined(__clang__) #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wc++14-extensions" @@ -33,7 +32,6 @@ #include #include #include -#include #include #if defined(__clang__) @@ -44,16 +42,96 @@ #include #include -using namespace crashpad; - static const std::string BACKTRACE_URL { CMAKE_BACKTRACE_URL }; static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN }; -static constexpr DWORD STATUS_MSVC_CPP_EXCEPTION = 0xE06D7363; -static constexpr ULONG_PTR MSVC_CPP_EXCEPTION_SIGNATURE = 0x19930520; +// ------------------------------------------------------------------------------------------------ +// SpinLock - a lock that can timeout attempting to lock a block of code, and is in a busy-wait cycle while trying to acquire +// note that this code will malfunction if you attempt to grab a lock while already holding it -CrashpadClient* client { nullptr }; -std::mutex annotationMutex; +class SpinLock { +public: + SpinLock(); + void lock(); + bool lock(int msecs); + void unlock(); +private: + QAtomicInteger _lock{ 0 }; + + Q_DISABLE_COPY(SpinLock) +}; + +class SpinLockLocker { +public: + SpinLockLocker(SpinLock& lock, int msecs = -1); + ~SpinLockLocker(); + bool isLocked() const; + void unlock(); + bool relock(int msecs = -1); + +private: + SpinLock* _lock; + bool _isLocked; + + Q_DISABLE_COPY(SpinLockLocker) +}; + +SpinLock::SpinLock() { +} + +void SpinLock::lock() { + while (!_lock.testAndSetAcquire(0, 1)) + ; +} + +bool SpinLock::lock(int msecs) { + QDeadlineTimer deadline(msecs); + for (;;) { + if (_lock.testAndSetAcquire(0, 1)) { + return true; + } + if (deadline.hasExpired()) { + return false; + } + } +} + +void SpinLock::unlock() { + _lock.storeRelease(0); +} + +SpinLockLocker::SpinLockLocker(SpinLock& lock, int msecs /* = -1 */ ) : _lock(&lock) { + _isLocked = _lock->lock(msecs); +} + +SpinLockLocker::~SpinLockLocker() { + if (_isLocked) { + _lock->unlock(); + } +} + +bool SpinLockLocker::isLocked() const { + return _isLocked; +} + +void SpinLockLocker::unlock() { + if (_isLocked) { + _lock->unlock(); + _isLocked = false; + } +} + +bool SpinLockLocker::relock(int msecs /* = -1 */ ) { + if (!_isLocked) { + _isLocked = _lock->lock(msecs); + } + return _isLocked; +} + +// ------------------------------------------------------------------------------------------------ + +crashpad::CrashpadClient* client{ nullptr }; +SpinLock crashpadAnnotationsProtect; crashpad::SimpleStringDictionary* crashpadAnnotations { nullptr }; #if defined(Q_OS_WIN) @@ -63,12 +141,18 @@ static const QString CRASHPAD_HANDLER_NAME { "crashpad_handler" }; #endif #ifdef Q_OS_WIN +// ------------------------------------------------------------------------------------------------ +// The area within this #ifdef is specific to the Microsoft C++ compiler + +static constexpr DWORD STATUS_MSVC_CPP_EXCEPTION = 0xE06D7363; +static constexpr ULONG_PTR MSVC_CPP_EXCEPTION_SIGNATURE = 0x19930520; +static constexpr int ANNOTATION_LOCK_WEAK_ATTEMPT = 5000; // attempt to lock the annotations list, but give up if it takes more than 5 seconds + #include #include void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo); - LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { if (!client) { return EXCEPTION_CONTINUE_SEARCH; @@ -123,6 +207,11 @@ struct CatchableTypeArray_internal { // the underlying C++ exception type (or at least its name) before throwing the whole // mess at crashpad void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { + SpinLockLocker guard(crashpadAnnotationsProtect, ANNOTATION_LOCK_WEAK_ATTEMPT); + if (!guard.isLocked()) { + return; + } + PEXCEPTION_RECORD ExceptionRecord = pExceptionInfo->ExceptionRecord; /* Exception arguments for Microsoft C++ exceptions: @@ -160,8 +249,6 @@ void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { } const std::type_info* type = reinterpret_cast(moduleBase + pCatchableType->pType); - // we're crashing, not really sure it matters who's currently holding the lock (although this could in theory really mess things up - annotationMutex.try_lock(); crashpadAnnotations->SetKeyValue("thrownObject", type->name()); // After annotating the name of the actual object type, go through the other entries in CatcahleTypeArray and itemize the list of possible @@ -182,6 +269,8 @@ void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { crashpadAnnotations->SetKeyValue("thrownObjectLike", compatibleObjects.toStdString()); } +// End of code specific to the Microsoft C++ compiler +// ------------------------------------------------------------------------------------------------ #endif bool startCrashHandler(std::string appPath) { @@ -190,7 +279,7 @@ bool startCrashHandler(std::string appPath) { } assert(!client); - client = new CrashpadClient(); + client = new crashpad::CrashpadClient(); std::vector arguments; std::map annotations; @@ -243,7 +332,7 @@ bool startCrashHandler(std::string appPath) { void setCrashAnnotation(std::string name, std::string value) { if (client) { - std::lock_guard guard(annotationMutex); + SpinLockLocker guard(crashpadAnnotationsProtect); if (!crashpadAnnotations) { crashpadAnnotations = new crashpad::SimpleStringDictionary(); // don't free this, let it leak crashpad::CrashpadInfo* crashpad_info = crashpad::CrashpadInfo::GetCrashpadInfo(); From b6e1c9e3afd97a6e0722d6296916be80a9ef8417 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Fri, 19 Jun 2020 18:26:26 -0700 Subject: [PATCH 4/7] added "c++ crash" to crash menu --- interface/src/Menu.cpp | 7 ++++++- interface/src/Menu.h | 2 ++ libraries/shared/src/CrashHelpers.cpp | 10 +++++++--- libraries/shared/src/CrashHelpers.h | 1 + 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index a1bb670837..bbb71cbea3 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -705,7 +705,7 @@ Menu::Menu() { // Developer > Crash >>> bool result = false; const QString HIFI_SHOW_DEVELOPER_CRASH_MENU("HIFI_SHOW_DEVELOPER_CRASH_MENU"); - result = QProcessEnvironment::systemEnvironment().contains(HIFI_SHOW_DEVELOPER_CRASH_MENU); + result = true;//QProcessEnvironment::systemEnvironment().contains(HIFI_SHOW_DEVELOPER_CRASH_MENU); if (result) { MenuWrapper* crashMenu = developerMenu->addMenu("Crash"); @@ -745,6 +745,11 @@ Menu::Menu() { action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNewFaultThreaded); connect(action, &QAction::triggered, qApp, []() { std::thread(crash::newFault).join(); }); + action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashThrownException); + connect(action, &QAction::triggered, qApp, []() { crash::throwException(); }); + action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashThrownExceptionThreaded); + connect(action, &QAction::triggered, qApp, []() { std::thread(crash::throwException).join(); }); + addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashOnShutdown, 0, qApp, SLOT(crashOnShutdown())); } diff --git a/interface/src/Menu.h b/interface/src/Menu.h index a3da179ad3..d33b3b0f5e 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -77,6 +77,8 @@ namespace MenuOption { const QString CrashOutOfBoundsVectorAccessThreaded = "Out of Bounds Vector Access (threaded)"; const QString CrashNewFault = "New Fault"; const QString CrashNewFaultThreaded = "New Fault (threaded)"; + const QString CrashThrownException = "Thrown C++ exception"; + const QString CrashThrownExceptionThreaded = "Thrown C++ exception (threaded)"; const QString CreateEntitiesGrabbable = "Create Entities As Grabbable (except Zones, Particles, and Lights)"; const QString DeadlockInterface = "Deadlock Interface"; const QString UnresponsiveInterface = "Unresponsive Interface"; diff --git a/libraries/shared/src/CrashHelpers.cpp b/libraries/shared/src/CrashHelpers.cpp index 1676318f3e..8a1a6df1d2 100644 --- a/libraries/shared/src/CrashHelpers.cpp +++ b/libraries/shared/src/CrashHelpers.cpp @@ -19,7 +19,7 @@ #else #include #endif - +#include namespace crash { @@ -86,7 +86,11 @@ void newFault() { const size_t GIGABYTE = 1024 * 1024 * 1024; new char[GIGABYTE]; } +} + +void throwException() { + qCDebug(shared) << "About to throw an exception"; + throw std::runtime_error("unexpected exception"); +} } - -} diff --git a/libraries/shared/src/CrashHelpers.h b/libraries/shared/src/CrashHelpers.h index 247aea5cde..ccda319819 100644 --- a/libraries/shared/src/CrashHelpers.h +++ b/libraries/shared/src/CrashHelpers.h @@ -25,6 +25,7 @@ void nullDeref(); void doAbort(); void outOfBoundsVectorCrash(); void newFault(); +void throwException(); } From 033726fc644fc77e25dc3b08fa8cfbd183c39f9e Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Fri, 19 Jun 2020 18:28:01 -0700 Subject: [PATCH 5/7] realizing the current logic is catching first-chance exceptions --- interface/src/CrashHandler_Crashpad.cpp | 31 +++++++++++++++++-------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index b11a3e119e..89984cd398 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -148,26 +148,32 @@ static constexpr DWORD STATUS_MSVC_CPP_EXCEPTION = 0xE06D7363; static constexpr ULONG_PTR MSVC_CPP_EXCEPTION_SIGNATURE = 0x19930520; static constexpr int ANNOTATION_LOCK_WEAK_ATTEMPT = 5000; // attempt to lock the annotations list, but give up if it takes more than 5 seconds +LPTOP_LEVEL_EXCEPTION_FILTER gl_nextUnhandledExceptionFilter = nullptr; + #include #include void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo); -LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { - if (!client) { - return EXCEPTION_CONTINUE_SEARCH; - } - - if (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_HEAP_CORRUPTION || - pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN) { +LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { + if (client && (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_HEAP_CORRUPTION || + pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN)) { client->DumpAndCrash(pExceptionInfo); } - if (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_MSVC_CPP_EXCEPTION) { + return EXCEPTION_CONTINUE_SEARCH; +} + +LONG WINAPI unhandledExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { + if (client && pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_MSVC_CPP_EXCEPTION) { fatalCxxException(pExceptionInfo); client->DumpAndCrash(pExceptionInfo); } + if (gl_nextUnhandledExceptionFilter != nullptr) { + return gl_nextUnhandledExceptionFilter(pExceptionInfo); + } + return EXCEPTION_CONTINUE_SEARCH; } @@ -323,11 +329,16 @@ bool startCrashHandler(std::string appPath) { // Enable automated uploads. database->GetSettings()->SetUploadsEnabled(true); + if (!client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true)) { + return false; + } + #ifdef Q_OS_WIN - AddVectoredExceptionHandler(0, vectoredExceptionHandler); + AddVectoredExceptionHandler(0, firstChanceExceptionHandler); + gl_nextUnhandledExceptionFilter = SetUnhandledExceptionFilter(unhandledExceptionHandler); #endif - return client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true); + return true; } void setCrashAnnotation(std::string name, std::string value) { From af6d2b2e567cae91d277cec484280e7262c33798 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Sat, 20 Jun 2020 20:54:24 -0700 Subject: [PATCH 6/7] added timer to detect if the unhandle exception hook has been broken added internal documentation on the structures and functions involved --- interface/src/CrashHandler.h | 2 + interface/src/CrashHandler_Breakpad.cpp | 3 + interface/src/CrashHandler_Crashpad.cpp | 78 +++++++++++++++++++------ interface/src/CrashHandler_None.cpp | 3 + interface/src/main.cpp | 1 + 5 files changed, 69 insertions(+), 18 deletions(-) diff --git a/interface/src/CrashHandler.h b/interface/src/CrashHandler.h index 6f8e9c3bf6..5b7e3b4438 100644 --- a/interface/src/CrashHandler.h +++ b/interface/src/CrashHandler.h @@ -13,8 +13,10 @@ #define hifi_CrashHandler_h #include +class QCoreApplication; bool startCrashHandler(std::string appPath); void setCrashAnnotation(std::string name, std::string value); +void startCrashHookMonitor(QCoreApplication* app); #endif // hifi_CrashHandler_h diff --git a/interface/src/CrashHandler_Breakpad.cpp b/interface/src/CrashHandler_Breakpad.cpp index c21bfa95e0..839e99ad3d 100644 --- a/interface/src/CrashHandler_Breakpad.cpp +++ b/interface/src/CrashHandler_Breakpad.cpp @@ -81,4 +81,7 @@ void setCrashAnnotation(std::string name, std::string value) { flushAnnotations(); } +void startCrashHookMonitor(QCoreApplication* app) { +} + #endif diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index 89984cd398..9671facf17 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -144,18 +144,27 @@ static const QString CRASHPAD_HANDLER_NAME { "crashpad_handler" }; // ------------------------------------------------------------------------------------------------ // The area within this #ifdef is specific to the Microsoft C++ compiler -static constexpr DWORD STATUS_MSVC_CPP_EXCEPTION = 0xE06D7363; -static constexpr ULONG_PTR MSVC_CPP_EXCEPTION_SIGNATURE = 0x19930520; -static constexpr int ANNOTATION_LOCK_WEAK_ATTEMPT = 5000; // attempt to lock the annotations list, but give up if it takes more than 5 seconds - -LPTOP_LEVEL_EXCEPTION_FILTER gl_nextUnhandledExceptionFilter = nullptr; +#include +#include +#include +#include #include #include -void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo); +static constexpr DWORD STATUS_MSVC_CPP_EXCEPTION = 0xE06D7363; +static constexpr ULONG_PTR MSVC_CPP_EXCEPTION_SIGNATURE = 0x19930520; +static constexpr int ANNOTATION_LOCK_WEAK_ATTEMPT = 5000; // attempt to lock the annotations list, but give up if it takes more than 5 seconds -LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { +LPTOP_LEVEL_EXCEPTION_FILTER gl_crashpadUnhandledExceptionFilter = nullptr; +QTimer unhandledExceptionTimer; // checks occasionally in case loading an external DLL reset the unhandled exception pointer + +void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo); // extracts type information from a thrown C++ exception +LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); // called on any thrown exception (whether or not it's caught) +LONG WINAPI unhandledExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); // called on any exception without a corresponding catch + +static LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { + // we're catching these exceptions on first-chance as the system state is corrupted at this point and they may not survive the exception handling mechanism if (client && (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_HEAP_CORRUPTION || pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN)) { client->DumpAndCrash(pExceptionInfo); @@ -164,35 +173,41 @@ LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { return EXCEPTION_CONTINUE_SEARCH; } -LONG WINAPI unhandledExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { +static LONG WINAPI unhandledExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { if (client && pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_MSVC_CPP_EXCEPTION) { fatalCxxException(pExceptionInfo); client->DumpAndCrash(pExceptionInfo); } - if (gl_nextUnhandledExceptionFilter != nullptr) { - return gl_nextUnhandledExceptionFilter(pExceptionInfo); + if (gl_crashpadUnhandledExceptionFilter != nullptr) { + return gl_crashpadUnhandledExceptionFilter(pExceptionInfo); } return EXCEPTION_CONTINUE_SEARCH; } +// The following structures are modified versions of structs defined inplicitly by the Microsoft C++ compiler +// as described at http://www.geoffchappell.com/studies/msvc/language/predefined/ +// They are redefined here as the definitions the compiler gives only work in 32-bit contexts and are out-of-sync +// with the internal structures when operating in a 64-bit environment +// as discovered and described here: https://stackoverflow.com/questions/39113168/c-rtti-in-a-windows-64-bit-vectoredexceptionhandler-ms-visual-studio-2015 + #pragma pack(push, ehdata, 4) -struct PMD_internal { +struct PMD_internal { // internal name: _PMD (no changes, so could in theory just use the original) int mdisp; int pdisp; int vdisp; }; -struct ThrowInfo_internal { +struct ThrowInfo_internal { // internal name: _ThrowInfo (changed all pointers into __int32) __int32 attributes; __int32 pmfnUnwind; // 32-bit RVA __int32 pForwardCompat; // 32-bit RVA __int32 pCatchableTypeArray; // 32-bit RVA }; -struct CatchableType_internal { +struct CatchableType_internal { // internal name: _CatchableType (changed all pointers into __int32) __int32 properties; __int32 pType; // 32-bit RVA PMD_internal thisDisplacement; @@ -201,7 +216,7 @@ struct CatchableType_internal { }; #pragma warning(disable : 4200) -struct CatchableTypeArray_internal { +struct CatchableTypeArray_internal { // internal name: _CatchableTypeArray (changed all pointers into __int32) int nCatchableTypes; __int32 arrayOfCatchableTypes[0]; // 32-bit RVA }; @@ -212,7 +227,12 @@ struct CatchableTypeArray_internal { // everything inside this function is extremely undocumented, attempting to extract // the underlying C++ exception type (or at least its name) before throwing the whole // mess at crashpad -void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { +// Some links describing how C++ exception handling works in an SEH context +// (since C++ exceptions are a figment of the Microsoft compiler): +// - https://www.codeproject.com/Articles/175482/Compiler-Internals-How-Try-Catch-Throw-are-Interpr +// - https://stackoverflow.com/questions/21888076/how-to-find-the-context-record-for-user-mode-exception-on-x64 + +static void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { SpinLockLocker guard(crashpadAnnotationsProtect, ANNOTATION_LOCK_WEAK_ATTEMPT); if (!guard.isLocked()) { return; @@ -275,9 +295,17 @@ void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { crashpadAnnotations->SetKeyValue("thrownObjectLike", compatibleObjects.toStdString()); } +void checkUnhandledExceptionHook() { + LPTOP_LEVEL_EXCEPTION_FILTER prevExceptionFilter = SetUnhandledExceptionFilter(unhandledExceptionHandler); + if (prevExceptionFilter != unhandledExceptionHandler) { + qWarning() << QString("Restored unhandled exception filter (which had been changed to %1)") + .arg(reinterpret_cast(prevExceptionFilter), 16, 16, QChar('0')); + } +} + // End of code specific to the Microsoft C++ compiler // ------------------------------------------------------------------------------------------------ -#endif +#endif // Q_OS_WIN bool startCrashHandler(std::string appPath) { if (BACKTRACE_URL.empty() || BACKTRACE_TOKEN.empty()) { @@ -335,7 +363,7 @@ bool startCrashHandler(std::string appPath) { #ifdef Q_OS_WIN AddVectoredExceptionHandler(0, firstChanceExceptionHandler); - gl_nextUnhandledExceptionFilter = SetUnhandledExceptionFilter(unhandledExceptionHandler); + gl_crashpadUnhandledExceptionFilter = SetUnhandledExceptionFilter(unhandledExceptionHandler); #endif return true; @@ -354,4 +382,18 @@ void setCrashAnnotation(std::string name, std::string value) { } } -#endif +void startCrashHookMonitor(QCoreApplication* app) { +#ifdef Q_OS_WIN + // create a timer that checks to see if our exception handler has been reset. This may occur when a new CRT + // is initialized, which could happen any time a DLL not compiled with the same compiler is loaded. + // It would be nice if this were replaced with a more intelligent response; this fires once a minute which + // may be too much (extra code running) and too little (leaving up to a 1min gap after the hook is broken) + checkUnhandledExceptionHook(); + + unhandledExceptionTimer.moveToThread(app->thread()); + QObject::connect(&unhandledExceptionTimer, &QTimer::timeout, checkUnhandledExceptionHook); + unhandledExceptionTimer.start(60000); +#endif // Q_OS_WIN +} + +#endif // HAS_CRASHPAD diff --git a/interface/src/CrashHandler_None.cpp b/interface/src/CrashHandler_None.cpp index 77b8ab332e..4fe56cd042 100644 --- a/interface/src/CrashHandler_None.cpp +++ b/interface/src/CrashHandler_None.cpp @@ -25,4 +25,7 @@ bool startCrashHandler(std::string appPath) { void setCrashAnnotation(std::string name, std::string value) { } +void startCrashHookMonitor(QCoreApplication* app) { +} + #endif diff --git a/interface/src/main.cpp b/interface/src/main.cpp index ae476b8142..bdbafbaeb8 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -381,6 +381,7 @@ int main(int argc, const char* argv[]) { #if defined(Q_OS_LINUX) app.setWindowIcon(QIcon(PathUtils::resourcesPath() + "images/vircadia-logo.svg")); #endif + startCrashHookMonitor(&app); QTimer exitTimer; if (traceDuration > 0.0f) { From 2f00c3e0bd873fde622ee465e3b6fbc0b4f24bed Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Fri, 24 Jul 2020 21:32:22 -0700 Subject: [PATCH 7/7] minor review of formatting --- interface/src/CrashHandler_Crashpad.cpp | 77 +++++++++++++------------ 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/interface/src/CrashHandler_Crashpad.cpp b/interface/src/CrashHandler_Crashpad.cpp index 9671facf17..a8195ce6e8 100644 --- a/interface/src/CrashHandler_Crashpad.cpp +++ b/interface/src/CrashHandler_Crashpad.cpp @@ -42,8 +42,8 @@ #include #include -static const std::string BACKTRACE_URL { CMAKE_BACKTRACE_URL }; -static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN }; +static const std::string BACKTRACE_URL{ CMAKE_BACKTRACE_URL }; +static const std::string BACKTRACE_TOKEN{ CMAKE_BACKTRACE_TOKEN }; // ------------------------------------------------------------------------------------------------ // SpinLock - a lock that can timeout attempting to lock a block of code, and is in a busy-wait cycle while trying to acquire @@ -55,6 +55,7 @@ public: void lock(); bool lock(int msecs); void unlock(); + private: QAtomicInteger _lock{ 0 }; @@ -132,12 +133,12 @@ bool SpinLockLocker::relock(int msecs /* = -1 */ ) { crashpad::CrashpadClient* client{ nullptr }; SpinLock crashpadAnnotationsProtect; -crashpad::SimpleStringDictionary* crashpadAnnotations { nullptr }; +crashpad::SimpleStringDictionary* crashpadAnnotations{ nullptr }; #if defined(Q_OS_WIN) -static const QString CRASHPAD_HANDLER_NAME { "crashpad_handler.exe" }; +static const QString CRASHPAD_HANDLER_NAME{ "crashpad_handler.exe" }; #else -static const QString CRASHPAD_HANDLER_NAME { "crashpad_handler" }; +static const QString CRASHPAD_HANDLER_NAME{ "crashpad_handler" }; #endif #ifdef Q_OS_WIN @@ -154,19 +155,19 @@ static const QString CRASHPAD_HANDLER_NAME { "crashpad_handler" }; static constexpr DWORD STATUS_MSVC_CPP_EXCEPTION = 0xE06D7363; static constexpr ULONG_PTR MSVC_CPP_EXCEPTION_SIGNATURE = 0x19930520; -static constexpr int ANNOTATION_LOCK_WEAK_ATTEMPT = 5000; // attempt to lock the annotations list, but give up if it takes more than 5 seconds +static constexpr int ANNOTATION_LOCK_WEAK_ATTEMPT = 5000; // attempt to lock the annotations list, but give up if it takes more than 5 seconds LPTOP_LEVEL_EXCEPTION_FILTER gl_crashpadUnhandledExceptionFilter = nullptr; -QTimer unhandledExceptionTimer; // checks occasionally in case loading an external DLL reset the unhandled exception pointer +QTimer unhandledExceptionTimer; // checks occasionally in case loading an external DLL reset the unhandled exception pointer -void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo); // extracts type information from a thrown C++ exception -LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); // called on any thrown exception (whether or not it's caught) -LONG WINAPI unhandledExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); // called on any exception without a corresponding catch +void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo); // extracts type information from a thrown C++ exception +LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); // called on any thrown exception (whether or not it's caught) +LONG WINAPI unhandledExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo); // called on any exception without a corresponding catch static LONG WINAPI firstChanceExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { // we're catching these exceptions on first-chance as the system state is corrupted at this point and they may not survive the exception handling mechanism if (client && (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_HEAP_CORRUPTION || - pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN)) { + pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN)) { client->DumpAndCrash(pExceptionInfo); } @@ -194,31 +195,31 @@ static LONG WINAPI unhandledExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) #pragma pack(push, ehdata, 4) -struct PMD_internal { // internal name: _PMD (no changes, so could in theory just use the original) +struct PMD_internal { // internal name: _PMD (no changes, so could in theory just use the original) int mdisp; int pdisp; int vdisp; }; -struct ThrowInfo_internal { // internal name: _ThrowInfo (changed all pointers into __int32) +struct ThrowInfo_internal { // internal name: _ThrowInfo (changed all pointers into __int32) __int32 attributes; __int32 pmfnUnwind; // 32-bit RVA __int32 pForwardCompat; // 32-bit RVA __int32 pCatchableTypeArray; // 32-bit RVA }; -struct CatchableType_internal { // internal name: _CatchableType (changed all pointers into __int32) +struct CatchableType_internal { // internal name: _CatchableType (changed all pointers into __int32) __int32 properties; - __int32 pType; // 32-bit RVA + __int32 pType; // 32-bit RVA PMD_internal thisDisplacement; __int32 sizeOrOffset; - __int32 copyFunction; // 32-bit RVA + __int32 copyFunction; // 32-bit RVA }; #pragma warning(disable : 4200) -struct CatchableTypeArray_internal { // internal name: _CatchableTypeArray (changed all pointers into __int32) +struct CatchableTypeArray_internal { // internal name: _CatchableTypeArray (changed all pointers into __int32) int nCatchableTypes; - __int32 arrayOfCatchableTypes[0]; // 32-bit RVA + __int32 arrayOfCatchableTypes[0]; // 32-bit RVA }; #pragma warning(default : 4200) @@ -255,23 +256,25 @@ static void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { // get the ThrowInfo struct from the exception arguments ThrowInfo_internal* pThrowInfo = reinterpret_cast(ExceptionRecord->ExceptionInformation[2]); ULONG_PTR moduleBase = ExceptionRecord->ExceptionInformation[3]; - if(moduleBase == 0 || pThrowInfo == NULL) { - return; // broken assumption + if (moduleBase == 0 || pThrowInfo == NULL) { + return; // broken assumption } // get the CatchableTypeArray* struct from ThrowInfo - if(pThrowInfo->pCatchableTypeArray == 0) { - return; // broken assumption + if (pThrowInfo->pCatchableTypeArray == 0) { + return; // broken assumption } - CatchableTypeArray_internal* pCatchableTypeArray = reinterpret_cast(moduleBase + pThrowInfo->pCatchableTypeArray); - if(pCatchableTypeArray->nCatchableTypes == 0 || pCatchableTypeArray->arrayOfCatchableTypes[0] == 0) { - return; // broken assumption + CatchableTypeArray_internal* pCatchableTypeArray = + reinterpret_cast(moduleBase + pThrowInfo->pCatchableTypeArray); + if (pCatchableTypeArray->nCatchableTypes == 0 || pCatchableTypeArray->arrayOfCatchableTypes[0] == 0) { + return; // broken assumption } // get the CatchableType struct for the actual exception type from CatchableTypeArray - CatchableType_internal* pCatchableType = reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[0]); - if(pCatchableType->pType == 0) { - return; // broken assumption + CatchableType_internal* pCatchableType = + reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[0]); + if (pCatchableType->pType == 0) { + return; // broken assumption } const std::type_info* type = reinterpret_cast(moduleBase + pCatchableType->pType); @@ -281,9 +284,10 @@ static void fatalCxxException(PEXCEPTION_POINTERS pExceptionInfo) { // catch() commands that could have caught this so we can find the list of its superclasses QString compatibleObjects; for (int catchTypeIdx = 1; catchTypeIdx < pCatchableTypeArray->nCatchableTypes; catchTypeIdx++) { - CatchableType_internal* pCatchableSuperclassType = reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[catchTypeIdx]); + CatchableType_internal* pCatchableSuperclassType = + reinterpret_cast(moduleBase + pCatchableTypeArray->arrayOfCatchableTypes[catchTypeIdx]); if (pCatchableSuperclassType->pType == 0) { - return; // broken assumption + return; // broken assumption } const std::type_info* superclassType = reinterpret_cast(moduleBase + pCatchableSuperclassType->pType); @@ -305,7 +309,7 @@ void checkUnhandledExceptionHook() { // End of code specific to the Microsoft C++ compiler // ------------------------------------------------------------------------------------------------ -#endif // Q_OS_WIN +#endif // Q_OS_WIN bool startCrashHandler(std::string appPath) { if (BACKTRACE_URL.empty() || BACKTRACE_TOKEN.empty()) { @@ -325,17 +329,16 @@ bool startCrashHandler(std::string appPath) { auto machineFingerPrint = uuidStringWithoutCurlyBraces(FingerprintUtils::getMachineFingerprint()); annotations["machine_fingerprint"] = machineFingerPrint.toStdString(); - arguments.push_back("--no-rate-limit"); // Setup Crashpad DB directory const auto crashpadDbName = "crashpad-db"; const auto crashpadDbDir = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation); - QDir(crashpadDbDir).mkpath(crashpadDbName); // Make sure the directory exists + QDir(crashpadDbDir).mkpath(crashpadDbName); // Make sure the directory exists const auto crashpadDbPath = crashpadDbDir.toStdString() + "/" + crashpadDbName; // Locate Crashpad handler - const QFileInfo interfaceBinary { QString::fromStdString(appPath) }; + const QFileInfo interfaceBinary{ QString::fromStdString(appPath) }; const QDir interfaceDir = interfaceBinary.dir(); assert(interfaceDir.exists(CRASHPAD_HANDLER_NAME)); const std::string CRASHPAD_HANDLER_PATH = interfaceDir.filePath(CRASHPAD_HANDLER_NAME).toStdString(); @@ -373,7 +376,7 @@ void setCrashAnnotation(std::string name, std::string value) { if (client) { SpinLockLocker guard(crashpadAnnotationsProtect); if (!crashpadAnnotations) { - crashpadAnnotations = new crashpad::SimpleStringDictionary(); // don't free this, let it leak + crashpadAnnotations = new crashpad::SimpleStringDictionary(); // don't free this, let it leak crashpad::CrashpadInfo* crashpad_info = crashpad::CrashpadInfo::GetCrashpadInfo(); crashpad_info->set_simple_annotations(crashpadAnnotations); } @@ -393,7 +396,7 @@ void startCrashHookMonitor(QCoreApplication* app) { unhandledExceptionTimer.moveToThread(app->thread()); QObject::connect(&unhandledExceptionTimer, &QTimer::timeout, checkUnhandledExceptionHook); unhandledExceptionTimer.start(60000); -#endif // Q_OS_WIN +#endif // Q_OS_WIN } -#endif // HAS_CRASHPAD +#endif // HAS_CRASHPAD