From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id CD6AB6ECC0; Wed, 15 Dec 2021 13:19:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CD6AB6ECC0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1639563568; bh=9vDFGrf8Dg5HJ01+CF/jCKrVydEx+KFis+LgS09bK8A=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=rx7h/sIC+yx5INKWQ//ljiW3SUyP9aRbQJzdcLXmwT8Z4On/XfQNbyow2JYv8dkje bD6/6AaAflNFKf2Y7mEbmotdYcRR7FsIP2kpju5qaI12JvoH2TfJb9cSc+quY8ZwyI DFWDm4q/zV+UZLer/QDBaHZptsrpcSmjqPZRDY40= Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A30416ECC0 for ; Wed, 15 Dec 2021 13:19:25 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A30416ECC0 Received: by smtp50.i.mail.ru with esmtpa (envelope-from ) id 1mxRNc-0003OF-Ho; Wed, 15 Dec 2021 13:19:25 +0300 To: Sergey Ostanevich , Igor Munkin Date: Wed, 15 Dec 2021 13:17:34 +0300 Message-Id: <20211215101734.6065-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9FFF274446F725B74A9CFE9BC757439B365EEF0A005CDE6D9182A05F538085040EED4DB6E5132F0936E5F2BCA9F54AA79898110622E418E90F93021639AC8BB75 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE730B41EED8119E12FEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006372B2228CC7F94022E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D86402F3A99380F6F48281DA4CC24E15A1117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3A703B70628EAD7BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A1DCCEB63E2F10FB089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C302CFC0D3F89A23B2B2577885F9E7EF67AEBBE0DD2C3B7E49C2B6934AE262D3EE7EAB7254005DCED695F253C9721EF041E0A4E2319210D9B64D260DF9561598F01A9E91200F654B0588BE6097D4A97A98E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3438BC6CF312DA731524DAC5F67648B1ED2C3DB285C302702087C7FD8CC0A4EE50D68CF51875287A121D7E09C32AA3244C2DF9CFB064C15207CCA65C6ABD0ACF8FC3B3ADDA61883BB5927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojqIoINCajonA8cGbDZvFFrw== X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB447042393E938748E69D697AC24826AC9D3D0EE26750CF6C1F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A84198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Fix write barrier for lua_setupvalue() and debug.setupvalue(). X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall (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