Skip to content

Commit

Permalink
Add missing coercion when recording select(string, ...)
Browse files Browse the repository at this point in the history
Thanks to Peter Cawley.

(cherry picked from commit 92b89d0)

Before the patch, the recording of `select()` with a string argument
leads to the following IR:
| rcx   >  int CONV   "1"   int.num index
Where the operand has string type instead of number type.
This leads to the corresponding mcode:
| cvttsd2si ecx, xmm1
Where xmm1 has an undefined value. Thus leads to the undefined behaviour
for the recording trace.

This patch adds the missing coercion.

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

Part of tarantool/tarantool#10199

Reviewed-by: Sergey Bronnikov <[email protected]>
Reviewed-by: Maxim Kokryashkin <[email protected]>
Signed-off-by: Sergey Kaplun <[email protected]>
  • Loading branch information
Mike Pall authored and Buristan committed Oct 16, 2024
1 parent a72e884 commit 088e2e1
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/lj_record.c
Original file line number Diff line number Diff line change
Expand Up @@ -1871,8 +1871,11 @@ static void rec_varg(jit_State *J, BCReg dst, ptrdiff_t nresults)
TRef tr = TREF_NIL;
ptrdiff_t idx = lj_ffrecord_select_mode(J, tridx, &J->L->base[dst-1]);
if (idx < 0) goto nyivarg;
if (idx != 0 && !tref_isinteger(tridx))
if (idx != 0 && !tref_isinteger(tridx)) {
if (tref_isstr(tridx))
tridx = emitir(IRTG(IR_STRTO, IRT_NUM), tridx, 0);
tridx = emitir(IRTGI(IR_CONV), tridx, IRCONV_INT_NUM|IRCONV_INDEX);
}
if (idx != 0 && tref_isk(tridx)) {
emitir(IRTGI(idx <= nvararg ? IR_GE : IR_LT),
fr, lj_ir_kint(J, frofs+8*(int32_t)idx));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
local tap = require('tap')

-- Test file to demonstrate LuaJIT incorrect recording of
-- `select()` fast function.
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1083.

local test = tap.test('lj-1083-missing-tostring-coercion-in-select'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
})

test:plan(1)

-- Simplify the `jit.dump()` output.
local select = select

local function test_select(...)
local result
for _ = 1, 4 do
-- Before the patch, with the missed coercion to string, the
-- recording of `select()` below leads to the following IR:
-- | rcx > int CONV "1" int.num index
-- Where the operand has string type instead of number type.
-- This leads to the corresponding mcode:
-- | cvttsd2si ecx, xmm1
-- Where xmm1 has an undefined value. Thus leads to the
-- incorrect result for the call below.
result = select('1', ...)
end
return result
end

jit.opt.start('hotloop=1')

-- XXX: amount of arguments is empirical, see the comment above.
local result = test_select(1, 2, 3, 4)

test:is(result, 1, 'correct select result after recording')

test:done(true)

0 comments on commit 088e2e1

Please sign in to comment.