[Tarantool-patches] [PATCH luajit] sysprof: disable runtime host symtab updates

[Tarantool-patches] [PATCH luajit] sysprof: disable runtime host symtab updates

[Maksim Kokryashkin via Tarantool-patches]

From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>

The symtab update for newly loaded shared libraries requires
memory allocation, which is not signal-safe and can cause
crashes. Updating symtab in a VM hook is not a viable option
either, as there are no guarantees that the symbol will be
dumped before its address is streamed. This patch completely
disables the runtime updates of the symtab for host symbols.
That means all Lua-C modules, FFI modules, and shared libraries
must be loaded before starting the profiler.

There is no test along with the patch for two main reasons:
1. The signal should land on an instruction inside the allocator
such that the second allocator call would cause a crash because of
an inconsistent inner state. Although we have ptrace-based
machinery for testing, the control is not that fine-grained. The
only option we have left is to rely on empirical offsets, which
is not a robust solution. Moreover, it is possible to build LuaJIT
with `malloc` as an allocator, and the test should be adapted to
that too. Needless to say, malloc sources may differ from
platform to platform, making the test unreliable.

2. Regression is unlikely here since this patch removes the only
call that could allocate memory inside the signal handler.

Resolves tarantool/tarantool#8140
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8140-crash-in-allocator
Issue: https://github.com/tarantool/tarantool/issues/8140
PR: https://github.com/tarantool/tarantool/pull/9460

 src/lj_sysprof.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index c6c20de2..88c7a41b 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -75,7 +75,6 @@ struct sysprof {
   luam_Sysprof_backtracer backtracer; /* Backtracing function for the host stack. */
   lj_profile_timer timer; /* Profiling timer. */
   int saved_errno; /* Saved errno when profiler failed. */
-  uint32_t lib_adds; /* Number of libs loaded. Monotonic. */
 };
 /*
 ** XXX: Only one VM can be profiled at a time.
@@ -100,7 +99,11 @@ static int is_unconfigured(struct sysprof *sp)
 
 static void stream_prologue(struct sysprof *sp)
 {
-  lj_symtab_dump(&sp->out, sp->g, &sp->lib_adds);
+  /*
+  ** XXX: Must be zero for the symtab module to dump all loaded libraries.
+  */
+  uint32_t unused_lib_adds = 0;
+  lj_symtab_dump(&sp->out, sp->g, &unused_lib_adds);
   lj_wbuf_addn(&sp->out, ljp_header, sizeof(ljp_header));
 }
 
@@ -256,8 +259,6 @@ 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);
   lj_wbuf_addbyte(&sp->out, (uint8_t)vmstate);
   stream_backtrace_host(sp);
 }
-- 
2.39.3 (Apple Git-145)

Re: [Tarantool-patches] [PATCH luajit] sysprof: disable runtime host symtab updates

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the patch!
LGTM!

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] sysprof: disable runtime host symtab updates

[Sergey Bronnikov via Tarantool-patches]

Hello, Max!

thanks for the patch! LGTM


On 12/8/23 09:10, Maksim Kokryashkin wrote:
> From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
<snipped>

Re: [Tarantool-patches] [PATCH luajit] sysprof: disable runtime host symtab updates

[Igor Munkin via Tarantool-patches]

Max,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/3.0 and
release/2.11.

On 08.12.23, Maksim Kokryashkin via Tarantool-patches wrote:
> From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
> 
> The symtab update for newly loaded shared libraries requires
> memory allocation, which is not signal-safe and can cause
> crashes. Updating symtab in a VM hook is not a viable option
> either, as there are no guarantees that the symbol will be
> dumped before its address is streamed. This patch completely
> disables the runtime updates of the symtab for host symbols.
> That means all Lua-C modules, FFI modules, and shared libraries
> must be loaded before starting the profiler.
> 
> There is no test along with the patch for two main reasons:
> 1. The signal should land on an instruction inside the allocator
> such that the second allocator call would cause a crash because of
> an inconsistent inner state. Although we have ptrace-based
> machinery for testing, the control is not that fine-grained. The
> only option we have left is to rely on empirical offsets, which
> is not a robust solution. Moreover, it is possible to build LuaJIT
> with `malloc` as an allocator, and the test should be adapted to
> that too. Needless to say, malloc sources may differ from
> platform to platform, making the test unreliable.
> 
> 2. Regression is unlikely here since this patch removes the only
> call that could allocate memory inside the signal handler.
> 
> Resolves tarantool/tarantool#8140
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8140-crash-in-allocator
> Issue: https://github.com/tarantool/tarantool/issues/8140
> PR: https://github.com/tarantool/tarantool/pull/9460
> 
>  src/lj_sysprof.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 

<snipped>

> -- 
> 2.39.3 (Apple Git-145)
> 

-- 
Best regards,
IM