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 3DA2E8024B1; Thu, 16 Nov 2023 11:54:35 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3DA2E8024B1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1700124875; bh=hzFxSFLRK+BF7Nn3cm9OamIvxDjdcMc03pYq+TPuTek=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=I/i7QHj+68PYgllOfFQiYkN11H4hMZrp4Cc7yMPSP0JtA3TE+Uf0qle53ZaQBaLw7 0Piz24C/hSbRmAOayQKJZXnCwzIG359o2khN8ZIi5ciIoaORZPxmKxlGj79EkFajls EkZiEnCgkA13DLVIHpwG+u8OI6zrunYFIu4hXY84= Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [95.163.41.98]) (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 C92D38024B1 for ; Thu, 16 Nov 2023 11:54:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C92D38024B1 Received: by smtp60.i.mail.ru with esmtpa (envelope-from ) id 1r3Y8t-00BuSN-36; Thu, 16 Nov 2023 11:54:32 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Thu, 16 Nov 2023 11:49:59 +0300 Message-ID: <20231116084959.24798-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9562B317E0136C8D382B5AB51A0CE7EC9F5A0ADFABA6D1A49182A05F5380850404C228DA9ACA6FE27F6673D8BE520A08C4480C2AD57C32AA79A2B3F6805C65EB24EFFC6CCBFB1FC01 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE74FC9C1E089083C84EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006370218B86C916BF3528638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D80ECF20245263B2A2D834D24EB7B93F10117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3A703B70628EAD7BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE4B6963042765DA4B985B8ACC81218E19D8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE36D3A1509E1113711040F9FF01DFDA4A8C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947CC2B5EEE3591E0D352E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89FBFBFE0634520CEB95E1C53F199C2BB95B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A50B59DA14A3EA1C2044E81E8B306F0F70C7AFFFE36AC0058DF87CCE6106E1FC07E67D4AC08A07B9B082B967D547A19D2F9C5DF10A05D560A950611B66E3DA6D700B0A020F03D25A0997E3FB2386030E77 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF1138764AF436E264FC5CF3BBDA9427BB27527E54ECB44963B041B3ADB2D1A709248F49047001ACAA8AC21204250FC4D01E607C01E02707B14772EB57B7552D16A74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojUgnpwJ++SjRlSWejWUXVug== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769858BD899263AF29A4480C2AD57C32AA781AE5054A97140B9DEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: [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: 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" 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/src/lj_opt_fold.c b/src/lj_opt_fold.c index 944a9ecc..61169397 100644 --- a/src/lj_opt_fold.c +++ b/src/lj_opt_fold.c @@ -1936,7 +1936,10 @@ LJFOLD(NE any any) LJFOLDF(comm_equal) { /* For non-numbers only: x == x ==> drop; x ~= x ==> fail */ - if (fins->op1 == fins->op2 && !irt_isnum(fins->t)) + if (fins->op1 == fins->op2 && + (!irt_isnum(fins->t) || + (fleft->o == IR_CONV && /* Converted integers cannot be NaN. */ + (uint32_t)(fleft->op2 & IRCONV_SRCMASK) - (uint32_t)IRT_I8 <= (uint32_t)(IRT_U64 - IRT_U8)))) return CONDFOLD(fins->o == IR_EQ); return fold_comm_swap(J); } diff --git a/src/lj_record.c b/src/lj_record.c index 3189a7c3..3e4914e1 100644 --- a/src/lj_record.c +++ b/src/lj_record.c @@ -1509,10 +1509,16 @@ TRef lj_record_idx(jit_State *J, RecordIndex *ix) lj_assertJ(!hasmm, "inconsistent metamethod handling"); if (oldv == niltvg(J2G(J))) { /* Need to insert a new key. */ TRef key = ix->key; - if (tref_isinteger(key)) /* NEWREF needs a TValue as a key. */ + if (tref_isinteger(key)) { /* NEWREF needs a TValue as a key. */ key = emitir(IRTN(IR_CONV), key, IRCONV_NUM_INT); - else if (tref_isnumber(key) && tref_isk(key) && tvismzero(&ix->keyv)) - key = lj_ir_knum_zero(J); /* Canonicalize -0.0 to +0.0. */ + } else if (tref_isnum(key)) { + if (tref_isk(key)) { + if (tvismzero(&ix->keyv)) + key = lj_ir_knum_zero(J); /* Canonicalize -0.0 to +0.0. */ + } else { + emitir(IRTG(IR_EQ, IRT_NUM), key, key); /* Check for !NaN. */ + } + } xref = emitir(IRT(IR_NEWREF, IRT_PGC), ix->tab, key); keybarrier = 0; /* NEWREF already takes care of the key barrier. */ #ifdef LUAJIT_ENABLE_TABLE_BUMP 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 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT misbehaviour for NEWREF IR +-- taken NaN value. +-- See also, https://github.com/LuaJIT/LuaJIT/issues/1069. + +local test = tap.test('lj-1069-newref-nan-key'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +local table_new = require('table.new') +local ffi = require('ffi') + +local NaN = tonumber('nan') + +test:plan(4) + +test:test('NaN on trace in the non-constant IR', function(subtest) + local NKEYS = 3 + + -- XXX: NaN isn't stored, so the number of tests is: + -- (NKEYS - 1) + test status + test error message + test keys + -- amount. + subtest:plan(NKEYS + 2) + + local tset_table = table_new(0, NKEYS) + + local function tset(t, k) + -- Value doesn't matter. + t[k] = true + end + + -- Compile the function. + jit.opt.start('hotloop=1') + + -- Use number keys to emit NEWREF. + tset(tset_table, 0.1) + tset(tset_table, 0.2) + + -- Insert NaN on the trace. + local ok, err = pcall(tset, tset_table, NaN) + + subtest:ok(not ok, 'function returns an error') + subtest:like(err, 'table index is NaN', 'correct error message') + + local nkeys = 0 + for k in pairs(tset_table) do + nkeys = nkeys + 1 + subtest:ok(k == k, ('not NaN key by number %d'):format(nkeys)) + end + subtest:is(nkeys, NKEYS - 1, 'correct amount of keys') +end) + +test:test('NaN on trace in the non-constant IR CONV', function(subtest) + local tonumber = tonumber + local NKEYS = 3 + + -- XXX: NaN isn't stored, so the number of tests is: + -- (NKEYS - 1) + test status + test error message + test keys + -- amount. + subtest:plan(NKEYS + 2) + + local tset_table = table_new(0, NKEYS) + + local function tset(t, k) + -- XXX: Emit CONV to number type. Value doesn't matter. + t[tonumber(k)] = true + end + + -- Compile the function. + jit.on() + jit.opt.start('hotloop=1') + + -- Use number keys to emit NEWREF. + tset(tset_table, ffi.new('float', 0.1)) + tset(tset_table, ffi.new('float', 0.2)) + + -- Insert NaN on the trace. + local ok, err = pcall(tset, tset_table, ffi.new('float', NaN)) + + subtest:ok(not ok, 'function returns an error') + subtest:like(err, 'table index is NaN', 'correct error message') + + local nkeys = 0 + for k in pairs(tset_table) do + nkeys = nkeys + 1 + subtest:ok(k == k, ('not NaN key by number %d'):format(nkeys)) + end + subtest:is(nkeys, NKEYS - 1, 'correct amount of keys') +end) + +-- Test the constant IR NaN value on the trace. + +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. + 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' + -- Rehash the table. + tab[counter] = 'unused' + -- 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:done(true) -- 2.42.0