From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 6FD1845C305 for ; Fri, 4 Dec 2020 16:42:39 +0300 (MSK) From: Igor Munkin Date: Fri, 4 Dec 2020 16:42:37 +0300 Message-Id: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit] x64: Fix __call metamethod return dispatch. List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich , Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org From: Mike Pall After linking new cframe to the chain KBASEa still stores the address of the previous one. If the execution proceeds to KBASE value (i.e. low 32 bits of the stored address) might be equal to the current BASE address value so the execution takes the invalid path. Such address clashing occurs only on x86_64 platform with disabled LJ_GC64, so 64-bit registers have to be compared in x64 build. NB: Though there is only 32-bit load to restore BASE value prior to the comparison, the high 32 bits of RDX are reset to zeros, according to x86 long mode semantics. Igor Munkin: * backported the original patch to tarantool/luajit repo * extended the original commit message with the rationale For more info and explanation see LuaJIT/LuaJIT#636. Relates to tarantool/tarantool#4518 Relates to tarantool/tarantool#4649 Signed-off-by: Igor Munkin --- Issues: * https://github.com/tarantool/tarantool/issues/4518 * https://github.com/tarantool/tarantool/issues/4649 Branch: * https://github.com/tarantool/luajit/tree/imun/gh-4518-cmp-64-bit-regs-in-vmeta-call CI is kinda green, considering C6 EOL and the corresponding failures: * https://gitlab.com/tarantool/tarantool/-/pipelines/225349795 @ChangeLog: * Fixed address clashing occurring while __call metamethod dispatching (gh-4518, gh-4649). Unfortunately, there is neither test nor reproducer for this failure, so we'll know that the patch works only on production installations. src/vm_x86.dasc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc index 56bee14..d76fbe3 100644 --- a/src/vm_x86.dasc +++ b/src/vm_x86.dasc @@ -1374,7 +1374,11 @@ static void build_subroutines(BuildCtx *ctx) | mov LFUNC:RB, [RA-8] | add NARGS:RD, 1 | // This is fragile. L->base must not move, KBASE must always be defined. + |.if x64 + | cmp KBASEa, rdx // Continue with CALLT if flag set. + |.else | cmp KBASE, BASE // Continue with CALLT if flag set. + |.endif | je ->BC_CALLT_Z | mov BASE, RA | ins_call // Otherwise call resolved metamethod. -- 2.25.0