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 A9BD65C2DBA; Mon, 28 Aug 2023 18:06:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A9BD65C2DBA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1693235218; bh=AqVOj9GP+tMWegEK4Fx3Q7zLJXnvobIfl5OrOD9CQOc=; 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=vNTD1qxhYAAT+Bs/0diCU2UyrbXPioecWNbgxVp1sql/34EtzE68E60n8wSbOKYNE QVtNzZDX6ffwj4ClndeiHS964p8zsPLxRyY5Agjs3PiyF9oVhrRrZFmYeUeVSB430S SgYDnpmvAae/AkmeYxNOfhaDRKTkM31Xc6/BB27Q= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [95.163.41.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 388955C2D87 for ; Mon, 28 Aug 2023 18:06:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 388955C2D87 Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1qadpR-0007up-1e; Mon, 28 Aug 2023 18:06:57 +0300 Date: Mon, 28 Aug 2023 18:02:12 +0300 To: Maxim Kokryashkin Message-ID: References: <20230825150024.23247-1-skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9954329A9C7AF96E9FD8545DF373B4CC37F8EB0C10E92C247182A05F538085040AE894B3CE97CA3DE34693A52703F0AFD2AE5980BB82AB8D1B2A5FB020275D3CD X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7BDF1FA55CCD598A3EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637829D9538242026C38638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8308584074C68A31ED3B8A2EF84264432117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC1F3D1E7C87716A07A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18F04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE140C956E756FBB7A1AD9CA79AA6DBBDDD8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE32A336C651863509103F1AB874ED89028C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F79006373568875668442E59EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A5F26A018D301B0642409017BDE52EAD927FFD362CAC26E3F2F87CCE6106E1FC07E67D4AC08A07B9B0A6C7FFFE744CA7FBCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3455AC8BF8E3153BA050F930C55241DFC5AE4D4714215514B712D3FF3B24A935394DBCCEBC6C99A9F21D7E09C32AA3244C1791D84C29B4A4E2BE974376D0DBBB07A995755A1445935E85A42E4C463514DC5DA084F8E80FEBD3202CD0F03380D9577A83BD0C44CE203720ABEDE4BBDD9CDD X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojrC6BcuK2ROzPEdJRH8loIg== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A7698CFA577F241C721834693A52703F0AFD1E1E156D000DABDBDEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Fix maxslots when recording BC_TSETM. 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, Maxim! Thanks for the review! I've updated the patch considering your comments and force-pushed the branch. On 28.08.23, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the patch! > Please consider my comments below. > > On Fri, Aug 25, 2023 at 06:00:24PM +0300, Sergey Kaplun wrote: > > From: Mike Pall > > > > Analyzed by Sergey Kaplun. > > > > (cherry-picked from commit 0cc5fdfbc0810073485150eb184dc358dab507d9) > > > > Recording of the `BC_TSETM` bytecode may keep too optimistic JIT > > maxslot. In that case, the slot above the top of the Lua stack may be > Typo: s/too optimistic JIT maxslot./the JIT maxslot too optimistic./ Fixed. > > considered used. When any VM event handler is called before the > > recording of the next instruction, this leads to an assertion failure in > > `rec_check_slots()`. > > > > This patch sets the `ra` as a maxslot, as far as the `ra` - 1 contains a > > table, which is always the highest slot after this bytecode. Also, it > > adds an assertion that we check slots below the top of the Lua stack. > > > > Sergey Kaplun: > > * added the description and the test for the problem > > > > Part of tarantool/tarantool#8825 > > --- > > + > > +local TEST_VALUE = '5' > > +local TEST_IDX = 5 > > + > > +local function slot5() > > + return nil, nil, nil, nil, TEST_VALUE > > +end > Why the fifth slot? Drop a comment. Fixed. See the iterative patch below. > > + > > +local storage > > +local function test_tsetm(...) > > + -- Usage of `TSETM` bytecode. > > + storage = {slot5()} > > + -- Use this function again to trick use-def analysis and avoid > > + -- cleaning JIT slots, so the last JIT slot contains > > + -- `TEST_VALUE`. > > + return slot5(...) > > +end > > + > > +-- Wrapper to avoid the recording of just the inner `slot5()` > > +-- function. > > +local function wrap() > > + test_tsetm() > > +end > > + > > +jit.opt.start('hotloop=1') > > +-- We need to call the VM event handler after each recorded bytecode > > +-- instruction to pollute the Lua stack and the issue > > +-- becomes observable. > Typo: s/and the issue becomes/and make the issue/ Fixed. See the iterative patch below: =================================================================== diff --git a/test/tarantool-tests/lj-1025-tsetm-maxslot.test.lua b/test/tarantool-tests/lj-1025-tsetm-maxslot.test.lua index 7ae0a99d..74625a79 100644 --- a/test/tarantool-tests/lj-1025-tsetm-maxslot.test.lua +++ b/test/tarantool-tests/lj-1025-tsetm-maxslot.test.lua @@ -15,6 +15,8 @@ local jit_dump = require('jit.dump') local TEST_VALUE = '5' local TEST_IDX = 5 +-- XXX: Use big enough slot numbewr to be overwritten by VM event +-- handler function. local function slot5() return nil, nil, nil, nil, TEST_VALUE end @@ -36,9 +38,9 @@ local function wrap() end jit.opt.start('hotloop=1') --- We need to call the VM event handler after each recorded bytecode --- instruction to pollute the Lua stack and the issue --- becomes observable. +-- We need to call the VM event handler after each recorded +-- bytecode instruction to pollute the Lua stack and make the +-- issue observable. jit_dump.start('b', '/dev/null') -- Compile and execute the trace with `TSETM`. =================================================================== > > +jit_dump.start('b', '/dev/null') > > + > > -- > > 2.41.0 > > -- Best regards, Sergey Kaplun