[Tarantool-patches] [PATCH v2 luajit] FFI: Add missing coercion when recording 64-bit bit.*().

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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