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 5E533464E99; Thu, 16 Jan 2025 19:18:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5E533464E99 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1737044309; bh=aPRXEexDcKX10eggXScFSdJvm/gxXyQPvj11nGthqUQ=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=q/w/dB2mE8ekOqLCGSuZaZKc0p0Bo352jpYE2AlBvMS0eiSpG4dBhJbvftOEi1t+b JTAJXQv71/Z4iZtQzIZGq+Y+3cx73rt1jOr482Damst3kEbc0YPW7hUK7ALD3bKdbP QlFWdNF73h5Mel39wS9DZPlXbr5BHxDo8i+6bi/g= Received: from send36.i.mail.ru (send36.i.mail.ru [89.221.237.131]) (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 1CDE4464E99 for ; Thu, 16 Jan 2025 19:18:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1CDE4464E99 Received: by exim-smtp-6758d5575c-cjkqz with esmtpa (envelope-from ) id 1tYSZe-00000000AS9-3hz7; Thu, 16 Jan 2025 19:18:27 +0300 Content-Type: multipart/alternative; boundary="------------wiLfJI0kTCC50FLYh7PFVGVr" Message-ID: <8eec6092-5844-4264-9912-bcc1a4c6f74a@tarantool.org> Date: Thu, 16 Jan 2025 19:18:25 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Maxim Kokryashkin , Sergey Bronnikov References: <3bd73ab3f3a0e8b200c493ec09e65f5ecb711a6b.1711466825.git.sergeyb@tarantool.org> <3nc2evmifho2awf6yzddx3uh7cskiwfafvtje46jnzhi47nwvv@x43uhh7wkbtx> Content-Language: en-US In-Reply-To: <3nc2evmifho2awf6yzddx3uh7cskiwfafvtje46jnzhi47nwvv@x43uhh7wkbtx> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9CAF828D4DCE9EB9516BB6BEA42E0431EF98641455DECBCC2182A05F53808504008A3332A13707E653DE06ABAFEAF6705B3EA8DFF7811F59AB9DF18C2B4F1053CC551C5DF466AD644 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE706EA9E10470DC775EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063771C846A5973DEE7E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8C405C2B7DB4C56B63E148CC555CA3870CB1A42D024E27AFBCC7F00164DA146DAFE8445B8C89999728AA50765F7900637F6B57BC7E64490618DEB871D839B7333395957E7521B51C2DFABB839C843B9C08941B15DA834481F8AA50765F7900637CAEE156C82D3D7D9389733CBF5DBD5E9B5C8C57E37DE458BD9DD9810294C998ED8FC6C240DEA76428AA50765F79006374B64FF5809E74304D81D268191BDAD3DBD4B6F7A4D31EC0BE2F48590F00D11D6D81D268191BDAD3D78DA827A17800CE7961EB98B2F29375EEC76A7562686271ED91E3A1F190DE8FD2E808ACE2090B5E14AD6D5ED66289B5259CC434672EE63711DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C30584FF81F342DA0735872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A54D09A6A031CBE70C5002B1117B3ED696656E7C15A6CA8F22C66B2B37046EC955823CB91A9FED034534781492E4B8EEAD47A3109F1ACFD409BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34F1257DC9690AEBA1FA581B7B2A5877F80001F3FEE2B7DE8BB3BAB14C4F86580A8DF82D47A848BFA91D7E09C32AA3244CAEDC70EA80B74A7977DD89D51EBB7742F15C5BA465449892EA455F16B58544A21C197AAF4D2E4732A5AE236DF995FB59978A700BF655EAEEED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyistvkELW9qXAkYTql7njQ== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140C7F571A18B7AAFD7C591814E25D11F9F7B588529A3B82E7C0152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Prevent loop in snap_usedef(). 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 Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------wiLfJI0kTCC50FLYh7PFVGVr Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, Maxim, thanks for review! Please see my comments below. Fixes applied and force-pushed. On 26.03.2024 19:04, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the patch! > Please consider my comments below. > On Tue, Mar 26, 2024 at 06:29:11PM +0300, Sergey Bronnikov wrote: >> From: Sergey Bronnikov >> >> Reported by XmiliaH. >> >> (cherry picked from commit 0e66fc96377853d898390f1a02723c54ec3a42f7) >> >> It is possible to get an infinite loop in a function `snap_usedef` >> when a `UCLO` makes a tight loop. > The description should include explanation for the cause of the issue > and should explain how it was resolved. Added more details about patch to commit message: Updated description:     It is possible to get an infinite loop in a function `snap_usedef`     when a `UCLO` makes a tight loop. This infinite loop could happen     when `snap_usedef()` is called on trace exit processes UCLO     bytecode instruction and this instruction attempts a jump with     negative value. The patch fixes the problem by checking a number     of slots in a jump argument and replace this value my `maxslot` if     a value is negative. >> Sergey Bronnikov: >> * added the description and the test for the problem >> >> Part of tarantool/tarantool#9595 >> --- >> Branch:https://github.com/tarantool/luajit/tree/ligurio/lj-736-prevent-loop-in-snap_usedef >> Issues: >> -https://github.com/LuaJIT/LuaJIT/issues/736 >> -https://github.com/tarantool/tarantool/issues/9595 >> >> src/lj_snap.c | 7 ++- >> .../lj-736-BC_UCLO-triggers-infinite-loop.lua | 59 +++++++++++++++++++ >> 2 files changed, 65 insertions(+), 1 deletion(-) >> create mode 100644 test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua >> >> diff --git a/src/lj_snap.c b/src/lj_snap.c >> index 5a00b5cd..0710e1f0 100644 >> --- a/src/lj_snap.c >> +++ b/src/lj_snap.c >> @@ -252,7 +252,12 @@ static BCReg snap_usedef(jit_State *J, uint8_t *udf, >> BCReg minslot = bc_a(ins); >> if (op >= BC_FORI && op <= BC_JFORL) minslot += FORL_EXT; >> else if (op >= BC_ITERL && op <= BC_JITERL) minslot += bc_b(pc[-2])-1; >> - else if (op == BC_UCLO) { pc += bc_j(ins); break; } >> + else if (op == BC_UCLO) { >> + ptrdiff_t delta = bc_j(ins); >> + if (delta < 0) return maxslot; /* Prevent loop. */ >> + pc += delta; >> + break; >> + } >> for (s = minslot; s < maxslot; s++) DEF_SLOT(s); >> return minslot < maxslot ? minslot : maxslot; >> } >> diff --git a/test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua b/test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua >> new file mode 100644 >> index 00000000..28a2b61b >> --- /dev/null >> +++ b/test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua >> @@ -0,0 +1,59 @@ >> +local tap = require('tap') >> +local test = tap.test('lj-736-BC_UCLO-triggers-infinite-loop'):skipcond({ >> + ['Test requires JIT enabled'] = not jit.status(), >> +}) >> + >> +test:plan(1) >> + >> +-- Test reproduces an issue when BC_UCLO triggers an infinite loop. >> +-- See details inhttps://github.com/LuaJIT/LuaJIT/issues/736. >> +-- >> +-- Listing below demonstrates a problem - >> +-- the bytecode UCLO on the line 13 makes a loop at 0013-0014: >> +-- >> +-- - BYTECODE -- bc_uclo.lua:0-20 >> +-- 0001 KPRI 0 0 >> +-- 0002 FNEW 1 0 ; bc_uclo.lua:5 >> +-- 0003 KSHORT 2 1 >> +-- 0004 KSHORT 3 4 >> +-- 0005 KSHORT 4 1 >> +-- 0006 FORI 2 => 0011 >> +-- 0007 => ISNEN 5 0 ; 2 >> +-- 0008 JMP 6 => 0010 >> +-- 0009 UCLO 0 => 0012 >> +-- 0010 => FORL 2 => 0007 >> +-- 0011 => UCLO 0 => 0012 >> +-- 0012 => KPRI 0 0 >> +-- 0013 UCLO 0 => 0012 >> +-- 0014 FNEW 1 1 ; bc_uclo.lua:18 >> +-- 0015 UCLO 0 => 0016 >> +-- 0016 => RET0 0 1 >> + >> +jit.opt.start('hotloop=1') >> + >> +do >> + local uv = 0 >> + local w = function() return uv end -- luacheck: no unused >> + for i = 1, 2 do > Add a comment that we have two iterations only because we only > need to record the trace. Added. >> + -- Infinite loop is here. >> + if i == 2 then >> + if i == 2 then >> + goto pass >> + end >> + goto unreachable >> + end >> + end >> +end >> + >> +::unreachable:: >> +-- Lua chunk below is required for reproducing a bug. >> +do >> + local uv = 0 -- luacheck: no unused >> + goto unreachable >> + local w = function() return uv end -- luacheck: ignore >> +end >> + >> +::pass:: > Please add a comment explaining why do we need a goto statement > and an unreachable code segment. Added more comments to the test. >> + >> +test:ok(true, 'BC_UCLO does not trigger an infinite loop') >> +os.exit(test:check() and 0 or 1) > The test executes without any failures for the x86 non-GC64 > build before the patch. The exact command: > $ cmake .. -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON && make -j && make test Fixed that. Now without patch test hangs and passed with applied patch. >> -- >> 2.34.1 >> --------------wiLfJI0kTCC50FLYh7PFVGVr Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi, Maxim,

thanks for review! Please see my comments below.

Fixes applied and force-pushed.

On 26.03.2024 19:04, Maxim Kokryashkin wrote:
Hi, Sergey!
Thanks for the patch!
Please consider my comments below.
On Tue, Mar 26, 2024 at 06:29:11PM +0300, Sergey Bronnikov wrote:

From: Sergey Bronnikov <sergeyb@tarantool.org>

Reported by XmiliaH.

(cherry picked from commit 0e66fc96377853d898390f1a02723c54ec3a42f7)

It is possible to get an infinite loop in a function `snap_usedef`
when a `UCLO` makes a tight loop.

The description should include explanation for the cause of the issue
and should explain how it was resolved.

Added more details about patch to commit message:

Updated description:

    It is possible to get an infinite loop in a function `snap_usedef`
    when a `UCLO` makes a tight loop. This infinite loop could happen
    when `snap_usedef()` is called on trace exit processes UCLO
    bytecode instruction and this instruction attempts a jump with
    negative value. The patch fixes the problem by checking a number
    of slots in a jump argument and replace this value my `maxslot` if
    a value is negative.


      

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

Part of tarantool/tarantool#9595
---
Branch: https://github.com/tarantool/luajit/tree/ligurio/lj-736-prevent-loop-in-snap_usedef
Issues:
- https://github.com/LuaJIT/LuaJIT/issues/736
- https://github.com/tarantool/tarantool/issues/9595

 src/lj_snap.c                                 |  7 ++-
 .../lj-736-BC_UCLO-triggers-infinite-loop.lua | 59 +++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua

diff --git a/src/lj_snap.c b/src/lj_snap.c
index 5a00b5cd..0710e1f0 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -252,7 +252,12 @@ static BCReg snap_usedef(jit_State *J, uint8_t *udf,
       BCReg minslot = bc_a(ins);
       if (op >= BC_FORI && op <= BC_JFORL) minslot += FORL_EXT;
       else if (op >= BC_ITERL && op <= BC_JITERL) minslot += bc_b(pc[-2])-1;
-      else if (op == BC_UCLO) { pc += bc_j(ins); break; }
+      else if (op == BC_UCLO) {
+	ptrdiff_t delta = bc_j(ins);
+	if (delta < 0) return maxslot;  /* Prevent loop. */
+	pc += delta;
+	break;
+      }
       for (s = minslot; s < maxslot; s++) DEF_SLOT(s);
       return minslot < maxslot ? minslot : maxslot;
       }
diff --git a/test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua b/test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua
new file mode 100644
index 00000000..28a2b61b
--- /dev/null
+++ b/test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua
@@ -0,0 +1,59 @@
+local tap = require('tap')
+local test = tap.test('lj-736-BC_UCLO-triggers-infinite-loop'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- Test reproduces an issue when BC_UCLO triggers an infinite loop.
+-- See details in https://github.com/LuaJIT/LuaJIT/issues/736.
+--
+-- Listing below demonstrates a problem -
+-- the bytecode UCLO on the line 13 makes a loop at 0013-0014:
+--
+-- - BYTECODE -- bc_uclo.lua:0-20
+-- 0001    KPRI     0   0
+-- 0002    FNEW     1   0      ; bc_uclo.lua:5
+-- 0003    KSHORT   2   1
+-- 0004    KSHORT   3   4
+-- 0005    KSHORT   4   1
+-- 0006    FORI     2 => 0011
+-- 0007 => ISNEN    5   0      ; 2
+-- 0008    JMP      6 => 0010
+-- 0009    UCLO     0 => 0012
+-- 0010 => FORL     2 => 0007
+-- 0011 => UCLO     0 => 0012
+-- 0012 => KPRI     0   0
+-- 0013    UCLO     0 => 0012
+-- 0014    FNEW     1   1      ; bc_uclo.lua:18
+-- 0015    UCLO     0 => 0016
+-- 0016 => RET0     0   1
+
+jit.opt.start('hotloop=1')
+
+do
+  local uv = 0
+  local w = function() return uv end -- luacheck: no unused
+  for i = 1, 2 do


      

      
Add a comment that we have two iterations only because we only
need to record the trace.

    

    Added.

    

      

      

        
+    -- Infinite loop is here.
+    if i == 2 then
+      if i == 2 then
+        goto pass
+      end
+      goto unreachable
+    end
+  end
+end
+
+::unreachable::
+-- Lua chunk below is required for reproducing a bug.
+do
+  local uv = 0 -- luacheck: no unused
+  goto unreachable
+  local w = function() return uv end -- luacheck: ignore
+end
+
+::pass::


      

      
Please add a comment explaining why do we need a goto statement
and an unreachable code segment.


    

    Added more comments to the test.

    

      

        
+
+test:ok(true, 'BC_UCLO does not trigger an infinite loop')
+os.exit(test:check() and 0 or 1)


      

      
The test executes without any failures for the x86 non-GC64
build before the patch. The exact command:
$ cmake .. -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON && make -j && make test


    

    


    

    
Fixed that. Now without patch test hangs and passed with applied
      patch.

    

    

      

      

        
--
2.34.1



      

    

  
  


--------------wiLfJI0kTCC50FLYh7PFVGVr--