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 55D066DA089; Mon, 13 Nov 2023 18:09:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 55D066DA089 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1699888178; bh=/FX8P7jxMrJ7srdjNMA3zoGQnbIPfdZbR0pVX0plE5g=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=XOuDe4YZAL73c6YnmChgyBwp9or7iwvT3BSSo+gQYos99VtKc1XCB1SeePj+Vx+2y xXi9DR94/aENtZeenezFe/4iJ8qXUlMiUsOkJKElVQrlpNmG9qi9vfQbjHChZmNkPq N5C7KjiDrgWrVyOjLuXeSeVIiYSl6Zg6SxsvVyNQ= Received: from smtp55.i.mail.ru (smtp55.i.mail.ru [95.163.41.93]) (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 5CE446B63EC for ; Mon, 13 Nov 2023 18:09:36 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5CE446B63EC Received: by smtp55.i.mail.ru with esmtpa (envelope-from ) id 1r2YZD-007cBR-0c; Mon, 13 Nov 2023 18:09:35 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Mon, 13 Nov 2023 18:05:01 +0300 Message-ID: <20231113150501.28143-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: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9C2A6B03AB739174C420C4E2B999D9DE4AE6436C68FF9734400894C459B0CD1B95CFD1029099644719A8D5D861701E8FFF139952104F82529485234136DCFB6CD X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE752E71F0C64B7C834EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063769AE3176B9E099158638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8D96040C594F9D6DA5A8621981C29C72E117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCE2CCD8F0CAA010FB389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8D166953D3EA3826BF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C643FE6A0CAC512C703F1AB874ED890284AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3461A96A57DA09AA9BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF4964A708C60C975A1DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C3C8F21CEC4765490D35872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A50BCB2F316FC05E760B83CA61187D7DD0E084E01C8DFB4349F87CCE6106E1FC07E67D4AC08A07B9B064E7220B7C5505929C5DF10A05D560A950611B66E3DA6D700B0A020F03D25A0997E3FB2386030E77 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF921AE72B6632A9109690118ADAF8C206C9921DA64BDE81551635CAE95C8C2AA21F5011C79ABBA8E772E3BA2733712135747EECA3A9B9D791DF3BB62E1DC493AEA74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojg8Y/IM6oQGJfSmMIh7QuWw== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769B64DCC5CADA68888D6BDEFA7709AA79921401CF93FD6FA55DEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Fix ABC FOLD rule with constants. 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 XmiliaH. (cherry-picked from commit c8bcf1e5fb8eb72c7e35604fdfd27bba512761bb) `fold_abc_k()` doesn't patch the first ABC check when the right constant operand is negative. This leads to out-of-bounds access from the array on a trace. This patch casts to uint32_t the operands to compare. If the right IR contains a negative integer, the second IR will always be patched. Also, because the ABC check on the trace is unordered, this guard will always fail. Also, this fold rule creates new instructions that reference operands across PHIs. This opens the room for other optimizations (like DCE), so some guards become eliminated, and we use out-of-bounds access from the array part of the table on trace. This patch adds the missing `PHIBARRIER()` check. 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-794-abc-fold-constants Tarantool PR: https://github.com/tarantool/tarantool/pull/9364 Related issues: * https://github.com/LuaJIT/LuaJIT/issues/794 * https://github.com/tarantool/tarantool/issues/9145 src/lj_opt_fold.c | 5 +- .../lj-794-abc-fold-constants.test.lua | 85 +++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 test/tarantool-tests/lj-794-abc-fold-constants.test.lua diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c index 944a9ecc..6175f7c1 100644 --- a/src/lj_opt_fold.c +++ b/src/lj_opt_fold.c @@ -1877,14 +1877,15 @@ LJFOLDF(abc_fwd) LJFOLD(ABC any KINT) LJFOLDF(abc_k) { + PHIBARRIER(fleft); if (LJ_LIKELY(J->flags & JIT_F_OPT_ABC)) { IRRef ref = J->chain[IR_ABC]; IRRef asize = fins->op1; while (ref > asize) { IRIns *ir = IR(ref); if (ir->op1 == asize && irref_isk(ir->op2)) { - int32_t k = IR(ir->op2)->i; - if (fright->i > k) + uint32_t k = (uint32_t)IR(ir->op2)->i; + if ((uint32_t)fright->i > k) ir->op2 = fins->op2; return DROPFOLD; } diff --git a/test/tarantool-tests/lj-794-abc-fold-constants.test.lua b/test/tarantool-tests/lj-794-abc-fold-constants.test.lua new file mode 100644 index 00000000..f8609933 --- /dev/null +++ b/test/tarantool-tests/lj-794-abc-fold-constants.test.lua @@ -0,0 +1,85 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT's incorrect fold optimization +-- for Array Bound Check for constants. +-- ABC(asize, k1), ABC(asize k2) ==> ABC(asize, max(k1, k2)). +-- See also https://github.com/LuaJIT/LuaJIT/issues/794. + +local test = tap.test('lj-794-abc-fold-constants'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +local MAGIC_UNUSED = 42 +test:plan(2) + +local function abc_check_sign() + local tab = {MAGIC_UNUSED} + local return_value = 0 + local abc_limit = 1 + -- No need to run the loop on the first call. We will take + -- the side exit anyway. + for i = 1, 3 do + -- Add an additional ABC check to be merged with. + if i > 1 then + -- luacheck: ignore + return_value = tab[1] + return_value = tab[abc_limit] + -- XXX: Just use some negative number. + abc_limit = -1000000 + end + end + return return_value +end + +jit.opt.start('hotloop=1') + +-- Compile the loop. +abc_check_sign() +-- Run the loop. +test:is(abc_check_sign(), nil, 'correct ABC constant rule for negative values') + +-- Now test the second issue, when ABC optimization applies for +-- operands across PHIs. + +-- XXX: Reset hotcounters to avoid collisions. +jit.opt.start('hotloop=1') + +local tab_array = {} +local small_tab = {MAGIC_UNUSED} +local full_tab = {} + +-- First, create tables with different asizes, to be used in PHI. +-- Create a large enough array part for the noticeable +-- out-of-bounds access. +for i = 1, 8 do + full_tab[i] = MAGIC_UNUSED +end + +-- We need 5 iterations to execute both the variant and the +-- invariant parts of the trace below. +for i = 1, 5 do + -- On the 3rd iteration, the recording is started. + if i > 3 then + tab_array[i] = small_tab + else + tab_array[i] = full_tab + end +end + +local result +local alias_tab = tab_array[1] +-- Compile a trace. +-- Run 5 iterations to execute both the variant and the invariant +-- parts. +for i = 1, 5 do + local local_tab = alias_tab + alias_tab = tab_array[i] + -- Additional ABC check to fold. + -- luacheck: ignore + result = alias_tab[1] + result = local_tab[8] +end + +test:is(result, nil, 'correct ABC constant rule across PHI') + +test:done(true) -- 2.42.0