<!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 the patch! LGTM with a minor comment.</p>
<p>Sergey</p>
<div class="moz-cite-prefix">On 2/25/26 15:11, Sergey Kaplun wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20260225121140.18847-1-skaplun@tarantool.org">
<pre wrap="" class="moz-quote-pre">From: Mike Pall <mike>
Thanks to Sergey Kaplun.
(cherry picked from commit d459c6ce503e880dc30aefb6b61aa7f2124c7a6e)
This patch is a follow-up to the commit
7505e78bd6c24cac6e93f5163675021734801b65 "Handle on-trace OOM errors
from helper functions.". Since this commit, the `pcall()` (and
`xpcall()`) emits an additional snapshot, but the arguments to the
pcall-ed function are momentarily purged from the snapshot since they
are not used. Hence, in several cases `debug.getlocal()` can't find the
corresponding slot for the previous frame.
This patch prevents purging right after `pcall()`, `xpcall()` recording.
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#12134
---
Branch: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/skaplun/lj-1425-pcall-snap-purge">https://github.com/tarantool/luajit/tree/skaplun/lj-1425-pcall-snap-purge</a>
Related issues:
* <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1425">https://github.com/LuaJIT/LuaJIT/issues/1425</a>
* <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/12134">https://github.com/tarantool/tarantool/issues/12134</a>
src/lj_record.c | 2 +-
.../lj-1425-pcall-snap-purge.test.lua | 93 +++++++++++++++++++
2 files changed, 94 insertions(+), 1 deletion(-)
create mode 100644 test/tarantool-tests/lj-1425-pcall-snap-purge.test.lua
diff --git a/src/lj_record.c b/src/lj_record.c
index ba409a61..e2e86f79 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -2120,7 +2120,7 @@ void lj_record_ins(jit_State *J)
/* Need snapshot before recording next bytecode (e.g. after a store). */
if (J->needsnap) {
J->needsnap = 0;
- if (J->pt) lj_snap_purge(J);
+ if (J->pt && bc_op(*J->pc) < BC_FUNCF) lj_snap_purge(J);
lj_snap_add(J);
J->mergesnap = 1;
}
diff --git a/test/tarantool-tests/lj-1425-pcall-snap-purge.test.lua b/test/tarantool-tests/lj-1425-pcall-snap-purge.test.lua
new file mode 100644
index 00000000..353842f1
--- /dev/null
+++ b/test/tarantool-tests/lj-1425-pcall-snap-purge.test.lua
@@ -0,0 +1,93 @@
+local tap = require('tap')
+
+-- Test file to demonstrate incorrect snapshot purge for the
+-- `pcall()` and `xpcall()`.
+-- See also: <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1425">https://github.com/LuaJIT/LuaJIT/issues/1425</a>.
+
+local test = tap.test('lj-1425-pcall-snap-purge'):skipcond({
+ ['Test requires JIT enabled'] = not jit.status(),
+})
+
+-- `pcall()` and `xpcall()`.
+test:plan(2)
+
+-- XXX: simplify `jit.dump()` output.
+local type = type
+local pcall = pcall
+local xpcall = xpcall
+local math_modf = math.modf
+local debug_getlocal = debug.getlocal
+
+local checkers = {}
+
+-- Called twice for the pseudo-type that aliases base Lua type via
+-- checkers map.
+local function checks(expected_type)
+ -- Value expected to be `checks_tab()` or `checks_obj()`
+ -- argument. It is always a table.
+ local _, value = debug_getlocal(2, 1)
+ -- Simple stitching function. Additional arguments are needed to
+ -- occupy the corresponding slot.</pre>
</blockquote>
<p>It is not clear for me why this corresponding slot is needed.</p>
<p>The bug can be reproduced without double nil's passed to
math_modf().</p>
<blockquote type="cite"
cite="mid:20260225121140.18847-1-skaplun@tarantool.org">
<pre wrap="" class="moz-quote-pre">
+ math_modf(0, nil, nil)
+ -- Start trace now, one iteration only.
+ -- luacheck: ignore 512
+ while true do
+ -- Base type?
+ if type(value) == expected_type then
+ return true
+ end
+ -- Pseudo types fallbacks to the map.
+ local checker = checkers[expected_type]
+ -- For the xpcall.
+ if checker(value) == true then
+ return true
+ end
+ break
+ end
+ error('Unreachable path taken')
+end
+
+-- Need to be pcalled.
+local function checks_tab(_)
+ checks('table')
+end
+
+local function checks_tab_p(map)
+ return pcall(checks_tab, map)
+end
+
+local function nop()
+end
+
+local function checks_tab_xp(map)
+ return xpcall(checks_tab, nop, map)
+end
+
+local function checks_obj(_)
+ checks('obj')
+end
+
+local function check_ff(name, checks_func)
+ <a class="moz-txt-link-freetext" href="test:test(name">test:test(name</a>, function(subtest)
+ <a class="moz-txt-link-freetext" href="subtest:plan(1)">subtest:plan(1)</a>
+
+ checkers['obj'] = checks_func
+
+ jit.flush()
+ jit.opt.start('hotloop=1')
+
+ checks_obj({})
+ -- Forcify stack reallocation on trace in `checks()`. The
+ -- source stack lacks the needed slot.
+ coroutine.wrap(function()
+ checks_obj({})
+ end)()
+
+ <a class="moz-txt-link-freetext" href="subtest:ok(true">subtest:ok(true</a>, 'No error for ' .. name)
+ end)
+end
+
+check_ff('pcall', checks_tab_p)
+check_ff('xpcall', checks_tab_xp)
+
+test:done(true)
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>