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 04061503767; Mon, 11 Dec 2023 18:39:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 04061503767 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1702309196; bh=ka6LlPDB9PInPkp3kaCHiOvwVLSMjAiRxXaBoLIQBEs=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=Uan54PnZuuO1elnvcuTIIrrxpNsbUC7T3uJGV4DgpbOPaNooEjqWPWGjAWVBBid+5 8AUhIKm48czU+Afn5j5TFuNVBVJEZODbJJyUE+ZwXObG3LPaQ7c5u+O1wkhQ+xJ/e6 qYFSlvJp5gRAdy0mVDt5Xv0YAq87XObPkxZYyCuk= 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 4D72F503767 for ; Mon, 11 Dec 2023 18:39:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4D72F503767 Received: by smtp36.i.mail.ru with esmtpa (envelope-from ) id 1rCiNt-00B1VE-1m; Mon, 11 Dec 2023 18:39:54 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Mon, 11 Dec 2023 18:35:20 +0300 Message-ID: <20231211153520.9322-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD97533543916A0F71AB5E10F7FD024E024A75B922889D28DA8CD62213F67905E7A52CC39746D39336B9583C23FD049A75A4D6B21BC79D0CED9B854597C55E94BBE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7C4470ABEE4BC4B50EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006378ABD31E9FF1CD53C8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D801FF178069B884085332172F0985A822117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC55B19328CBC4F849A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE26055571C92BF10FC0837EA9F3D197644AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3A7AC412AE061D85076E601842F6C81A1F004C906525384303E02D724532EE2C3F43C7A68FF6260569E8FC8737B5C2249F459A8243F1D1D44E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B603913CCF128142C9089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5DBDFF94E96070300B70500EE58233691E69048E04F6EDF1BF87CCE6106E1FC07E67D4AC08A07B9B01F9513A7CA91E5559C5DF10A05D560A950611B66E3DA6D700B0A020F03D25A0997E3FB2386030E77 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF10705DD7D894C2D2DBC82C73A7FFAD611A9C5E0AF042C98922BC0E1C53715574110A92F5D16EB67FFC43826C574F1C6FBF8174AFE98FB38992D0590ED46B4C2BA74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXgpSsePXpezQ4hjTLaEyXYj X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769B9887DF20A1C0099B6385D52CAB5A9CF87D154BA7A9C8918DEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Emit sunk IR_NEWREF only once per key on snapshot replay. 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 Sergey Kaplun and Peter Cawley. (cherry-picked from commit 1761fd2ef79ffe1778011c7e9cb03ed361b48c5e) Assume we have the parent trace with the following IRs: | 0001 } tab TNEW #0 #0 | 0002 } p32 NEWREF 0001 "key" | 0003 } fal HSTORE 0002 false | .... SNAP #1 [ ---- ---- 0001 ---- ] | 0004 > num SLOAD #1 T | .... SNAP #2 [ ---- ---- 0001 ] | 0005 > num EQ 0004 0004 | 0006 } tru HSTORE 0002 true | .... SNAP #3 [ ---- ---- 0001 true ] The side trace for the third snapshot emits the following IRs: | 0001 tab TNEW #0 #0 | 0002 p32 NEWREF 0001 "key" | 0003 fal HSTORE 0002 false | 0004 p32 NEWREF 0001 "key" | 0005 tru HSTORE 0004 true As we can see, `NEWREF` is emitted twice. This is a violation of its semantics, so the second store isn't noticeable. This patch prevents the second emitting of IR NEWREF by checking the last one emitted NEWREF IR. There is no need to check NEWREFs beyond since it guarantees the snapshot is taken after it, because it may cause table rehashing, so all prior results are invalidated. Sergey Kaplun: * added the description and the test for the problem Resolves tarantool/tarantool#7937 Part of tarantool/tarantool#9145 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1128-double-ir-newref-on-restore-sunk PR: https://github.com/tarantool/tarantool/pull/9466 Related issues: * https://github.com/LuaJIT/LuaJIT/issues/1128 * https://github.com/tarantool/tarantool/issues/7937 src/lj_snap.c | 16 ++++ ...-double-ir-newref-on-restore-sunk.test.lua | 81 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 test/tarantool-tests/lj-1128-double-ir-newref-on-restore-sunk.test.lua diff --git a/src/lj_snap.c b/src/lj_snap.c index 3f0fccec..73e18e69 100644 --- a/src/lj_snap.c +++ b/src/lj_snap.c @@ -583,9 +583,25 @@ void lj_snap_replay(jit_State *J, GCtrace *T) if (irr->o == IR_HREFK || irr->o == IR_AREF) { IRIns *irf = &T->ir[irr->op1]; tmp = emitir(irf->ot, tmp, irf->op2); + } else if (irr->o == IR_NEWREF) { + IRRef allocref = tref_ref(tr); + IRRef keyref = tref_ref(key); + IRRef newref_ref = J->chain[IR_NEWREF]; + IRIns *newref = &J->cur.ir[newref_ref]; + lj_assertJ(irref_isk(keyref), + "sunk store for parent IR %04d with bad key %04d", + refp - REF_BIAS, keyref - REF_BIAS); + if (newref_ref > allocref && newref->op2 == keyref) { + lj_assertJ(newref->op1 == allocref, + "sunk store for parent IR %04d with bad tab %04d", + refp - REF_BIAS, allocref - REF_BIAS); + tmp = newref_ref; + goto skip_newref; + } } } tmp = emitir(irr->ot, tmp, key); + skip_newref: val = snap_pref(J, T, map, nent, seen, irs->op2); if (val == 0) { IRIns *irc = &T->ir[irs->op2]; diff --git a/test/tarantool-tests/lj-1128-double-ir-newref-on-restore-sunk.test.lua b/test/tarantool-tests/lj-1128-double-ir-newref-on-restore-sunk.test.lua new file mode 100644 index 00000000..a89beab6 --- /dev/null +++ b/test/tarantool-tests/lj-1128-double-ir-newref-on-restore-sunk.test.lua @@ -0,0 +1,81 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT incorrect restoring of sunk +-- tables with double usage of IR_NEWREF. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1128. + +local test = tap.test('lj-1128-double-ir-newref-on-restore-sunk'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(3) + +local take_side + +local function trace_base(num) + local tab = {} + tab.key = false + -- This check can't be folded since `num` can be NaN. + tab.key = num == num + -- luacheck: ignore + -- This side trace emits the following IRs: + -- 0001 tab TNEW #0 #0 + -- 0002 p64 NEWREF 0001 "key" + -- 0003 fal HSTORE 0002 false + -- 0004 p64 NEWREF 0001 "key" + -- 0005 tru HSTORE 0004 true + -- As we can see, `NEWREF` is emitted twice. This is a violation + -- of its semantics, so the second store isn't noticeable. + if take_side then end + return tab.key +end + +-- Uncompiled function to end up side trace here. +local function trace_base_wp(num) + return trace_base(num) +end +jit.off(trace_base_wp) + +-- Same function as above, but with two IRs NEWREF emitted. +local function trace_2newref(num) + local tab = {} + tab.key = false + -- This + op can't be folded since `num` can be -0. + tab.key = num + 0 + tab.key2 = false + -- This check can't be folded since `num` can be NaN. + tab.key2 = num == num + -- luacheck: ignore + if take_side then end + return tab.key, tab.key2 +end + +-- Uncompiled function to end up side trace here. +local function trace_2newref_wp(num) + return trace_2newref(num) +end +jit.off(trace_2newref_wp) + +jit.opt.start('hotloop=1', 'hotexit=1', 'tryside=1') + +-- Compile parent traces. +trace_base_wp(0) +trace_base_wp(0) +trace_2newref_wp(0) +trace_2newref_wp(0) + +-- Compile side traces. +take_side = true +trace_base_wp(0) +trace_base_wp(0) +trace_2newref_wp(0) +trace_2newref_wp(0) + +test:is(trace_base(0), true, 'sunk value restored correctly') + +local arg = 0 +local r1, r2 = trace_2newref(arg) +test:is(r1, arg, 'sunk value restored correctly with 2 keys, first key') +test:is(r2, true, 'sunk value restored correctly with 2 keys, second key') + +test:done(true) -- 2.43.0