Skip to content

Commit

Permalink
crimson/common: skip first 4 frames when dumping a backtrace.
Browse files Browse the repository at this point in the history
It's all about these items:

```
 0# print_backtrace(std::basic_string_view<char, std::char_traits<char> >) at /home/rzarzynski/ceph1/build/../src/crimson/common/fatal_signal.cc:80
 1# FatalSignal::signaled(int, siginfo_t const&) at /opt/rh/gcc-toolset-9/root/usr/include/c++/9/ostream:570
 2# FatalSignal::install_oneshot_signal_handler<11>()::{lambda(int, siginfo_t*, void*)#1}::_FUN(int, siginfo_t*, void*) at /home/rzarzynski/ceph1/build/../src/crimson/common/fatal_signal.cc:
62
 3# 0x00007F16BBA13B30 in /lib64/libpthread.so.0
```

They are part of our backtrace handling and typically developers
are not interested in them. Let's be consistent with the classical
OSD and hide them.

Signed-off-by: Radoslaw Zarzynski <[email protected]>
  • Loading branch information
rzarzynski committed Sep 23, 2021
1 parent 2612268 commit 56208ed
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 11 deletions.
38 changes: 27 additions & 11 deletions src/crimson/common/fatal_signal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,26 @@ static void reraise_fatal(const int signum)
::_exit(1);
}

[[gnu::noinline]] void FatalSignal::signal_entry(
const int signum,
siginfo_t* const info,
void*)
{
if (static std::atomic_bool handled{false}; handled.exchange(true)) {
return;
}
assert(info);
FatalSignal::signaled(signum, *info);
reraise_fatal(signum);
}

template <int SigNum>
void FatalSignal::install_oneshot_signal_handler()
{
struct sigaction sa;
sa.sa_sigaction = [](int signum, siginfo_t *info, void *p) {
if (static std::atomic_bool handled{false}; handled.exchange(true)) {
return;
}
assert(info);
FatalSignal::signaled(signum, *info);
reraise_fatal(signum);
};
// it's a bad idea to use a lambda here. On GCC there are `operator()`
// and `_FUN()`. Controlling their inlineability is hard (impossible?).
sa.sa_sigaction = signal_entry;
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_SIGINFO | SA_RESTART | SA_NODEFER;
if constexpr (SigNum == SIGSEGV) {
Expand All @@ -71,13 +79,20 @@ void FatalSignal::install_oneshot_signal_handler()
}


static void print_backtrace(std::string_view cause) {
[[gnu::noinline]] static void print_backtrace(std::string_view cause) {
std::cerr << cause;
if (seastar::engine_is_ready()) {
std::cerr << " on shard " << seastar::this_shard_id();
}
// nobody wants to see things like `FatalSignal::signaled()` or
// `print_backtrace()` in our backtraces. `+ 1` is for the extra
// frame created by kernel (signal trampoline, it will take care
// about e.g. sigreturn(2) calling; see the man page).
constexpr std::size_t FRAMES_TO_SKIP = 3 + 1;
std::cerr << ".\nBacktrace:\n";
std::cerr << boost::stacktrace::stacktrace();
std::cerr << boost::stacktrace::stacktrace(
FRAMES_TO_SKIP,
static_cast<std::size_t>(-1)/* max depth same as the default one */);
std::cerr << std::flush;
// TODO: dump crash related meta data to $crash_dir
// see handle_fatal_signal()
Expand Down Expand Up @@ -138,7 +153,8 @@ static void print_proc_maps()
}
}

void FatalSignal::signaled(const int signum, const siginfo_t& siginfo)
[[gnu::noinline]] void FatalSignal::signaled(const int signum,
const siginfo_t& siginfo)
{
switch (signum) {
case SIGSEGV:
Expand Down
1 change: 1 addition & 0 deletions src/crimson/common/fatal_signal.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class FatalSignal {
FatalSignal();

private:
static void signal_entry(int signum, siginfo_t* siginfo, void* p);
static void signaled(int signum, const siginfo_t& siginfo);

template <int... SigNums>
Expand Down

0 comments on commit 56208ed

Please sign in to comment.