From 760e86664777796de61f9c1feb16fa95a71ee5fe Mon Sep 17 00:00:00 2001 From: moko256 Date: Fri, 16 Sep 2022 21:43:40 +0900 Subject: [PATCH 01/26] Add Windows support for `//flutter/fml/backtrace.h` --- fml/BUILD.gn | 4 +-- fml/backtrace.cc | 51 ++++++++++++++++++++++++++------------ fml/backtrace_unittests.cc | 3 +++ 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index f7b5be3dc6a62..e9de9f2b36d53 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -96,7 +96,7 @@ source_set("fml") { "wakeable.h", ] - if (is_mac || is_linux || (is_ios && is_debug)) { + if (is_mac || is_linux || is_win || (is_ios && is_debug)) { sources += [ "backtrace.cc" ] } else { sources += [ "backtrace_stub.cc" ] @@ -115,7 +115,7 @@ source_set("fml") { "//third_party/icu", ] - if (is_mac || is_linux || (is_ios && is_debug)) { + if (is_mac || is_linux || is_win || (is_ios && is_debug)) { # This abseil dependency is only used by backtrace.cc. deps += [ "//third_party/abseil-cpp/absl/debugging:symbolize" ] } diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 34b0ef06b146b..e9eaed61ebdfc 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -4,27 +4,32 @@ #include "flutter/fml/backtrace.h" -#include -#include -#include - #include #include -#if FML_OS_WIN -#include -#include -#endif - +#include "flutter/fml/build_config.h" #include "flutter/fml/logging.h" #include "flutter/fml/paths.h" #include "third_party/abseil-cpp/absl/debugging/symbolize.h" +#ifdef FML_OS_WIN +#include +#include +#include +#else // FML_OS_WIN +#include +#include +#include +#endif // FML_OS_WIN + namespace fml { static std::string kKUnknownFrameName = "Unknown"; static std::string DemangleSymbolName(const std::string& mangled) { +#if FML_OS_WIN + return mangled; +#else if (mangled == kKUnknownFrameName) { return kKUnknownFrameName; } @@ -44,6 +49,7 @@ static std::string DemangleSymbolName(const std::string& mangled) { auto demangled_string = std::string{demangled, length}; free(demangled); return demangled_string; +#endif // FML_OS_WIN } static std::string GetSymbolName(void* symbol) { @@ -55,10 +61,18 @@ static std::string GetSymbolName(void* symbol) { return DemangleSymbolName({name}); } +static int Backtrace(void** symbols, int size) { +#if FML_OS_WIN + return CaptureStackBackTrace(0, size, symbols, NULL); +#else + return ::backtrace(symbols, size); +#endif // FML_OS_WIN +} + std::string BacktraceHere(size_t offset) { constexpr size_t kMaxFrames = 256; void* symbols[kMaxFrames]; - const auto available_frames = ::backtrace(symbols, kMaxFrames); + const auto available_frames = Backtrace(symbols, kMaxFrames); if (available_frames <= 0) { return ""; } @@ -74,12 +88,15 @@ std::string BacktraceHere(size_t offset) { static size_t kKnownSignalHandlers[] = { SIGABRT, // abort program SIGFPE, // floating-point exception - SIGBUS, // bus error + SIGTERM, // software termination signal SIGSEGV, // segmentation violation +#if !FML_OS_WIN + SIGBUS, // bus error SIGSYS, // non-existent system call invoked SIGPIPE, // write on a pipe with no reader SIGALRM, // real-time timer expired - SIGTERM, // software termination signal + +#endif // !FML_OS_WIN }; static std::string SignalNameToString(int signal) { @@ -88,18 +105,20 @@ static std::string SignalNameToString(int signal) { return "SIGABRT"; case SIGFPE: return "SIGFPE"; - case SIGBUS: - return "SIGBUS"; case SIGSEGV: return "SIGSEGV"; + case SIGTERM: + return "SIGTERM"; +#if !FML_OS_WIN + case SIGBUS: + return "SIGBUS"; case SIGSYS: return "SIGSYS"; case SIGPIPE: return "SIGPIPE"; case SIGALRM: return "SIGALRM"; - case SIGTERM: - return "SIGTERM"; +#endif // !FML_OS_WIN }; return std::to_string(signal); } diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index f3b2a64114e5f..fb858bc4556db 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -19,18 +19,21 @@ TEST(BacktraceTest, CanGatherBacktrace) { auto trace = BacktraceHere(0); ASSERT_GT(trace.size(), 0u); ASSERT_NE(trace.find("Frame 0"), std::string::npos); + std::cout << trace << std::endl; } { auto trace = BacktraceHere(1); ASSERT_GT(trace.size(), 0u); ASSERT_NE(trace.find("Frame 0"), std::string::npos); + std::cout << trace << std::endl; } { auto trace = BacktraceHere(2); ASSERT_GT(trace.size(), 0u); ASSERT_NE(trace.find("Frame 0"), std::string::npos); + std::cout << trace << std::endl; } } From 6b26b252a7a48b760bc327345262048cca091f07 Mon Sep 17 00:00:00 2001 From: moko256 Date: Tue, 20 Sep 2022 15:51:41 +0900 Subject: [PATCH 02/26] remove empty break --- fml/backtrace.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index e9eaed61ebdfc..14a9446ca2733 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -95,7 +95,6 @@ static size_t kKnownSignalHandlers[] = { SIGSYS, // non-existent system call invoked SIGPIPE, // write on a pipe with no reader SIGALRM, // real-time timer expired - #endif // !FML_OS_WIN }; From aa9290331a1cd3274be57fd68f70780b563c047a Mon Sep 17 00:00:00 2001 From: moko256 Date: Tue, 20 Sep 2022 16:02:01 +0900 Subject: [PATCH 03/26] use absl::GetStackTrace --- fml/BUILD.gn | 5 ++++- fml/backtrace.cc | 16 +++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index e9de9f2b36d53..42ba933a83ba3 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -117,7 +117,10 @@ source_set("fml") { if (is_mac || is_linux || is_win || (is_ios && is_debug)) { # This abseil dependency is only used by backtrace.cc. - deps += [ "//third_party/abseil-cpp/absl/debugging:symbolize" ] + deps += [ + "//third_party/abseil-cpp/absl/debugging:stacktrace", + "//third_party/abseil-cpp/absl/debugging:symbolize" + ] } configs += [ "//third_party/icu:icu_config" ] diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 14a9446ca2733..6852f366becdc 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -10,6 +10,7 @@ #include "flutter/fml/build_config.h" #include "flutter/fml/logging.h" #include "flutter/fml/paths.h" +#include "third_party/abseil-cpp/absl/debugging/stacktrace.h" #include "third_party/abseil-cpp/absl/debugging/symbolize.h" #ifdef FML_OS_WIN @@ -61,25 +62,18 @@ static std::string GetSymbolName(void* symbol) { return DemangleSymbolName({name}); } -static int Backtrace(void** symbols, int size) { -#if FML_OS_WIN - return CaptureStackBackTrace(0, size, symbols, NULL); -#else - return ::backtrace(symbols, size); -#endif // FML_OS_WIN -} - std::string BacktraceHere(size_t offset) { constexpr size_t kMaxFrames = 256; void* symbols[kMaxFrames]; - const auto available_frames = Backtrace(symbols, kMaxFrames); + const auto available_frames = + absl::GetStackTrace(symbols, kMaxFrames, offset); if (available_frames <= 0) { return ""; } std::stringstream stream; - for (int i = 1 + offset; i < available_frames; ++i) { - stream << "Frame " << i - 1 - offset << ": " << symbols[i] << " " + for (int i = 0; i < available_frames; i++) { + stream << "Frame " << i << ": " << symbols[i] << " " << GetSymbolName(symbols[i]) << std::endl; } return stream.str(); From 1d5b4fc6d4f3529b075241a6b2869c353a53e11f Mon Sep 17 00:00:00 2001 From: moko256 Date: Tue, 20 Sep 2022 16:13:58 +0900 Subject: [PATCH 04/26] remove __cxxabiv1::__cxa_demangle --- fml/backtrace.cc | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 6852f366becdc..5016fb0390b25 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -27,39 +27,12 @@ namespace fml { static std::string kKUnknownFrameName = "Unknown"; -static std::string DemangleSymbolName(const std::string& mangled) { -#if FML_OS_WIN - return mangled; -#else - if (mangled == kKUnknownFrameName) { - return kKUnknownFrameName; - } - - int status = 0; - size_t length = 0; - char* demangled = __cxxabiv1::__cxa_demangle( - mangled.data(), // mangled name - nullptr, // output buffer (malloc-ed if nullptr) - &length, // demangled length - &status); - - if (demangled == nullptr || status != 0) { - return mangled; - } - - auto demangled_string = std::string{demangled, length}; - free(demangled); - return demangled_string; -#endif // FML_OS_WIN -} - static std::string GetSymbolName(void* symbol) { char name[1024]; if (!absl::Symbolize(symbol, name, sizeof(name))) { return kKUnknownFrameName; } - - return DemangleSymbolName({name}); + return name; } std::string BacktraceHere(size_t offset) { From 09649c1d07aa65a77b455c1d756ffdd5d339d1aa Mon Sep 17 00:00:00 2001 From: moko256 Date: Wed, 21 Sep 2022 01:14:25 +0900 Subject: [PATCH 05/26] replace with absil's signal handler --- fml/BUILD.gn | 2 +- fml/backtrace.cc | 110 +------------------------------------ fml/backtrace.h | 2 - fml/backtrace_unittests.cc | 24 +------- 4 files changed, 7 insertions(+), 131 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 42ba933a83ba3..f7daebad8577c 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -118,7 +118,7 @@ source_set("fml") { if (is_mac || is_linux || is_win || (is_ios && is_debug)) { # This abseil dependency is only used by backtrace.cc. deps += [ - "//third_party/abseil-cpp/absl/debugging:stacktrace", + "//third_party/abseil-cpp/absl/debugging:failure_signal_handler", "//third_party/abseil-cpp/absl/debugging:symbolize" ] } diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 5016fb0390b25..3f03b759cc9ed 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -4,118 +4,13 @@ #include "flutter/fml/backtrace.h" -#include -#include - #include "flutter/fml/build_config.h" -#include "flutter/fml/logging.h" #include "flutter/fml/paths.h" -#include "third_party/abseil-cpp/absl/debugging/stacktrace.h" +#include "third_party/abseil-cpp/absl/debugging/failure_signal_handler.h" #include "third_party/abseil-cpp/absl/debugging/symbolize.h" -#ifdef FML_OS_WIN -#include -#include -#include -#else // FML_OS_WIN -#include -#include -#include -#endif // FML_OS_WIN - namespace fml { -static std::string kKUnknownFrameName = "Unknown"; - -static std::string GetSymbolName(void* symbol) { - char name[1024]; - if (!absl::Symbolize(symbol, name, sizeof(name))) { - return kKUnknownFrameName; - } - return name; -} - -std::string BacktraceHere(size_t offset) { - constexpr size_t kMaxFrames = 256; - void* symbols[kMaxFrames]; - const auto available_frames = - absl::GetStackTrace(symbols, kMaxFrames, offset); - if (available_frames <= 0) { - return ""; - } - - std::stringstream stream; - for (int i = 0; i < available_frames; i++) { - stream << "Frame " << i << ": " << symbols[i] << " " - << GetSymbolName(symbols[i]) << std::endl; - } - return stream.str(); -} - -static size_t kKnownSignalHandlers[] = { - SIGABRT, // abort program - SIGFPE, // floating-point exception - SIGTERM, // software termination signal - SIGSEGV, // segmentation violation -#if !FML_OS_WIN - SIGBUS, // bus error - SIGSYS, // non-existent system call invoked - SIGPIPE, // write on a pipe with no reader - SIGALRM, // real-time timer expired -#endif // !FML_OS_WIN -}; - -static std::string SignalNameToString(int signal) { - switch (signal) { - case SIGABRT: - return "SIGABRT"; - case SIGFPE: - return "SIGFPE"; - case SIGSEGV: - return "SIGSEGV"; - case SIGTERM: - return "SIGTERM"; -#if !FML_OS_WIN - case SIGBUS: - return "SIGBUS"; - case SIGSYS: - return "SIGSYS"; - case SIGPIPE: - return "SIGPIPE"; - case SIGALRM: - return "SIGALRM"; -#endif // !FML_OS_WIN - }; - return std::to_string(signal); -} - -static void ToggleSignalHandlers(bool set); - -static void SignalHandler(int signal) { - // We are a crash signal handler. This can only happen once. Since we don't - // want to catch crashes while we are generating the crash reports, disable - // all set signal handlers to their default values before reporting the crash - // and re-raising the signal. - ToggleSignalHandlers(false); - - FML_LOG(ERROR) << "Caught signal " << SignalNameToString(signal) - << " during program execution." << std::endl - << BacktraceHere(3); - - ::raise(signal); -} - -static void ToggleSignalHandlers(bool set) { - for (size_t i = 0; i < sizeof(kKnownSignalHandlers) / sizeof(size_t); ++i) { - auto signal_name = kKnownSignalHandlers[i]; - auto handler = set ? &SignalHandler : SIG_DFL; - - if (::signal(signal_name, handler) == SIG_ERR) { - FML_LOG(ERROR) << "Could not attach signal handler for " << signal_name; - } - } -} - void InstallCrashHandler() { #if FML_OS_WIN if (!IsDebuggerPresent()) { @@ -127,7 +22,8 @@ void InstallCrashHandler() { if (exe_path.first) { absl::InitializeSymbolizer(exe_path.second.c_str()); } - ToggleSignalHandlers(true); + absl::FailureSignalHandlerOptions options; + absl::InstallFailureSignalHandler(options); } bool IsCrashHandlingSupported() { diff --git a/fml/backtrace.h b/fml/backtrace.h index 18f17933b355b..6ed5cc245c370 100644 --- a/fml/backtrace.h +++ b/fml/backtrace.h @@ -11,8 +11,6 @@ namespace fml { -std::string BacktraceHere(size_t offset = 0); - void InstallCrashHandler(); bool IsCrashHandlingSupported(); diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index fb858bc4556db..682c7e87898d8 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -4,8 +4,9 @@ #include "backtrace.h" +#include + #include "gtest/gtest.h" -#include "logging.h" namespace fml { namespace testing { @@ -15,26 +16,7 @@ TEST(BacktraceTest, CanGatherBacktrace) { GTEST_SKIP(); return; } - { - auto trace = BacktraceHere(0); - ASSERT_GT(trace.size(), 0u); - ASSERT_NE(trace.find("Frame 0"), std::string::npos); - std::cout << trace << std::endl; - } - - { - auto trace = BacktraceHere(1); - ASSERT_GT(trace.size(), 0u); - ASSERT_NE(trace.find("Frame 0"), std::string::npos); - std::cout << trace << std::endl; - } - - { - auto trace = BacktraceHere(2); - ASSERT_GT(trace.size(), 0u); - ASSERT_NE(trace.find("Frame 0"), std::string::npos); - std::cout << trace << std::endl; - } + EXPECT_DEATH_IF_SUPPORTED({ std::abort(); }, "raise"); } } // namespace testing From dca3616a9f9dd0e6f33624b6c53f3f49442de5f5 Mon Sep 17 00:00:00 2001 From: moko256 Date: Wed, 21 Sep 2022 15:59:08 +0900 Subject: [PATCH 06/26] fix format --- fml/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index f7daebad8577c..352e87bba837c 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -119,7 +119,7 @@ source_set("fml") { # This abseil dependency is only used by backtrace.cc. deps += [ "//third_party/abseil-cpp/absl/debugging:failure_signal_handler", - "//third_party/abseil-cpp/absl/debugging:symbolize" + "//third_party/abseil-cpp/absl/debugging:symbolize", ] } From dba40171547b46f99fcba2fc8cdaf7b83980a9f3 Mon Sep 17 00:00:00 2001 From: moko256 Date: Wed, 21 Sep 2022 16:42:07 +0900 Subject: [PATCH 07/26] cleanup unused vars --- fml/backtrace.h | 2 -- fml/backtrace_stub.cc | 6 ------ 2 files changed, 8 deletions(-) diff --git a/fml/backtrace.h b/fml/backtrace.h index 6ed5cc245c370..f83b856bd3e9b 100644 --- a/fml/backtrace.h +++ b/fml/backtrace.h @@ -5,8 +5,6 @@ #ifndef FLUTTER_FML_BACKTRACE_H_ #define FLUTTER_FML_BACKTRACE_H_ -#include - #include "flutter/fml/macros.h" namespace fml { diff --git a/fml/backtrace_stub.cc b/fml/backtrace_stub.cc index 2328ec45aa2a9..66d71980b400c 100644 --- a/fml/backtrace_stub.cc +++ b/fml/backtrace_stub.cc @@ -6,12 +6,6 @@ namespace fml { -static std::string kKUnknownFrameName = "Unknown"; - -std::string BacktraceHere(size_t offset) { - return ""; -} - void InstallCrashHandler() { // Not supported. } From a5b212cb6282b09637f0ed5ebe5852576e8285f4 Mon Sep 17 00:00:00 2001 From: moko256 Date: Wed, 21 Sep 2022 16:43:33 +0900 Subject: [PATCH 08/26] fix test to enable to run without symbol --- fml/backtrace_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index 682c7e87898d8..b94f7e642856c 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -16,7 +16,7 @@ TEST(BacktraceTest, CanGatherBacktrace) { GTEST_SKIP(); return; } - EXPECT_DEATH_IF_SUPPORTED({ std::abort(); }, "raise"); + EXPECT_DEATH_IF_SUPPORTED({ std::abort(); }, "SIGABRT"); } } // namespace testing From 4fe24c3e1b795aabc215389570c2ffd3b734cf6c Mon Sep 17 00:00:00 2001 From: moko256 Date: Wed, 21 Sep 2022 22:55:50 +0900 Subject: [PATCH 09/26] mark BacktraceTest as DeathTest --- fml/backtrace_unittests.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index b94f7e642856c..db98fbb70b003 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -11,11 +11,12 @@ namespace fml { namespace testing { -TEST(BacktraceTest, CanGatherBacktrace) { +TEST(BacktraceDeathTest, CanGatherBacktrace) { if (!IsCrashHandlingSupported()) { GTEST_SKIP(); return; } + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; EXPECT_DEATH_IF_SUPPORTED({ std::abort(); }, "SIGABRT"); } From f67dcf68624dad30b8851d47812d66e43d2e7754 Mon Sep 17 00:00:00 2001 From: moko256 Date: Fri, 23 Sep 2022 21:37:13 +0900 Subject: [PATCH 10/26] restore BacktraceHere --- fml/BUILD.gn | 1 + fml/backtrace.cc | 28 ++++++++++++++++++++++++++++ fml/backtrace.h | 4 ++++ fml/backtrace_stub.cc | 6 ++++++ fml/backtrace_unittests.cc | 12 +++++++----- 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 352e87bba837c..5f9e146096faa 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -119,6 +119,7 @@ source_set("fml") { # This abseil dependency is only used by backtrace.cc. deps += [ "//third_party/abseil-cpp/absl/debugging:failure_signal_handler", + "//third_party/abseil-cpp/absl/debugging:stacktrace", "//third_party/abseil-cpp/absl/debugging:symbolize", ] } diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 3f03b759cc9ed..cae5f9ad6ab11 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -7,10 +7,38 @@ #include "flutter/fml/build_config.h" #include "flutter/fml/paths.h" #include "third_party/abseil-cpp/absl/debugging/failure_signal_handler.h" +#include "third_party/abseil-cpp/absl/debugging/stacktrace.h" #include "third_party/abseil-cpp/absl/debugging/symbolize.h" namespace fml { +static std::string kKUnknownFrameName = "Unknown"; + +static std::string GetSymbolName(void* symbol) { + char name[1024]; + if (!absl::Symbolize(symbol, name, sizeof(name))) { + return kKUnknownFrameName; + } + return name; +} + +std::string BacktraceHere(size_t offset) { + constexpr size_t kMaxFrames = 256; + void* symbols[kMaxFrames]; + const auto available_frames = + absl::GetStackTrace(symbols, kMaxFrames, offset); + if (available_frames <= 0) { + return ""; + } + + std::stringstream stream; + for (int i = 0; i < available_frames; i++) { + stream << "Frame " << i << ": " << symbols[i] << " " + << GetSymbolName(symbols[i]) << std::endl; + } + return stream.str(); +} + void InstallCrashHandler() { #if FML_OS_WIN if (!IsDebuggerPresent()) { diff --git a/fml/backtrace.h b/fml/backtrace.h index f83b856bd3e9b..18f17933b355b 100644 --- a/fml/backtrace.h +++ b/fml/backtrace.h @@ -5,10 +5,14 @@ #ifndef FLUTTER_FML_BACKTRACE_H_ #define FLUTTER_FML_BACKTRACE_H_ +#include + #include "flutter/fml/macros.h" namespace fml { +std::string BacktraceHere(size_t offset = 0); + void InstallCrashHandler(); bool IsCrashHandlingSupported(); diff --git a/fml/backtrace_stub.cc b/fml/backtrace_stub.cc index 66d71980b400c..2328ec45aa2a9 100644 --- a/fml/backtrace_stub.cc +++ b/fml/backtrace_stub.cc @@ -6,6 +6,12 @@ namespace fml { +static std::string kKUnknownFrameName = "Unknown"; + +std::string BacktraceHere(size_t offset) { + return ""; +} + void InstallCrashHandler() { // Not supported. } diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index db98fbb70b003..feaa8bfef554b 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -4,20 +4,22 @@ #include "backtrace.h" -#include - #include "gtest/gtest.h" namespace fml { namespace testing { -TEST(BacktraceDeathTest, CanGatherBacktrace) { +TEST(BacktraceTest, CanGatherBacktrace) { if (!IsCrashHandlingSupported()) { GTEST_SKIP(); return; } - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - EXPECT_DEATH_IF_SUPPORTED({ std::abort(); }, "SIGABRT"); + { + auto trace = BacktraceHere(0); + ASSERT_GT(trace.size(), 0u); + ASSERT_NE(trace.find("Frame 0"), std::string::npos); + std::cout << trace << std::endl; + } } } // namespace testing From 393233ef09901d02d69c9ba4c347b381e03e7d56 Mon Sep 17 00:00:00 2001 From: moko256 Date: Sat, 24 Sep 2022 01:02:41 +0900 Subject: [PATCH 11/26] add comment --- fml/backtrace.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fml/backtrace.h b/fml/backtrace.h index 18f17933b355b..2f75834313d07 100644 --- a/fml/backtrace.h +++ b/fml/backtrace.h @@ -11,6 +11,9 @@ namespace fml { +// Retrieve the backtrace. It is maintained for debugging. +// +// If the |offset| is 0, the backtrace is included caller function. std::string BacktraceHere(size_t offset = 0); void InstallCrashHandler(); From edfe8c14746fbac170564ec537e6b2160354acac Mon Sep 17 00:00:00 2001 From: moko256 Date: Sat, 24 Sep 2022 01:09:49 +0900 Subject: [PATCH 12/26] test if stacktrace is supported --- fml/BUILD.gn | 1 + fml/backtrace_unittests.cc | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 5f9e146096faa..e8563ffd16016 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -361,6 +361,7 @@ if (enable_unittests) { "//flutter/runtime", "//flutter/runtime:libdart", "//flutter/testing", + "//third_party/abseil-cpp/absl/debugging:stacktrace", ] # This is needed for //third_party/googletest for linking zircon symbols. diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index feaa8bfef554b..fedc50d324289 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -5,6 +5,7 @@ #include "backtrace.h" #include "gtest/gtest.h" +#include "third_party/abseil-cpp/absl/debugging/stacktrace.h" namespace fml { namespace testing { @@ -22,5 +23,9 @@ TEST(BacktraceTest, CanGatherBacktrace) { } } +TEST(BacktraceTest, BacktraceEnabled) { + ASSERT_EQ(absl::debugging_internal::StackTraceWorksForTest(), true); +} + } // namespace testing } // namespace fml From 30105e701f69b7de04539186cef5fe7d16d0820a Mon Sep 17 00:00:00 2001 From: moko256 Date: Sat, 24 Sep 2022 01:50:27 +0900 Subject: [PATCH 13/26] Revert "test if stacktrace is supported" This reverts commit edfe8c14746fbac170564ec537e6b2160354acac. --- fml/BUILD.gn | 1 - fml/backtrace_unittests.cc | 5 ----- 2 files changed, 6 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index e8563ffd16016..5f9e146096faa 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -361,7 +361,6 @@ if (enable_unittests) { "//flutter/runtime", "//flutter/runtime:libdart", "//flutter/testing", - "//third_party/abseil-cpp/absl/debugging:stacktrace", ] # This is needed for //third_party/googletest for linking zircon symbols. diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index fedc50d324289..feaa8bfef554b 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -5,7 +5,6 @@ #include "backtrace.h" #include "gtest/gtest.h" -#include "third_party/abseil-cpp/absl/debugging/stacktrace.h" namespace fml { namespace testing { @@ -23,9 +22,5 @@ TEST(BacktraceTest, CanGatherBacktrace) { } } -TEST(BacktraceTest, BacktraceEnabled) { - ASSERT_EQ(absl::debugging_internal::StackTraceWorksForTest(), true); -} - } // namespace testing } // namespace fml From 51eaf5d061adca9c576c17b10c5175f097791d3a Mon Sep 17 00:00:00 2001 From: moko256 Date: Sun, 25 Sep 2022 23:24:33 +0900 Subject: [PATCH 14/26] Rollback to 6b26b25 --- fml/BUILD.gn | 6 +- fml/backtrace.cc | 127 ++++++++++++++++++++++++++++++++++--- fml/backtrace.h | 3 - fml/backtrace_unittests.cc | 15 +++++ 4 files changed, 134 insertions(+), 17 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 5f9e146096faa..e9de9f2b36d53 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -117,11 +117,7 @@ source_set("fml") { if (is_mac || is_linux || is_win || (is_ios && is_debug)) { # This abseil dependency is only used by backtrace.cc. - deps += [ - "//third_party/abseil-cpp/absl/debugging:failure_signal_handler", - "//third_party/abseil-cpp/absl/debugging:stacktrace", - "//third_party/abseil-cpp/absl/debugging:symbolize", - ] + deps += [ "//third_party/abseil-cpp/absl/debugging:symbolize" ] } configs += [ "//third_party/icu:icu_config" ] diff --git a/fml/backtrace.cc b/fml/backtrace.cc index cae5f9ad6ab11..14a9446ca2733 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -4,41 +4,151 @@ #include "flutter/fml/backtrace.h" +#include +#include + #include "flutter/fml/build_config.h" +#include "flutter/fml/logging.h" #include "flutter/fml/paths.h" -#include "third_party/abseil-cpp/absl/debugging/failure_signal_handler.h" -#include "third_party/abseil-cpp/absl/debugging/stacktrace.h" #include "third_party/abseil-cpp/absl/debugging/symbolize.h" +#ifdef FML_OS_WIN +#include +#include +#include +#else // FML_OS_WIN +#include +#include +#include +#endif // FML_OS_WIN + namespace fml { static std::string kKUnknownFrameName = "Unknown"; +static std::string DemangleSymbolName(const std::string& mangled) { +#if FML_OS_WIN + return mangled; +#else + if (mangled == kKUnknownFrameName) { + return kKUnknownFrameName; + } + + int status = 0; + size_t length = 0; + char* demangled = __cxxabiv1::__cxa_demangle( + mangled.data(), // mangled name + nullptr, // output buffer (malloc-ed if nullptr) + &length, // demangled length + &status); + + if (demangled == nullptr || status != 0) { + return mangled; + } + + auto demangled_string = std::string{demangled, length}; + free(demangled); + return demangled_string; +#endif // FML_OS_WIN +} + static std::string GetSymbolName(void* symbol) { char name[1024]; if (!absl::Symbolize(symbol, name, sizeof(name))) { return kKUnknownFrameName; } - return name; + + return DemangleSymbolName({name}); +} + +static int Backtrace(void** symbols, int size) { +#if FML_OS_WIN + return CaptureStackBackTrace(0, size, symbols, NULL); +#else + return ::backtrace(symbols, size); +#endif // FML_OS_WIN } std::string BacktraceHere(size_t offset) { constexpr size_t kMaxFrames = 256; void* symbols[kMaxFrames]; - const auto available_frames = - absl::GetStackTrace(symbols, kMaxFrames, offset); + const auto available_frames = Backtrace(symbols, kMaxFrames); if (available_frames <= 0) { return ""; } std::stringstream stream; - for (int i = 0; i < available_frames; i++) { - stream << "Frame " << i << ": " << symbols[i] << " " + for (int i = 1 + offset; i < available_frames; ++i) { + stream << "Frame " << i - 1 - offset << ": " << symbols[i] << " " << GetSymbolName(symbols[i]) << std::endl; } return stream.str(); } +static size_t kKnownSignalHandlers[] = { + SIGABRT, // abort program + SIGFPE, // floating-point exception + SIGTERM, // software termination signal + SIGSEGV, // segmentation violation +#if !FML_OS_WIN + SIGBUS, // bus error + SIGSYS, // non-existent system call invoked + SIGPIPE, // write on a pipe with no reader + SIGALRM, // real-time timer expired +#endif // !FML_OS_WIN +}; + +static std::string SignalNameToString(int signal) { + switch (signal) { + case SIGABRT: + return "SIGABRT"; + case SIGFPE: + return "SIGFPE"; + case SIGSEGV: + return "SIGSEGV"; + case SIGTERM: + return "SIGTERM"; +#if !FML_OS_WIN + case SIGBUS: + return "SIGBUS"; + case SIGSYS: + return "SIGSYS"; + case SIGPIPE: + return "SIGPIPE"; + case SIGALRM: + return "SIGALRM"; +#endif // !FML_OS_WIN + }; + return std::to_string(signal); +} + +static void ToggleSignalHandlers(bool set); + +static void SignalHandler(int signal) { + // We are a crash signal handler. This can only happen once. Since we don't + // want to catch crashes while we are generating the crash reports, disable + // all set signal handlers to their default values before reporting the crash + // and re-raising the signal. + ToggleSignalHandlers(false); + + FML_LOG(ERROR) << "Caught signal " << SignalNameToString(signal) + << " during program execution." << std::endl + << BacktraceHere(3); + + ::raise(signal); +} + +static void ToggleSignalHandlers(bool set) { + for (size_t i = 0; i < sizeof(kKnownSignalHandlers) / sizeof(size_t); ++i) { + auto signal_name = kKnownSignalHandlers[i]; + auto handler = set ? &SignalHandler : SIG_DFL; + + if (::signal(signal_name, handler) == SIG_ERR) { + FML_LOG(ERROR) << "Could not attach signal handler for " << signal_name; + } + } +} + void InstallCrashHandler() { #if FML_OS_WIN if (!IsDebuggerPresent()) { @@ -50,8 +160,7 @@ void InstallCrashHandler() { if (exe_path.first) { absl::InitializeSymbolizer(exe_path.second.c_str()); } - absl::FailureSignalHandlerOptions options; - absl::InstallFailureSignalHandler(options); + ToggleSignalHandlers(true); } bool IsCrashHandlingSupported() { diff --git a/fml/backtrace.h b/fml/backtrace.h index 2f75834313d07..18f17933b355b 100644 --- a/fml/backtrace.h +++ b/fml/backtrace.h @@ -11,9 +11,6 @@ namespace fml { -// Retrieve the backtrace. It is maintained for debugging. -// -// If the |offset| is 0, the backtrace is included caller function. std::string BacktraceHere(size_t offset = 0); void InstallCrashHandler(); diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index feaa8bfef554b..fb858bc4556db 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -5,6 +5,7 @@ #include "backtrace.h" #include "gtest/gtest.h" +#include "logging.h" namespace fml { namespace testing { @@ -20,6 +21,20 @@ TEST(BacktraceTest, CanGatherBacktrace) { ASSERT_NE(trace.find("Frame 0"), std::string::npos); std::cout << trace << std::endl; } + + { + auto trace = BacktraceHere(1); + ASSERT_GT(trace.size(), 0u); + ASSERT_NE(trace.find("Frame 0"), std::string::npos); + std::cout << trace << std::endl; + } + + { + auto trace = BacktraceHere(2); + ASSERT_GT(trace.size(), 0u); + ASSERT_NE(trace.find("Frame 0"), std::string::npos); + std::cout << trace << std::endl; + } } } // namespace testing From 8df181be8c6e6e25a08f7f240af8784c549becdb Mon Sep 17 00:00:00 2001 From: moko256 Date: Sun, 25 Sep 2022 23:25:19 +0900 Subject: [PATCH 15/26] Restore some changes. --- fml/backtrace.cc | 35 +++++------------------------------ fml/backtrace.h | 3 +++ 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 14a9446ca2733..71ed8f65da4d3 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -17,7 +17,6 @@ #include #include #else // FML_OS_WIN -#include #include #include #endif // FML_OS_WIN @@ -26,39 +25,12 @@ namespace fml { static std::string kKUnknownFrameName = "Unknown"; -static std::string DemangleSymbolName(const std::string& mangled) { -#if FML_OS_WIN - return mangled; -#else - if (mangled == kKUnknownFrameName) { - return kKUnknownFrameName; - } - - int status = 0; - size_t length = 0; - char* demangled = __cxxabiv1::__cxa_demangle( - mangled.data(), // mangled name - nullptr, // output buffer (malloc-ed if nullptr) - &length, // demangled length - &status); - - if (demangled == nullptr || status != 0) { - return mangled; - } - - auto demangled_string = std::string{demangled, length}; - free(demangled); - return demangled_string; -#endif // FML_OS_WIN -} - static std::string GetSymbolName(void* symbol) { char name[1024]; if (!absl::Symbolize(symbol, name, sizeof(name))) { return kKUnknownFrameName; } - - return DemangleSymbolName({name}); + return name; } static int Backtrace(void** symbols, int size) { @@ -77,6 +49,9 @@ std::string BacktraceHere(size_t offset) { return ""; } + // Exclude here. + offset++; + std::stringstream stream; for (int i = 1 + offset; i < available_frames; ++i) { stream << "Frame " << i - 1 - offset << ": " << symbols[i] << " " @@ -95,7 +70,7 @@ static size_t kKnownSignalHandlers[] = { SIGSYS, // non-existent system call invoked SIGPIPE, // write on a pipe with no reader SIGALRM, // real-time timer expired -#endif // !FML_OS_WIN +#endif // !FML_OS_WIN }; static std::string SignalNameToString(int signal) { diff --git a/fml/backtrace.h b/fml/backtrace.h index 18f17933b355b..2f75834313d07 100644 --- a/fml/backtrace.h +++ b/fml/backtrace.h @@ -11,6 +11,9 @@ namespace fml { +// Retrieve the backtrace. It is maintained for debugging. +// +// If the |offset| is 0, the backtrace is included caller function. std::string BacktraceHere(size_t offset = 0); void InstallCrashHandler(); From 6d5f7c2d9a591a9705b527e9f3cc9e53b7177e6b Mon Sep 17 00:00:00 2001 From: moko256 Date: Sun, 25 Sep 2022 23:26:59 +0900 Subject: [PATCH 16/26] Remove unused include. --- fml/backtrace.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 71ed8f65da4d3..b86c241c0eb42 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -17,7 +17,6 @@ #include #include #else // FML_OS_WIN -#include #include #endif // FML_OS_WIN From a5a124a5218584afaa741747f3eb40c8bc3b4d26 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 01:04:02 +0900 Subject: [PATCH 17/26] Unite adding to offset --- fml/backtrace.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index b86c241c0eb42..264fa7d0bcda5 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -49,11 +49,11 @@ std::string BacktraceHere(size_t offset) { } // Exclude here. - offset++; + offset += 2; std::stringstream stream; - for (int i = 1 + offset; i < available_frames; ++i) { - stream << "Frame " << i - 1 - offset << ": " << symbols[i] << " " + for (int i = offset; i < available_frames; ++i) { + stream << "Frame " << i - offset << ": " << symbols[i] << " " << GetSymbolName(symbols[i]) << std::endl; } return stream.str(); From 763d2116604edda9df68a7f7eb19d6f6a49ce5c1 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 01:04:46 +0900 Subject: [PATCH 18/26] Remove test output --- fml/backtrace_unittests.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index fb858bc4556db..f3b2a64114e5f 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -19,21 +19,18 @@ TEST(BacktraceTest, CanGatherBacktrace) { auto trace = BacktraceHere(0); ASSERT_GT(trace.size(), 0u); ASSERT_NE(trace.find("Frame 0"), std::string::npos); - std::cout << trace << std::endl; } { auto trace = BacktraceHere(1); ASSERT_GT(trace.size(), 0u); ASSERT_NE(trace.find("Frame 0"), std::string::npos); - std::cout << trace << std::endl; } { auto trace = BacktraceHere(2); ASSERT_GT(trace.size(), 0u); ASSERT_NE(trace.find("Frame 0"), std::string::npos); - std::cout << trace << std::endl; } } From 987eee5e1cdca9444e3b8f7f240f2b85d7621cdd Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 01:23:49 +0900 Subject: [PATCH 19/26] Add test for BacktraceHere(0) --- fml/backtrace_unittests.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index f3b2a64114e5f..56ccf388afa71 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -18,7 +18,12 @@ TEST(BacktraceTest, CanGatherBacktrace) { { auto trace = BacktraceHere(0); ASSERT_GT(trace.size(), 0u); - ASSERT_NE(trace.find("Frame 0"), std::string::npos); + + auto first_line_end = trace.find("\n"); + ASSERT_GT(first_line_end, 0u); + ASSERT_NE(trace.rfind("Frame 0", first_line_end), std::string::npos); + ASSERT_NE(trace.rfind("CanGatherBacktrace", first_line_end), + std::string::npos); } { From d6c28d9be8d0b5d3381ccd80e615d945484b7724 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 02:20:36 +0900 Subject: [PATCH 20/26] Skip test if binary has no symbol --- fml/backtrace_unittests.cc | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index 56ccf388afa71..a3e0607cd30c1 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -18,12 +18,7 @@ TEST(BacktraceTest, CanGatherBacktrace) { { auto trace = BacktraceHere(0); ASSERT_GT(trace.size(), 0u); - - auto first_line_end = trace.find("\n"); - ASSERT_GT(first_line_end, 0u); - ASSERT_NE(trace.rfind("Frame 0", first_line_end), std::string::npos); - ASSERT_NE(trace.rfind("CanGatherBacktrace", first_line_end), - std::string::npos); + ASSERT_NE(trace.find("Frame 0"), std::string::npos); } { @@ -39,5 +34,21 @@ TEST(BacktraceTest, CanGatherBacktrace) { } } +TEST(BacktraceTest, BacktraceStartsWithCallerName) { + auto trace = BacktraceHere(0); + ASSERT_GT(trace.size(), 0u); + + auto first_line_end = trace.find("\n"); + ASSERT_GT(first_line_end, 0u); + + if (trace.rfind("Unknown", first_line_end) != std::string::npos) { + GTEST_SKIP() << "The symbols has been stripped from the executable."; + return; + } + + ASSERT_NE(trace.rfind("Frame 0", first_line_end), std::string::npos); + ASSERT_NE(trace.rfind(__FUNCTION__, first_line_end), std::string::npos); +} + } // namespace testing } // namespace fml From 20b6dc45bb93579fb5511977e22034f8f7778d75 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 03:13:13 +0900 Subject: [PATCH 21/26] Use gmock's matcher --- fml/backtrace_unittests.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index a3e0607cd30c1..8f954f39af859 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -4,6 +4,7 @@ #include "backtrace.h" +#include "gmock/gmock-matchers.h" #include "gtest/gtest.h" #include "logging.h" @@ -34,20 +35,22 @@ TEST(BacktraceTest, CanGatherBacktrace) { } } -TEST(BacktraceTest, BacktraceStartsWithCallerName) { +TEST(BacktraceTest, BacktraceStartsWithCallerFunction) { auto trace = BacktraceHere(0); ASSERT_GT(trace.size(), 0u); auto first_line_end = trace.find("\n"); - ASSERT_GT(first_line_end, 0u); + ASSERT_NE(first_line_end, std::string::npos); - if (trace.rfind("Unknown", first_line_end) != std::string::npos) { + auto first_line = trace.substr(0, first_line_end); + + if (first_line.find("Unknown") != std::string::npos) { GTEST_SKIP() << "The symbols has been stripped from the executable."; return; } - ASSERT_NE(trace.rfind("Frame 0", first_line_end), std::string::npos); - ASSERT_NE(trace.rfind(__FUNCTION__, first_line_end), std::string::npos); + EXPECT_THAT(first_line, ::testing::HasSubstr("Frame 0")); + EXPECT_THAT(first_line, ::testing::HasSubstr(__FUNCTION__)); } } // namespace testing From ce81ec5bb8e01100d8ac08f4b9f15b81aeb8d604 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 03:19:57 +0900 Subject: [PATCH 22/26] Fix private header --- fml/backtrace_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index 8f954f39af859..923a7487a02e4 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -4,7 +4,7 @@ #include "backtrace.h" -#include "gmock/gmock-matchers.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "logging.h" From a4f3850f205ab3a4accb38009349703b83fed1a4 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 07:59:50 +0900 Subject: [PATCH 23/26] Force inlining --- fml/backtrace.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 264fa7d0bcda5..2dee2956aa545 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -32,7 +32,8 @@ static std::string GetSymbolName(void* symbol) { return name; } -static int Backtrace(void** symbols, int size) { +// Using force inlining to always exclude this function from the backtrace. +ABSL_ATTRIBUTE_ALWAYS_INLINE int Backtrace(void** symbols, int size) { #if FML_OS_WIN return CaptureStackBackTrace(0, size, symbols, NULL); #else @@ -49,7 +50,7 @@ std::string BacktraceHere(size_t offset) { } // Exclude here. - offset += 2; + offset += 1; std::stringstream stream; for (int i = offset; i < available_frames; ++i) { From b6c657eed8477e8f774fe6b4602df1fa63e46901 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 08:24:12 +0900 Subject: [PATCH 24/26] Skip if backtrace is not supported --- fml/backtrace_unittests.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index 923a7487a02e4..02f3dc7f1db1f 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -36,6 +36,11 @@ TEST(BacktraceTest, CanGatherBacktrace) { } TEST(BacktraceTest, BacktraceStartsWithCallerFunction) { + if (!IsCrashHandlingSupported()) { + GTEST_SKIP(); + return; + } + auto trace = BacktraceHere(0); ASSERT_GT(trace.size(), 0u); From a99f0dcbe04d57633e2121ab6d120084bc578d67 Mon Sep 17 00:00:00 2001 From: moko256 Date: Mon, 26 Sep 2022 22:06:24 +0900 Subject: [PATCH 25/26] Inline manually --- fml/backtrace.cc | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index 2dee2956aa545..f34056c9279df 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -32,19 +32,21 @@ static std::string GetSymbolName(void* symbol) { return name; } -// Using force inlining to always exclude this function from the backtrace. -ABSL_ATTRIBUTE_ALWAYS_INLINE int Backtrace(void** symbols, int size) { +std::string BacktraceHere(size_t offset) { + constexpr size_t kMaxFrames = 256; + void* symbols[kMaxFrames]; + +// A function isn't always inlined though the `inline` keyword present, and +// inlining behavior can be different between compilers. +// Therefore, calling this function directly instead of splitting into another +// function, and always enable to exclude here from stack. #if FML_OS_WIN - return CaptureStackBackTrace(0, size, symbols, NULL); + const auto available_frames = + CaptureStackBackTrace(0, kMaxFrames, symbols, NULL); #else - return ::backtrace(symbols, size); + const auto available_frames = ::backtrace(symbols, kMaxFrames); #endif // FML_OS_WIN -} -std::string BacktraceHere(size_t offset) { - constexpr size_t kMaxFrames = 256; - void* symbols[kMaxFrames]; - const auto available_frames = Backtrace(symbols, kMaxFrames); if (available_frames <= 0) { return ""; } From 24b2aa5b31db27eba76e5cbbe52125231758195a Mon Sep 17 00:00:00 2001 From: moko256 Date: Tue, 27 Sep 2022 00:42:02 +0900 Subject: [PATCH 26/26] Rollback to 763d211 and fix comment --- fml/backtrace.cc | 21 +++++++++------------ fml/backtrace.h | 2 +- fml/backtrace_unittests.cc | 24 ------------------------ 3 files changed, 10 insertions(+), 37 deletions(-) diff --git a/fml/backtrace.cc b/fml/backtrace.cc index f34056c9279df..264fa7d0bcda5 100644 --- a/fml/backtrace.cc +++ b/fml/backtrace.cc @@ -32,27 +32,24 @@ static std::string GetSymbolName(void* symbol) { return name; } -std::string BacktraceHere(size_t offset) { - constexpr size_t kMaxFrames = 256; - void* symbols[kMaxFrames]; - -// A function isn't always inlined though the `inline` keyword present, and -// inlining behavior can be different between compilers. -// Therefore, calling this function directly instead of splitting into another -// function, and always enable to exclude here from stack. +static int Backtrace(void** symbols, int size) { #if FML_OS_WIN - const auto available_frames = - CaptureStackBackTrace(0, kMaxFrames, symbols, NULL); + return CaptureStackBackTrace(0, size, symbols, NULL); #else - const auto available_frames = ::backtrace(symbols, kMaxFrames); + return ::backtrace(symbols, size); #endif // FML_OS_WIN +} +std::string BacktraceHere(size_t offset) { + constexpr size_t kMaxFrames = 256; + void* symbols[kMaxFrames]; + const auto available_frames = Backtrace(symbols, kMaxFrames); if (available_frames <= 0) { return ""; } // Exclude here. - offset += 1; + offset += 2; std::stringstream stream; for (int i = offset; i < available_frames; ++i) { diff --git a/fml/backtrace.h b/fml/backtrace.h index 2f75834313d07..167ce956206f6 100644 --- a/fml/backtrace.h +++ b/fml/backtrace.h @@ -11,7 +11,7 @@ namespace fml { -// Retrieve the backtrace. It is maintained for debugging. +// Retrieve the backtrace, for debugging. // // If the |offset| is 0, the backtrace is included caller function. std::string BacktraceHere(size_t offset = 0); diff --git a/fml/backtrace_unittests.cc b/fml/backtrace_unittests.cc index 02f3dc7f1db1f..f3b2a64114e5f 100644 --- a/fml/backtrace_unittests.cc +++ b/fml/backtrace_unittests.cc @@ -4,7 +4,6 @@ #include "backtrace.h" -#include "gmock/gmock.h" #include "gtest/gtest.h" #include "logging.h" @@ -35,28 +34,5 @@ TEST(BacktraceTest, CanGatherBacktrace) { } } -TEST(BacktraceTest, BacktraceStartsWithCallerFunction) { - if (!IsCrashHandlingSupported()) { - GTEST_SKIP(); - return; - } - - auto trace = BacktraceHere(0); - ASSERT_GT(trace.size(), 0u); - - auto first_line_end = trace.find("\n"); - ASSERT_NE(first_line_end, std::string::npos); - - auto first_line = trace.substr(0, first_line_end); - - if (first_line.find("Unknown") != std::string::npos) { - GTEST_SKIP() << "The symbols has been stripped from the executable."; - return; - } - - EXPECT_THAT(first_line, ::testing::HasSubstr("Frame 0")); - EXPECT_THAT(first_line, ::testing::HasSubstr(__FUNCTION__)); -} - } // namespace testing } // namespace fml