[Tarantool-patches] [PATCH luajit] Fix bit op coercion for shifts in DUALNUM builds.

[Tarantool-patches] [PATCH luajit] Fix bit op coercion for shifts in DUALNUM builds.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Junlong Li.

(cherry picked from commit 69bbf3c1b01de8239444b0c430a89fa868978fea)

This is a follow-up to the commit
8cd79d198df4b0e14882a663a1673e1308f09899 ("Fix bit op coercion in
DUALNUM builds."). After removing the coercion from
`lj_carith_check64()`, the bit shift operation may end in an infinite
loop in the case of infinite retrying to coerce the second operand from
number to integer TValue type.

This patch fixes that by unconditionally coercing the second argument in
the `LJLIB_ASM(bit_lshift)` fast function handler.

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

Part of tarantool/tarantool#11055
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/fix-bit-shift-dualnum

Note: CI is red due to problems with the integration testing.
See also: https://github.com/tarantool/tarantool/pull/11220

Related issue: https://github.com/tarantool/tarantool/issues/11055
ML: https://www.freelists.org/post/luajit/dead-loop-in-bitrshift.

How to build locally for reproducing:
| cmake -DLUAJIT_NUMMODE=2 -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DCMAKE_BUILD_TYPE=Debug . && make -j
And run the test like the following:
| ctest --timeout 1 -R fix-bit-shift-dualnum

 src/lib_bit.c                                 |  2 +-
 .../fix-bit-shift-dualnum.test.lua            | 27 +++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/fix-bit-shift-dualnum.test.lua

diff --git a/src/lib_bit.c b/src/lib_bit.c
index 6dbaf351..9ac5e645 100644
--- a/src/lib_bit.c
+++ b/src/lib_bit.c
@@ -98,7 +98,7 @@ LJLIB_ASM(bit_lshift)		LJLIB_REC(bit_shift IR_BSHL)
     x = lj_carith_shift64(x, sh, curr_func(L)->c.ffid - (int)FF_bit_lshift);
     return bit_result64(L, id, x);
   }
-  if (id2) setintV(L->base+1, sh);
+  setintV(L->base+1, sh);
   return FFH_RETRY;
 #else
   lj_lib_checknumber(L, 1);
diff --git a/test/tarantool-tests/fix-bit-shift-dualnum.test.lua b/test/tarantool-tests/fix-bit-shift-dualnum.test.lua
new file mode 100644
index 00000000..474a365f
--- /dev/null
+++ b/test/tarantool-tests/fix-bit-shift-dualnum.test.lua
@@ -0,0 +1,27 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT misbehaviour for bitshift
+-- operations in DUALNUM mode.
+-- See also:
+-- https://www.freelists.org/post/luajit/dead-loop-in-bitrshift.
+
+local test = tap.test('fix-bit-shift-dualnum')
+test:plan(5)
+
+-- This produces the number (not integer) `TValue` type for the
+-- DUALNUM build. If the second parameter of any of the shift
+-- functions is not an integer in the DUALNUM build, LuaJIT tries
+-- to convert it to an integer. In the case of a number, it does
+-- nothing and endlessly retries the call to the fallback
+-- function.
+local SHIFT_V = 1 - '0'
+
+-- Any of the shift calls below causes the infinite FFH retrying
+-- loop before the patch.
+test:ok(bit.arshift(0, SHIFT_V), 0, 'no infifnite loop in bit.arshift')
+test:ok(bit.lshift(0, SHIFT_V), 0, 'no infifnite loop in bit.lshift')
+test:ok(bit.rshift(0, SHIFT_V), 0, 'no infifnite loop in bit.rshift')
+test:ok(bit.rol(0, SHIFT_V), 0, 'no infifnite loop in bit.rol')
+test:ok(bit.ror(0, SHIFT_V), 0, 'no infifnite loop in bit.ror')
+
+test:done(true)
-- 
2.48.1

Re: [Tarantool-patches] [PATCH luajit] Fix bit op coercion for shifts in DUALNUM builds.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 3506 bytes --]

Hi, Sergey,

thanks for the patch! LGTM

On 12.03.2025 18:36, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Junlong Li.
>
> (cherry picked from commit 69bbf3c1b01de8239444b0c430a89fa868978fea)
>
> This is a follow-up to the commit
> 8cd79d198df4b0e14882a663a1673e1308f09899 ("Fix bit op coercion in
> DUALNUM builds."). After removing the coercion from
> `lj_carith_check64()`, the bit shift operation may end in an infinite
> loop in the case of infinite retrying to coerce the second operand from
> number to integer TValue type.
>
> This patch fixes that by unconditionally coercing the second argument in
> the `LJLIB_ASM(bit_lshift)` fast function handler.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#11055
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/fix-bit-shift-dualnum
>
> Note: CI is red due to problems with the integration testing.
> See also:https://github.com/tarantool/tarantool/pull/11220
>
> Related issue:https://github.com/tarantool/tarantool/issues/11055
> ML:https://www.freelists.org/post/luajit/dead-loop-in-bitrshift.
>
> How to build locally for reproducing:
> | cmake -DLUAJIT_NUMMODE=2 -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DCMAKE_BUILD_TYPE=Debug . && make -j
> And run the test like the following:
> | ctest --timeout 1 -R fix-bit-shift-dualnum
>
>   src/lib_bit.c                                 |  2 +-
>   .../fix-bit-shift-dualnum.test.lua            | 27 +++++++++++++++++++
>   2 files changed, 28 insertions(+), 1 deletion(-)
>   create mode 100644 test/tarantool-tests/fix-bit-shift-dualnum.test.lua
>
> diff --git a/src/lib_bit.c b/src/lib_bit.c
> index 6dbaf351..9ac5e645 100644
> --- a/src/lib_bit.c
> +++ b/src/lib_bit.c
> @@ -98,7 +98,7 @@ LJLIB_ASM(bit_lshift)		LJLIB_REC(bit_shift IR_BSHL)
>       x = lj_carith_shift64(x, sh, curr_func(L)->c.ffid - (int)FF_bit_lshift);
>       return bit_result64(L, id, x);
>     }
> -  if (id2) setintV(L->base+1, sh);
> +  setintV(L->base+1, sh);
>     return FFH_RETRY;
>   #else
>     lj_lib_checknumber(L, 1);
> diff --git a/test/tarantool-tests/fix-bit-shift-dualnum.test.lua b/test/tarantool-tests/fix-bit-shift-dualnum.test.lua
> new file mode 100644
> index 00000000..474a365f
> --- /dev/null
> +++ b/test/tarantool-tests/fix-bit-shift-dualnum.test.lua
> @@ -0,0 +1,27 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT misbehaviour for bitshift
> +-- operations in DUALNUM mode.
> +-- See also:
> +--https://www.freelists.org/post/luajit/dead-loop-in-bitrshift.
> +
> +local test = tap.test('fix-bit-shift-dualnum')
> +test:plan(5)
> +
> +-- This produces the number (not integer) `TValue` type for the
> +-- DUALNUM build. If the second parameter of any of the shift
> +-- functions is not an integer in the DUALNUM build, LuaJIT tries
> +-- to convert it to an integer. In the case of a number, it does
> +-- nothing and endlessly retries the call to the fallback
> +-- function.
> +local SHIFT_V = 1 - '0'
> +
> +-- Any of the shift calls below causes the infinite FFH retrying
> +-- loop before the patch.
> +test:ok(bit.arshift(0, SHIFT_V), 0, 'no infifnite loop in bit.arshift')
> +test:ok(bit.lshift(0, SHIFT_V), 0, 'no infifnite loop in bit.lshift')
> +test:ok(bit.rshift(0, SHIFT_V), 0, 'no infifnite loop in bit.rshift')
> +test:ok(bit.rol(0, SHIFT_V), 0, 'no infifnite loop in bit.rol')
> +test:ok(bit.ror(0, SHIFT_V), 0, 'no infifnite loop in bit.ror')
> +
> +test:done(true)

[-- Attachment #2: Type: text/html, Size: 4347 bytes --]

Re: [Tarantool-patches] [PATCH luajit] Fix bit op coercion for shifts in DUALNUM builds.

[Sergey Kaplun via Tarantool-patches]

I've applied the patch into all long-term branches in tarantool/luajit
and bumped a new version in master [1], release/3.3 [2], release/3.2 [3]
and release/2.11 [4].

[1]: https://github.com/tarantool/tarantool/pull/11281
[2]: https://github.com/tarantool/tarantool/pull/11282
[3]: https://github.com/tarantool/tarantool/pull/11283
[4]: https://github.com/tarantool/tarantool/pull/11284

-- 
Best regards,
Sergey Kaplun