From ff8d4883b10eafd167d96f6c63b9817bc848a96e Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 8 Jun 2016 14:56:18 -0700 Subject: [PATCH 1/5] Attempt to get better logging from pure virtual call crashes --- interface/src/CrashReporter.cpp | 44 +++++++++++++++++++++-- interface/src/FileLogger.cpp | 4 +++ interface/src/FileLogger.h | 1 + libraries/shared/src/GenericQueueThread.h | 27 ++++++++++++-- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp index 1fd534ffac..d683c5e4df 100644 --- a/interface/src/CrashReporter.cpp +++ b/interface/src/CrashReporter.cpp @@ -10,15 +10,20 @@ // -#ifdef HAS_BUGSPLAT #include "CrashReporter.h" +#ifdef _WIN32 +#include #include #include +#include #include - #include +#include "Application.h" + + +#pragma comment(lib, "Dbghelp.lib") // SetUnhandledExceptionFilter can be overridden by the CRT at the point that an error occurs. More information // can be found here: http://www.codeproject.com/Articles/154686/SetUnhandledExceptionFilter-and-the-C-C-Runtime-Li @@ -77,13 +82,43 @@ BOOL redirectLibraryFunctionToFunction(char* library, char* function, void* fn) return bRet; } +void printStackTrace(ULONG framesToSkip = 1) { + QString result; + unsigned int i; + void * stack[100]; + unsigned short frames; + SYMBOL_INFO * symbol; + HANDLE process; + + process = GetCurrentProcess(); + SymInitialize(process, NULL, TRUE); + frames = CaptureStackBackTrace(framesToSkip, 100, stack, NULL); + symbol = (SYMBOL_INFO *)calloc(sizeof(SYMBOL_INFO) + 256 * sizeof(char), 1); + symbol->MaxNameLen = 255; + symbol->SizeOfStruct = sizeof(SYMBOL_INFO); + + for (i = 0; i < frames; i++) { + SymFromAddr(process, (DWORD64)(stack[i]), 0, symbol); + qWarning() << QString("%1: %2 - 0x%0X").arg(QString::number(frames - i - 1), QString(symbol->Name), QString::number(symbol->Address, 16)); + } + + free(symbol); + + // Try to force the log to sync to the filesystem + auto app = qApp; + if (app && app->getLogger()) { + app->getLogger()->sync(); + } +} void handleSignal(int signal) { // Throw so BugSplat can handle throw(signal); } -void handlePureVirtualCall() { +void __cdecl handlePureVirtualCall() { + qWarning() << "Pure virtual function call detected"; + printStackTrace(2); // Throw so BugSplat can handle throw("ERROR: Pure virtual call"); } @@ -107,6 +142,8 @@ _purecall_handler __cdecl noop_set_purecall_handler(_purecall_handler pNew) { return nullptr; } +#ifdef HAS_BUGSPLAT + static const DWORD BUG_SPLAT_FLAGS = MDSF_PREVENTHIJACKING | MDSF_USEGUARDMEMORY; CrashReporter::CrashReporter(QString bugSplatDatabase, QString bugSplatApplicationName, QString version) @@ -133,3 +170,4 @@ CrashReporter::CrashReporter(QString bugSplatDatabase, QString bugSplatApplicati } } #endif +#endif \ No newline at end of file diff --git a/interface/src/FileLogger.cpp b/interface/src/FileLogger.cpp index f5f7ce6ebe..50f7d15d6b 100644 --- a/interface/src/FileLogger.cpp +++ b/interface/src/FileLogger.cpp @@ -115,3 +115,7 @@ QString FileLogger::getLogData() { } return result; } + +void FileLogger::sync() { + _persistThreadInstance->waitIdle(); +} diff --git a/interface/src/FileLogger.h b/interface/src/FileLogger.h index e9bae63a73..28ac6fba40 100644 --- a/interface/src/FileLogger.h +++ b/interface/src/FileLogger.h @@ -28,6 +28,7 @@ public: virtual void addMessage(const QString&) override; virtual QString getLogData() override; virtual void locateLog() override; + void sync(); signals: void rollingLogFile(QString newFilename); diff --git a/libraries/shared/src/GenericQueueThread.h b/libraries/shared/src/GenericQueueThread.h index 28fcdb0ed6..7c1bdccaa7 100644 --- a/libraries/shared/src/GenericQueueThread.h +++ b/libraries/shared/src/GenericQueueThread.h @@ -12,9 +12,10 @@ #include -#include -#include -#include +#include +#include +#include +#include #include "GenericThread.h" #include "NumericalConstants.h" @@ -35,6 +36,25 @@ public: _hasItems.wakeAll(); } + void waitIdle(uint32_t maxWaitMs = UINT32_MAX) { + QElapsedTimer timer; + timer.start(); + + // FIXME this will work as long as the thread doing the wait + // is the only thread which can add work to the queue. + // It would be better if instead we had a queue empty condition to wait on + // that would ensure that we get woken as soon as we're idle the very + // first time the queue was empty. + while (timer.elapsed() < maxWaitMs) { + lock(); + if (!_items.size()) { + unlock(); + return; + } + unlock(); + } + } + protected: virtual void queueItemInternal(const T& t) { _items.push_back(t); @@ -44,6 +64,7 @@ protected: return MSECS_PER_SECOND; } + virtual bool process() { lock(); if (!_items.size()) { From eee1ba09062216b0032110f5b1e6583a3ee4510c Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 9 Jun 2016 10:19:38 -0700 Subject: [PATCH 2/5] trying to get buildability --- interface/src/CrashReporter.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp index d683c5e4df..fce472e311 100644 --- a/interface/src/CrashReporter.cpp +++ b/interface/src/CrashReporter.cpp @@ -10,17 +10,16 @@ // +#include "Application.h" #include "CrashReporter.h" #ifdef _WIN32 -#include #include #include #include #include #include -#include "Application.h" #pragma comment(lib, "Dbghelp.lib") @@ -170,4 +169,4 @@ CrashReporter::CrashReporter(QString bugSplatDatabase, QString bugSplatApplicati } } #endif -#endif \ No newline at end of file +#endif From cd7c6be5906019c64a25876b33fd460f7e9eeefa Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 9 Jun 2016 12:25:21 -0700 Subject: [PATCH 3/5] Fix formatting --- interface/src/CrashReporter.cpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/interface/src/CrashReporter.cpp b/interface/src/CrashReporter.cpp index fce472e311..596c34ca92 100644 --- a/interface/src/CrashReporter.cpp +++ b/interface/src/CrashReporter.cpp @@ -82,21 +82,15 @@ BOOL redirectLibraryFunctionToFunction(char* library, char* function, void* fn) } void printStackTrace(ULONG framesToSkip = 1) { - QString result; - unsigned int i; - void * stack[100]; - unsigned short frames; - SYMBOL_INFO * symbol; - HANDLE process; - - process = GetCurrentProcess(); + HANDLE process = GetCurrentProcess(); SymInitialize(process, NULL, TRUE); - frames = CaptureStackBackTrace(framesToSkip, 100, stack, NULL); - symbol = (SYMBOL_INFO *)calloc(sizeof(SYMBOL_INFO) + 256 * sizeof(char), 1); + void* stack[100]; + uint16_t frames = CaptureStackBackTrace(framesToSkip, 100, stack, NULL); + SYMBOL_INFO* symbol = (SYMBOL_INFO *)calloc(sizeof(SYMBOL_INFO) + 256 * sizeof(char), 1); symbol->MaxNameLen = 255; symbol->SizeOfStruct = sizeof(SYMBOL_INFO); - for (i = 0; i < frames; i++) { + for (uint16_t i = 0; i < frames; ++i) { SymFromAddr(process, (DWORD64)(stack[i]), 0, symbol); qWarning() << QString("%1: %2 - 0x%0X").arg(QString::number(frames - i - 1), QString(symbol->Name), QString::number(symbol->Address, 16)); } From d7079fce8c78dc51af64f9f6646b7d2473cef00f Mon Sep 17 00:00:00 2001 From: Ken Cooke Date: Thu, 9 Jun 2016 15:07:24 -0700 Subject: [PATCH 4/5] Use shared CPUDetect.h for CPU detection --- libraries/audio/src/AudioHRTF.cpp | 53 ++----------------------------- 1 file changed, 2 insertions(+), 51 deletions(-) diff --git a/libraries/audio/src/AudioHRTF.cpp b/libraries/audio/src/AudioHRTF.cpp index 7fadf073a1..89c929011a 100644 --- a/libraries/audio/src/AudioHRTF.cpp +++ b/libraries/audio/src/AudioHRTF.cpp @@ -119,61 +119,12 @@ static void FIR_1x4_SSE(float* src, float* dst0, float* dst1, float* dst2, float } } -// -// Detect AVX/AVX2 support -// - -#if defined(_MSC_VER) - -#include - -static bool cpuSupportsAVX() { - int info[4]; - int mask = (1 << 27) | (1 << 28); // OSXSAVE and AVX - - __cpuidex(info, 0x1, 0); - - bool result = false; - if ((info[2] & mask) == mask) { - - if ((_xgetbv(_XCR_XFEATURE_ENABLED_MASK) & 0x6) == 0x6) { - result = true; - } - } - return result; -} - -#elif defined(__GNUC__) - -#include - -static bool cpuSupportsAVX() { - unsigned int eax, ebx, ecx, edx; - unsigned int mask = (1 << 27) | (1 << 28); // OSXSAVE and AVX - - bool result = false; - if (__get_cpuid(0x1, &eax, &ebx, &ecx, &edx) && ((ecx & mask) == mask)) { - - __asm__("xgetbv" : "=a"(eax), "=d"(edx) : "c"(0)); - if ((eax & 0x6) == 0x6) { - result = true; - } - } - return result; -} - -#else - -static bool cpuSupportsAVX() { - return false; -} - -#endif - // // Runtime CPU dispatch // +#include "CPUDetect.h" + typedef void FIR_1x4_t(float* src, float* dst0, float* dst1, float* dst2, float* dst3, float coef[4][HRTF_TAPS], int numFrames); FIR_1x4_t FIR_1x4_AVX; // separate compilation with VEX-encoding enabled From dd0d594524975d11ba6dac6152a4adf35dcdfcb8 Mon Sep 17 00:00:00 2001 From: Ken Cooke Date: Thu, 9 Jun 2016 16:31:09 -0700 Subject: [PATCH 5/5] cleanup --- libraries/audio/src/AudioHRTF.cpp | 8 +++----- libraries/audio/src/AudioHRTF.h | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/libraries/audio/src/AudioHRTF.cpp b/libraries/audio/src/AudioHRTF.cpp index 89c929011a..e7cf4436c6 100644 --- a/libraries/audio/src/AudioHRTF.cpp +++ b/libraries/audio/src/AudioHRTF.cpp @@ -10,7 +10,6 @@ // #include -#include #include #include @@ -125,13 +124,12 @@ static void FIR_1x4_SSE(float* src, float* dst0, float* dst1, float* dst2, float #include "CPUDetect.h" -typedef void FIR_1x4_t(float* src, float* dst0, float* dst1, float* dst2, float* dst3, float coef[4][HRTF_TAPS], int numFrames); -FIR_1x4_t FIR_1x4_AVX; // separate compilation with VEX-encoding enabled +void FIR_1x4_AVX(float* src, float* dst0, float* dst1, float* dst2, float* dst3, float coef[4][HRTF_TAPS], int numFrames); static void FIR_1x4(float* src, float* dst0, float* dst1, float* dst2, float* dst3, float coef[4][HRTF_TAPS], int numFrames) { - static FIR_1x4_t* f = cpuSupportsAVX() ? FIR_1x4_AVX : FIR_1x4_SSE; // init on first call - (*f)(src, dst0, dst1, dst2, dst3, coef, numFrames); // dispatch + static auto f = cpuSupportsAVX() ? FIR_1x4_AVX : FIR_1x4_SSE; + (*f)(src, dst0, dst1, dst2, dst3, coef, numFrames); // dispatch } // 4 channel planar to interleaved diff --git a/libraries/audio/src/AudioHRTF.h b/libraries/audio/src/AudioHRTF.h index f92f9c1602..d3b6237d0c 100644 --- a/libraries/audio/src/AudioHRTF.h +++ b/libraries/audio/src/AudioHRTF.h @@ -32,7 +32,7 @@ public: // input: mono source // output: interleaved stereo mix buffer (accumulates into existing output) // index: HRTF subject index - // azimuth: clockwise panning angle [0, 360] in degrees + // azimuth: clockwise panning angle in radians // gain: gain factor for distance attenuation // numFrames: must be HRTF_BLOCK in this version //