Hi, Sergey, thanks for the patch! LGTM Sergey On 3/2/26 10:52, Sergey Kaplun wrote: > From: Mike Pall > > Thanks to Sergey Kaplun. > > (cherry picked from commit 707c12bf00dafdfd3899b1a6c36435dbbf6c7022) > > In a single-number build (default for x86/x64), `-(0)` is always `-0` > due to its number type. However, in DUALNUM mode, the `0` may have the > integer type, and for this integer, `BC_UNM` has no effect. `0` may be > represented via number or integer type, where the resulting `0` is > constructed during initial parsing (`lj_strscan_scan()`) or arithmetic > operations (`BC_MODVV`). This inconsistency may break the JIT semantics > and lead to the assertion failure in the `lj_check_slots()`. > > This patch fixes the incorrect `BC_UNM` for integer operand 0. Also, the > narrowing optimization for the unary minus should be more strict to > avoid incorrect narrowing for integer slots and make JIT intact with the > VM semantics. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#12134 > --- > src/lj_opt_narrow.c | 5 +- > src/vm_arm.dasc | 5 +- > src/vm_arm64.dasc | 2 + > src/vm_mips.dasc | 6 ++- > src/vm_mips64.dasc | 6 ++- > src/vm_ppc.dasc | 7 ++- > src/vm_x64.dasc | 4 ++ > src/vm_x86.dasc | 5 ++ > .../lj-1422-incorrect-unm-for-mzero.test.lua | 54 +++++++++++++++++++ > .../tarantool-tests/lj-1422-unm-zero.test.lua | 12 +++++ > 10 files changed, 99 insertions(+), 7 deletions(-) > create mode 100644 test/tarantool-tests/lj-1422-incorrect-unm-for-mzero.test.lua > create mode 100644 test/tarantool-tests/lj-1422-unm-zero.test.lua > > diff --git a/src/lj_opt_narrow.c b/src/lj_opt_narrow.c > index 6e3e9533..3bf7320d 100644 > --- a/src/lj_opt_narrow.c > +++ b/src/lj_opt_narrow.c > @@ -553,10 +553,9 @@ TRef lj_opt_narrow_unm(jit_State *J, TRef rc, TValue *vc) > rc = conv_str_tonum(J, rc, vc); > if (tref_isinteger(rc)) { > uint32_t k = (uint32_t)numberVint(vc); > - if ((tvisint(vc) || k != 0) && k != 0x80000000u) { > + if (k != 0 && k != 0x80000000u) { > TRef zero = lj_ir_kint(J, 0); > - if (!tvisint(vc)) > - emitir(IRTGI(IR_NE), rc, zero); > + emitir(IRTGI(IR_NE), rc, zero); > return emitir(IRTGI(IR_SUBOV), zero, rc); > } > rc = emitir(IRTN(IR_CONV), rc, IRCONV_NUM_INT); > diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc > index 7095e660..785d13ac 100644 > --- a/src/vm_arm.dasc > +++ b/src/vm_arm.dasc > @@ -2930,13 +2930,16 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > | bhi ->vmeta_unm > | eorne CARG2, CARG2, #0x80000000 > | bne >5 > - | rsbseq CARG1, CARG1, #0 > + | rsbs CARG1, CARG1, #0 > + | ldrdeq CARG12, >8 > | ldrdvs CARG12, >9 > |5: > | strd CARG12, [BASE, RA] > | ins_next3 > | > |.align 8 > + |8: > + | .long 0x00000000, 0x80000000 // -0. > |9: > | .long 0x00000000, 0x41e00000 // 2^31. > break; > diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc > index 6600e226..c64b736b 100644 > --- a/src/vm_arm64.dasc > +++ b/src/vm_arm64.dasc > @@ -2481,6 +2481,8 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > | movz CARG3, #0x41e0, lsl #48 // 2^31. > | add TMP0, TMP0, TISNUM > | csel TMP0, TMP0, CARG3, vc > + | movz CARG3, #0x8000, lsl #48 // -0. > + | csel TMP0, TMP0, CARG3, ne > |5: > | str TMP0, [BASE, RA, lsl #3] > | ins_next > diff --git a/src/vm_mips.dasc b/src/vm_mips.dasc > index 32caabf7..dcb04ed8 100644 > --- a/src/vm_mips.dasc > +++ b/src/vm_mips.dasc > @@ -3319,7 +3319,8 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > | addu RA, BASE, RA > | bne SFARG1HI, TISNUM, >2 > |. lw SFARG1LO, LO(RB) > - | lui TMP1, 0x8000 > + | beqz SFARG1LO, >3 > + |. lui TMP1, 0x8000 > | beq SFARG1LO, TMP1, ->vmeta_unm // Meta handler deals with -2^31. > |. negu SFARG1LO, SFARG1LO > |1: > @@ -3333,6 +3334,9 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > |. lui TMP1, 0x8000 > | b <1 > |. xor SFARG1HI, SFARG1HI, TMP1 > + |3: > + | b <1 > + |. lui SFARG1HI, 0x8000 // -0. > break; > case BC_LEN: > | // RA = dst*8, RD = src*8 > diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc > index 44fba36c..34da6473 100644 > --- a/src/vm_mips64.dasc > +++ b/src/vm_mips64.dasc > @@ -3563,7 +3563,8 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > | sextw CARG1, CARG1 > | beq CARG1, TMP1, ->vmeta_unm // Meta handler deals with -2^31. > |. negu CARG1, CARG1 > - | zextw CARG1, CARG1 > + | beqz CARG1, >3 > + |. zextw CARG1, CARG1 > | settp CARG1, TISNUM > |1: > | ins_next1 > @@ -3575,6 +3576,9 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > |. dsll TMP1, TMP1, 32 > | b <1 > |. xor CARG1, CARG1, TMP1 > + |3: > + | b <1 > + |. dsll CARG1, TMP1, 32 > break; > case BC_LEN: > | // RA = dst*8, RD = src*8 > diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc > index 980ad897..88fa7b67 100644 > --- a/src/vm_ppc.dasc > +++ b/src/vm_ppc.dasc > @@ -3803,11 +3803,13 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > | bne >5 > |.if GPR64 > | lus TMP2, 0x8000 > - | neg TMP0, TMP0 > + | neg. TMP0, TMP0 > + | beq >8 > | cmplw TMP0, TMP2 > | beq >4 > |.else > | nego. TMP0, TMP0 > + | beq >8 > | bso >4 > |1: > |.endif > @@ -3834,6 +3836,9 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > | stw TMP0, 4(RA) > |.if DUALNUM > | b <3 > + |8: > + | lus TMP1, 0x8000 // -0. > + | b <7 > |.else > | ins_next2 > |.endif > diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc > index 8b6781a6..ee669887 100644 > --- a/src/vm_x64.dasc > +++ b/src/vm_x64.dasc > @@ -3179,11 +3179,15 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > |.if DUALNUM > | checkint RB, >5 > | neg RBd > + | jz >3 > | jo >4 > | setint RB > |9: > | mov [BASE+RA*8], RB > | ins_next > + |3: > + | mov64 RB, U64x(80000000,00000000) // -0. > + | jmp <9 > |4: > | mov64 RB, U64x(41e00000,00000000) // 2^31. > | jmp <9 > diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc > index 7c11c78e..027bdcd6 100644 > --- a/src/vm_x86.dasc > +++ b/src/vm_x86.dasc > @@ -3780,11 +3780,16 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) > | checkint RD, >5 > | mov RB, [BASE+RD*8] > | neg RB > + | jz >3 > | jo >4 > | mov dword [BASE+RA*8+4], LJ_TISNUM > + |8: > | mov dword [BASE+RA*8], RB > |9: > | ins_next > + |3: > + | mov dword [BASE+RA*8+4], 0x80000000 // -0. > + | jmp <8 > |4: > | mov dword [BASE+RA*8+4], 0x41e00000 // 2^31. > | mov dword [BASE+RA*8], 0 > diff --git a/test/tarantool-tests/lj-1422-incorrect-unm-for-mzero.test.lua b/test/tarantool-tests/lj-1422-incorrect-unm-for-mzero.test.lua > new file mode 100644 > index 00000000..7046784c > --- /dev/null > +++ b/test/tarantool-tests/lj-1422-incorrect-unm-for-mzero.test.lua > @@ -0,0 +1,54 @@ > +local tap = require('tap') > + > +-- This test demonstrates LuaJIT's inconsistencies in the VM and > +-- the JIT engine in the DUALNUM mode for 0. > +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1422. > + > +local test = tap.test('lj-1422-incorrect-unm-for-mzero'):skipcond({ > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +test:plan(3) > + > +local tonumber = tonumber > +local tostring = tostring > + > +local function always_number(val) > + return tonumber(val) or 1 > +end > + > +-- Yielded int type for non-x86 arches. > +local function modvn(v1) > + return always_number(v1) % 1 > +end > + > +local function unm(v) > + return -v > +end > + > +jit.opt.start('hotloop=1', 'hotexit=1') > + > +always_number(nil) -- Root trace. > +always_number(nil) > + > +local stack_slot = nil > +for _ = 1, 5 do > + always_number(0) -- Compile side trace. > + -- The side trace crashes in the `rec_check_slots()` for non-x86 > + -- arches before the patch. > + stack_slot = tostring(unm(modvn(stack_slot))) > +end > + > +test:is(stack_slot, '-0', 'correct result of the trace execution') > + > +-- `tonumber()` recording and conversion to number. > +local results = {nil, nil, nil, nil} > +for i = 1, 4 do > + local slot = -tonumber('0') > + results[i] = tostring(slot) > +end > + > +test:is(results[1], '-0', 'correct result of the expression') > +test:samevalues(results, 'correct result of the tonumber recording') > + > +test:done(true) > diff --git a/test/tarantool-tests/lj-1422-unm-zero.test.lua b/test/tarantool-tests/lj-1422-unm-zero.test.lua > new file mode 100644 > index 00000000..77062418 > --- /dev/null > +++ b/test/tarantool-tests/lj-1422-unm-zero.test.lua > @@ -0,0 +1,12 @@ > +local tap = require('tap') > + > +-- The test file to demonstrate LuaJIT's incorrect BC_UNM for the > +-- 0 operand for the DUALNUM mode. > +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1422. > + > +local test = tap.test('lj-1422-unm-zero') > +test:plan(1) > + > +test:ok(tostring(-0) == '-0', 'correct unary minus for 0') > + > +test:done(true)