From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Sergey Ostanevich <sergos@tarantool.org>, Maxim Kokryashkin <m.kokryashkin@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH luajit] LJ_GC64: Make ASMREF_L references 64 bit. Date: Tue, 11 Apr 2023 23:36:50 +0300 [thread overview] Message-ID: <20230411203650.10125-1-skaplun@tarantool.org> (raw) From: Mike Pall <mike> Reported by Yichun Zhang. (cherry picked from commit 850f8c59d3d04a9847f21f32a6c36d8269b5b6b1) The `ASMREF_L` reference is defined as `REF_NIL`, so it isn't considered as 64 bit address. On GC64 mode it may lead to the following assembly: | mov eax, edi so, high 32 bits of the reference are lost. This patch adds `IRT_NIL` to `IRT_IS64` mask, to consider `ASMREF_L` 64 bit long. Now the resulting assembly is the following: | mov rax, rdi False-positive `if` condition in <src/lj_asm.c> is OK, since `op12` already initialized as 0. False-positive `if` condition in <src/lj_opt_sink.c>, <src/lj_opt_split.c>, <src/lj_record.c> is OK, since `REF_NIL` is the last reference before `REF_BASE` and this iteration of a cycle is still the last one. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#8516 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/or-144-gc64-asmref-l Related issues: * https://github.com/openresty/lua-resty-core/issues/144 * https://github.com/tarantool/tarantool/issues/8516 PR: https://github.com/tarantool/tarantool/pull/8553 ML: https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode Don't restrict test case by GC64 mode, because want to test `IR_LREF` for any mode. Keep GC64 in the test name, to be clear where expect the SegFault. src/lj_asm.c | 1 + src/lj_ir.h | 4 ++- src/lj_opt_sink.c | 1 + .../or-144-gc64-asmref-l.test.lua | 28 +++++++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 test/tarantool-tests/or-144-gc64-asmref-l.test.lua diff --git a/src/lj_asm.c b/src/lj_asm.c index a154547b..fd31cd04 100644 --- a/src/lj_asm.c +++ b/src/lj_asm.c @@ -2013,6 +2013,7 @@ static void asm_setup_regsp(ASMState *as) ir->prev = REGSP_INIT; if (irt_is64(ir->t) && ir->o != IR_KNULL) { #if LJ_GC64 + /* The false-positive of irt_is64() for ASMREF_L (REF_NIL) is OK here. */ ir->i = 0; /* Will become non-zero only for RIP-relative addresses. */ #else /* Make life easier for backends by putting address of constant in i. */ diff --git a/src/lj_ir.h b/src/lj_ir.h index 4bad47ed..e8bca275 100644 --- a/src/lj_ir.h +++ b/src/lj_ir.h @@ -375,10 +375,12 @@ typedef struct IRType1 { uint8_t irt; } IRType1; #define irt_isint64(t) (irt_typerange((t), IRT_I64, IRT_U64)) #if LJ_GC64 +/* Include IRT_NIL, so IR(ASMREF_L) (aka REF_NIL) is considered 64 bit. */ #define IRT_IS64 \ ((1u<<IRT_NUM)|(1u<<IRT_I64)|(1u<<IRT_U64)|(1u<<IRT_P64)|\ (1u<<IRT_LIGHTUD)|(1u<<IRT_STR)|(1u<<IRT_THREAD)|(1u<<IRT_PROTO)|\ - (1u<<IRT_FUNC)|(1u<<IRT_CDATA)|(1u<<IRT_TAB)|(1u<<IRT_UDATA)) + (1u<<IRT_FUNC)|(1u<<IRT_CDATA)|(1u<<IRT_TAB)|(1u<<IRT_UDATA)|\ + (1u<<IRT_NIL)) #elif LJ_64 #define IRT_IS64 \ ((1u<<IRT_NUM)|(1u<<IRT_I64)|(1u<<IRT_U64)|(1u<<IRT_P64)|(1u<<IRT_LIGHTUD)) diff --git a/src/lj_opt_sink.c b/src/lj_opt_sink.c index 929ccb61..a16d112f 100644 --- a/src/lj_opt_sink.c +++ b/src/lj_opt_sink.c @@ -219,6 +219,7 @@ static void sink_sweep_ins(jit_State *J) for (ir = IR(J->cur.nk); ir < irbase; ir++) { irt_clearmark(ir->t); ir->prev = REGSP_INIT; + /* The false-positive of irt_is64() for ASMREF_L (REF_NIL) is OK here. */ if (irt_is64(ir->t) && ir->o != IR_KNULL) ir++; } diff --git a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua new file mode 100644 index 00000000..0c352c29 --- /dev/null +++ b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua @@ -0,0 +1,28 @@ +local tap = require('tap') +local test = tap.test('or-144-gc64-asmref-l'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +-- Test file to demonstrate LuaJIT `IR_LREF` assembling incorrect +-- behaviour. +-- See also: +-- * https://github.com/openresty/lua-resty-core/issues/144. +-- * https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode. + +jit.opt.start('hotloop=1') + +local global_env +local _ +for i = 1, 4 do + -- Test `IR_LREF` assembling: using `ASMREF_L` (`REF_NIL`). + global_env = getfenv(0) + -- Need to reuse the register, to cause emitting of `mov` + -- instruction (see `ra_left()` in <src/lj_asm.c>). + _ = tostring(i) +end + +test:ok(global_env == getfenv(0), 'IR_LREF assembling correctness') + +os.exit(test:check() and 0 or 1) -- 2.34.1
next reply other threads:[~2023-04-11 20:40 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-04-11 20:36 Sergey Kaplun via Tarantool-patches [this message] 2023-04-18 16:33 ` Maxim Kokryashkin via Tarantool-patches 2023-05-02 8:13 ` Sergey Kaplun via Tarantool-patches 2023-05-03 8:32 ` sergos via Tarantool-patches 2023-05-03 8:40 ` Sergey Kaplun via Tarantool-patches 2023-05-03 8:56 ` Maxim Kokryashkin via Tarantool-patches 2023-05-25 6:16 ` Igor Munkin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230411203650.10125-1-skaplun@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=m.kokryashkin@tarantool.org \ --cc=sergos@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] LJ_GC64: Make ASMREF_L references 64 bit.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox