Bonjour, Sergey,
fixed and force-pushed.
Sergey
Fixed.Hi, Sergey! Thanks for the fixes! LGTM, after fixing a bunch of nits below. On 06.02.25, Sergey Bronnikov wrote:From: Mike Pall <mike><snipped>--- Branch: https://github.com/tarantool/luajit/tree/ligurio/lj-1087-vm-handler-call Related issues: - https://github.com/LuaJIT/LuaJIT/issues/1087 - https://github.com/tarantool/tarantool/issues/10709 Changes in the v2: * Addressed comments by Sergey Kaplun<snipped>diff --git a/src/lj_trace.c b/src/lj_trace.c index 94cb27e5..6b97cc13 100644 --- a/src/lj_trace.c +++ b/src/lj_trace.c<snipped>diff --git a/test/tarantool-c-tests/lj-1087-vm-handler-call.test.c b/test/tarantool-c-tests/lj-1087-vm-handler-call.test.c new file mode 100644 index 00000000..ebdf63e1 --- /dev/null +++ b/test/tarantool-c-tests/lj-1087-vm-handler-call.test.c<snipped>+/* XXX: Still need normal assert inside writer functions. */Minor: Since there is no writer function, I would rather use the following wording: | Still need normal assert for sanity checks.
Fixed.+#undef NDEBUG +#include <assert.h> + +/* + * Test file to demonstrate a segmentation fault, when C functionTypo: s/C function/a C function/
+ * is used as a VM handler in LuaJIT:Minor: `as a VM handler for trace events` is a little bit more verbose. Feel free to ignore.
Fixed.
/*
- * Test file to demonstrate a segmentation fault, when C function
- * is used as a VM handler in LuaJIT:
+ * Test file to demonstrate a segmentation fault, when a C
+ * function is used as a VM handler for trace events in LuaJIT:
*
* Program received signal SIGSEGV, Segmentation fault.
* 0x000055555557e77d in trace_abort (J=0x7ffff7f9b6b8) at
lj_trace.c:615
+ * + * Program received signal SIGSEGV, Segmentation fault. + * 0x000055555557e77d in trace_abort (J=0x7ffff7f9b6b8) at lj_trace.c:615 + * 615 lj_vmevent_send(L, TRACE, + * (gdb) bt + * + * See details in https://github.com/LuaJIT/LuaJIT/issues/1087. + */ +<snipped>+ /* + * The number 32767 is `REF_DROP - REF_BIAS`.It will be better to rephrase the next sentence.See the commit + * 0fdf06b456e6 ("test: relax JIT setup in misc.getmetrics + * test") for the details. + */I suggest the following comment. | This is the maximum possible IR amount, so the trace is always | aborted. The amount of text to read is approximately the same, but we don't need to look into the mysterious commit anymore. :)
Fixed.
@@ -103,9 +103,9 @@ static int handle_cont_frame(void
*test_state)
jit_attach(L, nop, "trace");
/*
- * The number 32767 is `REF_DROP - REF_BIAS`. See the
commit
- * 0fdf06b456e6 ("test: relax JIT setup in misc.getmetrics
- * test") for the details.
+ * The number 32767 is `REF_DROP - REF_BIAS`. This is the
+ * maximum possible IR amount, so the trace is always
+ * aborted.
*/
+ int res = luaL_dostring(L, "jit.opt.start('minstitch=32767')");<snipped>