Skip to content

Commit

Permalink
sysprof: replace backtrace with libunwind
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
mkokryashkin committed Dec 4, 2023
1 parent fef60a1 commit 30843f7
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 48 deletions.
18 changes: 18 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ endif()
# tracebacks and unwinding are not affected -- the assembler part
# has frame unwind information and GCC emits it where needed (x64)
# or with -g.
# The '-fno-omit-frame-pointer` is set later, depending on sysprof
# and libunwind support.
AppendFlags(CMAKE_C_FLAGS -fomit-frame-pointer -fno-stack-protector)
AppendFlags(CMAKE_C_FLAGS -fno-stack-protector)

# Redefined to benefit from expanding macros in gdb.
set(CMAKE_C_FLAGS_DEBUG "-g -ggdb3")
Expand Down Expand Up @@ -212,6 +215,21 @@ endif()
option(LUAJIT_DISABLE_SYSPROF "LuaJIT platform and Lua profiler support" OFF)
if(LUAJIT_DISABLE_SYSPROF)
AppendFlags(TARGET_C_FLAGS -DLUAJIT_DISABLE_SYSPROF)
else()
if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND
(
CMAKE_SYSTEM_PROCESSOR STREQUAL "i386" OR
CMAKE_SYSTEM_PROCESSOR STREQUAL "i686" OR
CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64"
)
)
AppendFlags(CMAKE_C_FLAGS -fno-omit-frame-pointer -fasynchronous-unwind-tables)
if(ENABLE_BUNDLED_LUAJIT)
AppendFlags(TARGET_C_FLAGS -DLUAJIT_EXTERNAL_SYSPROF_BACKTRACER)
else()
find_package(LibUnwind MODULE REQUIRED)
endif()
endif()
endif()

# Switch to harder (and slower) hash function when a collision
Expand Down
43 changes: 43 additions & 0 deletions cmake/FindLibUnwind.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#[========================================================================[.rst:
FindLibUnwind
--------
Finds the libunwind library.
Result Variables
^^^^^^^^^^^^^^^^
``LIBUNWIND_FOUND``
True if the system has the libunwind library.
``LIBUNWIND_INCLUDE_DIR``
Include directory needed to use libunwind.
``LIBUNWIND_LIBRARIES``
Libraries needed to link to libunwind.
Cache Variables
^^^^^^^^^^^^^^^
``LIBUNWIND_INCLUDE_DIR``
The directory containing ``libunwind.h``.
``LIBUNWIND_LIBRARIES``
The paths to the libunwind libraries.
#]========================================================================]

include(FindPackageHandleStandardArgs)

find_package(PkgConfig QUIET)
pkg_check_modules(PC_LIBUNWIND QUIET libunwind)

find_path(LIBUNWIND_INCLUDE_DIR libunwind.h ${PC_LIBUNWIND_INCLUDE_DIRS})
if(LIBUNWIND_INCLUDE_DIR)
include_directories(${LIBUNWIND_INCLUDE_DIR})
endif()

find_library(LIBUNWIND_LIBRARY NAMES unwind PATHS ${PC_LIBUNWIND_LIBRARY_DIRS})

set(LIBUNWIND_PLATFORM_LIBRARY_NAME "unwind-${CMAKE_SYSTEM_PROCESSOR}")
find_library(LIBUNWIND_PLATFORM_LIBRARY ${LIBUNWIND_PLATFORM_LIBRARY_NAME}
${PC_LIBUNWIND_LIBRARY_DIRS})
set(LIBUNWIND_LIBRARIES ${LIBUNWIND_LIBRARY} ${LIBUNWIND_PLATFORM_LIBRARY})

find_package_handle_standard_args(LibUnwind
REQUIRED_VARS LIBUNWIND_INCLUDE_DIR LIBUNWIND_LIBRARIES)

mark_as_advanced(LIBUNWIND_INCLUDE_DIR LIBUNWIND_LIBRARIES)
4 changes: 4 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@ add_dependencies(core_shared buildvm_output)

list(APPEND TARGET_LIBS m)

if(NOT LUAJIT_DISABLE_SYSPROF)
list(APPEND TARGET_LIBS ${LIBUNWIND_LIBRARIES})
endif()

set(LIB_OBJECTS_STATIC
$<TARGET_OBJECTS:vm_static>
$<TARGET_OBJECTS:core_static>
Expand Down
12 changes: 12 additions & 0 deletions src/Makefile.original
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ ifneq (,$(findstring LJ_TARGET_PS3 1,$(TARGET_TESTARCH)))
TARGET_XLIBS+= -lpthread
endif

ifneq (,$(findstring LJ_HASSYSPROF 1,$(TARGET_TESTARCH)))
# Try to link against libunwind to determine whether it
# is present in the system.
HAS_LIBUNWIND=$(shell $(TARGET_LD) -lunwind -lunwind-x86_64 -c 2>/dev/null; echo $$?)
ifneq (,$(HAS_LIBUNWIND))
$(error libunwind is required for sysprof)
else
TARGET_XLIBS+= -lunwind -lunwind-x86_64
TARGET_XCFLAGS+= -fno-omit-frame-pointer -fasynchronous-unwind-tables
endif
endif

TARGET_XCFLAGS+= $(CCOPT_$(TARGET_LJARCH))
TARGET_ARCH+= $(patsubst %,-DLUAJIT_TARGET=LUAJIT_ARCH_%,$(TARGET_LJARCH))

Expand Down
100 changes: 69 additions & 31 deletions src/lj_sysprof.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,17 @@

#include <pthread.h>
#include <errno.h>
#include <execinfo.h>

#ifndef LUAJIT_EXTERNAL_SYSPROF_BACKTRACER
/*
** We only need local unwinding, then a special implementation
** can be selected which may run much faster than the generic
** implementation which supports both kinds of unwinding, local
** and remote.
*/
#define UNW_LOCAL_ONLY
#include <libunwind.h>
#endif

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

/* --- Stream ------------------------------------------------------------- */

#ifndef LUAJIT_EXTERNAL_SYSPROF_BACKTRACER

static ssize_t collect_stack(void **buffer, int size)
{
int frame_no = 0;
unw_context_t unw_ctx;
unw_cursor_t unw_cur;

int rc = unw_getcontext(&unw_ctx);
if (rc != 0)
return -1;

rc = unw_init_local(&unw_cur, &unw_ctx);
if (rc != 0)
return -1;

for (; frame_no < size; ++frame_no) {
unw_word_t ip;
rc = unw_get_reg(&unw_cur, UNW_REG_IP, &ip);
if (rc != 0)
return -1;

buffer[frame_no] = (void *)ip;
rc = unw_step(&unw_cur);
if (rc <= 0)
break;
}
return frame_no;
}

static void default_backtrace_host(void *(writer)(int frame_no, void *addr))
{
static void *backtrace_buf[SYSPROF_BACKTRACE_FRAME_MAX] = {};

struct sysprof *sp = &sysprof;
int max_depth = sp->opt.mode == LUAM_SYSPROF_LEAF
? SYSPROF_HANDLER_STACK_DEPTH + 1
: SYSPROF_BACKTRACE_FRAME_MAX;
const int depth = collect_stack(backtrace_buf, max_depth);
int level;

lj_assertX(depth <= max_depth, "backtrace is too deep");
lj_assertX(depth != -1, "failed to collect backtrace");
for (level = SYSPROF_HANDLER_STACK_DEPTH; level < depth; ++level) {
if (!writer(level - SYSPROF_HANDLER_STACK_DEPTH + 1, backtrace_buf[level]))
return;
}
}
#endif

static const uint8_t ljp_header[] = {'l', 'j', 'p', LJP_FORMAT_VERSION,
0x0, 0x0, 0x0};

Expand Down Expand Up @@ -205,24 +265,6 @@ static void *stream_frame_host(int frame_no, void *addr)
return addr;
}

static void default_backtrace_host(void *(writer)(int frame_no, void *addr))
{
static void *backtrace_buf[SYSPROF_BACKTRACE_FRAME_MAX] = {};

struct sysprof *sp = &sysprof;
int max_depth = sp->opt.mode == LUAM_SYSPROF_LEAF
? SYSPROF_HANDLER_STACK_DEPTH + 1
: SYSPROF_BACKTRACE_FRAME_MAX;
const int depth = backtrace(backtrace_buf, max_depth);
int level;

lj_assertX(depth <= max_depth, "depth of C stack is too big");
for (level = SYSPROF_HANDLER_STACK_DEPTH; level < depth; ++level) {
if (!writer(level - SYSPROF_HANDLER_STACK_DEPTH + 1, backtrace_buf[level]))
return;
}
}

static void stream_backtrace_host(struct sysprof *sp)
{
lj_assertX(sp->backtracer != NULL, "uninitialized sysprof backtracer");
Expand Down Expand Up @@ -427,20 +469,16 @@ int lj_sysprof_set_backtracer(luam_Sysprof_backtracer backtracer) {

if (sp->state != SPS_IDLE)
return PROFILE_ERRUSE;
if (backtracer == NULL) {

if (backtracer == NULL)
#ifndef LUAJIT_EXTERNAL_SYSPROF_BACKTRACER
sp->backtracer = default_backtrace_host;
/*
** XXX: `backtrace` is not signal-safe, according to man,
** because it is lazy loaded on the first call, which triggers
** allocations. We need to call `backtrace` before starting profiling
** to avoid lazy loading.
*/
void *dummy = NULL;
backtrace(&dummy, 1);
}
else {
#else
return PROFILE_ERRUSE;
#endif
else
sp->backtracer = backtracer;
}

if (!is_unconfigured(sp)) {
sp->state = SPS_IDLE;
}
Expand Down
17 changes: 0 additions & 17 deletions test/tarantool-c-tests/misclib-sysprof-capi.test.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,6 @@ static int validation(void *test_state)
return TEST_EXIT_SUCCESS;
}

/*
* FIXME: The following two tests are disabled because sometimes
* `backtrace` dynamically loads a platform-specific unwinder,
* which is not signal-safe.
*/

#if 0
/*
* Structure given as ctx to sysprof writer and on_stop callback.
*/
Expand Down Expand Up @@ -239,7 +232,6 @@ static int check_profile_func(lua_State *L)
== PROFILE_SUCCESS);
assert_true(luaM_sysprof_set_on_stop(on_stop_cb_default)
== PROFILE_SUCCESS);
assert_true(luaM_sysprof_set_backtracer(NULL) == PROFILE_SUCCESS);

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

return TEST_EXIT_SUCCESS;
}
#endif

static int profile_func_jitoff(void *test_state)
{
UNUSED(test_state);
return todo("Need to replace backtrace with libunwind first");
#if 0
lua_State *L = test_state;
utils_get_aux_lfunc(L);
(void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_OFF);
(void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_FLUSH);
check_profile_func(L);
(void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_ON);
return TEST_EXIT_SUCCESS;
#endif
}

static int profile_func_jiton(void *test_state)
{
UNUSED(test_state);
return todo("Need to replace backtrace with libunwind first");
#if 0
lua_State *L = test_state;
utils_get_aux_lfunc(L);
check_profile_func(L);
return TEST_EXIT_SUCCESS;
#endif
}

int main(void)
Expand Down

0 comments on commit 30843f7

Please sign in to comment.