Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fml/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
Expand All @@ -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" ]
}
Expand Down
74 changes: 33 additions & 41 deletions fml/backtrace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,68 +4,56 @@

#include "flutter/fml/backtrace.h"

#include <cxxabi.h>
#include <dlfcn.h>
#include <execinfo.h>

#include <csignal>
#include <sstream>

#if FML_OS_WIN
#include <crtdbg.h>
#include <debugapi.h>
#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 <Windows.h>
#include <crtdbg.h>
#include <debugapi.h>
#else // FML_OS_WIN
#include <execinfo.h>
#endif // FML_OS_WIN

namespace fml {

static std::string kKUnknownFrameName = "Unknown";

static std::string DemangleSymbolName(const std::string& mangled) {
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;
}

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 = ::backtrace(symbols, kMaxFrames);
const auto available_frames = Backtrace(symbols, kMaxFrames);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should/could we use abseil's GetStackTrace here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks nice! I'm replacing with this. I'll check output on linux...

Copy link
Contributor Author

@moko256 moko256 Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also find absl::InstallFailureSignalHandler. The engine doesn't use BacktraceHere except in signal handler, so we can replace backtrace.cc with absl::InstallFailureSignalHandler.

cc: @chinmaygarde @cbracken

Copy link
Contributor Author

@moko256 moko256 Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source code (prototype): moko256@3ca3f10
Windows:

*** SIGABRT received at time=1663690051 ***
    @   000000014073933C  (unknown)  absl::AbslFailureSignalHandler
    @   0000000142957225  (unknown)  raise
    @   00000001428D5DE9  (unknown)  abort
    @   00000001400E74A9  (unknown)  fml::testing::BacktracePrinter
    @   00000001400E748E  (unknown)  fml::testing::BacktraceTest_BacktraceHere_Test::TestBody
    @   00000001427BA210  (unknown)  testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,void>
    @   00000001427A4201  (unknown)  testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void>
    @   000000014278A59F  (unknown)  testing::Test::Run
    @   000000014278ADA8  (unknown)  testing::TestInfo::Run
    @   000000014278B41C  (unknown)  testing::TestSuite::Run
    @   00000001427956E5  (unknown)  testing::internal::UnitTestImpl::RunAllTests
    @   00000001427BE6F0  (unknown)  testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool>
    @   00000001427A6E91  (unknown)  testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool>
    @   0000000142795274  (unknown)  testing::UnitTest::Run
    @   0000000140279BC1  (unknown)  RUN_ALL_TESTS
    @   0000000140279848  (unknown)  main
    @   00000001428636A9  (unknown)  invoke_main
    @   000000014286358E  (unknown)  __scrt_common_main_seh
    @   000000014286344E  (unknown)  __scrt_common_main
    @   000000014286373E  (unknown)  mainCRTStartup
    @   00007FFFC2AB7034  (unknown)  BaseThreadInitThunk
    @   00007FFFC3A426A1  (unknown)  RtlUserThreadStart

Linux:

*** SIGABRT received at time=1663721941 on cpu 2 ***
PC: @     0x7fc0709537bb  (unknown)  raise
    @     0x55a71acd5eec         64  absl::WriteFailureInfo()
    @     0x55a71acd5c02         64  absl::AbslFailureSignalHandler()
    @     0x7fc070c71730  1470387120  (unknown)
    @     0x55a71a74e941         32  fml::testing::BacktraceTest_BacktraceHere_Test::TestBody()
    @     0x55a71c87107b         96  testing::internal::HandleSehExceptionsInMethodIfSupported<>()
    @     0x55a71c863c27        112  testing::internal::HandleExceptionsInMethodIfSupported<>()
    @     0x55a71c853073         96  testing::Test::Run()
    @     0x55a71c853734        112  testing::TestInfo::Run()
    @     0x55a71c853d3d        112  testing::TestSuite::Run()
    @     0x55a71c85d4e1        208  testing::internal::UnitTestImpl::RunAllTests()
    @     0x55a71c8741fb         96  testing::internal::HandleSehExceptionsInMethodIfSupported<>()
    @     0x55a71c865617        112  testing::internal::HandleExceptionsInMethodIfSupported<>()
    @     0x55a71c85d0df         96  testing::UnitTest::Run()
    @     0x55a71a8d5961         16  RUN_ALL_TESTS()
    @     0x55a71a8d5832        720  main
    @     0x7fc07094009b  (unknown)  __libc_start_main
    @ 0x5541d68949564100  (unknown)  (unknown)
Aborted

if (available_frames <= 0) {
return "";
}

// Exclude here.
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();
Expand All @@ -74,12 +62,14 @@ 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) {
Expand All @@ -88,18 +78,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);
}
Expand Down
3 changes: 3 additions & 0 deletions fml/backtrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

namespace fml {

// Retrieve the backtrace, for debugging.
//
// If the |offset| is 0, the backtrace is included caller function.
std::string BacktraceHere(size_t offset = 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this function? I presume the intent was for use when debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the #16450, which introduced BacktraceHere, this is only used in signal handler (and test for BacktraceHere). I thought it was made public to test.
The handler from absl::InstallFailureSignalHandler also shows stacktrace when receive signal, same as from fml's InstallCrashHandler before. The stacktrace's format is a little different from BacktraceHere, but it seems that the existing tests doesn't depend on the format.

I just intended to avoid bitrot to follow the style guide. I know this can be used to debug. If any engine's contributors use it to debug, I'll restore it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm -- I don't use it to debug, so personally I'm fine if we remove it (we can always restore it in a followup patch with appropriate tweaks if desired)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't have code that isn't used, but I do end up using this call frequently when debugging/testing locally. Does absl have a similar function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This is not blocking for landing, but I'd like to know for my own edification)

Copy link
Member

@cbracken cbracken Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If people are using it for development, and if absl doesn't have an equivalent then let's restore it (the existing test will avoid bitrot, at least to some extent).

If absl does have an equivalent call, maybe we could add a "// See: absl::SomethingOrOther" type comment to the class docs as a little breadcrumb to help developers find it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


void InstallCrashHandler();
Expand Down