Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit][v2] Prevent loop in snap_usedef().
@ 2025-01-16 16:36 Sergey Bronnikov via Tarantool-patches
  2025-01-21 12:27 ` Sergey Kaplun via Tarantool-patches
  2025-01-31  9:29 ` Sergey Kaplun via Tarantool-patches
  0 siblings, 2 replies; 5+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-01-16 16:36 UTC (permalink / raw)
  To: tarantool-patches, Sergey Kaplun, Maxim Kokryashkin

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. 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#10709
---

Branch: https://github.com/tarantool/luajit/tree/ligurio/lj-736-prevent-loop-in-snap_usedef

Related issues:
* https://github.com/tarantool/tarantool/issues/10709
* https://github.com/LuaJIT/LuaJIT/issues/736

v2 changes:

- Updated test, now it hangs without patch with fix.
- Added more comments to the test with explanations.

 src/lj_snap.c                                 |  7 +-
 .../lj-736-BC_UCLO-triggers-infinite-loop.lua | 82 +++++++++++++++++++
 2 files changed, 88 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 8a33dc22..8d7bd868 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..fb053e9a
--- /dev/null
+++ b/test/tarantool-tests/lj-736-BC_UCLO-triggers-infinite-loop.lua
@@ -0,0 +1,82 @@
+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(2)
+
+-- 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')
+
+local assert_msg = 'Infinite loop is not reproduced.'
+local assert = assert
+
+local function testcase()
+  -- The code in the first scope `do`/`end` is a prerequisite.
+  -- It is needed so that we have a trace at the exit from which
+  -- the creation of the snapshot will begin.
+  do
+    -- Upvalue below is not used actually, but it is required
+    -- for calling `snap_usedef()` on trace exit.
+    local uv1 -- luacheck: ignore
+    local _ = function() return uv1 end
+
+    -- The loop below is required for recording a trace.
+    -- The condition inside a loop executes `goto` to a label
+    -- outside of the loop when the code executed by JIT and
+    -- this triggers snapshotting.
+    for i = 1, 2 do
+      -- Exit to interpreter once trace is compiled.
+      if i == 2 then
+        goto x
+      end
+    end
+  end
+
+::x::
+  do
+    local uv2 -- luacheck: no unused
+
+    -- `goto` if not executed without a patch and generates an
+    -- UCLO bytecode that makes an infinite loop in a function
+    -- `snap_usedef` when patch is not applied. `goto` must point
+    -- to the label on one of the previous lines. `assert()` is
+    -- executed when patch is applied.
+    assert(nil, assert_msg)
+    goto x
+
+    -- Line below is required, it makes `uv` upvalue, and must be
+    -- placed after `goto`, otherwise reproducer become broken.
+    local _ = function() return uv2 end -- luacheck: ignore
+  end
+end
+
+local ok, err = pcall(testcase)
+
+test:is(ok, false, 'assertion is triggered in a function with testcase')
+test:ok(err:match(assert_msg), 'BC_UCLO does not trigger an infinite loop')
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-01-31  9:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-16 16:36 [Tarantool-patches] [PATCH luajit][v2] Prevent loop in snap_usedef() Sergey Bronnikov via Tarantool-patches
2025-01-21 12:27 ` Sergey Kaplun via Tarantool-patches
2025-01-21 14:43   ` Sergey Bronnikov via Tarantool-patches
2025-01-21 15:01     ` Sergey Kaplun via Tarantool-patches
2025-01-31  9:29 ` Sergey Kaplun 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