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 C5D19EDCC62; Mon, 13 Jan 2025 18:26:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C5D19EDCC62 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1736781987; bh=KKJXwv1iu4j3JfLH7HDajRxIFz3jByXt/EkgRMmHHcM=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=K+/nMS/SGYW9o72W+IcBe2Kx/OL5ULIRasXUp/G6PAbq/wIWxo20wVaQuQ8bOLhQ7 Ct3WesoQqKJQC08W5nFb3rrhtrmBg770Ni6N/v5owgGxcJxZ/IOugA/iDxe0iCNCVd 0mg4bFWckL8EF7r0YtF/Q+UIXwm7r4BBVmWYPCl0= Received: from send126.i.mail.ru (send126.i.mail.ru [89.221.237.221]) (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 EBE27EDCC62 for ; Mon, 13 Jan 2025 18:26:25 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org EBE27EDCC62 Received: by exim-smtp-bcf9586c5-58zll with esmtpa (envelope-from ) id 1tXMKb-000000000DL-3KHJ; Mon, 13 Jan 2025 18:26:25 +0300 Date: Mon, 13 Jan 2025 18:25:45 +0300 To: Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org Message-ID: References: <20250109150124.23841-1-skaplun@tarantool.org> <64c5cff1-e038-4844-8950-f255df8de717@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <64c5cff1-e038-4844-8950-f255df8de717@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD97C62A2C8925CC647B11223E654DDE28102A451326EF43BBE182A05F53808504043455D8ED1CD6D213DE06ABAFEAF6705E898290C75018D3EC9CABDAFE3DBE4E3ED51035647AA255C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE766DBE83FD69AB645EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637D07BBD2EBFB7BF888638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8FD9626C3C17AA11546F20FEE803C3BA9F8D2868901F26BB2CC7F00164DA146DAFE8445B8C89999728AA50765F79006372A3B24BF85B2E607389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8D2DCF9CF1F528DBCF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8B2303E78B907142AC75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A51BC98F3009B336835002B1117B3ED696FF3D4B1581015CEF72305013E4AE841E823CB91A9FED034534781492E4B8EEAD0D89974173551D4FBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34D8FD11F74BDD6E8D9A2C077724788EDD19CFC5B9FC6B17B70AD57742A35886C06504915007BB86D61D7E09C32AA3244C66BB96D46B0B818077DD89D51EBB77423EF0D26B8FDEDA67EA455F16B58544A2E30DDF7C44BCB90DA5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojtuBC1linLcji9EoVIkbblw== X-DA7885C5: 87E92DB4EEFE88BAF255D290C0D534F983B7CC3EEA42B07E5D98717C444C9019828441ED6CDEDFB95B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F739381B31377CF4CA2196AC968C57066A682FEC1A09792B9D37125CA1EDEA3815384E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Fix predict_next() in parser (for real now). 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 Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the review! On 13.01.25, Sergey Bronnikov wrote: > Hi, Sergey > > thanks for the patch, LGTM with a minor question below. Please consider my answers below. > > On 09.01.2025 18:01, Sergey Kaplun wrote: > > From: Mike Pall > > > > Reported by Sergey Kaplun. > > > > (cherry picked from commit f602f0154b644211283cfeea92a570ca38f71947) > > > > Before the patch `predict_next()` uses the pc allocation limit > > (`fs->bclim`) instead of the real limit of the defined bytecodes > > (`fs->pc`). This leads to the use of undefined value and possible > > crash. This patch fixes the check. > > > > Sergey Kaplun: > > * added the description and the test for the problem > > > > Part of tarantool/tarantool#10709 > > --- > > > > Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1226-fix-predict-next > > Related issues: > > *https://github.com/LuaJIT/LuaJIT/issues/1226 > > *https://github.com/tarantool/tarantool/issues/10709 > > +-- The resulting bytecode is the following: > > +-- > > +-- 0001 KNIL 0 3 > > +-- 0002 JMP 4 => 0003 > > +-- 0003 => ITERC 4 2 3 > > +-- 0004 ITERL 4 => 0003 > > +-- > > +-- The parsing of the `for` iterator uses the incorrect check for > > +-- `fs->bclim`, which allows the usage of an uninitialized value, > > +-- so the test fails under Valgrind. > > +local res_f = loadstring([[ > > +-- This local variable is necessary, because it emits `KPRI` > > +-- bytecode, with which the next `KPRI` bytecode will be merged. > > +local _ > > +for _ in nil do end > > +]]) > > + > > +test:ok(res_f, 'chunk loaded successfully') > > + > > +local res, err = pcall(res_f) > > + > > +-- Check consistency with PUC Rio Lua 5.1 behaviour. > What for? It is not related to the bug and fix for it. I considered this a simple canary test. The main check is done during parsing by Valgrind, obviously. But still don't see anything crucial in checking the parsing behaviour this way too (just to be sure). > > +test:ok(not res, 'loaded function not executed') > > +test:like(err, 'attempt to call a nil value', 'correct error message') > > + > > +test:done(true) -- Best regards, Sergey Kaplun