[Tarantool-patches] [PATCH luajit] Fix write barrier for lua_setupvalue() and debug.setupvalue().

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

From: Mike Pall <mike>

(cherry picked from e613105ca92fe25e7bd63031b409faa8c908ac35)

Child function inherits parents upvalues. Assume parent function is
marked first (all closed upvalues and function are colored to black),
and then `debug.setupvalue()`/`lua_setupvalue()` is called for an
unmarked child function with inherited upvalues. The barrier is tried to
move forward (but not actually move, due to the colors of operands) for
a non-marked function (instead marked upvalue). Now black upvalue refers
to a white object. Black objects can't refer white objects due to GC
invariant, so the invariant is violated.

This patch changes a function object to an upvalue for barrier movement.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#6548
---
Related issue: https://github.com/tarantool/tarantool/issues/6548
Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-gc-setupvalue-full-ci
Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-gc-setupvalue-full-ci

Note: CI is red. But this job is red on master too...

 src/lj_api.c                                  |  8 ++-
 src/lj_debug.c                                |  7 ++-
 src/lj_debug.h                                |  3 +-
 .../fix-gc-setupvalue.test.lua                | 60 +++++++++++++++++++
 test/tarantool-tests/utils.lua                | 32 ++++++++++
 5 files changed, 104 insertions(+), 6 deletions(-)
 create mode 100644 test/tarantool-tests/fix-gc-setupvalue.test.lua

diff --git a/src/lj_api.c b/src/lj_api.c
index c7a0b327..ba38881f 100644
--- a/src/lj_api.c
+++ b/src/lj_api.c
@@ -943,7 +943,8 @@ LUA_API int lua_next(lua_State *L, int idx)
 LUA_API const char *lua_getupvalue(lua_State *L, int idx, int n)
 {
   TValue *val;
-  const char *name = lj_debug_uvnamev(index2adr(L, idx), (uint32_t)(n-1), &val);
+  GCobj *o;
+  const char *name = lj_debug_uvnamev(index2adr(L, idx), (uint32_t)(n-1), &val, &o);
   if (name) {
     copyTV(L, L->top, val);
     incr_top(L);
@@ -1129,13 +1130,14 @@ LUA_API const char *lua_setupvalue(lua_State *L, int idx, int n)
 {
   cTValue *f = index2adr(L, idx);
   TValue *val;
+  GCobj *o;
   const char *name;
   api_checknelems(L, 1);
-  name = lj_debug_uvnamev(f, (uint32_t)(n-1), &val);
+  name = lj_debug_uvnamev(f, (uint32_t)(n-1), &val, &o);
   if (name) {
     L->top--;
     copyTV(L, val, L->top);
-    lj_gc_barrier(L, funcV(f), L->top);
+    lj_gc_barrier(L, o, L->top);
   }
   return name;
 }
diff --git a/src/lj_debug.c b/src/lj_debug.c
index bb9ab288..8eb5983b 100644
--- a/src/lj_debug.c
+++ b/src/lj_debug.c
@@ -221,19 +221,22 @@ const char *lj_debug_uvname(GCproto *pt, uint32_t idx)
 }
 
 /* Get name and value of upvalue. */
-const char *lj_debug_uvnamev(cTValue *o, uint32_t idx, TValue **tvp)
+const char *lj_debug_uvnamev(cTValue *o, uint32_t idx, TValue **tvp, GCobj **op)
 {
   if (tvisfunc(o)) {
     GCfunc *fn = funcV(o);
     if (isluafunc(fn)) {
       GCproto *pt = funcproto(fn);
       if (idx < pt->sizeuv) {
-	*tvp = uvval(&gcref(fn->l.uvptr[idx])->uv);
+	GCobj *uvo = gcref(fn->l.uvptr[idx]);
+	*tvp = uvval(&uvo->uv);
+	*op = uvo;
 	return lj_debug_uvname(pt, idx);
       }
     } else {
       if (idx < fn->c.nupvalues) {
 	*tvp = &fn->c.upvalue[idx];
+	*op = obj2gco(fn);
 	return "";
       }
     }
diff --git a/src/lj_debug.h b/src/lj_debug.h
index a157d284..e037728a 100644
--- a/src/lj_debug.h
+++ b/src/lj_debug.h
@@ -29,7 +29,8 @@ typedef struct lj_Debug {
 LJ_FUNC cTValue *lj_debug_frame(lua_State *L, int level, int *size);
 LJ_FUNC BCLine LJ_FASTCALL lj_debug_line(GCproto *pt, BCPos pc);
 LJ_FUNC const char *lj_debug_uvname(GCproto *pt, uint32_t idx);
-LJ_FUNC const char *lj_debug_uvnamev(cTValue *o, uint32_t idx, TValue **tvp);
+LJ_FUNC const char *lj_debug_uvnamev(cTValue *o, uint32_t idx, TValue **tvp,
+				     GCobj **op);
 LJ_FUNC const char *lj_debug_slotname(GCproto *pt, const BCIns *pc,
 				      BCReg slot, const char **name);
 LJ_FUNC const char *lj_debug_funcname(lua_State *L, cTValue *frame,
diff --git a/test/tarantool-tests/fix-gc-setupvalue.test.lua b/test/tarantool-tests/fix-gc-setupvalue.test.lua
new file mode 100644
index 00000000..8d83ee6e
--- /dev/null
+++ b/test/tarantool-tests/fix-gc-setupvalue.test.lua
@@ -0,0 +1,60 @@
+local tap = require('tap')
+local utils = require('utils')
+
+local test = tap.test('fix-gc-setupvalue')
+test:plan(1)
+
+-- Test file to demonstrate LuaJIT GC invariant violation
+-- for inherited upvalues.
+
+-- The bug is about the situation, when black upvalue refers to
+-- a white object. This happens due to parent function is marked
+-- first (all closed upvalues and function are colored to black),
+-- and then `debug.setupvalue()` is called for a child function
+-- with inherited upvalues. The barrier is move forward for a
+-- non-marked function (instead upvalue) and invariant is
+-- violated.
+
+-- Create to functions with closed upvalue.
+do
+  local uv = 1
+  local function f_parent()
+    local function f()
+      return uv + 1
+    end
+    _G.f = f
+    return uv + 1
+  end
+  -- Set up `f()`.
+  f_parent()
+  _G.f_parent = f_parent
+end
+
+-- Set GC on start.
+collectgarbage()
+-- Set minimally possible stepmul.
+-- 1024/10 * stepmul == 10 < sizeof(GCfuncL), so it guarantees,
+-- that 2 functions will be marked in different time.
+local oldstepmul = collectgarbage('setstepmul', 1)
+
+-- `f_parent()` function is marked before `f()`, so wait until
+-- it becomes black and proceed with the test.
+while not utils.gc_isblack(_G.f_parent) do
+  collectgarbage('step')
+end
+
+-- Set created string (white) for the upvalue.
+debug.setupvalue(_G.f, 1, '4'..'1')
+_G.f = nil
+
+-- Lets finish it faster.
+collectgarbage('setstepmul', oldstepmul)
+-- Finish GC cycle to be sure that the object is collected.
+while not collectgarbage('step') do end
+
+-- Generate some garbage to reuse freed memory.
+for i = 1, 1e2 do local _ = {string.rep('0', i)} end
+
+test:ok(_G.f_parent() == 42, 'correct set up of upvalue')
+
+os.exit(test:check() and 0 or 1)
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index 5bd42b30..68781f28 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -3,11 +3,43 @@ local M = {}
 local ffi = require('ffi')
 local tap = require('tap')
 local bc = require('jit.bc')
+local bit = require('bit')
+
+local GCRef = ffi.abi('gc64') and 'uint64_t' or 'uint32_t'
+local LJ_GC_BLACK = 0x04
 
 ffi.cdef([[
   int setenv(const char *name, const char *value, int overwrite);
+  typedef struct {
+  ]]..GCRef..[[ nextgc;
+  uint8_t marked;
+  uint8_t gct;
+  /* Need this fields for correct alignment and sizeof. */
+  uint8_t misc1;
+  uint8_t misc2;
+  } GCHeader;
 ]])
 
+function M.gc_isblack(obj)
+  local objtype = type(obj)
+  assert(objtype ~= 'number' and objtype ~= 'boolean',
+         'can proceed only with GC objects')
+  local address
+  if objtype == 'string' then
+    -- XXX: get strdata first and go back to GCHeader.
+    address = ffi.cast('char *', obj)
+    address = address - (ffi.sizeof('GCHeader') + 8)
+  else
+    -- XXX: FFI ABI forbids to cast functions objects
+    -- to non-functional pointers, but we can get their address
+    -- via tostring.
+    local str_address = tostring(obj):gsub(objtype .. ': ', '')
+    address = tonumber(str_address)
+  end
+  local marked = ffi.cast('GCHeader *', address).marked
+  return bit.band(marked, LJ_GC_BLACK) == LJ_GC_BLACK
+end
+
 local function luacmd(args)
   -- arg[-1] is guaranteed to be not nil.
   local idx = -2
-- 
2.34.1