[Tarantool-patches] [PATCH luajit] sysprof: move symtab update into profile hook
Maxim Kokryashkin
max.kokryashkin at gmail.com
Tue May 23 18:27:48 MSK 2023
Before the patch, the symtab update was done in the signal
handler. That update requires memory allocation, which
can't be done safely in a signal handler. This patch reuses
LuaJIT's HOOK_PROFILE for symtab update routine execution, so
it is now possible to update it safely after the signal
handler exit.
Resolves tarantool/tarantool#8140
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8140-sysprof-allocator-crash
Issue: https://github.com/tarantool/tarantool/issues/8140
PR: https://github.com/tarantool/tarantool/pull/8691
src/lj_dispatch.c | 10 ++++
src/lj_sysprof.c | 34 +++++++++++++-
src/lj_sysprof.h | 4 ++
test/tarantool-tests/CMakeLists.txt | 1 +
.../gh-8140-sysprof-allocator-crash.test.lua | 46 +++++++++++++++++++
.../CMakeLists.txt | 9 ++++
.../sysprofalloc.c | 17 +++++++
7 files changed, 119 insertions(+), 2 deletions(-)
create mode 100644 test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua
create mode 100644 test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt
create mode 100644 test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c
diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c
index ee735450..65948a0c 100644
--- a/src/lj_dispatch.c
+++ b/src/lj_dispatch.c
@@ -30,6 +30,9 @@
#if LJ_HASPROFILE
#include "lj_profile.h"
#endif
+#if LJ_HASSYSPROF
+#include "lj_sysprof.h"
+#endif
#include "lj_vm.h"
#include "luajit.h"
@@ -552,7 +555,14 @@ void LJ_FASTCALL lj_dispatch_profile(lua_State *L, const BCIns *pc)
global_State *g;
setcframe_pc(cf, pc);
L->top = L->base + cur_topslot(pt, pc, cframe_multres_n(cf));
+#if LJ_HASSYSPROF
+ if (lj_symtab_update_requested())
+ lj_symtab_update_hook(L);
+ else
+ lj_profile_interpreter(L);
+#else
lj_profile_interpreter(L);
+#endif
setcframe_pc(cf, oldpc);
g = G(L);
setgcref(g->cur_L, obj2gco(L));
diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 2e9ed9b3..5b5cbe2a 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -76,6 +76,7 @@ struct sysprof {
lj_profile_timer timer; /* Profiling timer. */
int saved_errno; /* Saved errno when profiler failed. */
uint32_t lib_adds; /* Number of libs loaded. Monotonic. */
+ volatile sig_atomic_t symtab_update_needed; /* Symtab update request flag. */
};
/*
** XXX: Only one VM can be profiled at a time.
@@ -88,6 +89,36 @@ static struct sysprof sysprof = {0};
static const uint8_t ljp_header[] = {'l', 'j', 'p', LJP_FORMAT_VERSION,
0x0, 0x0, 0x0};
+void lj_symtab_update_hook(lua_State *L) {
+ struct sysprof *sp = &sysprof;
+ global_State *g = G(L);
+ uint8_t mask;
+ mask = (g->hookmask & ~HOOK_PROFILE);
+ sp->symtab_update_needed = 0;
+ if (!(mask & HOOK_VMEVENT)) {
+ g->hookmask = HOOK_VMEVENT;
+ lj_dispatch_update(g);
+ lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_CFUNC_EVENT, L);
+ }
+ g->hookmask = mask;
+ lj_dispatch_update(g);
+}
+
+int lj_symtab_update_requested() {
+ struct sysprof *sp = &sysprof;
+ return sp->symtab_update_needed;
+}
+
+static void setup_symtab_update_hook(struct sysprof *sp) {
+ global_State *g = sp->g;
+ uint8_t mask = g->hookmask;
+ if (!(mask & (HOOK_PROFILE|HOOK_VMEVENT|HOOK_GC))) {
+ sp->symtab_update_needed = 1;
+ g->hookmask = (mask | HOOK_PROFILE);
+ lj_dispatch_update(g);
+ }
+}
+
static int stream_is_needed(struct sysprof *sp)
{
return sp->opt.mode != LUAM_SYSPROF_DEFAULT;
@@ -240,8 +271,7 @@ static void stream_guest(struct sysprof *sp, uint32_t vmstate)
static void stream_host(struct sysprof *sp, uint32_t vmstate)
{
- struct lua_State *L = gco2th(gcref(sp->g->cur_L));
- lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_CFUNC_EVENT, L);
+ setup_symtab_update_hook(sp);
lj_wbuf_addbyte(&sp->out, (uint8_t)vmstate);
stream_backtrace_host(sp);
}
diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h
index 7e8c2e6e..456ca76f 100644
--- a/src/lj_sysprof.h
+++ b/src/lj_sysprof.h
@@ -108,4 +108,8 @@ void lj_sysprof_add_proto(const struct GCproto *pt);
void lj_sysprof_add_trace(const struct GCtrace *tr);
#endif /* LJ_HASJIT */
+void lj_symtab_update_hook(lua_State *L);
+
+int lj_symtab_update_requested();
+
#endif
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index a428d009..25244733 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -61,6 +61,7 @@ add_subdirectory(gh-5813-resolving-of-c-symbols/gnuhash)
add_subdirectory(gh-5813-resolving-of-c-symbols/stripped)
add_subdirectory(gh-6098-fix-side-exit-patching-on-arm64)
add_subdirectory(gh-6189-cur_L)
+add_subdirectory(gh-8140-sysprof-allocator-crash)
add_subdirectory(lj-49-bad-lightuserdata)
add_subdirectory(lj-416-xor-before-jcc)
add_subdirectory(lj-601-fix-gc-finderrfunc)
diff --git a/test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua b/test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua
new file mode 100644
index 00000000..afce83e2
--- /dev/null
+++ b/test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua
@@ -0,0 +1,46 @@
+local tap = require('tap')
+local test = tap.test('gh-8140-sysprof-allocator-crash'):skipcond({
+ ["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
+ jit.arch ~= "x64",
+ ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
+})
+test:plan(2)
+
+local profilename = require("utils").profilename
+local profile = require('jit.profile')
+
+local TMP_BINFILE = profilename("sysprofdata.tmp.bin")
+local callback_called = false
+
+local function payload()
+ local r = 0
+ for i = 1, 1e8 do
+ r = r + i
+ end
+ return r
+end
+
+local function callback(_, _, _)
+ callback_called = true
+end
+
+profile.start('f', callback)
+payload()
+profile.stop()
+
+test:ok(callback_called, 'LuaJIT profiler callback was not called.')
+
+jit.off()
+misc.sysprof.start{mode='C', interval=1, path=TMP_BINFILE}
+for _ = 1, 1e4 do
+ require('sysprofalloc').get_string()
+ -- Make sure that C library is collected, so it will be loaded
+ -- again on the next iteration.
+ collectgarbage()
+end
+misc.sysprof.stop()
+
+test:ok(true, 'Sysprof has crashed.')
+
+os.remove(TMP_BINFILE)
+os.exit(test:check() and 0 or 1)
diff --git a/test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt
new file mode 100644
index 00000000..dbe60219
--- /dev/null
+++ b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt
@@ -0,0 +1,9 @@
+if (NOT(CMAKE_SYSTEM_NAME STREQUAL "Darwin"))
+ BuildTestCLib(sysprofalloc sysprofalloc.c)
+ # Unfortunately, <target_link_options> command is introduced
+ # since CMake 3.13, so we can't use it now considering ancient
+ # distros support. Just build linker flags by hands.
+ set(CMAKE_SHARED_LINKER_FLAGS
+ "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--hash-style=both"
+ )
+endif()
diff --git a/test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c
new file mode 100644
index 00000000..d3c41d42
--- /dev/null
+++ b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c
@@ -0,0 +1,17 @@
+#include <lua.h>
+#include <lauxlib.h>
+
+int get_string(lua_State *L) {
+ lua_pushstring(L, "test string");
+ return 1;
+}
+
+static const struct luaL_Reg sysprofalloc [] = {
+ {"get_string", get_string},
+ {NULL, NULL}
+};
+
+int luaopen_sysprofalloc(lua_State *L) {
+ luaL_register(L, "sysprofalloc", sysprofalloc);
+ return 1;
+}
--
2.40.1
More information about the Tarantool-patches
mailing list