[Tarantool-patches] [PATCH luajit] LJ_GC64: Always snapshot functions for non-base frames.

[Sergey Bronnikov]

From: Sergey Bronnikov <sergeyb at tarantool.org>

Reported by Arseny Vakhrushev.
Analysis and fix contributed by Peter Cawley.

(cherry picked from commit ff1e72acead01df7d8ed0fbb31efd32f57953618)

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
---
Branch: https://github.com/tarantool/luajit/commits/ligurio/lj-611-always-snapshot-functions-for-non-base-frames
PR: https://github.com/tarantool/tarantool/pull/9254
LJ issue: https://github.com/LuaJIT/LuaJIT/issues/611

 src/lj_record.c                               |  1 +
 src/lj_snap.c                                 |  9 +++-
 ...t-functions-for-non-base-frames-1.test.lua | 36 +++++++++++++++
 ...hot-functions-for-non-base-frames.test.lua | 45 +++++++++++++++++++
 4 files changed, 89 insertions(+), 2 deletions(-)
 create mode 100644 test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua
 create mode 100644 test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua

diff --git a/src/lj_record.c b/src/lj_record.c
index 48a5481b..55785e23 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -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);
 }
diff --git a/src/lj_snap.c b/src/lj_snap.c
index 6c5e5e53..06ae17eb 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -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)) &&
diff --git a/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua
new file mode 100644
index 00000000..759c2862
--- /dev/null
+++ b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames-1.test.lua
@@ -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)
diff --git a/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua
new file mode 100644
index 00000000..7305c185
--- /dev/null
+++ b/test/tarantool-tests/lj-611-always-snapshot-functions-for-non-base-frames.test.lua
@@ -0,0 +1,45 @@
+local tap = require('tap')
+local test = tap.test('lj-611-always-snapshot-functions-for-non-base-frames'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+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)
-- 
2.34.1