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 39204580F72; Wed, 16 Aug 2023 18:57:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 39204580F72 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1692201460; bh=7hxsi8/42Hlvrb9LjM8cHMDkJUn0TXlHpHXLlqmezSg=; 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=jErqgrMI1Roof4+nbRh28kj74jzlNIpunnHv8ld/QdSl1qCcPruM6vZmqf5rVVLCN zynQ6m5/vfahgljYfXMsvmWQd6VNV1b+n4DXfMsIRDsgYXMbXk7+PSXwKKL4IQ2vDj 6yn3pW8BattMXb9/bWmkzvjQ1lKO9JqZPQoXhdRM= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 4851F580F72 for ; Wed, 16 Aug 2023 18:57:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4851F580F72 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1qWItt-0001we-BU; Wed, 16 Aug 2023 18:57:37 +0300 Date: Wed, 16 Aug 2023 18:52:50 +0300 To: Maxim Kokryashkin Message-ID: References: <20230815142541.29855-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: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9700E0DCE2907754D12A1D0741C7E82ED7F70A80435AA08BE182A05F53808504051FB91EFF0113FE8F87CBF6D76D02A2D2D57D5125E94BFFD43F10575AC02696E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7952C4D7BD0BF3359EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637389D8DDD54F43F7A8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8E1C2ECA4B767571064C11E8EA74C4BBA117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCC2ED6D5310B1F811A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8B3A703B70628EAD7BAAAE862A0553A39223F8577A6DFFEA7C8BDE37D78FCB031643847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5A60E7FA009480BAFDB87C6FDA8A84D2CA2CC1F1F71803711F87CCE6106E1FC07E67D4AC08A07B9B02A336C6518635091CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D348F343DA43F62289F1F710D0691B70E1E196367C6F1565BCCDB94986998F64EA9780915E62E77CD1D1D7E09C32AA3244C974341DDF08D7352FAC48FDD033967F7408A6A02710B730485A42E4C463514DC5DA084F8E80FEBD3202CD0F03380D9577A83BD0C44CE203720ABEDE4BBDD9CDD X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojFRrmMqSMPxo8FhnpRurWRw== X-DA7885C5: 574C282E08D9BB4772A066AB63FCEB163B3E620A919B451D505A5357F34FDA41262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986B98AD3EDA4ABE4AE65855231C5ACA52400FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Fix predict_next() in parser. 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! Fixed your comments inline. Branch is force-pushed. On 16.08.23, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the patch! > LGTM, except for a few nits below. > On Tue, Aug 15, 2023 at 05:25:41PM +0300, Sergey Kaplun wrote: > > From: Mike Pall > > > > Reported by Sergey Kaplun. > > > > The `0001 KNIL` is a result of merging two `KPRI` instructions: one for > > the local variable, one for the slot with `nil` object. During parsing in > > `predict_next()` the second `MOV` bytecode is examined to set `pairs` or > > `next` local variable. But, as far as it moves `nil` value, that isn't > > an actual variable, so it has no the name this leads to the crash. > Typo: s/variable, so it/variable and/ > Typo: s/the name this/name, that move/ Fixed. > > > > This patch adds the check to be sure that `RD` in the `MOV` bytecode is > Typo: s/the check/a check/ Fixed. > > an actual variable. > Please mention the lj_bc.h here, so it is obvious what `RD` is. Fixed. > > > > Sergey Kaplun: > > * added the description and the test for the problem > > > > Part of tarantool/tarantool#8825 > > --- > > > > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1033-fix-parsing-predict-next > > PR: https://github.com/tarantool/tarantool/pull/8987 > > Related issues: > > * https://github.com/LuaJIT/LuaJIT/issues/1033 > > * https://github.com/tarantool/tarantool/issues/8825 > > > > src/lj_parse.c | 1 + > > .../lj-1033-fix-parsing-predict-next.test.lua | 30 +++++++++++++++++++ > > 2 files changed, 31 insertions(+) > > create mode 100644 test/tarantool-tests/lj-1033-fix-parsing-predict-next.test.lua > > > > diff --git a/src/lj_parse.c b/src/lj_parse.c > > index 3f6caaec..420b95cb 100644 > > --- a/src/lj_parse.c > > +++ b/src/lj_parse.c > > +-- The resulting bytecode is the following: > > +-- > > +-- 0001 KNIL 0 1 > > +-- 0002 MOV 2 1 > > +-- 0003 TGETS 1 1 0 ; "foo" > > +-- 0004 CALL 1 4 2 > > +-- > > +-- This MOV don't use any variable value from the stack, so the > Typo: s/don't/doesn't/ Fixed. See the iterative diff below: =================================================================== diff --git a/test/tarantool-tests/lj-1033-fix-parsing-predict-next.test.lua b/test/tarantool-tests/lj-1033-fix-parsing-predict-next.test.lua index 624344eb..63998d8c 100644 --- a/test/tarantool-tests/lj-1033-fix-parsing-predict-next.test.lua +++ b/test/tarantool-tests/lj-1033-fix-parsing-predict-next.test.lua @@ -14,7 +14,7 @@ local res_f = loadstring([[ -- 0003 TGETS 1 1 0 ; "foo" -- 0004 CALL 1 4 2 -- --- This MOV don't use any variable value from the stack, so the +-- This MOV doesn't use any variable value from the stack, so the -- attempt to get the name in `predict_next() leads to the crash. local _ for _ in (nil):foo() do end =================================================================== > > -- Best regards, Sergey Kaplun