From b74b69d752c10eb82d1716d41fc8e789bfc4e8c6 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 23 Jun 2017 10:00:12 -0700 Subject: [PATCH 1/7] Add Andrew Schwartzmeyer to authors and contributors --- AUTHORS | 1 + CONTRIBUTORS | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index 8e654101b..879d13734 100644 --- a/AUTHORS +++ b/AUTHORS @@ -10,6 +10,7 @@ Abhishek Dasgupta Abhishek Parmar +Andrew Schwartzmeyer Andy Ying Brian Silverman Google Inc. diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 70c0ace6a..70ddbec4c 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -24,6 +24,7 @@ Abhishek Dasgupta Abhishek Parmar +Andrew Schwartzmeyer Andy Ying Brian Silverman Fumitoshi Ukai From ff0132c27b5e7d42a96dfb361cd3252d4dfca191 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 20 Dec 2016 15:24:50 -0800 Subject: [PATCH 2/7] Copy stacktrace_generic-inl.h for Windows --- src/stacktrace_windows-inl.h | 59 ++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 src/stacktrace_windows-inl.h diff --git a/src/stacktrace_windows-inl.h b/src/stacktrace_windows-inl.h new file mode 100644 index 000000000..fad81d3e3 --- /dev/null +++ b/src/stacktrace_windows-inl.h @@ -0,0 +1,59 @@ +// Copyright (c) 2000 - 2007, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +// Portable implementation - just use glibc +// +// Note: The glibc implementation may cause a call to malloc. +// This can cause a deadlock in HeapProfiler. +#include +#include +#include "stacktrace.h" + +_START_GOOGLE_NAMESPACE_ + +// If you change this function, also change GetStackFrames below. +int GetStackTrace(void** result, int max_depth, int skip_count) { + static const int kStackLength = 64; + void * stack[kStackLength]; + int size; + + size = backtrace(stack, kStackLength); + skip_count++; // we want to skip the current frame as well + int result_count = size - skip_count; + if (result_count < 0) + result_count = 0; + if (result_count > max_depth) + result_count = max_depth; + for (int i = 0; i < result_count; i++) + result[i] = stack[i + skip_count]; + + return result_count; +} + +_END_GOOGLE_NAMESPACE_ From 6a13f089c9e8cf116ae7090dc3a7bfc61ff3c981 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 21 Dec 2016 13:48:58 -0800 Subject: [PATCH 3/7] Support stacktrace on Windows --- CMakeLists.txt | 5 +++ src/stacktrace_windows-inl.h | 63 +++++++++++++++++++++++++----------- src/utilities.h | 7 ++++ 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7415eab3d..d353c00e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -455,6 +455,11 @@ if (HAVE_EXECINFO_H) set (HAVE_STACKTRACE 1) endif (HAVE_EXECINFO_H) +if (WIN32) + set (HAVE_STACKTRACE 1) + target_link_libraries (glog PUBLIC Dbghelp.lib) +endif (WIN32) + if (UNIX OR (APPLE AND HAVE_DLADDR)) set (HAVE_SYMBOLIZE 1) endif (UNIX OR (APPLE AND HAVE_DLADDR)) diff --git a/src/stacktrace_windows-inl.h b/src/stacktrace_windows-inl.h index fad81d3e3..56064089d 100644 --- a/src/stacktrace_windows-inl.h +++ b/src/stacktrace_windows-inl.h @@ -27,33 +27,60 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // -// Portable implementation - just use glibc +// Author: Andrew Schwartzmeyer // -// Note: The glibc implementation may cause a call to malloc. -// This can cause a deadlock in HeapProfiler. -#include -#include +// Windows implementation - just use CaptureStackBackTrace + +#include "port.h" #include "stacktrace.h" +#include "Dbghelp.h" +#include _START_GOOGLE_NAMESPACE_ +static bool ready_to_run = false; +class StackTraceInit { +public: + HANDLE hProcess; + StackTraceInit() { + // Initialize the symbol handler + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680344(v=vs.85).aspx + hProcess = GetCurrentProcess(); + SymSetOptions(SYMOPT_UNDNAME | SYMOPT_DEFERRED_LOADS); + SymInitialize(hProcess, NULL, true); + ready_to_run = true; + } + ~StackTraceInit() { + SymCleanup(hProcess); + ready_to_run = false; + } +}; + +static const StackTraceInit module_initializer; // Force initialization + // If you change this function, also change GetStackFrames below. int GetStackTrace(void** result, int max_depth, int skip_count) { - static const int kStackLength = 64; - void * stack[kStackLength]; - int size; - - size = backtrace(stack, kStackLength); + if (!ready_to_run) { + return 0; + } skip_count++; // we want to skip the current frame as well - int result_count = size - skip_count; - if (result_count < 0) - result_count = 0; - if (result_count > max_depth) - result_count = max_depth; - for (int i = 0; i < result_count; i++) - result[i] = stack[i + skip_count]; + if (max_depth > 64) { + max_depth = 64; + } + std::vector stack(max_depth); + // This API is thread-safe (moreover it walks only the current thread). + int size = CaptureStackBackTrace(skip_count, max_depth, &stack[0], NULL); + for (int i = 0; i < size; ++i) { + // Resolve symbol information from address. + char buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME * sizeof(TCHAR)]; + SYMBOL_INFO* symbol = reinterpret_cast(buffer); + symbol->SizeOfStruct = sizeof(SYMBOL_INFO); + symbol->MaxNameLen = MAX_SYM_NAME; + SymFromAddr(module_initializer.hProcess, reinterpret_cast(stack[i]), 0, symbol); + result[i] = stack[i]; + } - return result_count; + return size; } _END_GOOGLE_NAMESPACE_ diff --git a/src/utilities.h b/src/utilities.h index 101ca641c..a10735b7a 100644 --- a/src/utilities.h +++ b/src/utilities.h @@ -99,6 +99,8 @@ // malloc() from the unwinder. This is a problem because we're // trying to use the unwinder to instrument malloc(). // +// 4) The Windows API CaptureStackTrace. +// // Note: if you add a new implementation here, make sure it works // correctly when GetStackTrace() is called with max_depth == 0. // Some code may do that. @@ -112,6 +114,8 @@ # define STACKTRACE_H "stacktrace_x86_64-inl.h" # elif (defined(__ppc__) || defined(__PPC__)) && __GNUC__ >= 2 # define STACKTRACE_H "stacktrace_powerpc-inl.h" +# elif defined(OS_WINDOWS) +# define STACKTRACE_H "stacktrace_windows-inl.h" # endif #endif @@ -143,6 +147,9 @@ namespace glog_internal_namespace_ { #ifdef HAVE___ATTRIBUTE__ # define ATTRIBUTE_NOINLINE __attribute__ ((noinline)) # define HAVE_ATTRIBUTE_NOINLINE +#elif defined(OS_WINDOWS) +# define ATTRIBUTE_NOINLINE __declspec(noinline) +# define HAVE_ATTRIBUTE_NOINLINE #else # define ATTRIBUTE_NOINLINE #endif From af31a0a7e3ee140683d582b309ac9f46fe0d381f Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 3 Mar 2017 16:50:08 -0800 Subject: [PATCH 4/7] Support symbolizer and demangler on Windows --- CMakeLists.txt | 1 + src/demangle.cc | 31 +++++++++++++++ src/demangle_unittest.cc | 41 +++++++++++++++----- src/stacktrace_windows-inl.h | 42 ++------------------ src/symbolize.cc | 60 +++++++++++++++++++++++++++++ src/symbolize_unittest.cc | 74 +++++++++++++++++++++++++----------- src/utilities.h | 3 ++ 7 files changed, 181 insertions(+), 71 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d353c00e5..b8f8bd8c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -457,6 +457,7 @@ endif (HAVE_EXECINFO_H) if (WIN32) set (HAVE_STACKTRACE 1) + set (HAVE_SYMBOLIZE 1) target_link_libraries (glog PUBLIC Dbghelp.lib) endif (WIN32) diff --git a/src/demangle.cc b/src/demangle.cc index e858181a6..78520433c 100644 --- a/src/demangle.cc +++ b/src/demangle.cc @@ -35,10 +35,16 @@ // Note that we only have partial C++0x support yet. #include // for NULL +#include "utilities.h" #include "demangle.h" +#if defined(OS_WINDOWS) +#include +#endif + _START_GOOGLE_NAMESPACE_ +#if !defined(OS_WINDOWS) typedef struct { const char *abbrev; const char *real_name; @@ -1293,12 +1299,37 @@ static bool ParseTopLevelMangledName(State *state) { } return false; } +#endif // The demangler entry point. bool Demangle(const char *mangled, char *out, int out_size) { +#if defined(OS_WINDOWS) + // When built with incremental linking, the Windows debugger + // library provides a more complicated `Symbol->Name` with the + // Incremental Linking Table offset, which looks like + // `@ILT+1105(?func@Foo@@SAXH@Z)`. However, the demangler expects + // only the mangled symbol, `?func@Foo@@SAXH@Z`. Fortunately, the + // mangled symbol is guaranteed not to have parentheses, + // so we search for `(` and extract up to `)`. + // + // Since we may be in a signal handler here, we cannot use `std::string`. + char buffer[1024]; // Big enough for a sane symbol. + const char *lparen = strchr(mangled, '('); + if (lparen) { + // Extract the string `(?...)` + const char *rparen = strchr(lparen, ')'); + size_t length = rparen - lparen - 1; + strncpy(buffer, lparen + 1, length); + buffer[length] = '\0'; + mangled = buffer; + } // Else the symbol wasn't inside a set of parentheses + // We use the ANSI version to ensure the string type is always `char *`. + return UnDecorateSymbolName(mangled, out, out_size, UNDNAME_COMPLETE); +#else State state; InitState(&state, mangled, out, out_size); return ParseTopLevelMangledName(&state) && !state.overflowed; +#endif } _END_GOOGLE_NAMESPACE_ diff --git a/src/demangle_unittest.cc b/src/demangle_unittest.cc index 32f322101..be483411f 100644 --- a/src/demangle_unittest.cc +++ b/src/demangle_unittest.cc @@ -62,18 +62,37 @@ static const char *DemangleIt(const char * const mangled) { } } +#if defined(OS_WINDOWS) + +TEST(Demangle, Windows) { + EXPECT_STREQ( + "public: static void __cdecl Foo::func(int)", + DemangleIt("?func@Foo@@SAXH@Z")); + EXPECT_STREQ( + "public: static void __cdecl Foo::func(int)", + DemangleIt("@ILT+1105(?func@Foo@@SAXH@Z)")); + EXPECT_STREQ( + "int __cdecl foobarArray(int * const)", + DemangleIt("?foobarArray@@YAHQAH@Z")); +} + +#else + // Test corner cases of bounary conditions. TEST(Demangle, CornerCases) { - char tmp[10]; - EXPECT_TRUE(Demangle("_Z6foobarv", tmp, sizeof(tmp))); - // sizeof("foobar()") == 9 - EXPECT_STREQ("foobar()", tmp); - EXPECT_TRUE(Demangle("_Z6foobarv", tmp, 9)); - EXPECT_STREQ("foobar()", tmp); - EXPECT_FALSE(Demangle("_Z6foobarv", tmp, 8)); // Not enough. - EXPECT_FALSE(Demangle("_Z6foobarv", tmp, 1)); - EXPECT_FALSE(Demangle("_Z6foobarv", tmp, 0)); - EXPECT_FALSE(Demangle("_Z6foobarv", NULL, 0)); // Should not cause SEGV. + const size_t size = 10; + char tmp[size] = { 0 }; + const char *demangled = "foobar()"; + const char *mangled = "_Z6foobarv"; + EXPECT_TRUE(Demangle(mangled, tmp, sizeof(tmp))); + // sizeof("foobar()") == size - 1 + EXPECT_STREQ(demangled, tmp); + EXPECT_TRUE(Demangle(mangled, tmp, size - 1)); + EXPECT_STREQ(demangled, tmp); + EXPECT_FALSE(Demangle(mangled, tmp, size - 2)); // Not enough. + EXPECT_FALSE(Demangle(mangled, tmp, 1)); + EXPECT_FALSE(Demangle(mangled, tmp, 0)); + EXPECT_FALSE(Demangle(mangled, NULL, 0)); // Should not cause SEGV. } // Test handling of functions suffixed with .clone.N, which is used by GCC @@ -123,6 +142,8 @@ TEST(Demangle, FromFile) { } } +#endif + int main(int argc, char **argv) { #ifdef HAVE_LIB_GFLAGS ParseCommandLineFlags(&argc, &argv, true); diff --git a/src/stacktrace_windows-inl.h b/src/stacktrace_windows-inl.h index 56064089d..f329a7c8c 100644 --- a/src/stacktrace_windows-inl.h +++ b/src/stacktrace_windows-inl.h @@ -33,54 +33,18 @@ #include "port.h" #include "stacktrace.h" -#include "Dbghelp.h" -#include +#include _START_GOOGLE_NAMESPACE_ -static bool ready_to_run = false; -class StackTraceInit { -public: - HANDLE hProcess; - StackTraceInit() { - // Initialize the symbol handler - // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680344(v=vs.85).aspx - hProcess = GetCurrentProcess(); - SymSetOptions(SYMOPT_UNDNAME | SYMOPT_DEFERRED_LOADS); - SymInitialize(hProcess, NULL, true); - ready_to_run = true; - } - ~StackTraceInit() { - SymCleanup(hProcess); - ready_to_run = false; - } -}; - -static const StackTraceInit module_initializer; // Force initialization - // If you change this function, also change GetStackFrames below. int GetStackTrace(void** result, int max_depth, int skip_count) { - if (!ready_to_run) { - return 0; - } - skip_count++; // we want to skip the current frame as well if (max_depth > 64) { max_depth = 64; } - std::vector stack(max_depth); + skip_count++; // we want to skip the current frame as well // This API is thread-safe (moreover it walks only the current thread). - int size = CaptureStackBackTrace(skip_count, max_depth, &stack[0], NULL); - for (int i = 0; i < size; ++i) { - // Resolve symbol information from address. - char buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME * sizeof(TCHAR)]; - SYMBOL_INFO* symbol = reinterpret_cast(buffer); - symbol->SizeOfStruct = sizeof(SYMBOL_INFO); - symbol->MaxNameLen = MAX_SYM_NAME; - SymFromAddr(module_initializer.hProcess, reinterpret_cast(stack[i]), 0, symbol); - result[i] = stack[i]; - } - - return size; + return CaptureStackBackTrace(skip_count, max_depth, result, NULL); } _END_GOOGLE_NAMESPACE_ diff --git a/src/symbolize.cc b/src/symbolize.cc index f83c30973..24dcddcc9 100644 --- a/src/symbolize.cc +++ b/src/symbolize.cc @@ -837,6 +837,66 @@ static ATTRIBUTE_NOINLINE bool SymbolizeAndDemangle(void *pc, char *out, _END_GOOGLE_NAMESPACE_ +#elif defined(OS_WINDOWS) + +#include + +_START_GOOGLE_NAMESPACE_ + +class SymInitializer { +public: + HANDLE process = NULL; + bool ready = false; + SymInitializer() { + // Initialize the symbol handler. + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680344(v=vs.85).aspx + process = GetCurrentProcess(); + // Defer symbol loading. + // We do not request undecorated symbols with SYMOPT_UNDNAME + // because the mangling library calls UnDecorateSymbolName. + SymSetOptions(SYMOPT_DEFERRED_LOADS); + if (SymInitialize(process, NULL, true)) { + ready = true; + } + } + ~SymInitializer() { + SymCleanup(process); + // We do not need to close `HANDLE process` because it's a "pseudo handle." + } +private: + SymInitializer(const SymInitializer&); + SymInitializer& operator=(const SymInitializer&); +}; + +static ATTRIBUTE_NOINLINE bool SymbolizeAndDemangle(void *pc, char *out, + int out_size) { + const static SymInitializer symInitializer; + if (!symInitializer.ready) { + return false; + } + // Resolve symbol information from address. + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680578(v=vs.85).aspx + char buf[sizeof(SYMBOL_INFO) + MAX_SYM_NAME]; + SYMBOL_INFO *symbol = reinterpret_cast(buf); + symbol->SizeOfStruct = sizeof(SYMBOL_INFO); + symbol->MaxNameLen = MAX_SYM_NAME; + // We use the ANSI version to ensure the string type is always `char *`. + // This could break if a symbol has Unicode in it. + BOOL ret = SymFromAddr(symInitializer.process, + reinterpret_cast(pc), 0, symbol); + if (ret == 1 && static_cast(symbol->NameLen) < out_size) { + // `NameLen` does not include the null terminating character. + strncpy(out, symbol->Name, static_cast(symbol->NameLen) + 1); + out[static_cast(symbol->NameLen)] = '\0'; + // Symbolization succeeded. Now we try to demangle the symbol. + DemangleInplace(out, out_size); + return true; + } + return false; +} + +_END_GOOGLE_NAMESPACE_ + #else # error BUG: HAVE_SYMBOLIZE was wrongly set #endif diff --git a/src/symbolize_unittest.cc b/src/symbolize_unittest.cc index bdd2f03ce..a0a9737b2 100644 --- a/src/symbolize_unittest.cc +++ b/src/symbolize_unittest.cc @@ -49,10 +49,22 @@ using namespace GFLAGS_NAMESPACE; using namespace std; using namespace GOOGLE_NAMESPACE; -#if defined(HAVE_STACKTRACE) && defined(__ELF__) +#if defined(HAVE_STACKTRACE) #define always_inline +// A wrapper function for Symbolize() to make the unit test simple. +static const char *TrySymbolize(void *pc) { + static char symbol[4096]; + if (Symbolize(pc, symbol, sizeof(symbol))) { + return symbol; + } else { + return NULL; + } +} + +# if defined(__ELF__) + // This unit tests make sense only with GCC. // Uses lots of GCC specific features. #if defined(__GNUC__) && !defined(__OPENCC__) @@ -70,16 +82,6 @@ using namespace GOOGLE_NAMESPACE; # endif // defined(__i386__) || defined(__x86_64__) #endif -// A wrapper function for Symbolize() to make the unit test simple. -static const char *TrySymbolize(void *pc) { - static char symbol[4096]; - if (Symbolize(pc, symbol, sizeof(symbol))) { - return symbol; - } else { - return NULL; - } -} - // Make them C linkage to avoid mangled names. extern "C" { void nonstatic_func() { @@ -355,11 +357,42 @@ void ATTRIBUTE_NOINLINE TestWithReturnAddress() { #endif } +# elif defined(OS_WINDOWS) + +#include +#pragma intrinsic(_ReturnAddress) + +struct Foo { + static void func(int x); +}; + +__declspec(noinline) void Foo::func(int x) { + volatile int a = x; + ++a; +} + +TEST(Symbolize, SymbolizeWithDemangling) { + Foo::func(100); + const char* ret = TrySymbolize((void *)(&Foo::func)); + EXPECT_STREQ("public: static void __cdecl Foo::func(int)", ret); +} + +__declspec(noinline) void TestWithReturnAddress() { + void *return_address = _ReturnAddress(); + const char *symbol = TrySymbolize(return_address); + CHECK(symbol != NULL); + CHECK_STREQ(symbol, "main"); + cout << "Test case TestWithReturnAddress passed." << endl; +} +# endif // __ELF__ +#endif // HAVE_STACKTRACE + int main(int argc, char **argv) { FLAGS_logtostderr = true; InitGoogleLogging(argv[0]); InitGoogleTest(&argc, argv); -#ifdef HAVE_SYMBOLIZE +#if defined(HAVE_SYMBOLIZE) +# if defined(__ELF__) // We don't want to get affected by the callback interface, that may be // used to install some callback function at InitGoogle() time. InstallSymbolizeCallback(NULL); @@ -368,18 +401,15 @@ int main(int argc, char **argv) { TestWithPCInsideNonInlineFunction(); TestWithReturnAddress(); return RUN_ALL_TESTS(); -#else - return 0; -#endif -} - -#else -int main() { -#ifdef HAVE_SYMBOLIZE +# elif defined(OS_WINDOWS) + TestWithReturnAddress(); + return RUN_ALL_TESTS(); +# else // OS_WINDOWS printf("PASS (no symbolize_unittest support)\n"); + return 0; +# endif // __ELF__ #else printf("PASS (no symbolize support)\n"); -#endif return 0; +#endif // HAVE_SYMBOLIZE } -#endif // HAVE_STACKTRACE diff --git a/src/utilities.h b/src/utilities.h index a10735b7a..be2cff46e 100644 --- a/src/utilities.h +++ b/src/utilities.h @@ -133,6 +133,9 @@ #elif defined(OS_MACOSX) && defined(HAVE_DLADDR) // Use dladdr to symbolize. # define HAVE_SYMBOLIZE +#elif defined(OS_WINDOWS) +// Use DbgHelp to symbolize +# define HAVE_SYMBOLIZE #endif #ifndef ARRAYSIZE From 9095856ea8da4cdf41fe22f2ac0075f8be571b02 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 25 May 2017 13:43:39 -0700 Subject: [PATCH 5/7] Export GetStackTrace Necessary when building with BUILD_SHARED_LIBS=1. --- src/stacktrace_windows-inl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/stacktrace_windows-inl.h b/src/stacktrace_windows-inl.h index f329a7c8c..c747d96a3 100644 --- a/src/stacktrace_windows-inl.h +++ b/src/stacktrace_windows-inl.h @@ -31,13 +31,14 @@ // // Windows implementation - just use CaptureStackBackTrace +#include "config.h" #include "port.h" #include "stacktrace.h" #include _START_GOOGLE_NAMESPACE_ -// If you change this function, also change GetStackFrames below. +GOOGLE_GLOG_DLL_DECL int GetStackTrace(void** result, int max_depth, int skip_count) { if (max_depth > 64) { max_depth = 64; From 38f8ff9a49797c5b6351033fbf6c30c4a874a908 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 23 Jun 2017 12:39:49 -0700 Subject: [PATCH 6/7] Link to DbgHelp using pre-processor directive This method ensure that all users of glog get automatically linked to the DbgHelp library without needing to set compiler flags. --- CMakeLists.txt | 1 - src/demangle.cc | 1 + src/symbolize.cc | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b8f8bd8c9..af4680da3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -458,7 +458,6 @@ endif (HAVE_EXECINFO_H) if (WIN32) set (HAVE_STACKTRACE 1) set (HAVE_SYMBOLIZE 1) - target_link_libraries (glog PUBLIC Dbghelp.lib) endif (WIN32) if (UNIX OR (APPLE AND HAVE_DLADDR)) diff --git a/src/demangle.cc b/src/demangle.cc index 78520433c..4b33580b8 100644 --- a/src/demangle.cc +++ b/src/demangle.cc @@ -40,6 +40,7 @@ #if defined(OS_WINDOWS) #include +#pragma comment(lib, "DbgHelp") #endif _START_GOOGLE_NAMESPACE_ diff --git a/src/symbolize.cc b/src/symbolize.cc index 24dcddcc9..953f1dbc4 100644 --- a/src/symbolize.cc +++ b/src/symbolize.cc @@ -840,6 +840,7 @@ _END_GOOGLE_NAMESPACE_ #elif defined(OS_WINDOWS) #include +#pragma comment(lib, "DbgHelp") _START_GOOGLE_NAMESPACE_ From 4c14eb179cf444423f90c995fdd4b12a39eeaa6e Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Mon, 26 Jun 2017 17:13:21 -0700 Subject: [PATCH 7/7] Support signal handler on Windows --- CMakeLists.txt | 4 ++-- src/logging.cc | 9 +++----- src/signalhandler.cc | 42 +++++++++++++++++++++++++++++------ src/signalhandler_unittest.cc | 12 +++++++++- src/utilities.cc | 6 +++-- 5 files changed, 55 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index af4680da3..7f282959c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -364,9 +364,9 @@ set (GLOG_SRCS src/vlog_is_on.cc ) -if (HAVE_PTHREAD) +if (HAVE_PTHREAD OR WIN32) list (APPEND GLOG_SRCS src/signalhandler.cc) -endif (HAVE_PTHREAD) +endif (HAVE_PTHREAD OR WIN32) if (WIN32) list (APPEND GLOG_SRCS diff --git a/src/logging.cc b/src/logging.cc index 0b5e6ee97..32a5791a8 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -1463,16 +1463,13 @@ void LogMessage::RecordCrashReason( # define ATTRIBUTE_NORETURN #endif +#if defined(OS_WINDOWS) +__declspec(noreturn) +#endif static void logging_fail() ATTRIBUTE_NORETURN; static void logging_fail() { -#if defined(_DEBUG) && defined(_MSC_VER) - // When debugging on windows, avoid the obnoxious dialog and make - // it possible to continue past a LOG(FATAL) in the debugger - __debugbreak(); -#else abort(); -#endif } typedef void (*logging_fail_func_t)() ATTRIBUTE_NORETURN; diff --git a/src/signalhandler.cc b/src/signalhandler.cc index a7aef8b99..72fd92baf 100644 --- a/src/signalhandler.cc +++ b/src/signalhandler.cc @@ -48,9 +48,6 @@ _START_GOOGLE_NAMESPACE_ -// TOOD(hamaji): Use signal instead of sigaction? -#ifdef HAVE_SIGACTION - namespace { // We'll install the failure signal handler for these signals. We could @@ -66,10 +63,14 @@ const struct { { SIGILL, "SIGILL" }, { SIGFPE, "SIGFPE" }, { SIGABRT, "SIGABRT" }, +#if !defined(OS_WINDOWS) { SIGBUS, "SIGBUS" }, +#endif { SIGTERM, "SIGTERM" }, }; +static bool kFailureSignalHandlerInstalled = false; + // Returns the program counter from signal context, NULL if unknown. void* GetPC(void* ucontext_in_void) { #if (defined(HAVE_UCONTEXT_H) || defined(HAVE_SYS_UCONTEXT_H)) && defined(PC_FROM_UCONTEXT) @@ -168,6 +169,9 @@ void DumpTimeInfo() { g_failure_writer(buf, formatter.num_bytes_written()); } +// TOOD(hamaji): Use signal instead of sigaction? +#ifdef HAVE_SIGACTION + // Dumps information about the signal to STDERR. void DumpSignalInfo(int signal_number, siginfo_t *siginfo) { // Get the signal name. @@ -213,6 +217,8 @@ void DumpSignalInfo(int signal_number, siginfo_t *siginfo) { g_failure_writer(buf, formatter.num_bytes_written()); } +#endif // HAVE_SIGACTION + // Dumps information about the stack frame to STDERR. void DumpStackFrameInfo(const char* prefix, void* pc) { // Get the symbol name. @@ -240,12 +246,17 @@ void DumpStackFrameInfo(const char* prefix, void* pc) { // Invoke the default signal handler. void InvokeDefaultSignalHandler(int signal_number) { +#ifdef HAVE_SIGACTION struct sigaction sig_action; memset(&sig_action, 0, sizeof(sig_action)); sigemptyset(&sig_action.sa_mask); sig_action.sa_handler = SIG_DFL; sigaction(signal_number, &sig_action, NULL); kill(getpid(), signal_number); +#elif defined(OS_WINDOWS) + signal(signal_number, SIG_DFL); + raise(signal_number); +#endif } // This variable is used for protecting FailureSignalHandler() from @@ -256,9 +267,14 @@ static pthread_t* g_entered_thread_id_pointer = NULL; // Dumps signal and stack frame information, and invokes the default // signal handler once our job is done. +#if defined(OS_WINDOWS) +void FailureSignalHandler(int signal_number) +#else void FailureSignalHandler(int signal_number, siginfo_t *signal_info, - void *ucontext) { + void *ucontext) +#endif +{ // First check if we've already entered the function. We use an atomic // compare and swap operation for platforms that support it. For other // platforms, we use a naive method that could lead to a subtle race. @@ -298,16 +314,20 @@ void FailureSignalHandler(int signal_number, // First dump time info. DumpTimeInfo(); +#if !defined(OS_WINDOWS) // Get the program counter from ucontext. void *pc = GetPC(ucontext); DumpStackFrameInfo("PC: ", pc); +#endif #ifdef HAVE_STACKTRACE // Get the stack traces. void *stack[32]; // +1 to exclude this function. const int depth = GetStackTrace(stack, ARRAYSIZE(stack), 1); +# ifdef HAVE_SIGACTION DumpSignalInfo(signal_number, signal_info); +# endif // Dump the stack traces. for (int i = 0; i < depth; ++i) { DumpStackFrameInfo(" ", stack[i]); @@ -333,18 +353,19 @@ void FailureSignalHandler(int signal_number, } // namespace -#endif // HAVE_SIGACTION - namespace glog_internal_namespace_ { bool IsFailureSignalHandlerInstalled() { #ifdef HAVE_SIGACTION + // TODO(andschwa): Return kFailureSignalHandlerInstalled? struct sigaction sig_action; memset(&sig_action, 0, sizeof(sig_action)); sigemptyset(&sig_action.sa_mask); sigaction(SIGABRT, NULL, &sig_action); if (sig_action.sa_sigaction == &FailureSignalHandler) return true; +#elif defined(OS_WINDOWS) + return kFailureSignalHandlerInstalled; #endif // HAVE_SIGACTION return false; } @@ -363,11 +384,18 @@ void InstallFailureSignalHandler() { for (size_t i = 0; i < ARRAYSIZE(kFailureSignals); ++i) { CHECK_ERR(sigaction(kFailureSignals[i].number, &sig_action, NULL)); } + kFailureSignalHandlerInstalled = true; +#elif defined(OS_WINDOWS) + for (size_t i = 0; i < ARRAYSIZE(kFailureSignals); ++i) { + CHECK_NE(signal(kFailureSignals[i].number, &FailureSignalHandler), + SIG_ERR); + } + kFailureSignalHandlerInstalled = true; #endif // HAVE_SIGACTION } void InstallFailureWriter(void (*writer)(const char* data, int size)) { -#ifdef HAVE_SIGACTION +#if defined(HAVE_SIGACTION) || defined(OS_WINDOWS) g_failure_writer = writer; #endif // HAVE_SIGACTION } diff --git a/src/signalhandler_unittest.cc b/src/signalhandler_unittest.cc index 59be2315f..e85f523af 100644 --- a/src/signalhandler_unittest.cc +++ b/src/signalhandler_unittest.cc @@ -34,7 +34,9 @@ #include "utilities.h" -#include +#if defined(HAVE_PTHREAD) +# include +#endif #include #include #include @@ -87,12 +89,20 @@ int main(int argc, char **argv) { fprintf(stderr, "looping\n"); while (true); } else if (command == "die_in_thread") { +#if defined(HAVE_PTHREAD) pthread_t thread; pthread_create(&thread, NULL, &DieInThread, NULL); pthread_join(thread, NULL); +#else + fprintf(stderr, "no pthread\n"); + return 1; +#endif } else if (command == "dump_to_stdout") { InstallFailureWriter(WriteToStdout); abort(); + } else if (command == "installed") { + fprintf(stderr, "signal handler installed: %s\n", + IsFailureSignalHandlerInstalled() ? "true" : "false"); } else { // Tell the shell script puts("OK"); diff --git a/src/utilities.cc b/src/utilities.cc index 0d686eb46..e73cee021 100644 --- a/src/utilities.cc +++ b/src/utilities.cc @@ -137,17 +137,19 @@ static void DumpStackTraceAndExit() { DumpStackTrace(1, DebugWriteToStderr, NULL); // TOOD(hamaji): Use signal instead of sigaction? -#ifdef HAVE_SIGACTION if (IsFailureSignalHandlerInstalled()) { // Set the default signal handler for SIGABRT, to avoid invoking our // own signal handler installed by InstallFailureSignalHandler(). +#ifdef HAVE_SIGACTION struct sigaction sig_action; memset(&sig_action, 0, sizeof(sig_action)); sigemptyset(&sig_action.sa_mask); sig_action.sa_handler = SIG_DFL; sigaction(SIGABRT, &sig_action, NULL); - } +#elif defined(OS_WINDOWS) + signal(SIGABRT, SIG_DFL); #endif // HAVE_SIGACTION + } abort(); }