Skip to content

Commit

Permalink
Restore state when recording __concat metamethod throws an error.
Browse files Browse the repository at this point in the history
Thanks to Sergey Kaplun.

(cherry picked from commit 7421a1b)

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
  • Loading branch information
Mike Pall authored and Buristan committed Oct 16, 2024
1 parent 82820a6 commit a3078c5
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 2 deletions.
26 changes: 24 additions & 2 deletions src/lj_record.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -1939,12 +1941,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. */
Expand Down Expand Up @@ -1980,8 +1997,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. */
}

Expand Down Expand Up @@ -2303,6 +2323,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 --------------------------------------------- */
Expand Down
45 changes: 45 additions & 0 deletions test/tarantool-tests/lj-1234-err-in-record-concat.test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
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()
return ''
end

-- Need to use metamethod call in the concat recording.
-- We may use any object with a metamethod, but let's use a table
-- as the most common one.
local concatable_t = setmetatable({}, {
__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 _ = {} .. (concatable_t .. concatable_t)
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)

0 comments on commit a3078c5

Please sign in to comment.