From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergos@tarantool.org Subject: [Tarantool-patches] [PATCH luajit] sysprof: move symtab update into profile hook Date: Tue, 23 May 2023 18:27:48 +0300 [thread overview] Message-ID: <20230523152748.203212-1-m.kokryashkin@tarantool.org> (raw) 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
next reply other threads:[~2023-05-23 15:27 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-23 15:27 Maxim Kokryashkin via Tarantool-patches [this message] 2023-05-24 9:46 ` Sergey Kaplun via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230523152748.203212-1-m.kokryashkin@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=max.kokryashkin@gmail.com \ --cc=sergos@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] sysprof: move symtab update into profile hook' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox