Skip to content

Commit

Permalink
Fix predict_next() in parser (for real now).
Browse files Browse the repository at this point in the history
Reported by Sergey Kaplun.

(cherry picked from commit f602f01)

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
  • Loading branch information
Mike Pall authored and Buristan committed Jan 16, 2025
1 parent ffede1b commit 3a2c2af
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
6 changes: 2 additions & 4 deletions src/lj_parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
31 changes: 31 additions & 0 deletions test/tarantool-tests/lj-1226-fix-predict-next.test.lua
Original file line number Diff line number Diff line change
@@ -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.
test:ok(not res, 'loaded function not executed')
test:like(err, 'attempt to call a nil value', 'correct error message')

test:done(true)

0 comments on commit 3a2c2af

Please sign in to comment.