Skip to content

Commit fe23eb2

Browse files
Mike PallBuristan
authored andcommitted
Restore state when recording __concat metamethod throws an error.
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 Reviewed-by: Sergey Bronnikov <[email protected]> Reviewed-by: Maxim Kokryashkin <[email protected]> Signed-off-by: Sergey Kaplun <[email protected]> (cherry picked from commit 420748c)
1 parent 689f6c3 commit fe23eb2

File tree

2 files changed

+69
-2
lines changed

2 files changed

+69
-2
lines changed

src/lj_record.c

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,9 @@ void lj_record_ret(jit_State *J, BCReg rbase, ptrdiff_t gotresults)
940940
J->L->base = b + baseadj;
941941
copyTV(J->L, b-(2<<LJ_FR2), &save);
942942
}
943-
if (tr) { /* Store final result. */
943+
if (tr >= 0xffffff00) {
944+
lj_err_throw(J->L, -(int32_t)tr); /* Propagate errors. */
945+
} else if (tr) { /* Store final result. */
944946
BCReg dst = bc_a(*(frame_contpc(frame)-1));
945947
J->base[dst] = tr;
946948
if (dst >= J->maxslot) {
@@ -1939,12 +1941,27 @@ static TRef rec_tnew(jit_State *J, uint32_t ah)
19391941

19401942
/* -- Concatenation ------------------------------------------------------- */
19411943

1944+
typedef struct RecCatDataCP {
1945+
jit_State *J;
1946+
RecordIndex *ix;
1947+
} RecCatDataCP;
1948+
1949+
static TValue *rec_mm_concat_cp(lua_State *L, lua_CFunction dummy, void *ud)
1950+
{
1951+
RecCatDataCP *rcd = (RecCatDataCP *)ud;
1952+
UNUSED(L); UNUSED(dummy);
1953+
rec_mm_arith(rcd->J, rcd->ix, MM_concat); /* Call __concat metamethod. */
1954+
return NULL;
1955+
}
1956+
19421957
static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
19431958
{
19441959
TRef *top = &J->base[topslot];
19451960
TValue savetv[5+LJ_FR2];
19461961
BCReg s;
19471962
RecordIndex ix;
1963+
RecCatDataCP rcd;
1964+
int errcode;
19481965
lj_assertJ(baseslot < topslot, "bad CAT arg");
19491966
for (s = baseslot; s <= topslot; s++)
19501967
(void)getslot(J, s); /* Ensure all arguments have a reference. */
@@ -1980,8 +1997,11 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
19801997
ix.tab = top[-1];
19811998
ix.key = top[0];
19821999
memcpy(savetv, &J->L->base[topslot-1], sizeof(savetv)); /* Save slots. */
1983-
rec_mm_arith(J, &ix, MM_concat); /* Call __concat metamethod. */
2000+
rcd.J = J;
2001+
rcd.ix = &ix;
2002+
errcode = lj_vm_cpcall(J->L, NULL, &rcd, rec_mm_concat_cp);
19842003
memcpy(&J->L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */
2004+
if (errcode) return (TRef)(-errcode);
19852005
return 0; /* No result yet. */
19862006
}
19872007

@@ -2303,6 +2323,8 @@ void lj_record_ins(jit_State *J)
23032323

23042324
case BC_CAT:
23052325
rc = rec_cat(J, rb, rc);
2326+
if (rc >= 0xffffff00)
2327+
lj_err_throw(J->L, -(int32_t)rc); /* Propagate errors. */
23062328
break;
23072329

23082330
/* -- Constant and move ops --------------------------------------------- */
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
local tap = require('tap')
2+
3+
-- Test file to demonstrate the crash during the concat recording
4+
-- if it throws an error.
5+
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1234.
6+
7+
local test = tap.test('lj-1234-err-in-record-concat'):skipcond({
8+
['Test requires JIT enabled'] = not jit.status(),
9+
})
10+
11+
test:plan(2)
12+
13+
jit.opt.start('hotloop=1')
14+
15+
local __concat = function()
16+
return ''
17+
end
18+
19+
-- Need to use metamethod call in the concat recording.
20+
-- We may use any object with a metamethod, but let's use a table
21+
-- as the most common one.
22+
local concatable_t = setmetatable({}, {
23+
__concat = __concat,
24+
})
25+
26+
local function test_concat_p()
27+
local counter = 0
28+
while counter < 1 do
29+
counter = counter + 1
30+
-- The first result is placed on the Lua stack before the
31+
-- error is raised. When the error is raised, it is handled by
32+
-- the trace recorder, but since neither `rec_cat()` nor
33+
-- `lj_record_ret()` restore the Lua stack (before the patch),
34+
-- it becomes unbalanced after the instruction recording
35+
-- attempt.
36+
local _ = {} .. (concatable_t .. concatable_t)
37+
end
38+
end
39+
40+
local result, errmsg = pcall(test_concat_p)
41+
42+
test:ok(not result, 'the error is raised')
43+
test:like(errmsg, 'attempt to concatenate a table value', 'correct error')
44+
45+
test:done(true)

0 commit comments

Comments
 (0)