Skip to content

Commit 4309932

Browse files
Mike PallBuristan
authored andcommitted
Avoid recording interference due to invocation of VM hooks.
Thanks to Sergey Kaplun. (cherry picked from commit ab834de) If the VM event contains a trace, it may cause several inconsistencies during the recording of another trace: - If there is an exit from the trace in the VM event for the 'trace start' VM event, the JIT engine converts the newly recorded trace to the "side trace". So, when this side exit is taken, the JIT returns from the VM event in the middle of another frame. - Stitching semantics are broken for the VM events due to an inconsistent frame link chain. This patch fixes these issues by forbidding stitching in the VM event and saving the context of the JIT engine at the VM event for trace start. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#12134 Reviewed-by: Sergey Bronnikov <sergeyb@tarantool.org> Signed-off-by: Sergey Kaplun <skaplun@tarantool.org> (cherry picked from commit ac306b3)
1 parent 4882468 commit 4309932

4 files changed

Lines changed: 97 additions & 11 deletions

File tree

src/lj_dispatch.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -523,16 +523,18 @@ ASMFunction LJ_FASTCALL lj_dispatch_call(lua_State *L, const BCIns *pc)
523523
/* Stitch a new trace. */
524524
void LJ_FASTCALL lj_dispatch_stitch(jit_State *J, const BCIns *pc)
525525
{
526-
ERRNO_SAVE
527-
lua_State *L = J->L;
528-
void *cf = cframe_raw(L->cframe);
529-
const BCIns *oldpc = cframe_pc(cf);
530-
setcframe_pc(cf, pc);
531-
/* Before dispatch, have to bias PC by 1. */
532-
L->top = L->base + cur_topslot(curr_proto(L), pc+1, cframe_multres_n(cf));
533-
lj_trace_stitch(J, pc-1); /* Point to the CALL instruction. */
534-
setcframe_pc(cf, oldpc);
535-
ERRNO_RESTORE
526+
if (!(J2G(J)->hookmask & HOOK_VMEVENT)) {
527+
ERRNO_SAVE
528+
lua_State *L = J->L;
529+
void *cf = cframe_raw(L->cframe);
530+
const BCIns *oldpc = cframe_pc(cf);
531+
setcframe_pc(cf, pc);
532+
/* Before dispatch, have to bias PC by 1. */
533+
L->top = L->base + cur_topslot(curr_proto(L), pc+1, cframe_multres_n(cf));
534+
lj_trace_stitch(J, pc-1); /* Point to the CALL instruction. */
535+
setcframe_pc(cf, oldpc);
536+
ERRNO_RESTORE
537+
}
536538
}
537539
#endif
538540

src/lj_trace.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,11 @@ static void trace_start(jit_State *J)
459459
J->ktrace = 0;
460460
setgcref(J->cur.startpt, obj2gco(J->pt));
461461

462-
lj_vmevent_send(J2G(J), TRACE,
462+
lj_vmevent_send_(J2G(J), TRACE,
463+
TValue savetv = J2G(J)->tmptv;
464+
TValue savetv2 = J2G(J)->tmptv2;
465+
TraceNo parent = J->parent;
466+
ExitNo exitno = J->exitno;
463467
setstrV(V, V->top++, lj_str_newlit(V, "start"));
464468
setintV(V->top++, traceno);
465469
setfuncV(V, V->top++, J->fn);
@@ -474,6 +478,11 @@ static void trace_start(jit_State *J)
474478
setintV(V->top++, -1);
475479
}
476480
}
481+
,
482+
J2G(J)->tmptv = savetv;
483+
J2G(J)->tmptv2 = savetv2;
484+
J->parent = parent;
485+
J->exitno = exitno;
477486
);
478487
lj_record_setup(J);
479488
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
local tap = require('tap')
2+
3+
-- The test file to demonstrate the incorrect recording of the
4+
-- trace when stitching in the VM event.
5+
-- See also https://github.com/LuaJIT/LuaJIT/issues/1429.
6+
7+
local test = tap.test('lj-1429-stitching-to-vm-event'):skipcond({
8+
['Test requires JIT enabled'] = not jit.status(),
9+
})
10+
11+
test:plan(1)
12+
13+
local function always_number(val)
14+
return tonumber(val) or 1
15+
end
16+
17+
-- This handler leads to stitching in the VM event.
18+
local function hdl()
19+
always_number('')
20+
end
21+
22+
jit.opt.start('hotloop=1', 'hotexit=1')
23+
24+
jit.attach(hdl, 'trace')
25+
26+
coroutine.wrap(function()
27+
always_number('')
28+
always_number('')
29+
always_number(0) -- Start side trace, invoke handler.
30+
-- This breaks the recording semantics before the patch.
31+
end)()
32+
33+
test:ok(true, 'no assertion failure')
34+
35+
test:done(true)
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
local tap = require('tap')
2+
3+
-- The test file to demonstrate the incorrect recording of the
4+
-- trace when facing the trace exit in the VM event (start).
5+
-- See also https://github.com/LuaJIT/LuaJIT/issues/1434.
6+
7+
local test = tap.test('lj-1434-trace-start-interference'):skipcond({
8+
['Test requires JIT enabled'] = not jit.status(),
9+
})
10+
11+
test:plan(1)
12+
13+
local function call(self)
14+
return self
15+
end
16+
17+
local function cb()
18+
-- Side exit for trace 1.
19+
call(nil)
20+
end
21+
22+
jit.opt.start('hotloop=1', 'hotexit=1');
23+
24+
jit.attach(cb, 'trace')
25+
26+
coroutine.wrap(function()
27+
for i = 1, 4 do
28+
-- Record trace 1.
29+
call(call(i))
30+
-- Start trace 2. Side exit from trace 1 in the 'trace start'
31+
-- VM event converts the second trace to the "side trace".
32+
-- After that the VM assertion `lj_assert_bad_for_arg_type()`
33+
-- fails, since we return from the VM event in the middle of
34+
-- another frame.
35+
end
36+
end)()
37+
38+
test:ok(true, 'no assertion failure')
39+
40+
test:done(true)

0 commit comments

Comments
 (0)