From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Maxim Kokryashkin <m.kokryashkin@tarantool.org>, Sergey Bronnikov <sergeyb@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH luajit] Restore state when recording __concat metamethod throws an error. Date: Wed, 21 Aug 2024 11:23:16 +0300 [thread overview] Message-ID: <20240821082316.4880-1-skaplun@tarantool.org> (raw) From: Mike Pall <mike> Thanks to Sergey Kaplun. (cherry picked from commit 7421a1b33c7ea46f12bba9700c15b5c90253fee0) Since neither `rec_cat()` nor `lj_record_ret()` restore the Lua stack, if the error is raised, it leads either to a crash in `BC_RET` or to the "unbalanced stack" assertion failure. This patch protects the `rec_mm_arith()`, which can raise an error. Its caller returns the negated error code to be rethrown in case of the caught error. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#10199 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1234-err-in-record-concat Related issues: * https://github.com/tarantool/tarantool/issues/10199 * https://github.com/LuaJIT/LuaJIT/issues/1234 src/lj_record.c | 26 ++++++++++- .../lj-1234-err-in-record-concat.test.lua | 43 +++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 test/tarantool-tests/lj-1234-err-in-record-concat.test.lua diff --git a/src/lj_record.c b/src/lj_record.c index 96fe26d8..64da3256 100644 --- a/src/lj_record.c +++ b/src/lj_record.c @@ -940,7 +940,9 @@ void lj_record_ret(jit_State *J, BCReg rbase, ptrdiff_t gotresults) J->L->base = b + baseadj; copyTV(J->L, b-(2<<LJ_FR2), &save); } - if (tr) { /* Store final result. */ + if (tr >= 0xffffff00) { + lj_err_throw(J->L, -(int32_t)tr); /* Propagate errors. */ + } else if (tr) { /* Store final result. */ BCReg dst = bc_a(*(frame_contpc(frame)-1)); J->base[dst] = tr; if (dst >= J->maxslot) { @@ -1936,12 +1938,27 @@ static TRef rec_tnew(jit_State *J, uint32_t ah) /* -- Concatenation ------------------------------------------------------- */ +typedef struct RecCatDataCP { + jit_State *J; + RecordIndex *ix; +} RecCatDataCP; + +static TValue *rec_mm_concat_cp(lua_State *L, lua_CFunction dummy, void *ud) +{ + RecCatDataCP *rcd = (RecCatDataCP *)ud; + UNUSED(L); UNUSED(dummy); + rec_mm_arith(rcd->J, rcd->ix, MM_concat); /* Call __concat metamethod. */ + return NULL; +} + static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot) { TRef *top = &J->base[topslot]; TValue savetv[5+LJ_FR2]; BCReg s; RecordIndex ix; + RecCatDataCP rcd; + int errcode; lj_assertJ(baseslot < topslot, "bad CAT arg"); for (s = baseslot; s <= topslot; s++) (void)getslot(J, s); /* Ensure all arguments have a reference. */ @@ -1977,8 +1994,11 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot) ix.tab = top[-1]; ix.key = top[0]; memcpy(savetv, &J->L->base[topslot-1], sizeof(savetv)); /* Save slots. */ - rec_mm_arith(J, &ix, MM_concat); /* Call __concat metamethod. */ + rcd.J = J; + rcd.ix = &ix; + errcode = lj_vm_cpcall(J->L, NULL, &rcd, rec_mm_concat_cp); memcpy(&J->L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */ + if (errcode) return (TRef)(-errcode); return 0; /* No result yet. */ } @@ -2300,6 +2320,8 @@ void lj_record_ins(jit_State *J) case BC_CAT: rc = rec_cat(J, rb, rc); + if (rc >= 0xffffff00) + lj_err_throw(J->L, -(int32_t)rc); /* Propagate errors. */ break; /* -- Constant and move ops --------------------------------------------- */ diff --git a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua new file mode 100644 index 00000000..9abaeba5 --- /dev/null +++ b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua @@ -0,0 +1,43 @@ +local tap = require('tap') + +-- Test file to demonstrate the crash during the concat recording +-- if it throws an error. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1234. + +local test = tap.test('lj-1234-err-in-record-concat'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(2) + +jit.opt.start('hotloop=1') + +local __concat = function(v1, v2) + return tostring(v1) .. tostring(v2) +end + +-- Need to use metamethod call in the concat recording. +debug.setmetatable(nil, { + __concat = __concat, +}) + +local function test_concat_p() + local counter = 0 + while counter < 1 do + counter = counter + 1 + -- The first result is placed on the Lua stack before the + -- error is raised. When the error is raised, it is handled by + -- the trace recorder, but since neither `rec_cat()` nor + -- `lj_record_ret()` restore the Lua stack (before the patch), + -- it becomes unbalanced after the instruction recording + -- attempt. + local _ = {} .. (nil .. nil) + end +end + +local result, errmsg = pcall(test_concat_p) + +test:ok(not result, 'the error is raised') +test:like(errmsg, 'attempt to concatenate a table value', 'correct error') + +test:done(true) -- 2.45.2
next reply other threads:[~2024-08-21 8:23 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-08-21 8:23 Sergey Kaplun via Tarantool-patches [this message] 2024-09-13 13:07 ` Sergey Bronnikov via Tarantool-patches 2024-09-13 15:27 ` Sergey Kaplun via Tarantool-patches 2024-09-13 15:48 ` Sergey Kaplun via Tarantool-patches 2024-09-17 6:33 ` Sergey Bronnikov via Tarantool-patches 2024-09-23 6:30 ` Maxim Kokryashkin via Tarantool-patches 2024-10-18 15:14 ` Sergey Kaplun 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=20240821082316.4880-1-skaplun@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=m.kokryashkin@tarantool.org \ --cc=sergeyb@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] Restore state when recording __concat metamethod throws an error.' \ /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