Bonjour, Sergey,

fixed and force-pushed.

Sergey


On 07.02.2025 13:50, Sergey Kaplun via Tarantool-patches wrote:
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 function
Typo: s/C function/a C function/
Fixed.

+ * 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>