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 97CF36ECE3; Fri, 10 Jun 2022 11:27:52 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 97CF36ECE3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1654849672; bh=Mpgf4oSOjpA+WMUvaaCyANht/ZNd6EIaS51hEKGuG30=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=WavIKlucKH7DdMijudd7G7h4yC9LpS6uCQFFqRNBUzxcfab7mA0DC63/aXIJpJOBw xxgsWSA8X7S0OLfSfsh640gNhSIqE+ZfFdD5yhpS6SJ6ghPYnib87PGcqhURALHwS5 yVIlou7EJG4B93L1waLhtfRY/YWq+MaVAypv+egU= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [94.100.177.149]) (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 524646ECE3 for ; Fri, 10 Jun 2022 11:27:50 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 524646ECE3 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1nzZzh-0004PI-7N; Fri, 10 Jun 2022 11:27:49 +0300 Date: Fri, 10 Jun 2022 11:25:33 +0300 To: sergos Message-ID: References: <20211215101734.6065-1-skaplun@tarantool.org> <68FC92D8-C62F-4625-8331-5AB943D9E7FF@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <68FC92D8-C62F-4625-8331-5AB943D9E7FF@tarantool.org> X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9C073F92845E8ABAAD183CF044C355D4E9C7FC99E6BC70892182A05F5380850404C228DA9ACA6FE2752521C4DA0465FC6F1BD85751C1BBDDC7B4778DFEA650A1FD6936F2A14CFC32A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE770FEBAE29342FA8FEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637651D61939D0B3DD78638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8604811B0136BBD1EED3A5A4ACDEA1FDC117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCECADA55FE5B58BB7A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B60A62CEF541B197C8089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A599AAC8248CCBEC6B9C9661D39AE81A02F0B3286B8CB17C8BD59269BC5F550898D99A6476B3ADF6B4886A5961035A09600383DAD389E261318FB05168BE4CE3AF X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3455049D7B43D89D6464CFAF55C688F29BD9B97C893A1CAFAF293F825A2FDE3D7CCA124D9F54C82D721D7E09C32AA3244C000FFA6525E39F21EDE411A87F8FA60869B6CAE0477E908DFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj/W+skaHQI8MMkw+nM2Elew== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D679FBE0654EBC89414966D51B5BBF64010FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [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" Hi, Sergos! Thanks for the review. I updated commit message considerring your comments. The branch is force-pushed. | Fix write barrier for lua_setupvalue() and debug.setupvalue(). | | (cherry picked from e613105ca92fe25e7bd63031b409faa8c908ac35) | | At the first GC state (`GCSpause`) all objects should be marked with the | current white (`LJ_GC_WHITE0` or `LJ_GC_WHITE1`). The main lua_State, | globals table, registry, and metatables are marked gray and added to the | gray list. The state now changes to `GCSpropagate`. | | At `GCSpropagate` each object in the gray list is removed and marked | black (`LJ_GC_BLACK`), then any white (type 0 or 1) objects it references are marked | gray and added to the gray list. Once the gray list is empty the current | white is switched to the other white. All objects marked with the old | white type are now dead objects. | | 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 by marking object gray (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 On 06.06.22, sergos wrote: > Hi! > > Thanks for the patch! > > LGTM with minor updates to the description. > > Sergos > > > On 15 Dec 2021, at 13:17, Sergey Kaplun wrote: > > > > From: Mike Pall > > > > (cherry picked from e613105ca92fe25e7bd63031b409faa8c908ac35) > > > > Child function inherits parents upvalues. Assume parent function is > > parents’ upvalues. > > > marked first (all closed upvalues and function are colored to black), > > I believe coloring should be described, or at least GC mentioned > > > and then `debug.setupvalue()`/`lua_setupvalue()` is called for an > > unmarked child function with inherited upvalues. The barrier is tried to > > Not quite clear which way it tries to move - mark the upvalue white? > > > 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 > > > > 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. > > + > > +-- 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 > > + > > If I’m not missing the point, the below will never be executed since > invariant is violated. GC invariant is violated, but there is no defined behaviour in this case (the object may be remarked later and assertion isn't failed). So the simplest way is to create some other objects to reuse memory pointed by dead object. > > > +-- 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 > > -- > > 2.34.1 > > > -- Best regards, Sergey Kaplun