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