From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>,
Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH luajit] Add missing LJ_MAX_JSLOTS check.
Date: Fri, 22 Oct 2021 14:46:53 +0300 [thread overview]
Message-ID: <20211022114653.4225-1-skaplun@tarantool.org> (raw)
From: Mike Pall <mike>
Thanks to Yichun Zhang.
(cherry picked from commit 630ff3196a06353c6a7ccd1e9ac3958f4a8ca13c)
Before the patch, JIT compiler doesn't check slots overflow for
recording of metamethods call. So the assertion in `rec_check_slots()`
checking that we don't overflow the slots limit (the limit
`LJ_MAX_JSLOTS` is 250) is failing, when we record metamethod call
(`J->baseslot` diff + `J->maxslot` ~ 5-8 stack slots), while almost all
slots of JIT engine are occupied.
This patch adds the corresponding check in `lj_record_call()`.
Sergey Kaplun:
* added the description and the test for the problem
---
Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-slot-check-for-mm-record
Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-slot-check-for-mm-record
src/lj_record.c | 2 +
.../fix-slot-check-for-mm-record.test.lua | 81 +++++++++++++++++++
2 files changed, 83 insertions(+)
create mode 100644 test/tarantool-tests/fix-slot-check-for-mm-record.test.lua
diff --git a/src/lj_record.c b/src/lj_record.c
index 42af09e5..adf2370e 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -731,6 +731,8 @@ void lj_record_call(jit_State *J, BCReg func, ptrdiff_t nargs)
J->framedepth++;
J->base += func+1+LJ_FR2;
J->baseslot += func+1+LJ_FR2;
+ if (J->baseslot + J->maxslot >= LJ_MAX_JSLOTS)
+ lj_trace_err(J, LJ_TRERR_STACKOV);
}
/* Record tail call. */
diff --git a/test/tarantool-tests/fix-slot-check-for-mm-record.test.lua b/test/tarantool-tests/fix-slot-check-for-mm-record.test.lua
new file mode 100644
index 00000000..e361830d
--- /dev/null
+++ b/test/tarantool-tests/fix-slot-check-for-mm-record.test.lua
@@ -0,0 +1,81 @@
+local tap = require('tap')
+
+local test = tap.test('fix-slot-check-for-mm-record')
+test:plan(2)
+
+-- Before the patch, JIT compiler doesn't check slots overflow
+-- for recording of metamethods call. So the assertion checking
+-- that we don't overflow the slots limit (the limit
+-- `LJ_MAX_JSLOTS` is 250) is failing, when we record metamethod
+-- call (`J->baseslot` diff + `J->maxslot` ~ 5-8 stack slots),
+-- while almost all slots of JIT engine are occupied.
+
+-- Table with the simplest metamethod to call.
+local a0 = setmetatable({}, {
+ __add = function(t, arg1)
+ t[arg1] = arg1
+ end
+})
+_G.a0 = a0
+
+-- Fixarg function with call to metamethod.
+local function a1()
+ -- This constant is not setted as an upvalue to simplify stack
+ -- slots counting. Just remember that it is 42.
+ return a0 + 42
+end
+_G.a1 = a1
+
+-- Generate bunch of functions to call them recursively.
+-- Each function is a vararg function bumps slots on
+-- 2 (4) = 1 (2) * 2 for usual Lua frame and vararg frame
+-- recording for GC32 (GC64).
+for i = 2, 121 do
+ local f, err = load(('local r = a%d() return r'):format(i - 1))
+ assert(f, err)
+ _G['a'..i] = f
+end
+
+-- Trace is long enough, so we need to increase maxrecord.
+jit.opt.start('hotloop=1', 'maxrecord=2048')
+
+local function test_gc32()
+ -- 1 - Base slot.
+ -- 3 slots for cycle start, stop, step.
+ for _ = 1, 4 do
+ -- Occupy 1 slot for the function itself + 2 next slots will
+ -- occupied for a call to the vararg function.
+ -- Need 121 calls: 7 (baseslot after `a121()` is recorded)
+ -- + 119 * 2 + 1 (`a1` -- is not vararg function) = 246 slots.
+ -- The next call of metamethod in `a0` to record have 2 args
+ -- + 2 slots for metamethod function + 1 slot for frame.
+ -- luacheck: no global
+ a121()
+ assert(a0[42] == 42)
+ a0[42] = nil
+ end
+ return true
+end
+
+local function test_gc64()
+ -- 2 - Base slot.
+ -- 3 slots for cycle start, stop, step.
+ for _ = 1, 4 do
+ -- Occupy 1 slot for the function itself + 4 next slots will
+ -- occupied for a call to the vararg function.
+ -- Need 60 calls: 10 (baseslot after `a60()` is recorded)
+ -- + 58 * 4 + 2 (`a1` -- is not vararg function) = 244 slots.
+ -- The next call of metamethod in `a0` to record have 2 args
+ -- + 3 slots for metamethod function + 2 slots for frame.
+ -- luacheck: no global
+ a60()
+ assert(a0[42] == 42)
+ a0[42] = nil
+ end
+ return true
+end
+
+test:ok(test_gc32())
+test:ok(test_gc64())
+
+os.exit(test:check() and 0 or 1)
--
2.31.0
next reply other threads:[~2021-10-22 11:48 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-22 11:46 Sergey Kaplun via Tarantool-patches [this message]
2022-03-25 15:51 ` sergos via Tarantool-patches
2022-03-28 10:02 ` Sergey Kaplun via Tarantool-patches
2022-03-30 10:02 ` sergos via Tarantool-patches
2022-04-06 7:21 ` Sergey Kaplun via Tarantool-patches
2022-06-27 19:58 ` Igor Munkin via Tarantool-patches
2022-06-28 6:39 ` Sergey Kaplun via Tarantool-patches
2022-06-30 12:08 ` Igor Munkin via Tarantool-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211022114653.4225-1-skaplun@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=imun@tarantool.org \
--cc=sergos@tarantool.org \
--cc=skaplun@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH luajit] Add missing LJ_MAX_JSLOTS check.' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox