Skip to content

Commit 7817ca8

Browse files
committed
sysprof: replace backtrace with libunwind
`backtrace` fails to unwind the host stack in LuaJIT since there are no frame pointers during the VM calls, even with the `-fno-omit-frame-pointer` option set. Sometimes, those failures cause crashes. Moreover, `backtrace` is not signal-safe, which is vital for the sysprof. This commit replaces it with the libunwind-based unwinder, which makes use of additional runtime information to provide robust unwinding even if there are no frame pointers and is signal-safe. Because Tarantool uses its own fiber unwinder, the new default unwinder is not built into bundled builds. Also, this commit enables C API tests, which used to crash with `backtrace`, and disables an assertion regarding the successful backtracer function setting, as it is unnecessary. Part of tarantool/tarantool#781
1 parent fef60a1 commit 7817ca8

File tree

6 files changed

+145
-48
lines changed

6 files changed

+145
-48
lines changed

Diff for: CMakeLists.txt

+17
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ endif()
9797
# tracebacks and unwinding are not affected -- the assembler part
9898
# has frame unwind information and GCC emits it where needed (x64)
9999
# or with -g.
100+
# The '-fno-omit-frame-pointer` is set later, depending on sysprof
101+
# and libunwind support.
100102
AppendFlags(CMAKE_C_FLAGS -fomit-frame-pointer -fno-stack-protector)
101103

102104
# Redefined to benefit from expanding macros in gdb.
@@ -212,6 +214,21 @@ endif()
212214
option(LUAJIT_DISABLE_SYSPROF "LuaJIT platform and Lua profiler support" OFF)
213215
if(LUAJIT_DISABLE_SYSPROF)
214216
AppendFlags(TARGET_C_FLAGS -DLUAJIT_DISABLE_SYSPROF)
217+
else()
218+
if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND
219+
(
220+
CMAKE_SYSTEM_PROCESSOR STREQUAL "i386" OR
221+
CMAKE_SYSTEM_PROCESSOR STREQUAL "i686" OR
222+
CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64"
223+
)
224+
)
225+
AppendFlags(CMAKE_C_FLAGS -fno-omit-frame-pointer -fasynchronous-unwind-tables)
226+
if(ENABLE_BUNDLED_LUAJIT)
227+
AppendFlags(TARGET_C_FLAGS -DLUAJIT_EXTERNAL_SYSPROF_BACKTRACER)
228+
else()
229+
find_package(LibUnwind MODULE REQUIRED)
230+
endif()
231+
endif()
215232
endif()
216233

217234
# Switch to harder (and slower) hash function when a collision

Diff for: cmake/FindLibUnwind.cmake

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
#[========================================================================[.rst:
2+
FindLibUnwind
3+
--------
4+
Finds the libunwind library.
5+
6+
Result Variables
7+
^^^^^^^^^^^^^^^^
8+
``LIBUNWIND_FOUND``
9+
True if the system has the libunwind library.
10+
``LIBUNWIND_INCLUDE_DIR``
11+
Include directory needed to use libunwind.
12+
``LIBUNWIND_LIBRARIES``
13+
Libraries needed to link to libunwind.
14+
15+
Cache Variables
16+
^^^^^^^^^^^^^^^
17+
``LIBUNWIND_INCLUDE_DIR``
18+
The directory containing ``libunwind.h``.
19+
``LIBUNWIND_LIBRARIES``
20+
The paths to the libunwind libraries.
21+
#]========================================================================]
22+
23+
include(FindPackageHandleStandardArgs)
24+
25+
find_package(PkgConfig QUIET)
26+
pkg_check_modules(PC_LIBUNWIND QUIET libunwind)
27+
28+
find_path(LIBUNWIND_INCLUDE_DIR libunwind.h ${PC_LIBUNWIND_INCLUDE_DIRS})
29+
if(LIBUNWIND_INCLUDE_DIR)
30+
include_directories(${LIBUNWIND_INCLUDE_DIR})
31+
endif()
32+
33+
find_library(LIBUNWIND_LIBRARY NAMES unwind PATHS ${PC_LIBUNWIND_LIBRARY_DIRS})
34+
35+
set(LIBUNWIND_PLATFORM_LIBRARY_NAME "unwind-${CMAKE_SYSTEM_PROCESSOR}")
36+
find_library(LIBUNWIND_PLATFORM_LIBRARY ${LIBUNWIND_PLATFORM_LIBRARY_NAME}
37+
${PC_LIBUNWIND_LIBRARY_DIRS})
38+
set(LIBUNWIND_LIBRARIES ${LIBUNWIND_LIBRARY} ${LIBUNWIND_PLATFORM_LIBRARY})
39+
40+
find_package_handle_standard_args(LibUnwind
41+
REQUIRED_VARS LIBUNWIND_INCLUDE_DIR LIBUNWIND_LIBRARIES)
42+
43+
mark_as_advanced(LIBUNWIND_INCLUDE_DIR LIBUNWIND_LIBRARIES)

Diff for: src/CMakeLists.txt

+4
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,10 @@ add_dependencies(core_shared buildvm_output)
284284

285285
list(APPEND TARGET_LIBS m)
286286

287+
if(NOT LUAJIT_DISABLE_SYSPROF)
288+
list(APPEND TARGET_LIBS ${LIBUNWIND_LIBRARIES})
289+
endif()
290+
287291
set(LIB_OBJECTS_STATIC
288292
$<TARGET_OBJECTS:vm_static>
289293
$<TARGET_OBJECTS:core_static>

Diff for: src/Makefile.original

+12
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,18 @@ ifneq (,$(findstring LJ_TARGET_PS3 1,$(TARGET_TESTARCH)))
292292
TARGET_XLIBS+= -lpthread
293293
endif
294294

295+
ifneq (,$(findstring LJ_HASSYSPROF 1,$(TARGET_TESTARCH)))
296+
# Try to link against libunwind to determine whether it
297+
# is present in the system.
298+
HAS_LIBUNWIND=$(shell $(TARGET_LD) -lunwind -lunwind-x86_64 -c 2>/dev/null; echo $$?)
299+
ifneq (0,$(HAS_LIBUNWIND))
300+
$(error libunwind is required for sysprof)
301+
else
302+
TARGET_XLIBS+= -lunwind -lunwind-x86_64
303+
TARGET_XCFLAGS+= -fno-omit-frame-pointer -fasynchronous-unwind-tables
304+
endif
305+
endif
306+
295307
TARGET_XCFLAGS+= $(CCOPT_$(TARGET_LJARCH))
296308
TARGET_ARCH+= $(patsubst %,-DLUAJIT_TARGET=LUAJIT_ARCH_%,$(TARGET_LJARCH))
297309

Diff for: src/lj_sysprof.c

+69-31
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,17 @@
2626

2727
#include <pthread.h>
2828
#include <errno.h>
29-
#include <execinfo.h>
29+
30+
#ifndef LUAJIT_EXTERNAL_SYSPROF_BACKTRACER
31+
/*
32+
** We only need local unwinding, then a special implementation
33+
** can be selected which may run much faster than the generic
34+
** implementation which supports both kinds of unwinding, local
35+
** and remote.
36+
*/
37+
#define UNW_LOCAL_ONLY
38+
#include <libunwind.h>
39+
#endif
3040

3141
/*
3242
** Number of profiler frames we need to omit during stack
@@ -85,6 +95,56 @@ static struct sysprof sysprof = {0};
8595

8696
/* --- Stream ------------------------------------------------------------- */
8797

98+
#ifndef LUAJIT_EXTERNAL_SYSPROF_BACKTRACER
99+
100+
static ssize_t collect_stack(void **buffer, int size)
101+
{
102+
int frame_no = 0;
103+
unw_context_t unw_ctx;
104+
unw_cursor_t unw_cur;
105+
106+
int rc = unw_getcontext(&unw_ctx);
107+
if (rc != 0)
108+
return -1;
109+
110+
rc = unw_init_local(&unw_cur, &unw_ctx);
111+
if (rc != 0)
112+
return -1;
113+
114+
for (; frame_no < size; ++frame_no) {
115+
unw_word_t ip;
116+
rc = unw_get_reg(&unw_cur, UNW_REG_IP, &ip);
117+
if (rc != 0)
118+
return -1;
119+
120+
buffer[frame_no] = (void *)ip;
121+
rc = unw_step(&unw_cur);
122+
if (rc <= 0)
123+
break;
124+
}
125+
return frame_no;
126+
}
127+
128+
static void default_backtrace_host(void *(writer)(int frame_no, void *addr))
129+
{
130+
static void *backtrace_buf[SYSPROF_BACKTRACE_FRAME_MAX] = {};
131+
132+
struct sysprof *sp = &sysprof;
133+
int max_depth = sp->opt.mode == LUAM_SYSPROF_LEAF
134+
? SYSPROF_HANDLER_STACK_DEPTH + 1
135+
: SYSPROF_BACKTRACE_FRAME_MAX;
136+
const int depth = collect_stack(backtrace_buf, max_depth);
137+
int level;
138+
139+
lj_assertX(depth <= max_depth, "backtrace is too deep");
140+
lj_assertX(depth != -1, "failed to collect backtrace");
141+
for (level = SYSPROF_HANDLER_STACK_DEPTH; level < depth; ++level) {
142+
if (!writer(level - SYSPROF_HANDLER_STACK_DEPTH + 1, backtrace_buf[level]))
143+
return;
144+
}
145+
}
146+
#endif
147+
88148
static const uint8_t ljp_header[] = {'l', 'j', 'p', LJP_FORMAT_VERSION,
89149
0x0, 0x0, 0x0};
90150

@@ -205,24 +265,6 @@ static void *stream_frame_host(int frame_no, void *addr)
205265
return addr;
206266
}
207267

208-
static void default_backtrace_host(void *(writer)(int frame_no, void *addr))
209-
{
210-
static void *backtrace_buf[SYSPROF_BACKTRACE_FRAME_MAX] = {};
211-
212-
struct sysprof *sp = &sysprof;
213-
int max_depth = sp->opt.mode == LUAM_SYSPROF_LEAF
214-
? SYSPROF_HANDLER_STACK_DEPTH + 1
215-
: SYSPROF_BACKTRACE_FRAME_MAX;
216-
const int depth = backtrace(backtrace_buf, max_depth);
217-
int level;
218-
219-
lj_assertX(depth <= max_depth, "depth of C stack is too big");
220-
for (level = SYSPROF_HANDLER_STACK_DEPTH; level < depth; ++level) {
221-
if (!writer(level - SYSPROF_HANDLER_STACK_DEPTH + 1, backtrace_buf[level]))
222-
return;
223-
}
224-
}
225-
226268
static void stream_backtrace_host(struct sysprof *sp)
227269
{
228270
lj_assertX(sp->backtracer != NULL, "uninitialized sysprof backtracer");
@@ -427,20 +469,16 @@ int lj_sysprof_set_backtracer(luam_Sysprof_backtracer backtracer) {
427469

428470
if (sp->state != SPS_IDLE)
429471
return PROFILE_ERRUSE;
430-
if (backtracer == NULL) {
472+
473+
if (backtracer == NULL)
474+
#ifndef LUAJIT_EXTERNAL_SYSPROF_BACKTRACER
431475
sp->backtracer = default_backtrace_host;
432-
/*
433-
** XXX: `backtrace` is not signal-safe, according to man,
434-
** because it is lazy loaded on the first call, which triggers
435-
** allocations. We need to call `backtrace` before starting profiling
436-
** to avoid lazy loading.
437-
*/
438-
void *dummy = NULL;
439-
backtrace(&dummy, 1);
440-
}
441-
else {
476+
#else
477+
return PROFILE_ERRUSE;
478+
#endif
479+
else
442480
sp->backtracer = backtracer;
443-
}
481+
444482
if (!is_unconfigured(sp)) {
445483
sp->state = SPS_IDLE;
446484
}

Diff for: test/tarantool-c-tests/misclib-sysprof-capi.test.c

-17
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,6 @@ static int validation(void *test_state)
121121
return TEST_EXIT_SUCCESS;
122122
}
123123

124-
/*
125-
* FIXME: The following two tests are disabled because sometimes
126-
* `backtrace` dynamically loads a platform-specific unwinder,
127-
* which is not signal-safe.
128-
*/
129-
130-
#if 0
131124
/*
132125
* Structure given as ctx to sysprof writer and on_stop callback.
133126
*/
@@ -239,7 +232,6 @@ static int check_profile_func(lua_State *L)
239232
== PROFILE_SUCCESS);
240233
assert_true(luaM_sysprof_set_on_stop(on_stop_cb_default)
241234
== PROFILE_SUCCESS);
242-
assert_true(luaM_sysprof_set_backtracer(NULL) == PROFILE_SUCCESS);
243235

244236
status = luaM_sysprof_start(L, &opt);
245237
assert_true(status == PROFILE_SUCCESS);
@@ -270,33 +262,24 @@ static int check_profile_func(lua_State *L)
270262

271263
return TEST_EXIT_SUCCESS;
272264
}
273-
#endif
274265

275266
static int profile_func_jitoff(void *test_state)
276267
{
277-
UNUSED(test_state);
278-
return todo("Need to replace backtrace with libunwind first");
279-
#if 0
280268
lua_State *L = test_state;
281269
utils_get_aux_lfunc(L);
282270
(void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_OFF);
283271
(void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_FLUSH);
284272
check_profile_func(L);
285273
(void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_ON);
286274
return TEST_EXIT_SUCCESS;
287-
#endif
288275
}
289276

290277
static int profile_func_jiton(void *test_state)
291278
{
292-
UNUSED(test_state);
293-
return todo("Need to replace backtrace with libunwind first");
294-
#if 0
295279
lua_State *L = test_state;
296280
utils_get_aux_lfunc(L);
297281
check_profile_func(L);
298282
return TEST_EXIT_SUCCESS;
299-
#endif
300283
}
301284

302285
int main(void)

0 commit comments

Comments
 (0)