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 D32B815ABB1; Thu, 8 Dec 2022 08:49:35 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D32B815ABB1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1670478576; bh=FGJCsO3GBWMXRp8WNY/8QTtp1rfger+qHsw/ShtuwgU=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=KZryWwvdhvZHZ3Kfyl/N1XoH533X5LRXEHd6XHebvhgoM/AXg+qzDJ7F1QrBEBl7h GgVgC11UCiKHmQhLSUyKG/qhkQ+QdvZst4Lrv8bOlqXUzelDBfDH6jMnGaS6w8tL0n lgfdO+LJYb3wavKTIA+8Au3iLEwocmzBGqJdoMQ8= Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 02A206FFCC for ; Thu, 8 Dec 2022 08:49:35 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 02A206FFCC Received: by smtp41.i.mail.ru with esmtpa (envelope-from ) id 1p39mn-000535-Vc; Thu, 08 Dec 2022 08:49:34 +0300 To: Sergey Ostanevich , Maksim Kokryashkin Date: Thu, 8 Dec 2022 08:46:18 +0300 Message-Id: <20221208054618.9104-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD908190A22B884CF14F728BD2D4F82E7258B41E11569A987E5182A05F538085040C936EE7C4E44716EB6D1CA19D0FB9AFFBBA16BC8860067BAA1753B196031FBCE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7AED985C8E545F588EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637DC205F3977E1285D8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8E153F2BCFBAFDCF0BC91AEC5AB4F1FAB117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCE1488AC3D4DED311A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735200AC5B80A05675ACDF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C64E7220B7C550592AD7EC71F1DB884274AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C3F8A13C3983F94D3BBA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7588D3C263EAE74EA731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A52946532879DDDE0358B2275CE9D097FED1461958C15F2A824EAF44D9B582CE87C8A4C02DF684249CC203C45FEA855C8F X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D343D50AEDB859DBAD99F823AD42DFC0A0A00588B789B0B70F4AD0EB050A2F94BA721D000AA43E5D1DF1D7E09C32AA3244CDFBB2138AC051D61E82546A9D374A135A8CE788DE6831205927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojuVvIalmfL4cqOf/IBogViA== X-Mailru-Sender: F16D9CAFEEA6770E7B6EAD4ADB3BCAF05B5573F48CE10AF3C4DFA16ABFBD30FD8EBC2AC92E4C644FF2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A84198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] LJ_GC64: Fix ir_khash for non-string GCobj. 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 Contributed by Peter Cawley. (cherry picked from commit b4ed3219a1a98dd9fe7d1e3eeea3b82f5a780948) When emitting `IR_HREF` for constant value to lookup the `ir_khash()` function is used to calculate hash for the corresponding object. This calculation must be the same as in the corresponding `hashkey()` function from . Hash calculating via passing two arguments `lo`, and `hi` to `hashrot()` routine. For non-string GC objects the first `lo` argument is the same for GC64 and not GC64 mode -- lower 32 bits of the object address. For GC64 mode `hi` argument is upper 32 bits of the object address, including specific type NaN-tag. This `hi` argument in `ir_khash()` function is miscalculated in GC64 using non-GC64 value (`lo` + `HASH_BIAS`). As a result, the hash for the GC object is miscalculated on trace and we exit from trace due to assertion guard on the type or value check. This patch fixes calculation of hash value on trace for GC64 mode by making it consistent with `hashkey()`. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#7230 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-356-ir-khash-non-string-obj-full-ci Issue/PR: * https://github.com/tarantool/tarantool/issues/7230 * https://github.com/LuaJIT/LuaJIT/pull/356 Tarantool PR: https://github.com/tarantool/tarantool/pull/8020 Side note: Problems with red fuzzer jobs look irrelevant to the patch. src/lj_asm.c | 4 + .../lj-356-ir-khash-non-string-obj.test.lua | 90 +++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua diff --git a/src/lj_asm.c b/src/lj_asm.c index 1a7fb0c8..a154547b 100644 --- a/src/lj_asm.c +++ b/src/lj_asm.c @@ -1016,7 +1016,11 @@ static uint32_t ir_khash(IRIns *ir) } else { lua_assert(irt_isgcv(ir->t)); lo = u32ptr(ir_kgc(ir)); +#if LJ_GC64 + hi = (uint32_t)(u64ptr(ir_kgc(ir)) >> 32) | (irt_toitype(ir->t) << 15); +#else hi = lo + HASH_BIAS; +#endif } return hashrot(lo, hi); } diff --git a/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua b/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua new file mode 100644 index 00000000..fff0b1a5 --- /dev/null +++ b/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua @@ -0,0 +1,90 @@ +local tap = require('tap') +local traceinfo = require('jit.util').traceinfo +local table_new = require('table.new') + +-- Test file to demonstrate the incorrect GC64 JIT behaviour +-- for `IR_HREF` for on-trace-constant key lookup. +-- See also https://github.com/LuaJIT/LuaJIT/pull/356. +local test = tap.test('lj-356-ir-khash-non-string-obj') +local N_ITERATIONS = 4 + +-- Amount of iteration for trace compilation and execution and +-- additional check, that there is no new trace compiled. +test:plan(N_ITERATIONS + 1) + +-- To reproduce the issue we need to compile a trace with +-- `IR_HREF`, with a lookup of constant hash key GC value. To +-- prevent `IR_HREFK` to be emitted instead, we need a table with +-- a huge hash part. Delta of address between the start of the +-- hash part of the table and the current node to lookup must be +-- more than `(1024 * 64 - 1) * sizeof(Node)`. +-- See , for details. +-- XXX: This constant is well suited to prevent test to be flaky, +-- because the aforementioned delta is always large enough. +local N_HASH_FIELDS = 1024 * 1024 * 8 +local MAGIC = 42 + +local filled_tab = table_new(0, N_HASH_FIELDS + 1) + +-- The function returns constant cdata pinned to `GCproto` to be +-- used as a key for table lookup. +local function get_const_cdata() + return 0LL +end + +-- XXX: don't set `hotexit` to prevent compilation of trace after +-- exiting the main test cycle. +jit.opt.start('hotloop=1') + +-- Prevent `get_const_cdata()` become hot and be compiled before +-- the main test cycle. +jit.off() + +filled_tab[get_const_cdata()] = MAGIC + +-- Speed up table filling-up. +jit.on() + +-- Filling-up the table with GC values to minimize the amount of +-- hash collisions and increases delta between the start of the +-- hash part of the table and currently stored node. +for i = 1, N_HASH_FIELDS do + filled_tab[1LL] = i +end + +-- Prevent JIT misbehaviour before the main test chunk. +jit.off() + +-- Allocate a table with exact array part to be sure that there +-- is no side exit from the trace, due to table reallocation. +local result_tab = table_new(N_ITERATIONS, 0) + +jit.flush() + +assert(not traceinfo(1), 'no traces compiled after flush') + +jit.on() + +for _ = 1, N_ITERATIONS do + -- If the hash for table lookup is miscalculated, then we get + -- `nil` (most possibly) value from the table and the side exit + -- will be taken and we continue execution from the call to + -- `get_const_cdata()`, this function is already hot after the + -- first cycle iteration, and the new trace is recorded. + table.insert(result_tab, filled_tab[get_const_cdata()]) +end + +jit.off() + +test:ok(not traceinfo(2), 'the second trace should not be compiled') + +-- No more need to prevent trace compilation. +jit.on() + +for i = 1, N_ITERATIONS do + -- Check that that all lookups are correct and there is no + -- value from other cdata stored in the table. + test:ok(result_tab[i] == MAGIC, 'correct hash lookup from the table') +end + +os.exit(test:check() and 0 or 1) -- 2.34.1