Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] Fix JIT slot overflow during up-recursion.
@ 2025-06-05  9:41 Sergey Kaplun via Tarantool-patches
  2025-06-06 10:44 ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-06-05  9:41 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit 048972dbfdb6b441fe8a9bfe4d1f048966579ba8)

In the case when LuaJIT is recording the side trace after the
up-recursion call, there is no check that the updated `maxslot` value
doesn't overflow the `LJ_MAX_JSLOTS` limit. If it records several huge
returns in a row, the overflow of the aforementioned limit may occur.
This triggers an assertion failure in `rec_check_slots()`.

This patch fixes it by adding the corresponding check in the
`lj_record_ret()`.

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

Part of tarantool/tarantool#11278
---
Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1358-jslot-overflow-uprecursion
Related issues:
* https://github.com/tarantool/tarantool/issues/11278
* https://github.com/LuaJIT/LuaJIT/issues/1358

 src/lj_record.c                               |  3 +-
 ...j-1358-jslot-overflow-uprecursion.test.lua | 82 +++++++++++++++++++
 2 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-1358-jslot-overflow-uprecursion.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index d83fa38f..1dd22dac 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -889,7 +889,8 @@ void lj_record_ret(jit_State *J, BCReg rbase, ptrdiff_t gotresults)
       lj_trace_err(J, LJ_TRERR_LLEAVE);
     } else if (J->needsnap) {  /* Tailcalled to ff with side-effects. */
       lj_trace_err(J, LJ_TRERR_NYIRETL);  /* No way to insert snapshot here. */
-    } else if (1 + pt->framesize >= LJ_MAX_JSLOTS) {
+    } else if (1 + pt->framesize >= LJ_MAX_JSLOTS ||
+	       J->baseslot + J->maxslot >= LJ_MAX_JSLOTS) {
       lj_trace_err(J, LJ_TRERR_STACKOV);
     } else {  /* Return to lower frame. Guard for the target we return to. */
       TRef trpt = lj_ir_kgc(J, obj2gco(pt), IRT_PROTO);
diff --git a/test/tarantool-tests/lj-1358-jslot-overflow-uprecursion.test.lua b/test/tarantool-tests/lj-1358-jslot-overflow-uprecursion.test.lua
new file mode 100644
index 00000000..1b151b56
--- /dev/null
+++ b/test/tarantool-tests/lj-1358-jslot-overflow-uprecursion.test.lua
@@ -0,0 +1,82 @@
+local tap = require('tap')
+
+-- The test file to demonstrate JIT slots overflow when compiling
+-- the return from the trace with up-recursion.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1358.
+
+local test = tap.test('lj-1358-jslot-overflow-uprecursion'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- The test generates the functions with the following workload:
+--
+-- | local uprec_func()
+-- |   if cond then return end
+-- |   return 'x', --[[...]] 'x', uprec_func()
+-- | end
+-- |
+-- | local function empty() end
+-- | empty('x', --[[...]] 'x', uprec_func())
+--
+-- The recording of the return from `uprec_func()` before the call
+-- to `empty()` causes the assertion failure in the
+-- `rec_check_slots()`.
+
+-- Generate a function with many return values plus up-recursion.
+local function generate_uprec_payload(n_returns)
+  local str_func = [[
+  local counter = 0
+  local function payload_f()
+    counter = counter + 1
+    if counter > 5 then return end
+    return
+  ]]
+  for _ = 1, n_returns do
+    str_func = str_func .. '"x", '
+  end
+  str_func = str_func .. [[
+    payload_f()
+  end
+  return payload_f
+  ]]
+  local f = assert(loadstring(str_func))
+  return f()
+end
+
+-- Generate the necessary number of locals for a huge enough
+-- `cbase`.
+local function generate_nloc_payload(n_locals)
+  local str_func = [[
+  -- Function to be called after return with all stack slots used.
+  local function empty() end
+  empty(
+  ]]
+  for _ = 1, n_locals do
+    str_func = str_func .. '"x", '
+  end
+  str_func = str_func .. [[
+    _G.uprec_func()
+  )
+  ]]
+  local f = assert(loadstring(str_func))
+  return f
+end
+
+-- Avoid an unrelated JIT output.
+jit.off()
+-- 30 * 5 = 150 returned values for the first call.
+_G.uprec_func = generate_uprec_payload(30)
+-- Plus 100 slots for locals, plus a slot for the function to be
+-- called causes JIT stack slots overflow.
+local test_func = generate_nloc_payload(100)
+
+jit.on()
+jit.opt.start('hotloop=1', 'hotexit=1', 'recunroll=1')
+
+test_func()
+
+test:ok(true, 'no assertion on JIT slots overflow')
+
+test:done(true)
-- 
2.49.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Fix JIT slot overflow during up-recursion.
  2025-06-05  9:41 [Tarantool-patches] [PATCH luajit] Fix JIT slot overflow during up-recursion Sergey Kaplun via Tarantool-patches
@ 2025-06-06 10:44 ` Sergey Bronnikov via Tarantool-patches
  2025-06-06 12:00   ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-06-06 10:44 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

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

Hello, Sergey,

thanks for the patch!

LGTM with minor comment below.

Sergey

On 6/5/25 12:41, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Sergey Kaplun.
>
> (cherry picked from commit 048972dbfdb6b441fe8a9bfe4d1f048966579ba8)
>
> In the case when LuaJIT is recording the side trace after the
> up-recursion call, there is no check that the updated `maxslot` value
> doesn't overflow the `LJ_MAX_JSLOTS` limit. If it records several huge
> returns in a row, the overflow of the aforementioned limit may occur.
> This triggers an assertion failure in `rec_check_slots()`.
>
> This patch fixes it by adding the corresponding check in the
> `lj_record_ret()`.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#11278
Please add a "Closes tarantool/security#145".
> ---
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1358-jslot-overflow-uprecursion
> Related issues:
> *https://github.com/tarantool/tarantool/issues/11278
> *https://github.com/LuaJIT/LuaJIT/issues/1358

Also https://github.com/tarantool/security/issues/145.


<snipped>


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Fix JIT slot overflow during up-recursion.
  2025-06-06 10:44 ` Sergey Bronnikov via Tarantool-patches
@ 2025-06-06 12:00   ` Sergey Kaplun via Tarantool-patches
  2025-06-06 12:18     ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-06-06 12:00 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hello, Sergey!
Thanks for the review!
Updated the commit message as you suggested and rebased branch on the
current tarantool/master.

On 06.06.25, Sergey Bronnikov wrote:
> Hello, Sergey,
> 
> thanks for the patch!
> 
> LGTM with minor comment below.
> 
> Sergey
> 
> On 6/5/25 12:41, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Reported by Sergey Kaplun.
> >
> > (cherry picked from commit 048972dbfdb6b441fe8a9bfe4d1f048966579ba8)
> >
> > In the case when LuaJIT is recording the side trace after the
> > up-recursion call, there is no check that the updated `maxslot` value
> > doesn't overflow the `LJ_MAX_JSLOTS` limit. If it records several huge
> > returns in a row, the overflow of the aforementioned limit may occur.
> > This triggers an assertion failure in `rec_check_slots()`.
> >
> > This patch fixes it by adding the corresponding check in the
> > `lj_record_ret()`.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> >
> > Part of tarantool/tarantool#11278
> Please add a "Closes tarantool/security#145".

Added:
| Resolves tarantool/security#145

> > ---
> > Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1358-jslot-overflow-uprecursion
> > Related issues:
> > *https://github.com/tarantool/tarantool/issues/11278
> > *https://github.com/LuaJIT/LuaJIT/issues/1358
> 
> Also https://github.com/tarantool/security/issues/145.
> 
> 
> <snipped>
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Fix JIT slot overflow during up-recursion.
  2025-06-06 12:00   ` Sergey Kaplun via Tarantool-patches
@ 2025-06-06 12:18     ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 4+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-06-06 12:18 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

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

Thanks!

On 6/6/25 15:00, Sergey Kaplun wrote:
> Hello, Sergey!
> Thanks for the review!
> Updated the commit message as you suggested and rebased branch on the
> current tarantool/master.
LGTM
>
<snipped>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-06-06 12:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-05  9:41 [Tarantool-patches] [PATCH luajit] Fix JIT slot overflow during up-recursion Sergey Kaplun via Tarantool-patches
2025-06-06 10:44 ` Sergey Bronnikov via Tarantool-patches
2025-06-06 12:00   ` Sergey Kaplun via Tarantool-patches
2025-06-06 12:18     ` Sergey Bronnikov 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