Skip to content

Commit

Permalink
LJ_GC64: Always snapshot functions for non-base frames.
Browse files Browse the repository at this point in the history
Reported by Arseny Vakhrushev.
Analysis and fix contributed by Peter Cawley.

(cherry picked from commit ff1e72a)

The problem is GC64-specific and could be reproduced with enabled
compiler options LUA_USE_ASSERT and LUA_USE_APICHECK.

Sergey Kaplun:
  * minimized reproducer made by fuzzing test

Sergey Bronnikov:
  * added the description (see a comment in the test)
  * added tests for the problem: first one based on the original
    reproducer and second one based on a reproducer made by fuzzing test.

Part of tarantool/tarantool#8825
  • Loading branch information
Mike Pall authored and ligurio committed Oct 12, 2023
1 parent 7d5901d commit d668177
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/lj_record.c
Expand Up @@ -211,6 +211,7 @@ static TRef getcurrf(jit_State *J)
{
if (J->base[-1-LJ_FR2])
return J->base[-1-LJ_FR2];
/* Non-base frame functions ought to be loaded already. */
lj_assertJ(J->baseslot == 1+LJ_FR2, "bad baseslot");
return sloadt(J, -1-LJ_FR2, IRT_FUNC, IRSLOAD_READONLY);
}
Expand Down
9 changes: 7 additions & 2 deletions src/lj_snap.c
Expand Up @@ -85,8 +85,13 @@ static MSize snapshot_slots(jit_State *J, SnapEntry *map, BCReg nslots)
IRIns *ir = &J->cur.ir[ref];
if ((LJ_FR2 || !(sn & (SNAP_CONT|SNAP_FRAME))) &&
ir->o == IR_SLOAD && ir->op1 == s && ref > retf) {
/* No need to snapshot unmodified non-inherited slots. */
if (!(ir->op2 & IRSLOAD_INHERIT))
/*
** No need to snapshot unmodified non-inherited slots.
** But always snapshot the function below a frame in LJ_FR2 mode.
*/
if (!(ir->op2 & IRSLOAD_INHERIT) &&
(!LJ_FR2 || s == 0 || s+1 == nslots ||
!(J->slot[s+1] & (TREF_CONT|TREF_FRAME))))
continue;
/* No need to restore readonly slots and unmodified non-parent slots. */
if (!(LJ_DUALNUM && (ir->op2 & IRSLOAD_CONVERT)) &&
Expand Down
@@ -0,0 +1,36 @@
local tap = require('tap')
local test = tap.test('lj-611-always-snapshot-functions-for-non-base-frames-1'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
})

-- GC64: Function missing in snapshot for non-base frame
-- https://github.com/LuaJIT/LuaJIT/issues/611

test:plan(1)

jit.opt.start('hotloop=1', 'hotexit=1')

local inner_counter = 0
local SIDE_START = 1
-- Lower frame to return from `inner()` function side trace.
-- TODO: Give a reason for vararg func.
local function lower_frame(...)
local inner = function()
if inner_counter > SIDE_START then
return
end
inner_counter = inner_counter + 1
end
inner(..., inner(inner()))
end

-- Compile `inner()` function.
lower_frame()
lower_frame()
-- Compile hotexit
lower_frame()
-- Take side exit from side trace.
lower_frame(1)

test:ok(true, 'function is present in snapshot')
test:done(true)
@@ -0,0 +1,43 @@
local tap = require('tap')
local test = tap.test('lj-611-always-snapshot-functions-for-non-base-frames')

test:plan(1)

jit.opt.start('hotloop=1', 'hotexit=1')

-- Test reproduces a bug "GC64: Function missing in snapshot for non-base
-- frame" [1], and based on reproducer described in [2].
--
-- [1]: https://github.com/LuaJIT/LuaJIT/issues/611
-- [2]: https://github.com/LuaJIT/LuaJIT/issues/611#issuecomment-679228156
--
-- Function `outer` is recorded to a trace and calls a builtin function that is
-- not JIT-compilable and therefore triggers exit to interpreter, and then it
-- resumes tracing just after the call returns - this is a trace stitching.
-- Then, within the call, we need the potential for a side trace. Finally, we need
-- that side exit to be taken enough for the exit to be compiled into a trace.
-- The loop at the bottom has enough iterations to trigger JIT compilation, and
-- enough more on top on trigger compilation of the not case. Compilation of
-- this case hits the assertion failure.

local inner
for _ = 1, 3 do
inner = function(_, i)
return i < 4
end
end

local function outer(i)
-- The function `string.gsub` is not JIT-compilable and triggers a trace
-- exit. For example, `string.gmatch` and `string.match` are suitable as
-- well.
-- See https://github.com/tarantool/tarantool/wiki/LuaJIT-Not-Yet-Implemented.
inner(string.gsub('', '', ''), i)
end

for i = 1, 4 do
outer(i)
end

test:ok(true, 'function is present in snapshot')
test:done(true)

0 comments on commit d668177

Please sign in to comment.