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 9AC97D5DED4; Wed, 13 Mar 2024 11:49:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9AC97D5DED4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1710319768; bh=8ll/oG/w3U2EaKb6SN8Ul8qdtr5cDB4gFG1ENB5XG3s=; 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=zh4qsqd7sz+vvcqi95bSkkWkNHZ+tiqyeCgAA+vEJJ13jdgbsdANoaXGUepf0rAjQ M/YEHMucz9XByL/Zi1+wGN3YUKYmvyDESHBUMhBguQVF1hLQz05BvcGKcqF2BWUDdE Udwr/X+SpaSKkAlQIlSy6JQzrUYym889/+n8xv9I= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [95.163.41.65]) (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 A859DD5DED4 for ; Wed, 13 Mar 2024 11:49:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A859DD5DED4 Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1rkKIg-00000001AB5-47dA; Wed, 13 Mar 2024 11:49:27 +0300 Date: Wed, 13 Mar 2024 11:49:26 +0300 To: Sergey Kaplun Message-ID: <7fbw6e7cgrz7ll5tmmjb6746fy2rdzxpxnsaeezt66d4r4hdby@f3i2heonivix> References: <20240311103701.24502-1-skaplun@tarantool.org> <3fdmcpw5ar4shu4om6ue2xdk7b2jjopuwf2sltf635ewgywbyz@ehjelhcmcwjn> 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: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD987C0EE6E7F0A597D21AFE2E000C69B1FBAF020A234192257182A05F5380850400596D3AC9C01920F9487ABAC94A94B541E922F8C2972BFD805A355599DF1B17B141B3585208100AE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7922D113DFDC6D5A3EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006376A7AE72AA03243C6EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38B73AB1701401CD8716CDE247FD46C0E43ABEB59464C0C99ECE1A69F61769E06ADA471835C12D1D9774AD6D5ED66289B5278DA827A17800CE767883B903EA3BAEA9FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3E478A468B35FE767117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CF5E208EBACFE9213EBA3038C0950A5D36C8A9BA7A39EFB766D91E3A1F190DE8FDBA3038C0950A5D36D5E8D9A59859A8B6E57745532406118876E601842F6C81A1F004C906525384303E02D724532EE2C3F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7C837C4FEFBD186071C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A57D92D7A4BF527E635002B1117B3ED6960419048FBEB5B080B48B7A7F94616420823CB91A9FED034534781492E4B8EEAD0942DC5495D0595EBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3458239C6F30CE7ACFD3AEF283B7F458A91DABA471F7FD1FD316B8223D3AABFA26425B5AB33BD1DE0F1D7E09C32AA3244C8B3F170DA02FD00DA9A6C043C55A4F6365ABD8D5B3D5E316EA455F16B58544A2557BDE0DD54B3590965026E5D17F6739C77C69D99B9914278E50E1F0597A6FD5CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojtv/CB9kl0S0fL/6BsxQVww== X-Mailru-Sender: 7940E2A4EB16C9976F80DCDCD59EC227E435B4FF32A6B4AB9487ABAC94A94B541E922F8C2972BFD8E2527C969975515CFF9FCECFB8D89CB6C77752E0C033A69E235A20A81F3B0E39AB3C5F247CB2F7F93A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Handle stack reallocation in debug.setmetatable() and lua_setmetatable(). 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the fixes! LGTM On Wed, Mar 13, 2024 at 10:46:32AM +0300, Sergey Kaplun wrote: > Maxim, > > On 12.03.24, Maxim Kokryashkin wrote: > > Hi, Sergey! > > Thanks for the clarifications! > > See my answers below. > > On Tue, Mar 12, 2024 at 08:43:44AM +0300, Sergey Kaplun wrote: > > > Hi, Maxim! > > > Thanks for the review! > > > Please consider my answers below. > > > > > > On 11.03.24, Maxim Kokryashkin wrote: > > > > Hi, Sergey! > > > > Thanks for the patch! > > > > Please consider my comments below. > > > > > > > > The test passes before the patch, as can be seen in CI for this branch: > > > > https://github.com/tarantool/luajit/tree/mkokryashkin/test > > > > > > I see quite the opposite [1][2]. > > Then add a comment mentioning that test fails only for the ASAN build. > > It is quite easy to miss. > > It is already mentioned at the beggining of the file [1]. > > > > > > > > > > > > On Mon, Mar 11, 2024 at 01:37:01PM +0300, Sergey Kaplun wrote: > > > > > > > > > > > > > > --- > > > > > > > > > > > +local jdump = require('jit.dump') > > > > > + > > > > > +test:plan(1) > > > > > + > > > > > +jdump.start('t', '/dev/null') > > > > Why do we need that call here? > > > > > > Because we need to trigger the VM event, see the comment below. > > Please drop a comment mentioning that. > > Sure, see the iterative patch below. Branch is force-pushed. > > =================================================================== > diff --git a/test/tarantool-tests/lj-1172-debug-handling-ref.test.lua b/test/tarantool-tests/lj-1172-debug-handling-ref.test.lua > index cac1c223..cf892011 100644 > --- a/test/tarantool-tests/lj-1172-debug-handling-ref.test.lua > +++ b/test/tarantool-tests/lj-1172-debug-handling-ref.test.lua > @@ -13,6 +13,8 @@ local jdump = require('jit.dump') > > test:plan(1) > > +-- We need to trigger the `TRACE` vmevent handler during > +-- `debug.setmetatable()`. It will cause Lua stack reallocation. > jdump.start('t', '/dev/null') > > -- Use `coroutine.wrap()` to create a new Lua stack with a minimum > =================================================================== > > > > > > > > > + > > > > > +-- Use `coroutine.wrap()` to create a new Lua stack with a minimum > > > > > +-- number of stack slots. > > > > > +coroutine.wrap(function() > > > > > + -- "TRACE flush" event handler causes stack reallocation and > > > > How is flush event caused? > > > > > > By the `jit.dump()` as most VM events. > > > > > > > > + -- leads to heap-use-after-free. This event handler is called > > > > > + -- because all traces are specialized to base metatables, so > > > > > + -- if we update any base metatable, we must flush all traces. > > > > > + debug.setmetatable(1, {}) > > > > > +end)() > > > > > > -- > > > Best regards, > > > Sergey Kaplun > > [1]: https://github.com/tarantool/luajit/blob/fead6df178f5b7a8384e217720647025eaf66e75/test/tarantool-tests/lj-1172-debug-handling-ref.test.lua#L5 > > -- > Best regards, > Sergey Kaplun