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 D61D96EFDF3; Sat, 18 Nov 2023 19:24:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D61D96EFDF3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1700324688; bh=mc9eQNbriEj/yljERQCdNg9yDOl5gJBKH4DcJjf+qaY=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=oe41NlR3cMf9nRrsGTj1AmBYtT9pntttFJ5swk+/jIHYmk9kfCED23AZS1wLdhBsz kcKI1B0s/CvhxHgmPdGswh9y+cXrmHIniwth1iDOWFpmZk+Av7KU5nq620fePwuoob Q7ibuWjJ58mWPLuKjDcr0RfUmD6FF3sbgSO/erEI= Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [95.163.41.96]) (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 5B4A76EFDF3 for ; Sat, 18 Nov 2023 19:24:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5B4A76EFDF3 Received: by smtp58.i.mail.ru with esmtpa (envelope-from ) id 1r4O7i-00E85x-0K; Sat, 18 Nov 2023 19:24:46 +0300 Message-ID: <53dc17fe-77c9-412f-952a-32f519277b93@tarantool.org> Date: Sat, 18 Nov 2023 19:24:45 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun , Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org References: <20231113150501.28143-1-skaplun@tarantool.org> In-Reply-To: <20231113150501.28143-1-skaplun@tarantool.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD93F1575C7510F554758DA7B170E5CC37B35781EB1F1E8D37200894C459B0CD1B91D6A1160FCA91A2E5D459E6AEFA9A32EB03C9B52580B368F04D08576228FC625 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7BB17EE3498E810FEEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637BB2557E27C12D3EF8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D83403D8BED664694DC9A9F210ECAB1E3C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3A703B70628EAD7BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE140C956E756FBB7A07FB45A5F6E725C8D8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE364E7220B7C5505929735652A29929C6CC4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947C062BEEFFB5F8EA3E2E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F72BE6798D6036352CE5475246E174218B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A58CEC1D01388CF564C42F32F8C5923ABC866A1C66C4FBB915F87CCE6106E1FC07E67D4AC08A07B9B065B78C30F681404DCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFC27B0590ABD94EECB19BA49509B68E316FE97A0F301C0DA6CC8687C54240422C141C0AB2DA7D0DDAFCC135EDF961F15FD15180BA1379053BBD61B4A2DA621FA8E48CAC7CA610320002C26D483E81D6BE0DBAE6F56676BC7117BB6831D7356A2DEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojQSTOZvjxQBUFqGxN0p+Hzg== X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F2588458858219029D6C646798BE945FEE6DB5833D55658DEFB9AEE8282EC151BADDC1D3523A6D01B4765B2DFB59E2DDD9FE06B14FA522850F29BC30B0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [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 Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hello, Sergey! thanks for the patch! LGTM, see minor comments below. Sergey On 11/13/23 18:05, Sergey Kaplun wrote: > 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 IR output would be useful in a test, what do you think? > 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 AFAIK we put all test-related stuff after "test:plan". Feel free to ignore. > +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 With -1 works too, I would replace -10^6 with -1 for simplification.