<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey,</p>
<p>thanks for review! See comments below. The branch was
force-pushed.</p>
<p>Sergey</p>
<div class="moz-cite-prefix">On 3/12/26 12:36, Sergey Kaplun via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Thanks for the patch!
LGTM, after fixing my nits below.
Please add the iterational diff for the fixes.
On 12.03.26, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">From: Mike Pall <mike>
Thanks to Peter Cawley.
(cherry picked from commit d1a2fef8a8f53b0055ee041f7f63d83a27444ffa)
Stack overflow can cause a segmentation fault in a vararg
function on ARM64 and MIPS64 in LJ_FR2 mode. This happens
because the stack check in BC_IFUNCV is off by one on these
platforms without the patch. The original stack check
for ARM64 and MIPS64 was incorrect:
| RA == BASE + (RD=NARGS)*8 + framesize * 8 >= maxstack
while the stack check on x86_64 is correct and therefore is
not affected by the problem:
| RA == BASE + (RD=NARGS+1)*8 + framesize * 8 +8 > maxstack
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Typo: s/ +8/ + 8/
</pre>
</blockquote>
Fixed, thanks!
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
The patch partially fixes the aforementioned issue by bumping
LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a
vararg function as the __newindex metamethod.
A fixup for a number of required slots in `call_init()` was added
for consistency with non-GC64 flavor. The check is too strict, so
this can't lead to any crash.
This patch also corrects the number of redzone slots in
luajit-gdb.py to match the updated LJ_STACK_EXTRA and adds the test
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
luajit_lldb.py should be updated as well.</pre>
</blockquote>
<p>Right, fixed:</p>
<p>--- a/src/luajit_lldb.py<br>
+++ b/src/luajit_lldb.py<br>
@@ -833,7 +833,7 @@ def dump_stack(L, base=None, top=None):<br>
top = top or L.top<br>
stack = mref(TValuePtr, L.stack)<br>
maxstack = mref(TValuePtr, L.maxstack)<br>
- red = 5 + 2 * LJ_FR2<br>
+ red = 5 + 3 * LJ_FR2<br>
<br>
dump = [<br>
'{padding} Red zone: {nredslots: >2} slots
{padding}'.format(<br>
<br>
</p>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre"><gh-1402-call_init-regression.test.lua> that will help to avoid
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue
tracker.
</pre>
</blockquote>
<p>Ah, right, I've overlooked it is a LuaJIT issue, not Tarantool.
Thanks!</p>
<p>Renamed.</p>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">a regression in the future, see details in [1].
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Just mention details here like the following:
| The patch partially fixes the aforementioned issue by bumping
| LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a
| vararg function as the __newindex metamethod.
|
| A fixup for a number of required slots in `call_init()` was added for
| consistency with the non-GC64 flavor. The check is too strict (if
| comparing the corresponding checks in the VM BC_IFUNCV), so this can't
| lead to any crash. To avoid possible regression in the future the
| corresponding test is added.
|
| This patch also corrects the number of redzone slots in luajit-gdb.py
| and luajit_lldb.py to match the updated LJ_STACK_EXTRA.
</pre>
</blockquote>
Updated.
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
Sergey Bronnikov:
* added the description and the test for the problem
Part of tarantool/tarantool#12134
1. <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1402">https://github.com/LuaJIT/LuaJIT/issues/1402</a>
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Please, don't mention the issue during backporting, to avoid messing the
issue tracker.
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">---
src/lj_def.h | 2 +-
src/lj_dispatch.c | 2 +-
src/luajit-gdb.py | 2 +-
src/vm_arm64.dasc | 1 +
src/vm_mips64.dasc | 1 +
.../gh-1402-call_init-regression.test.lua | 36 +++++++++++++
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue
tracker.</pre>
</blockquote>
renamed
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre"> ...048-fix-stack-checks-vararg-calls.test.lua | 53 +++++++++++++++++++
7 files changed, 94 insertions(+), 3 deletions(-)
create mode 100644 test/tarantool-tests/gh-1402-call_init-regression.test.lua
create mode 100644 test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
diff --git a/src/lj_def.h b/src/lj_def.h
index a5bca6b0..7e4f251e 100644
--- a/src/lj_def.h
+++ b/src/lj_def.h
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c
index a44a5adf..431cb3c2 100644
--- a/src/lj_dispatch.c
+++ b/src/lj_dispatch.c
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index 0ae2a6e0..dab07b35 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
index 6600e226..5ef37243 100644
--- a/src/vm_arm64.dasc
+++ b/src/vm_arm64.dasc
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc
index da187a7a..6c2975b4 100644
--- a/src/vm_mips64.dasc
+++ b/src/vm_mips64.dasc
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">diff --git a/test/tarantool-tests/gh-1402-call_init-regression.test.lua b/test/tarantool-tests/gh-1402-call_init-regression.test.lua
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Please, avoid _ in the file names, lets name it like:
lj-1402-vararg-stkov-check-gc64.test.lua
Same for the name of the test.</pre>
</blockquote>
ok, renamed once again
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">new file mode 100644
index 00000000..b20f9e39
--- /dev/null
+++ b/test/tarantool-tests/gh-1402-call_init-regression.test.lua
@@ -0,0 +1,36 @@
+local tap = require('tap')
+
+-- A test file to demonstrate a probably quite strict stack
+-- check for vararg functions in call_init.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
This is not about quite strict stack check. We need this to test the
behaviour of the LuaJIT while recording the vararg function. Let's
rephrase like the following:
| -- The test file to verify correctness of stack size check during
| -- recording of vararg functions.</pre>
</blockquote>
Updated.
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
The test file to verify correctness of stack size check during recording of vararg functions.
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+-- See also <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1402">https://github.com/LuaJIT/LuaJIT/issues/1402</a>
+local test = tap.test('gh-1402-call_init-regression.test.lua'):skipcond({
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue
tracker.
</pre>
</blockquote>
<p>renamed</p>
<p>---
a/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua<br>
+++
b/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua<br>
@@ -1,15 +1,16 @@<br>
local tap = require('tap')<br>
<br>
--- A test file to demonstrate a probably quite strict stack<br>
--- check for vararg functions in call_init.<br>
+-- The test file to verify correctness of stack size check during<br>
+-- recording of vararg functions.<br>
-- See also <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1402">https://github.com/LuaJIT/LuaJIT/issues/1402</a><br>
-local test =
tap.test('gh-1402-call_init-regression.test.lua'):skipcond({<br>
+local test =
tap.test('lj-1402-vararg-stkov-check-gc64.test.lua'):skipcond({<br>
['Test requires JIT enabled'] = not jit.status(),<br>
})<br>
<br>
<br>
</p>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+local function vararg(...) -- luacheck: no unused
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Let's use this comment before the vararg declaration.
It helps with the _ below as well.</pre>
</blockquote>
<p>Updated:</p>
<p>---
a/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua<br>
+++
b/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua<br>
@@ -1,15 +1,16 @@<br>
local tap = require('tap')<br>
<br>
--- A test file to demonstrate a probably quite strict stack<br>
--- check for vararg functions in call_init.<br>
+-- The test file to verify correctness of stack size check during<br>
+-- recording of vararg functions.<br>
-- See also <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1402">https://github.com/LuaJIT/LuaJIT/issues/1402</a><br>
-local test =
tap.test('gh-1402-call_init-regression.test.lua'):skipcond({<br>
+local test =
tap.test('lj-1402-vararg-stkov-check-gc64.test.lua'):skipcond({<br>
['Test requires JIT enabled'] = not jit.status(),<br>
})<br>
<br>
<a class="moz-txt-link-freetext" href="test:plan(1)">test:plan(1)</a><br>
<br>
-local function vararg(...) -- luacheck: no unused<br>
+-- luacheck: no unused<br>
+local function vararg(...)<br>
-- None.<br>
end<br>
</p>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ -- None.
+end
+
+-- Make compilation aggressive.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Excess comment. It's quite general approach in our tests.
</pre>
</blockquote>
<p><br>
</p>
<p>Updated:</p>
<p> <a class="moz-txt-link-freetext" href="test:plan(1)">test:plan(1)</a><br>
<br>
-local function vararg(...) -- luacheck: no unused<br>
+-- luacheck: no unused<br>
+local function vararg(...)<br>
-- None.<br>
end<br>
<br>
<br>
</p>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+jit.opt.start("hotloop=1")
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Typo: s/"/'/g
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+
</pre>
</blockquote>
</blockquote>
end<br>
<br>
--- Make compilation aggressive.<br>
-jit.opt.start("hotloop=1")<br>
+jit.opt.start('hotloop=1')<br>
<br>
local function caller()<br>
-- luacheck: push no unused<br>
<br>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
Please add the following comment:
| -- This function utilizes the exact amount of stack slots
| -- to cause the stack reallocation during `call_init()` in the
| -- GC64 mode.
</pre>
</blockquote>
--- Make compilation aggressive.<br>
-jit.opt.start("hotloop=1")<br>
+jit.opt.start('hotloop=1')<br>
<br>
+-- This function utilizes the exact amount of stack slots to cause<br>
+-- the stack reallocation during `call_init()` in the GC64 mode.<br>
local function caller()<br>
-- luacheck: push no unused<br>
local _, _, _, _, _, _, _, _, _, _<br>
<br>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+local function caller()
+ -- luacheck: push no unused
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Lets drop this luacheck suppression, see the comment above.
</pre>
</blockquote>
<p>Updated:</p>
<p>+-- This function utilizes the exact amount of stack slots to
cause<br>
+-- the stack reallocation during `call_init()` in the GC64 mode.<br>
local function caller()<br>
- -- luacheck: push no unused<br>
local _, _, _, _, _, _, _, _, _, _<br>
local _, _, _, _, _, _, _, _, _, _<br>
local _, _, _, _, _, _, _, _, _, _<br>
- -- luacheck: pop<br>
local n = 1<br>
while n < 3 do<br>
vararg()<br>
<br>
</p>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+ local _, _, _, _, _, _, _, _, _, _
+ local _, _, _, _, _, _, _, _, _, _
+ local _, _, _, _, _, _, _, _, _, _
+ -- luacheck: pop
+ local n = 1
+ while n < 3 do
+ vararg()
+ n = n + 1
+ end
+end
+
+pcall(coroutine.wrap(caller))
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
The pcall is excess lets do it without it:
| coroutine.wrap(caller)()
</pre>
</blockquote>
@@ -29,7 +29,7 @@ local function caller()<br>
end<br>
end<br>
<br>
-pcall(coroutine.wrap(caller))<br>
+coroutine.wrap(caller)()<br>
<br>
<a class="moz-txt-link-freetext" href="test:ok(true">test:ok(true</a>, 'no assertion for vararg functions in call_init')<br>
<br>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+
+test:ok(true, 'no assertion for vararg functions in call_init')
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
Just mention 'no assertion failure' (this assertion isn't in the
`call_init()`, but during recording in `rec_check_slots()`).
</pre>
</blockquote>
<p>Updated:</p>
<p> <br>
-test:ok(true, 'no assertion for vararg functions in call_init')<br>
+test:ok(true, 'no assertion')<br>
<br>
<a class="moz-txt-link-freetext" href="test:done(true)">test:done(true)</a><br>
<br>
</p>
<blockquote type="cite" cite="mid:abKJM3PapGNa6BMR@root">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">+
+test:done(true)
diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
new file mode 100644
index 00000000..3a8ad63d
--- /dev/null
+++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">--
2.43.0
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>