<!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 question below.<br>
</p>
<div class="moz-cite-prefix">On 09.01.2025 18:01, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20250109150124.23841-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">From: Mike Pall <mike>
Reported by Sergey Kaplun.
(cherry picked from commit f602f0154b644211283cfeea92a570ca38f71947)
Before the patch `predict_next()` uses the pc allocation limit
(`fs->bclim`) instead of the real limit of the defined bytecodes
(`fs->pc`). This leads to the use of undefined value and possible
crash. This patch fixes the check.
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#10709
---
Branch: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/skaplun/lj-1226-fix-predict-next">https://github.com/tarantool/luajit/tree/skaplun/lj-1226-fix-predict-next</a>
Related issues:
* <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1226">https://github.com/LuaJIT/LuaJIT/issues/1226</a>
* <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/10709">https://github.com/tarantool/tarantool/issues/10709</a>
src/lj_parse.c | 6 ++--
.../lj-1226-fix-predict-next.test.lua | 31 +++++++++++++++++++
2 files changed, 33 insertions(+), 4 deletions(-)
create mode 100644 test/tarantool-tests/lj-1226-fix-predict-next.test.lua
diff --git a/src/lj_parse.c b/src/lj_parse.c
index 9b45b103..ec85ac9b 100644
--- a/src/lj_parse.c
+++ b/src/lj_parse.c
@@ -2527,11 +2527,9 @@ static void parse_for_num(LexState *ls, GCstr *varname, BCLine line)
*/
static int predict_next(LexState *ls, FuncState *fs, BCPos pc)
{
- BCIns ins;
+ BCIns ins = fs->bcbase[pc].ins;
GCstr *name;
cTValue *o;
- if (pc >= fs->bclim) return 0;
- ins = fs->bcbase[pc].ins;
switch (bc_op(ins)) {
case BC_MOV:
if (bc_d(ins) >= fs->nactvar) return 0;
@@ -2580,7 +2578,7 @@ static void parse_for_iter(LexState *ls, GCstr *indexname)
assign_adjust(ls, 3, expr_list(ls, &e), &e);
/* The iterator needs another 3 [4] slots (func [pc] | state ctl). */
bcreg_bump(fs, 3+LJ_FR2);
- isnext = (nvars <= 5 && predict_next(ls, fs, exprpc));
+ isnext = (nvars <= 5 && fs->pc > exprpc && predict_next(ls, fs, exprpc));
var_add(ls, 3); /* Hidden control variables. */
lex_check(ls, TK_do);
loop = bcemit_AJ(fs, isnext ? BC_ISNEXT : BC_JMP, base, NO_JMP);
diff --git a/test/tarantool-tests/lj-1226-fix-predict-next.test.lua b/test/tarantool-tests/lj-1226-fix-predict-next.test.lua
new file mode 100644
index 00000000..3cd2c8f5
--- /dev/null
+++ b/test/tarantool-tests/lj-1226-fix-predict-next.test.lua
@@ -0,0 +1,31 @@
+local tap = require('tap')
+local test = tap.test('lj-1226-fix-predict-next')
+
+test:plan(3)
+
+-- The resulting bytecode is the following:
+--
+-- 0001 KNIL 0 3
+-- 0002 JMP 4 => 0003
+-- 0003 => ITERC 4 2 3
+-- 0004 ITERL 4 => 0003
+--
+-- The parsing of the `for` iterator uses the incorrect check for
+-- `fs->bclim`, which allows the usage of an uninitialized value,
+-- so the test fails under Valgrind.
+local res_f = loadstring([[
+-- This local variable is necessary, because it emits `KPRI`
+-- bytecode, with which the next `KPRI` bytecode will be merged.
+local _
+for _ in nil do end
+]])
+
+test:ok(res_f, 'chunk loaded successfully')
+
+local res, err = pcall(res_f)
+
+-- Check consistency with PUC Rio Lua 5.1 behaviour.</pre>
</blockquote>
What for? It is not related to the bug and fix for it.<br>
<blockquote type="cite"
cite="mid:20250109150124.23841-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">
+test:ok(not res, 'loaded function not executed')
+test:like(err, 'attempt to call a nil value', 'correct error message')
+
+test:done(true)
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>