Skip to content

Commit

Permalink
FFI: Add missing coercion when recording 64-bit bit.*().
Browse files Browse the repository at this point in the history
Thanks to Peter Cawley.

(cherry picked from commit 304da39)

Before the patch, with the missed coercion from string, there is the
cast to `i64` from `p64`, where the last one is the string address. This
leads to an incorrect result of the bit operation.

This patch adds the missing coercion everywhere for bit operations
recording. Only the `recff_bit64_nary()` is affected, since all other
routines have the corresponding type check and cast emitting if
necessary. However, for the consistency, all functions have the same
checking routine `crec_bit64_arg()` now.

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

Part of tarantool/tarantool#10709

Reviewed-by: Sergey Bronnikov <[email protected]>
Signed-off-by: Sergey Kaplun <[email protected]>
(cherry picked from commit c698ff5)
  • Loading branch information
Mike Pall authored and Buristan committed Jan 27, 2025
1 parent 5c8aa9a commit 89e78a6
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/Makefile.dep.original
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ lj_crecord.o: lj_crecord.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
lj_cdata.h lj_cparse.h lj_cconv.h lj_carith.h lj_clib.h lj_ccall.h \
lj_ff.h lj_ffdef.h lj_ir.h lj_jit.h lj_ircall.h lj_iropt.h lj_trace.h \
lj_dispatch.h lj_traceerr.h lj_record.h lj_ffrecord.h lj_snap.h \
lj_crecord.h lj_strfmt.h
lj_crecord.h lj_strfmt.h lj_strscan.h
lj_ctype.o: lj_ctype.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \
lj_gc.h lj_err.h lj_errmsg.h lj_str.h lj_tab.h lj_strfmt.h lj_ctype.h \
lj_ccallback.h lj_buf.h
Expand Down
31 changes: 21 additions & 10 deletions src/lj_crecord.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "lj_crecord.h"
#include "lj_dispatch.h"
#include "lj_strfmt.h"
#include "lj_strscan.h"

/* Some local macros to save typing. Undef'd at the end. */
#define IR(ref) (&J->cur.ir[(ref)])
Expand Down Expand Up @@ -1782,11 +1783,21 @@ static CTypeID crec_bit64_type(CTState *cts, cTValue *tv)
return 0; /* Use regular 32 bit ops. */
}

static TRef crec_bit64_arg(jit_State *J, CType *d, TRef sp, TValue *sval)
{
if (LJ_UNLIKELY(tref_isstr(sp))) {
if (lj_strscan_num(strV(sval), sval)) {
sp = emitir(IRTG(IR_STRTO, IRT_NUM), sp, 0);
} /* else: interpreter will throw. */
}
return crec_ct_tv(J, d, 0, sp, sval);
}

void LJ_FASTCALL recff_bit64_tobit(jit_State *J, RecordFFData *rd)
{
CTState *cts = ctype_ctsG(J2G(J));
TRef tr = crec_ct_tv(J, ctype_get(cts, CTID_INT64), 0,
J->base[0], &rd->argv[0]);
TRef tr = crec_bit64_arg(J, ctype_get(cts, CTID_INT64),
J->base[0], &rd->argv[0]);
if (!tref_isinteger(tr))
tr = emitconv(tr, IRT_INT, tref_type(tr), 0);
J->base[0] = tr;
Expand All @@ -1797,7 +1808,7 @@ int LJ_FASTCALL recff_bit64_unary(jit_State *J, RecordFFData *rd)
CTState *cts = ctype_ctsG(J2G(J));
CTypeID id = crec_bit64_type(cts, &rd->argv[0]);
if (id) {
TRef tr = crec_ct_tv(J, ctype_get(cts, id), 0, J->base[0], &rd->argv[0]);
TRef tr = crec_bit64_arg(J, ctype_get(cts, id), J->base[0], &rd->argv[0]);
tr = emitir(IRT(rd->data, id-CTID_INT64+IRT_I64), tr, 0);
J->base[0] = emitir(IRTG(IR_CNEWI, IRT_CDATA), lj_ir_kint(J, id), tr);
return 1;
Expand All @@ -1817,9 +1828,9 @@ int LJ_FASTCALL recff_bit64_nary(jit_State *J, RecordFFData *rd)
if (id) {
CType *ct = ctype_get(cts, id);
uint32_t ot = IRT(rd->data, id-CTID_INT64+IRT_I64);
TRef tr = crec_ct_tv(J, ct, 0, J->base[0], &rd->argv[0]);
TRef tr = crec_bit64_arg(J, ct, J->base[0], &rd->argv[0]);
for (i = 1; J->base[i] != 0; i++) {
TRef tr2 = crec_ct_tv(J, ct, 0, J->base[i], &rd->argv[i]);
TRef tr2 = crec_bit64_arg(J, ct, J->base[i], &rd->argv[i]);
tr = emitir(ot, tr, tr2);
}
J->base[0] = emitir(IRTG(IR_CNEWI, IRT_CDATA), lj_ir_kint(J, id), tr);
Expand All @@ -1834,15 +1845,15 @@ int LJ_FASTCALL recff_bit64_shift(jit_State *J, RecordFFData *rd)
CTypeID id;
TRef tsh = 0;
if (J->base[0] && tref_iscdata(J->base[1])) {
tsh = crec_ct_tv(J, ctype_get(cts, CTID_INT64), 0,
J->base[1], &rd->argv[1]);
tsh = crec_bit64_arg(J, ctype_get(cts, CTID_INT64),
J->base[1], &rd->argv[1]);
if (!tref_isinteger(tsh))
tsh = emitconv(tsh, IRT_INT, tref_type(tsh), 0);
J->base[1] = tsh;
}
id = crec_bit64_type(cts, &rd->argv[0]);
if (id) {
TRef tr = crec_ct_tv(J, ctype_get(cts, id), 0, J->base[0], &rd->argv[0]);
TRef tr = crec_bit64_arg(J, ctype_get(cts, id), J->base[0], &rd->argv[0]);
uint32_t op = rd->data;
if (!tsh) tsh = lj_opt_narrow_tobit(J, J->base[1]);
if (!(op < IR_BROL ? LJ_TARGET_MASKSHIFT : LJ_TARGET_MASKROT) &&
Expand Down Expand Up @@ -1872,7 +1883,7 @@ TRef recff_bit64_tohex(jit_State *J, RecordFFData *rd, TRef hdr)
CTypeID id2 = 0;
n = (int32_t)lj_carith_check64(J->L, 2, &id2);
if (id2)
trsf = crec_ct_tv(J, ctype_get(cts, CTID_INT32), 0, trsf, &rd->argv[1]);
trsf = crec_bit64_arg(J, ctype_get(cts, CTID_INT32), trsf, &rd->argv[1]);
else
trsf = lj_opt_narrow_tobit(J, trsf);
emitir(IRTGI(IR_EQ), trsf, lj_ir_kint(J, n)); /* Specialize to n. */
Expand All @@ -1883,7 +1894,7 @@ TRef recff_bit64_tohex(jit_State *J, RecordFFData *rd, TRef hdr)
if ((uint32_t)n > 254) n = 254;
sf |= ((SFormat)((n+1)&255) << STRFMT_SH_PREC);
if (id) {
tr = crec_ct_tv(J, ctype_get(cts, id), 0, J->base[0], &rd->argv[0]);
tr = crec_bit64_arg(J, ctype_get(cts, id), J->base[0], &rd->argv[0]);
if (n < 16)
tr = emitir(IRT(IR_BAND, IRT_U64), tr,
lj_ir_kint64(J, ((uint64_t)1 << 4*n)-1));
Expand Down
30 changes: 30 additions & 0 deletions test/tarantool-tests/lj-1252-missing-bit64-coercion.test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
local tap = require('tap')

-- Test file to demonstrate LuaJIT incorrect recording of
-- `bit` library with needed coercion from string.
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1252.

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

test:plan(1)

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

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

-- Before the patch, with the missed coercion from string, there
-- is the cast to `i64` from `p64`, where the last one is the
-- string address. This leads to an incorrect result of the bit
-- operation.

local results = {}
for i = 1, 4 do
results[i] = bor('0', 0LL)
end

test:samevalues(results, 'correct recording of the bit operation with coercion')

test:done(true)

0 comments on commit 89e78a6

Please sign in to comment.