From 5ab16302cac5a63f6ac3aa084850a145b43f4785 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 24 Apr 2018 15:09:30 -0700 Subject: [PATCH 1/3] Keep CrashpadClient around --- interface/src/Crashpad.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/interface/src/Crashpad.cpp b/interface/src/Crashpad.cpp index e39cd42d81..27b6f4b937 100644 --- a/interface/src/Crashpad.cpp +++ b/interface/src/Crashpad.cpp @@ -11,6 +11,8 @@ #include "Crashpad.h" +#include + #include #if HAS_CRASHPAD @@ -20,7 +22,7 @@ #include #include -#include +#include #include #include @@ -28,28 +30,26 @@ #include #include +#include + using namespace crashpad; static const std::string BACKTRACE_URL { CMAKE_BACKTRACE_URL }; static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN }; -static std::wstring gIPCPipe; - extern QString qAppFileName(); std::mutex annotationMutex; crashpad::SimpleStringDictionary* crashpadAnnotations { nullptr }; -#include + +std::unique_ptr client; LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) { if (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_HEAP_CORRUPTION || pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN) { - CrashpadClient client; - if (gIPCPipe.length()) { - client.SetHandlerIPCPipe(gIPCPipe); - } - client.DumpAndCrash(pExceptionInfo); + assert(client); + client->DumpAndCrash(pExceptionInfo); } return EXCEPTION_CONTINUE_SEARCH; @@ -60,7 +60,7 @@ bool startCrashHandler() { return false; } - CrashpadClient client; + client.reset(new CrashpadClient()); std::vector arguments; std::map annotations; @@ -96,12 +96,9 @@ bool startCrashHandler() { // Enable automated uploads. database->GetSettings()->SetUploadsEnabled(true); - bool result = client.StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true); - gIPCPipe = client.GetHandlerIPCPipe(); - AddVectoredExceptionHandler(0, vectoredExceptionHandler); - return result; + return client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true); } void setCrashAnnotation(std::string name, std::string value) { From 250806252e526889820549e6ac13ce259a340ebd Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 24 Apr 2018 15:09:30 -0700 Subject: [PATCH 2/3] Don't use unique_ptr to store CrashpadClient --- interface/src/Crashpad.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/interface/src/Crashpad.cpp b/interface/src/Crashpad.cpp index 27b6f4b937..45f1d0778f 100644 --- a/interface/src/Crashpad.cpp +++ b/interface/src/Crashpad.cpp @@ -39,16 +39,17 @@ static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN }; extern QString qAppFileName(); +CrashpadClient* client { nullptr }; std::mutex annotationMutex; crashpad::SimpleStringDictionary* crashpadAnnotations { nullptr }; - -std::unique_ptr client; - 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) { - assert(client); client->DumpAndCrash(pExceptionInfo); } @@ -60,7 +61,8 @@ bool startCrashHandler() { return false; } - client.reset(new CrashpadClient()); + assert(!client); + client = new CrashpadClient(); std::vector arguments; std::map annotations; From f3fe1f3e58386363ddb2e2b5a48e5a08bd73f4c3 Mon Sep 17 00:00:00 2001 From: Clement Date: Mon, 30 Apr 2018 13:51:53 -0700 Subject: [PATCH 3/3] Force crash helpers not inlined --- libraries/shared/src/CrashHelpers.cpp | 77 +++++++++++++++++++++++++++ libraries/shared/src/CrashHelpers.h | 66 +++-------------------- 2 files changed, 83 insertions(+), 60 deletions(-) create mode 100644 libraries/shared/src/CrashHelpers.cpp diff --git a/libraries/shared/src/CrashHelpers.cpp b/libraries/shared/src/CrashHelpers.cpp new file mode 100644 index 0000000000..f8ca90bc4c --- /dev/null +++ b/libraries/shared/src/CrashHelpers.cpp @@ -0,0 +1,77 @@ +// +// CrashHelpers.cpp +// libraries/shared/src +// +// Created by Clement Brisset on 4/30/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "CrashHelpers.h" + +namespace crash { + +class B; +class A { +public: + A(B* b) : _b(b) { } + ~A(); + virtual void virtualFunction() = 0; + +private: + B* _b; +}; + +class B : public A { +public: + B() : A(this) { } + virtual void virtualFunction() override { } +}; + +A::~A() { + _b->virtualFunction(); +} + +void pureVirtualCall() { + qCDebug(shared) << "About to make a pure virtual call"; + B b; +} + +void doubleFree() { + qCDebug(shared) << "About to double delete memory"; + int* blah = new int(200); + delete blah; + delete blah; +} + +void nullDeref() { + qCDebug(shared) << "About to dereference a null pointer"; + int* p = nullptr; + *p = 1; +} + +void doAbort() { + qCDebug(shared) << "About to abort"; + abort(); +} + +void outOfBoundsVectorCrash() { + qCDebug(shared) << "std::vector out of bounds crash!"; + std::vector v; + v[0] = 42; +} + +void newFault() { + qCDebug(shared) << "About to crash inside new fault"; + + // Force crash with multiple large allocations + while (true) { + const size_t GIGABYTE = 1024 * 1024 * 1024; + new char[GIGABYTE]; + } + +} + +} diff --git a/libraries/shared/src/CrashHelpers.h b/libraries/shared/src/CrashHelpers.h index 1cc6749182..ad988c8906 100644 --- a/libraries/shared/src/CrashHelpers.h +++ b/libraries/shared/src/CrashHelpers.h @@ -18,66 +18,12 @@ namespace crash { -class B; -class A { -public: - A(B* b) : _b(b) { } - ~A(); - virtual void virtualFunction() = 0; - -private: - B* _b; -}; - -class B : public A { -public: - B() : A(this) { } - virtual void virtualFunction() override { } -}; - -A::~A() { - _b->virtualFunction(); -} - -void pureVirtualCall() { - qCDebug(shared) << "About to make a pure virtual call"; - B b; -} - -void doubleFree() { - qCDebug(shared) << "About to double delete memory"; - int* blah = new int(200); - delete blah; - delete blah; -} - -void nullDeref() { - qCDebug(shared) << "About to dereference a null pointer"; - int* p = nullptr; - *p = 1; -} - -void doAbort() { - qCDebug(shared) << "About to abort"; - abort(); -} - -void outOfBoundsVectorCrash() { - qCDebug(shared) << "std::vector out of bounds crash!"; - std::vector v; - v[0] = 42; -} - -void newFault() { - qCDebug(shared) << "About to crash inside new fault"; - - // Force crash with multiple large allocations - while (true) { - const size_t GIGABYTE = 1024 * 1024 * 1024; - new char[GIGABYTE]; - } - -} +void pureVirtualCall(); +void doubleFree(); +void nullDeref(); +void doAbort(); +void outOfBoundsVectorCrash(); +void newFault(); }