Skip to content

Commit 04c6aa4

Browse files
arnej27959arnej27959
authored
Merge pull request #34825 from vespa-engine/vekterli/async-signal-safe-backtrace-with-unwind-h
Add async signal safe backtracing using `unwind.h` where available
2 parents 95e4526 + ca6d09a commit 04c6aa4

4 files changed

Lines changed: 96 additions & 21 deletions

File tree

‎vespalib/src/tests/signalhandler/signalhandler_test.cpp‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "my_shared_library.h"
44
#include <vespa/vespalib/util/count_down_latch.h>
55
#include <vespa/vespalib/util/signalhandler.h>
6+
#include <vespa/vespalib/util/backtrace.h>
67
#include <vespa/vespalib/gtest/gtest.h>
78
#include <gmock/gmock.h>
89
#include <thread>
@@ -14,8 +15,7 @@ LOG_SETUP("signalhandler_test");
1415
using namespace vespalib;
1516
using namespace ::testing;
1617

17-
TEST(SignalHandlerTest, signal_handler_can_intercept_hooked_signals)
18-
{
18+
TEST(SignalHandlerTest, signal_handler_can_intercept_hooked_signals) {
1919
EXPECT_TRUE(!SignalHandler::INT.check());
2020
EXPECT_TRUE(!SignalHandler::TERM.check());
2121
SignalHandler::INT.ignore();
@@ -36,10 +36,12 @@ TEST(SignalHandlerTest, signal_handler_can_intercept_hooked_signals)
3636
EXPECT_EQ(0, system("res=`./vespalib_victim_app`; test \"$res\" = \"GOT TERM\""));
3737
}
3838

39-
TEST(SignalHandlerTest, can_dump_stack_of_another_thread)
40-
{
41-
vespalib::CountDownLatch arrival_latch(2);
42-
vespalib::CountDownLatch departure_latch(2);
39+
TEST(SignalHandlerTest, can_dump_stack_of_another_thread) {
40+
if (!has_signal_safe_collect_stack_frames()) {
41+
GTEST_SKIP() << "async signal safe stack dumping not supported; skipping test";
42+
}
43+
CountDownLatch arrival_latch(2);
44+
CountDownLatch departure_latch(2);
4345

4446
std::thread t([&]{
4547
my_cool_function(arrival_latch, departure_latch);
@@ -55,8 +57,10 @@ TEST(SignalHandlerTest, can_dump_stack_of_another_thread)
5557
t.join();
5658
}
5759

58-
TEST(SignalHandlerTest, can_get_stack_trace_of_own_thread)
59-
{
60+
TEST(SignalHandlerTest, can_get_stack_trace_of_own_thread) {
61+
if (!has_signal_safe_collect_stack_frames()) {
62+
GTEST_SKIP() << "async signal safe stack dumping not supported; skipping test";
63+
}
6064
auto trace = my_totally_tubular_and_groovy_function();
6165
EXPECT_THAT(trace, HasSubstr("my_totally_tubular_and_groovy_function"));
6266
}

‎vespalib/src/vespa/vespalib/util/backtrace.cpp‎

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@
88
#include <cstdlib>
99
#include <string>
1010

11+
#if defined (__has_include)
12+
#if __has_include(<unwind.h>)
13+
#define VESPA_BACKTRACE_HAS_UNWIND_H
14+
#include <unwind.h>
15+
#endif
16+
#endif
17+
18+
namespace vespalib {
19+
1120
namespace {
1221

1322
/**
@@ -44,15 +53,67 @@ demangleBacktraceLine(const std::string& line)
4453
return line;
4554
}
4655

56+
#ifdef VESPA_BACKTRACE_HAS_UNWIND_H
57+
58+
struct UnwindState {
59+
// Could be done with just a current+end ptr pair, but this is more obvious
60+
void** frames_out = nullptr;
61+
size_t frames_written = 0;
62+
size_t frames_max = 0;
63+
};
64+
65+
_Unwind_Reason_Code unwind_callback(_Unwind_Context* ctx, void* caller_arg) {
66+
auto* my_state = static_cast<UnwindState*>(caller_arg);
67+
// We do "top of stack" frame skipping on a higher level, and therefore don't
68+
// bother with that detail here.
69+
void* frame_addr = reinterpret_cast<void*>(_Unwind_GetIP(ctx));
70+
if (frame_addr == nullptr) {
71+
return _URC_END_OF_STACK;
72+
}
73+
my_state->frames_out[my_state->frames_written] = frame_addr;
74+
my_state->frames_written++;
75+
if (my_state->frames_written == my_state->frames_max) {
76+
return _URC_END_OF_STACK;
77+
}
78+
return _URC_NO_REASON;
79+
}
80+
81+
#endif
82+
83+
} // anon ns
84+
85+
bool has_signal_safe_collect_stack_frames() noexcept {
86+
#ifdef VESPA_BACKTRACE_HAS_UNWIND_H
87+
return true;
88+
#else
89+
return false;
90+
#endif
91+
}
92+
93+
size_t signal_safe_collect_stack_frames(void** frames_out, size_t frames_max) {
94+
#ifdef VESPA_BACKTRACE_HAS_UNWIND_H
95+
// The unwind callback must have room for at least 1 frame.
96+
if (frames_max == 0) {
97+
return 0;
98+
}
99+
UnwindState my_state{frames_out, 0, frames_max};
100+
_Unwind_Backtrace(unwind_callback, &my_state);
101+
return my_state.frames_written;
102+
#else
103+
// No known async signal safe unwinding; bail out without doing anything.
104+
(void)frames_out;
105+
(void)frames_max;
106+
return 0;
107+
#endif
47108
}
48109

49110
int
50-
vespalib::getStackTraceFrames(void** framesOut, int maxFrames) {
111+
getStackTraceFrames(void** framesOut, int maxFrames) {
51112
return backtrace(framesOut, maxFrames);
52113
}
53114

54115
std::string
55-
vespalib::getStackTrace(int ignoreTop, void* const* stack, int size)
116+
getStackTrace(int ignoreTop, void* const* stack, int size)
56117
{
57118
asciistream ost;
58119
char** symbols = backtrace_symbols(stack, size);
@@ -67,9 +128,11 @@ vespalib::getStackTrace(int ignoreTop, void* const* stack, int size)
67128
}
68129

69130
std::string
70-
vespalib::getStackTrace(int ignoreTop) {
131+
getStackTrace(int ignoreTop) {
71132
ignoreTop += 1;
72133
void* stack[25];
73134
int size = backtrace(stack, 25);
74135
return getStackTrace(ignoreTop, stack, size);
75136
}
137+
138+
} // vespalib

‎vespalib/src/vespa/vespalib/util/backtrace.h‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,21 @@ std::string getStackTrace(int ignoreTop, void* const* stack, int size);
3636
*/
3737
int getStackTraceFrames(void** framesOut, int maxFrames);
3838

39+
/**
40+
* Returns true iff `signal_safe_collect_stack_frames()` is supported on
41+
* this platform.
42+
*/
43+
[[nodiscard]] bool has_signal_safe_collect_stack_frames() noexcept;
44+
45+
/**
46+
* Collects up to and including `frames_max` stack frames into `frames_out` in
47+
* an async signal safe way. The number of collected frames is returned.
48+
*
49+
* If `has_signal_safe_collect_stack_frames() == false`, the function will
50+
* return 0 and `frames_out` is not modified.
51+
*/
52+
[[nodiscard]] size_t signal_safe_collect_stack_frames(void** frames_out, size_t frames_max);
53+
3954

4055
}
4156

‎vespalib/src/vespa/vespalib/util/signalhandler.cpp‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22

33
#include "signalhandler.h"
44
#include "backtrace.h"
5-
#ifdef __APPLE__
6-
#define BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED
7-
#endif
8-
#include <boost/stacktrace/safe_dump_to.hpp> // Header-only dependency
9-
#include <boost/stacktrace/frame.hpp>
105
#include <array>
116
#include <atomic>
127
#include <cassert>
@@ -94,9 +89,7 @@ SignalHandler::dump_current_thread_stack_to_shared_state() noexcept
9489
return; // Someone else is already inside the house...!
9590
}
9691
auto& frames_buf = _shared_backtrace_data._stack_frames;
97-
static_assert(std::is_same_v<const void*, boost::stacktrace::frame::native_frame_ptr_t>);
98-
// Note: safe_dump_to() takes in buffer size in _bytes_, not in number of frames.
99-
const auto n_frames = boost::stacktrace::safe_dump_to(frames_buf.data(), frames_buf.size() * sizeof(void*));
92+
const size_t n_frames = signal_safe_collect_stack_frames(frames_buf.data(), frames_buf.size());
10093
_shared_backtrace_data._n_dumped_frames = static_cast<uint32_t>(n_frames);
10194
_shared_backtrace_data._signal_handler_done.store(true);
10295
}
@@ -211,8 +204,8 @@ SignalHandler::get_cross_thread_stack_trace(pthread_t thread_id)
211204
expected_done = true;
212205
}
213206
constexpr int frames_to_skip = 4; // handleSignal() -> gotSignal() -> dump_current_thread_...() -> backtrace()
214-
return vespalib::getStackTrace(frames_to_skip, _shared_backtrace_data._stack_frames.data(),
215-
static_cast<int>(_shared_backtrace_data._n_dumped_frames));
207+
return getStackTrace(frames_to_skip, _shared_backtrace_data._stack_frames.data(),
208+
static_cast<int>(_shared_backtrace_data._n_dumped_frames));
216209
}
217210

218211
void

0 commit comments

Comments
 (0)