Skip to content

Commit

Permalink
Fix recording of BC_VARG.
Browse files Browse the repository at this point in the history
Reported by Bachir Bendrissou.

(cherry picked from commit 62e362a)

When the trace is started after the stitching, it may not have some
slots from the first one. That slot may be the first slot given to the
`select()` function (if it is determined by the call that caused the
stitch). Before the patch, there is no loading of this slot in the
`rec_varg()` for the trace, and the 0 value from slots is taken instead.
Hence, the following recording of `BC_VARG` is incorrect.

This patch fixes this by using the `getslot()` instead of taking the
value directly.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11055

Reviewed-by: Sergey Bronnikov <[email protected]>
Signed-off-by: Sergey Kaplun <[email protected]>
(cherry picked from commit 2ce227a)
  • Loading branch information
Mike Pall authored and Buristan committed Feb 11, 2025
1 parent 6a00abe commit eb5c41e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/lj_record.c
Original file line number Diff line number Diff line change
Expand Up @@ -1869,7 +1869,7 @@ static void rec_varg(jit_State *J, BCReg dst, ptrdiff_t nresults)
J->maxslot = dst + (BCReg)nresults;
}
} else if (select_detect(J)) { /* y = select(x, ...) */
TRef tridx = J->base[dst-1];
TRef tridx = getslot(J, dst-1);
TRef tr = TREF_NIL;
ptrdiff_t idx = lj_ffrecord_select_mode(J, tridx, &J->L->base[dst-1]);
if (idx < 0) goto nyivarg;
Expand Down
36 changes: 36 additions & 0 deletions test/tarantool-tests/fix-recording-bc-varg-used-in-select.test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
local tap = require('tap')

-- Test file to demonstrate incorrect recording of `BC_VARG` that
-- is given to the `select()` function. See also:
-- https://www.freelists.org/post/luajit/Possible-issue-during-register-allocation-ra-alloc1.

local test = tap.test('fix-recording-bc-varg-used-in-select'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
})

test:plan(1)

-- XXX: Simplify `jit.dump` output.
local modf = math.modf

local EXPECTED = 'canary'
local function test_func(...)
local first_varg_item
for _ = 1, 4 do
-- `modf()` is used to create a stitching with a meaningful
-- index value, that equals 1, i.e. refers to the first item
-- in `...`. The second trace started after stitching does not
-- contain the stack slot for the first argument of the
-- `select()`. Before the patch, there is no loading of
-- this slot for the trace and the 0 value is taken instead.
-- Hence, this leads to an incorrect recording of the
-- `BC_VARG` with detected `select()`.
first_varg_item = select(modf(1, 0), ...)
end
return first_varg_item
end

jit.opt.start('hotloop=1')
test:is(test_func(EXPECTED), EXPECTED, 'correct BC_VARG recording')

test:done(true)

0 comments on commit eb5c41e

Please sign in to comment.