* [Tarantool-patches] [PATCH luajit] Prevent loop in snap_usedef().
@ 2024-03-26 15:29 Sergey Bronnikov via Tarantool-patches
2024-03-26 16:04 ` Maxim Kokryashkin via Tarantool-patches
0 siblings, 1 reply; 2+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-03-26 15:29 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, Maxim Kokryashkin
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.
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
+ -- 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::
+
+test:ok(true, 'BC_UCLO does not trigger an infinite loop')
+os.exit(test:check() and 0 or 1)
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit] Prevent loop in snap_usedef().
2024-03-26 15:29 [Tarantool-patches] [PATCH luajit] Prevent loop in snap_usedef() Sergey Bronnikov via Tarantool-patches
@ 2024-03-26 16:04 ` Maxim Kokryashkin via Tarantool-patches
0 siblings, 0 replies; 2+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2024-03-26 16:04 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
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.
>
> 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.
> + -- 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.
> +
> +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
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-03-26 16:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-26 15:29 [Tarantool-patches] [PATCH luajit] Prevent loop in snap_usedef() Sergey Bronnikov via Tarantool-patches
2024-03-26 16:04 ` Maxim Kokryashkin via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox