From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 559AD12A9401; Fri, 14 Mar 2025 15:38:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 559AD12A9401 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1741955939; bh=C5uMNUYZRQnN3akDQx9gETtlwt8j2z8xOyKwxaE+Vgw=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=pWEs0pFt66P9/tT9mNGXkTMyujV1hFzSBYqjK4vVIfy5qXEjldW4d90YWuslPf6LM iekCj4Mr+yAUB0IOANiz3R/IcVItSiPnYylYBunJrD08NbEfzETX4rrz469NcG9UJK GNONgvLGbI3o9qZkg/cgZ14kro0FuErV7jdkC8lo= Received: from send173.i.mail.ru (send173.i.mail.ru [95.163.59.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3FEC812A9401 for ; Fri, 14 Mar 2025 15:38:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3FEC812A9401 Received: by exim-smtp-69cc44787d-np9gk with esmtpa (envelope-from ) id 1tt4JV-00000000DnN-0YPN; Fri, 14 Mar 2025 15:38:57 +0300 Content-Type: multipart/alternative; boundary="------------09HFr60HaHMEjGGzlhL2vGNR" Message-ID: <57d54d47-4931-405c-bce1-207ec6d81634@tarantool.org> Date: Fri, 14 Mar 2025 15:38:56 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <20250312153616.13143-1-skaplun@tarantool.org> In-Reply-To: <20250312153616.13143-1-skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9CB00722163A93FBB8B3405974EA1BF38D198EB2EA5703649CD62213F67905E7A51934A9463BC71B4F56B0A8670164AF4C7623CA58FEA907DDC62A744CEC8F8E251CE0648C9E8651D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7ACA11F7F2395C8CCEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375665A3E9B1844DE94599BDCECF00259277E4DD49ACFBB9B435BC4F3E7FD1A995907389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0D9442B0B5983000E8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B652D31B9D28593E51CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C224958C1606C78F2434E76E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B6A4E49BB0F3BA1413AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E7356D8C47C27EEC5E9FB5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A5173BAEF873BF38BE5002B1117B3ED6965EB3F8ADEA23EC5C69995D676B7B4CBE823CB91A9FED034534781492E4B8EEAD47A3109F1ACFD409BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3467D08F30473A584248EACC0FADA14E3B28E1401411A04E4059DFD365D4C53D6F283D3A72F2D611ED1D7E09C32AA3244C12D484EAE41BC44C77DD89D51EBB774280923C651901267BEA455F16B58544A2E30DDF7C44BCB90DA5AE236DF995FB59978A700BF655EAEEED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVQiWK+2I7Y2s6MNfS6jl0xo= X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140F5DBDE5A5560B3D6AC8EDD30083ED68ECDEF77C2A0B2E68F0152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Fix bit op coercion for shifts in DUALNUM builds. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------09HFr60HaHMEjGGzlhL2vGNR Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey, thanks for the patch! LGTM On 12.03.2025 18:36, Sergey Kaplun wrote: > From: Mike Pall > > 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) --------------09HFr60HaHMEjGGzlhL2vGNR Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

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)
--------------09HFr60HaHMEjGGzlhL2vGNR--