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 6B90A141D6B; Fri, 2 Dec 2022 11:45:35 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6B90A141D6B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1669970735; bh=rSH7iKYNZnXquUn1w06nhHghCyOZ6rBK8LS8J6fgpFM=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=VqZrn96iypHCW4Xf8bMyoFIPzgK7iwtAefvd4IeiE7y05VZLYwakI030kJsRz1H4Q CY4hymXnz/IEGPoffOuVY3iWukvZ/Aof/9whsZ1RLIdzwkMn19Ko9QQGxFyNlQWtfX Lb9Z2DVQXZkBDvLLjO0Zpmw/xBLQFlEu23SKN25c= Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (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 57CF8141D45 for ; Fri, 2 Dec 2022 11:45:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 57CF8141D45 Received: by smtp61.i.mail.ru with esmtpa (envelope-from ) id 1p11fp-00073B-9x; Fri, 02 Dec 2022 11:45:33 +0300 To: Sergey Ostanevich , Maxim Kokryashkin Date: Fri, 2 Dec 2022 11:42:20 +0300 Message-Id: <20221202084220.23122-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: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD908190A22B884CF14D0AA3A673247BF999E1DF6A3D5584681182A05F5380850400D9E3E412EF1B14871365E65E943B7F1B472A9F8DE5167D60F9FB313F3C35F0F X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE78D182E101C1D8075C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7E3137013C338FE3BEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38B6F1F7B995052D5CE5B3A70955442ECC8C03370266EA690ACCC7F00164DA146DAFE8445B8C89999728AA50765F790063798EAA0EED4DE4A60389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8AC7A5202D916A59BF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947CD72B8CFDDF92C982AD7EC71F1DB884274AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C3A795DF33A22FB294BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE732FCE54C4D9A645443847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5D8EC888B959A8CA2D922BD746EA2FB49179407F1014811974EAF44D9B582CE87C8A4C02DF684249CC203C45FEA855C8F X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3416EA6E382A5BB17617D6BFE943E94A5553374A4E7894259849115239906117C8E7A7A0032F57453C1D7E09C32AA3244C09957A9CB6A9C557A2EBE26B4A12817FF26BFA4C8A6946B8927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojdH6LJGuFcapq/Ifv747Hpg== X-Mailru-Sender: F6248FDC0389C5118060C76C4E47A9E0CECF669284E20188BFE6F0BDEE8B0859B7CBEF92542CD7C88B0A2698F12F5C9EC77752E0C033A69E86920BD37369036789A8C6A0E60D2BB63A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] x64/LJ_GC64: Fix type-check-only variant of SLOAD. 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 Peter Cawley. (cherry picked from commit 05fbdf565c700365d22e38f11478101a0d92a23e) To reproduce the issue we need to assemble `IR_SLOAD` with `IRSLOAD_TYPECHECK` flag set. Also, this IR shouldn't be used later and be not pri, not any number, not any integer type. For GC64 mode, we get the following mcode for this typecheck only variant of the IR: | 55557f6bffc9 mov rdi, [rdx+0x4] | 55557f6bffcd sar rdi, 0x2f | 55557f6bffd1 cmp edi, -0x0b | 55557f6bffd4 jnz 0x55557f6b0010 ->0 This 0x4 addition is excess and crucial: We got the invalid irtype value to compare (due wrong addressing) -- so the assertion guard is always failed and we always exit from the trace. This patch removes this addition. 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-350-fix-sload-typecheck-full-ci Issues: * https://github.com/tarantool/tarantool/issues/7230 * https://github.com/LuaJIT/LuaJIT/pull/350 Tarantool PR: https://github.com/tarantool/tarantool/pull/7995 src/lj_asm_x86.h | 2 +- .../lj-350-sload-typecheck.test.lua | 42 +++++++++++++++++++ .../lj-408-tonumber-cdata-record.test.lua | 10 ----- 3 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 test/tarantool-tests/lj-350-sload-typecheck.test.lua diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h index 8a4d4025..8efda8e5 100644 --- a/src/lj_asm_x86.h +++ b/src/lj_asm_x86.h @@ -1759,7 +1759,7 @@ static void asm_sload(ASMState *as, IRIns *ir) emit_i8(as, irt_toitype(t)); emit_rr(as, XO_ARITHi8, XOg_CMP, tmp); emit_shifti(as, XOg_SAR|REX_64, tmp, 47); - emit_rmro(as, XO_MOV, tmp|REX_64, base, ofs+4); + emit_rmro(as, XO_MOV, tmp|REX_64, base, ofs); #else } else { emit_i8(as, irt_toitype(t)); diff --git a/test/tarantool-tests/lj-350-sload-typecheck.test.lua b/test/tarantool-tests/lj-350-sload-typecheck.test.lua new file mode 100644 index 00000000..6ffc61fb --- /dev/null +++ b/test/tarantool-tests/lj-350-sload-typecheck.test.lua @@ -0,0 +1,42 @@ +local tap = require('tap') +local traceinfo = require('jit.util').traceinfo + +-- Test file to demonstrate the incorrect GC64 JIT asembling +-- `IR_SLOAD`. +-- See also https://github.com/LuaJIT/LuaJIT/pull/350. +local test = tap.test('lj-350-sload-typecheck') + +test:plan(1) + +-- Contains only IR_SLOAD after recording. +local function sload(arg) + return arg +end + +local tab_arg = {} + +-- Reset JIT, remove any other traces. +jit.off() +jit.flush() + +assert(not traceinfo(1), 'no traces compiled after flush') + +-- Try to executed compiled trace wiht IR_SLOAD, if emitted mcode +-- is incorrect, assertion guard type check will failed even for +-- correct type of argument and a new trace is recorded. +jit.opt.start('hotloop=1', 'hotexit=1') + +jit.on() + +-- Make the function hot. +sload(tab_arg) +-- Compile the trace. +sload(tab_arg) +-- Execute trace and try to compile a trace from the side exit. +sload(tab_arg) + +jit.off() + +test:ok(not traceinfo(2), 'the second trace should not be compiled') + +os.exit(test:check() and 0 or 1) diff --git a/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua b/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua index bf9e8e46..a8235e93 100644 --- a/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua +++ b/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua @@ -6,7 +6,6 @@ local tap = require('tap') -- conversions. -- See also https://github.com/LuaJIT/LuaJIT/issues/408, -- https://github.com/LuaJIT/LuaJIT/pull/412, --- https://github.com/LuaJIT/LuaJIT/pull/412, -- https://github.com/tarantool/tarantool/issues/7655. local test = tap.test('lj-408-tonumber-cdata-record') @@ -14,15 +13,6 @@ local NULL = ffi.cast('void *', 0) test:plan(4) --- This test won't fail for GC64 on x86_64. This happens due to --- wrong instruction emitting for SLOAD IR -- we always exit by --- the assertion guard on the argument type check. See also --- https://github.com/LuaJIT/LuaJIT/pull/350. --- The test fails without fix in the current commit, if the --- following commit is backported: --- https://github.com/LuaJIT/LuaJIT/commit/05fbdf56 --- Feel free to remove this comment after backporting of the --- aforementioned commit. local function check(x) -- Don't use a tail call to avoid "leaving loop in root trace" -- error, so the trace will be compiled. -- 2.34.1