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 DA79F125DDAB; Thu, 27 Feb 2025 19:00:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DA79F125DDAB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1740672060; bh=k1kIh3bauD2jIY2N3HvzZX/UcW8kWGgut2fGrg8ZEpk=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=tmd42fc5UJ7v/1rMfEj8i4Uil/7oqwF0G52DkPB+u6SCHkXZ+HMYhozpqPtSb7Csu CJQfGTzesX3m2+E9H4HaB9Eq8XksTYMXJrcZxBpbbHQ3AHD9WLZZntlrEYKe8epkQR MJU3PJaqTqjXQa3dijFx1stIJcv15N7j9FfUWujo= Received: from send194.i.mail.ru (send194.i.mail.ru [95.163.59.33]) (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 12C37125DDA8 for ; Thu, 27 Feb 2025 19:00:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 12C37125DDA8 Received: by exim-smtp-6559db9d9c-b4bgz with esmtpa (envelope-from ) id 1tngJl-00000000AGc-0SjO; Thu, 27 Feb 2025 19:00:57 +0300 To: Sergey Bronnikov Date: Thu, 27 Feb 2025 19:00:13 +0300 Message-ID: <20250227160013.2040-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.48.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD94C6F4C9F2485915148C6813F9D0BFE22706994934ADC424B00894C459B0CD1B91E1E2B8E202BC64CC591814E25D11F9F41C694026C7BF66BE30DC022C37A180F854E8B091AFA3944 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7FEAC828D2BF6EC3CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375662B0B15A2BD47BC347C064DCC89F7F77401DF4C38237AD596376D2E28E0A88ABC389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C045A75973B56231AD8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B601F8F2FECC0250C8CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7E1BCFB2C0BE3F189731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5982EA3F2677970165002B1117B3ED69632B061D1B278DF1BE20DC3F561CE4150823CB91A9FED034534781492E4B8EEAD13926AAAFEFA6645C79554A2A72441328621D336A7BC284946AD531847A6065A535571D14F44ED41 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34B9F55CA4D2956E3079256C46310EF2447EBC2432DF53D6A270A5317EED66B83467C979D2A05754AB1D7E09C32AA3244C4E9ED8A13B18958977DD89D51EBB774266C3870336B9E8A6EA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVQQG/FugD0/Cd+/jpbivvrs= X-DA7885C5: 8A9CA7EEDCB2B7D6F255D290C0D534F949D424B0241FE2349FA61968C5504986E42206D4A1389B3D5B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F739381B31377CF4CA219844153907CA98A425EA37C16AF8EB01996E1D5F710375621E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Fix IR_ABC hoisting. 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 Reported by pwnhacker0x18. Fixed by Peter Cawley. (cherry picked from commit 7369eff67d46d7f5fac9ee064e3fbf97a15458de) The `IR_ABC` has 2 different types: * `IRT_INT` for generic condition check. * `IRT_P32` as a marker for the invariant checks for upper and lower boundaries of the loop index. These checks may be dropped during the folding optimization in recording the variant part of the loop. The checks may be dropped if the instruction is not PHI, the first operand is invariant, and the upper bound of the array part of the table isn't changed, or the table itself isn't changed on the trace. The checks in the `abc_invar` fold rule assume that the constant values of the first operand of ABC may be dropped. But when this constant value comes from the folding chain that does CSE, the check is still necessary if the original instruction (not resulting after loop optimization substitution and the folding chain) isn't a constant. Hence, this patch additionally singularizes the `IRT_U32` type for the `IR_ABC` in the case when the asize is a constant on the trace so it can be simply dropped from the invariant part of the loop. For `IRT_P32` type, this patch adds a check that the first operand isn't a constant to be sure that it isn't a result of the fold chain. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11055 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1194-abc-hoisting Related issues: * https://github.com/LuaJIT/LuaJIT/issues/1194 * https://github.com/tarantool/tarantool/issues/11055 src/lj_opt_fold.c | 5 +- src/lj_record.c | 5 +- .../lj-1194-abc-hoisting.test.lua | 77 +++++++++++++++++++ 3 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 test/tarantool-tests/lj-1194-abc-hoisting.test.lua diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c index cd4395bb..83105816 100644 --- a/src/lj_opt_fold.c +++ b/src/lj_opt_fold.c @@ -1906,9 +1906,10 @@ LJFOLDF(abc_k) LJFOLD(ABC any any) LJFOLDF(abc_invar) { - /* Invariant ABC marked as PTR. Drop if op1 is invariant, too. */ + /* Invariant ABC marked as P32 or U32. Drop if op1 is invariant too. */ if (!irt_isint(fins->t) && fins->op1 < J->chain[IR_LOOP] && - !irt_isphi(IR(fins->op1)->t)) + (irt_isu32(fins->t) || + (!irref_isk(fins->op1) && !irt_isphi(IR(fins->op1)->t)))) return DROPFOLD; return NEXTFOLD; } diff --git a/src/lj_record.c b/src/lj_record.c index 5345fa63..3dc6e840 100644 --- a/src/lj_record.c +++ b/src/lj_record.c @@ -1315,12 +1315,13 @@ static void rec_idx_abc(jit_State *J, TRef asizeref, TRef ikey, uint32_t asize) /* Runtime value for stop of loop is within bounds? */ if ((uint64_t)stop + ofs < (uint64_t)asize) { /* Emit invariant bounds check for stop. */ - emitir(IRTG(IR_ABC, IRT_P32), asizeref, ofs == 0 ? J->scev.stop : + uint32_t abc = IRTG(IR_ABC, tref_isk(asizeref) ? IRT_U32 : IRT_P32); + emitir(abc, asizeref, ofs == 0 ? J->scev.stop : emitir(IRTI(IR_ADD), J->scev.stop, ofsref)); /* Emit invariant bounds check for start, if not const or negative. */ if (!(J->scev.dir && J->scev.start && (int64_t)IR(J->scev.start)->i + ofs >= 0)) - emitir(IRTG(IR_ABC, IRT_P32), asizeref, ikey); + emitir(abc, asizeref, ikey); return; } } diff --git a/test/tarantool-tests/lj-1194-abc-hoisting.test.lua b/test/tarantool-tests/lj-1194-abc-hoisting.test.lua new file mode 100644 index 00000000..3a78c34e --- /dev/null +++ b/test/tarantool-tests/lj-1194-abc-hoisting.test.lua @@ -0,0 +1,77 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT's incorrect hoisting out of the +-- loop for Array Bound Check. +-- See also https://github.com/LuaJIT/LuaJIT/issues/1194. + +local test = tap.test('lj-1194-abc-hoisting'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +local table_new = require('table.new') + +-- Before the patch, the `for` cycle in the `test_func()` below +-- produces the following trace: +-- +-- | 0006 int FLOAD 0005 tab.asize +-- | 0007 > p32 ABC 0006 0001 +-- | 0008 p32 FLOAD 0005 tab.array +-- | 0009 p32 AREF 0008 0003 +-- | 0010 tab FLOAD 0005 tab.meta +-- | 0011 > tab EQ 0010 NULL +-- | 0012 nil ASTORE 0009 nil +-- | 0013 >+ tab TNEW #0 #0 +-- | 0014 + int ADD 0003 +1 +-- | 0015 > int LE 0014 0001 +-- | 0016 ------ LOOP ------------ +-- | 0017 > int NE 0014 +0 +-- | 0018 p32 FLOAD 0013 tab.array +-- | 0019 p32 AREF 0018 0014 +-- | 0020 nil ASTORE 0019 nil +-- +-- As you can see, the `0007 ABC` instruction is dropped from the +-- variant part of the loop. + +-- Disable fusing to simplify the reproducer it now will be crash +-- on loading of an address from the `t->array`. +jit.opt.start('hotloop=1', '-fuse') + +local function test_func() + -- 1 iteration for hotcount warm-up. 2 and 3 to record invariant + -- and variant parts of the loop. The trace is run via an + -- additional call to this function. + local limit = 3 + -- Create a table with a fixed array size (`limit` + 1), so all + -- access to it fits into the array part. + local s = table_new(limit, 0) + for i = 1, limit do + -- Don't change the table on hotcount warm-up. + if i ~= 1 then + -- `0020 ASTORE` causes the SegFault on the trace on the + -- last (3rd) iteration, since the table (set on the first + -- iteration) has `asize == 0`, and t->array == NULL`. + -- luacheck: no unused + s[i] = nil + s = {} + end + end +end + +-- Compile the `for` trace inside `test_func()`. +-- The invariant part of this trace contains the ABC check, while +-- the invariant does not. So, first compile the trace, then use +-- the second call to run it from the beginning with all guards +-- passing in the invariant part. +test_func() + +-- Avoid compilation of any other traces. +jit.off() + +-- Run that trace. +test_func() + +test:ok(true, 'no crash on the trace due to incorrect ABC hoisting') + +test:done(true) -- 2.48.1