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 BD0CD704267; Fri, 17 Nov 2023 13:23:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BD0CD704267 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1700216599; bh=G+hINSGDNrwtebECSqRNH2D8VVGXuL6hSqMRPm0C/R8=; 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=Px56N/MFYgJw2Rekvg3rS6txMdvR5Dhc5maAv0zONhF/FvhxLDFVLaCKt1J1xyj4d 5pDY7ux4qqX9mQsvqXKEynxpsbIlDg8+PFQ0fwCsiOO+Gfti3KlD1rmdDSwl1OHSdE 83fIwkhQs/tcDHXTPOOu2oB/2ahhqfRKhpO04sNA= Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [95.163.41.77]) (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 3BA9F64ED28 for ; Fri, 17 Nov 2023 13:23:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3BA9F64ED28 Received: by smtp36.i.mail.ru with esmtpa (envelope-from ) id 1r3w0L-00HSvp-0e; Fri, 17 Nov 2023 13:23:17 +0300 Date: Fri, 17 Nov 2023 13:23:16 +0300 To: Sergey Kaplun Message-ID: References: <20231116084959.24798-1-skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231116084959.24798-1-skaplun@tarantool.org> X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9C2A6B03AB739174CFB513953A26686ECD2CB4A01364DB00800894C459B0CD1B9577A07FE5F4AADF51F92C7AAEDCE5CDE5196EA129AE73E833ECC478399AFAD77 X-C1DE0DAB: 0D63561A33F958A5519739AA6B7F28A33B36891FC0260776AB7CA050525296F8F87CCE6106E1FC07E67D4AC08A07B9B0CE135D2742255B35CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34D1AE09A115117C96634A307EA3A72AAEA13AC308A5533458C66F172BD2B2979631D68B7E181EF07F1D7E09C32AA3244C8321421B4992AD66B390CFC9F8B09126A995755A1445935E85A42E4C463514DC5DA084F8E80FEBD396F07DFE06A4A8314E894E437E78228B66933FA05BD8EF0CAD958392AE682691 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojWWC/ZYCun+XWuz2RsLz9/w== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE40752C271CD6913A1037E7E96896365731E75434CE7A01A4145D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit] Add NaN check to IR_NEWREF. 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 patch! LGTM, except for a single nit and a question below. On Thu, Nov 16, 2023 at 11:49:59AM +0300, Sergey Kaplun wrote: > From: Mike Pall > > Thanks to Peter Cawley. > > (cherry-picked from commit 7f9907b4ed0870ba64342bcc4b26cff0a94540da) > > When emitting IR NEWREF, there is no check for a non-NaN stored key > value. Thus, when the NaN number value is given to trace, it may be > stored as a key. This patch adds the corresponding check. If fold > optimization is enabled, this IR EQ check is dropped if it references > CONV IR from any (unsigned) integer type since NaN can be created via > conversion from an integer. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#9145 > --- > > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1069-newref-nan-key > Tarantool PR: https://github.com/tarantool/tarantool/pull/9374 > Fuzzer link: https://oss-fuzz.com/testcase-detail/5251574662037504 > Relate issues: > * https://github.com/LuaJIT/LuaJIT/issues/1069 > * https://github.com/tarantool/tarantool/issues/9145 > > src/lj_opt_fold.c | 5 +- > src/lj_record.c | 12 +- > .../lj-1069-newref-nan-key.test.lua | 151 ++++++++++++++++++ > 3 files changed, 164 insertions(+), 4 deletions(-) > create mode 100644 test/tarantool-tests/lj-1069-newref-nan-key.test.lua > diff --git a/test/tarantool-tests/lj-1069-newref-nan-key.test.lua b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua > new file mode 100644 > index 00000000..ec28b274 > --- /dev/null > +++ b/test/tarantool-tests/lj-1069-newref-nan-key.test.lua > @@ -0,0 +1,151 @@ > +-- Test the constant IR NaN value on the trace. Nit: This comment seems a bit redundant, as it duplicates the test name. Feel free to ignore. > +test:test('constant NaN on the trace', function(subtest) > + -- Test the status and the error message. > + subtest:plan(2) > + local function protected() > + local counter = 0 > + -- Use a number key to emit NEWREF. > + local key = 0.1 > + > + jit.opt.start('hotloop=1') > + > + while counter < 2 do > + counter = counter + 1 > + -- luacheck: ignore > + local tab = {_ = 'unused'} > + tab[key] = 'NaN' > + -- XXX: Use the constant NaN value on the trace. > + key = 0/0 > + end > + end > + > + local ok, err = pcall(protected) > + subtest:ok(not ok, 'function returns an error') > + subtest:like(err, 'table index is NaN', 'NaN index error message') > +end) > + > +test:test('constant NaN on the trace and rehash of the table', function(subtest) > + -- A little bit different test case: after rehashing the table, > + -- the node is lost, and a hash part of the table isn't freed. > + -- This leads to the assertion failure, which checks memory > + -- leaks on shutdown. > + -- XXX: The test didn't fail even before the patch. Check the > + -- same values as in the previous test for consistency. What do you mean by "didn't fail"? AFAICS, it leads to the assertion failure. If you've meant no fails for a build with no assertions, then it worth clarifying it in this comment.