* [Tarantool-patches] [PATCH v2 luajit] FFI: Add missing coercion when recording 64-bit bit.*().
@ 2025-01-22 11:58 Sergey Kaplun via Tarantool-patches
2025-01-23 13:36 ` Sergey Bronnikov via Tarantool-patches
2025-01-31 9:31 ` Sergey Kaplun via Tarantool-patches
0 siblings, 2 replies; 3+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-01-22 11:58 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
From: Mike Pall <mike>
Thanks to Peter Cawley.
(cherry picked from commit 304da39cc5ee43491f7b1f4e0c9c52d477ce0d98)
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
---
Changes in the v2:
* Fixed several typos in the comments.
* Added the clarification why the commit adds the test only for the bor
(`recfh_bit64_nary()`) operation.
Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1252-missing-bit64-coercion
Related issues:
* https://github.com/tarantool/tarantool/issues/10709
* https://github.com/LuaJIT/LuaJIT/issues/1252
src/Makefile.dep.original | 2 +-
src/lj_crecord.c | 31 +++++++++++++------
.../lj-1252-missing-bit64-coercion.test.lua | 30 ++++++++++++++++++
3 files changed, 52 insertions(+), 11 deletions(-)
create mode 100644 test/tarantool-tests/lj-1252-missing-bit64-coercion.test.lua
diff --git a/src/Makefile.dep.original b/src/Makefile.dep.original
index 53426a7e..b96de1b9 100644
--- a/src/Makefile.dep.original
+++ b/src/Makefile.dep.original
@@ -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
diff --git a/src/lj_crecord.c b/src/lj_crecord.c
index 81ae6dfa..dcb90216 100644
--- a/src/lj_crecord.c
+++ b/src/lj_crecord.c
@@ -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)])
@@ -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;
@@ -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;
@@ -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);
@@ -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) &&
@@ -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. */
@@ -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));
diff --git a/test/tarantool-tests/lj-1252-missing-bit64-coercion.test.lua b/test/tarantool-tests/lj-1252-missing-bit64-coercion.test.lua
new file mode 100644
index 00000000..e193fa0c
--- /dev/null
+++ b/test/tarantool-tests/lj-1252-missing-bit64-coercion.test.lua
@@ -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)
--
2.47.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-31 9:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-22 11:58 [Tarantool-patches] [PATCH v2 luajit] FFI: Add missing coercion when recording 64-bit bit.*() Sergey Kaplun via Tarantool-patches
2025-01-23 13:36 ` Sergey Bronnikov via Tarantool-patches
2025-01-31 9:31 ` Sergey Kaplun via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox