[Tarantool-patches] [PATCH luajit] Fix write barrier for lua_setupvalue() and debug.setupvalue().
Sergey Kaplun
skaplun at tarantool.org
Wed Dec 15 13:17:34 MSK 2021
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
More information about the Tarantool-patches
mailing list