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 0464327E4C; Fri, 11 Nov 2022 13:19:55 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0464327E4C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1668161996; bh=GicqW0mFWwrrKGkc9iEozEG0hJx6PQedUGmwKi0R9p8=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=VCVLHP5RgpwVMRoVx0QOTZtCPX0r1lO4ezadyRyFdda3vuNsbvQh19m4RxJ/26/FP H9rahZ8IBs3iP1zOmvWeH+VZlC5oUy9Clp5xcu3D4KlapKfmiSrumSzQJwrOX53CWv lJ1HlfqKCTJ5zpfpgC2ikYL9MnHvDRCmDmGoSbow= Received: from mail-lf1-f42.google.com (mail-lf1-f42.google.com [209.85.167.42]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 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 B424527E46 for ; Fri, 11 Nov 2022 13:19:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B424527E46 Received: by mail-lf1-f42.google.com with SMTP id l12so7665750lfp.6 for ; Fri, 11 Nov 2022 02:19:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fzbK8Vk7Bwh1dFzd44WtxtVwe+dQQLIFrxbzgAtgHLg=; b=2rcDH6/AUs+rvPSP36cx6KUGOveNtkvEy5BT7HJYaYHXyhP0E1jN78gA+Wswe1MgXQ PxxyZIQg4pHTb5tR6yJIXE09TH/9Zzub8ku4RrJ4121FsFG4EHTM6QGxaAOuEMyhRGtI Ice+rkcMX2jmWuUNcQUgqnf4pfE85akGQIMP01Rz7mDTmkR6nO5vCWbRB7R3HAFw9+8m CFu84HaH20nb7vVGxyZVPtMbe4rioLtW1cBL5TQbQAl6l17IBtp0RwYOqF3B0EeH/gpr zz6Z5C/D5QhcHaAmlQqpvIYJBw2YHnMShX+47fPaTgcn7X67uAkXWwK+IBlRmbqxnuQT 8+kQ== X-Gm-Message-State: ANoB5pkrQ7wk7VUB1z/f9yfL0bMEleIVKLC+a7LBKZJXGmL+JSSl9OqC UhFC74Y+tiyIJwG57l2rYywmq+/2XCUyOA== X-Google-Smtp-Source: AA0mqf7kbE1tcW8QyxjQVV0UBy5wcGx0M9T1q3mF0JUnyLoxbDz8GANPJ6kblqvO63tM1rfrzkShtA== X-Received: by 2002:a05:6512:258c:b0:4af:5088:95a7 with SMTP id bf12-20020a056512258c00b004af508895a7mr481309lfb.544.1668161993498; Fri, 11 Nov 2022 02:19:53 -0800 (PST) Received: from localhost.localdomain (89-179-107-73.broadband.corbina.ru. [89.179.107.73]) by smtp.gmail.com with ESMTPSA id a26-20020ac2505a000000b00497aa190523sm247075lfm.248.2022.11.11.02.19.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Nov 2022 02:19:52 -0800 (PST) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, sergos@tarantool.org, skaplun@tarantool.org Date: Fri, 11 Nov 2022 13:19:48 +0300 Message-Id: <20221111101948.6773-1-m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.38.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit] GC64: enable sysprof support 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This patch reverts changes introduced via commit 71e7020637bb15024ceb7e93ce66b61870567339 ('GC64: disable sysprof support'), fixing the incorrect behavior in vm by adding the `LJ_HASSYSPROF` flag to the DynASM flags. Resolves tarantool/tarantool#7919 Related to tarantool/tarantool#781 --- Issue: https://github.com/tarantool/tarantool/issues/7919 Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-7919-sysprof-gc64 PR: https://github.com/tarantool/tarantool/pull/7922 cmake/SetDynASMFlags.cmake | 5 +++ src/lj_arch.h | 2 +- src/lj_obj.h | 2 + src/vm_x64.dasc | 42 ++++++++++++++----- src/vm_x86.dasc | 8 ++++ .../misclib-sysprof-capi.test.lua | 7 ++-- .../misclib-sysprof-lapi.test.lua | 5 +-- 7 files changed, 52 insertions(+), 19 deletions(-) diff --git a/cmake/SetDynASMFlags.cmake b/cmake/SetDynASMFlags.cmake index 8e10758f..142d7e64 100644 --- a/cmake/SetDynASMFlags.cmake +++ b/cmake/SetDynASMFlags.cmake @@ -113,6 +113,11 @@ if(NOT FOUND EQUAL -1) AppendFlags(HOST_C_FLAGS -DLUAJIT_NO_UNWIND) endif() +string(FIND "${TESTARCH}" "LJ_HASSYSPROF 1" FOUND) +if(NOT FOUND EQUAL -1) + list(APPEND DYNASM_FLAGS -D LJ_HASSYSPROF) +endif() + string(REGEX MATCH "LJ_ARCH_VERSION ([0-9]+)" LUAJIT_ARCH_VERSION ${TESTARCH}) list(APPEND DYNASM_FLAGS -D VER=${CMAKE_MATCH_1}) diff --git a/src/lj_arch.h b/src/lj_arch.h index 40129d9e..cd02fee1 100644 --- a/src/lj_arch.h +++ b/src/lj_arch.h @@ -595,7 +595,7 @@ #endif /* Disable or enable the platform and Lua profiler. */ -#if defined(LUAJIT_DISABLE_SYSPROF) || defined(LJ_ARCH_NOSYSPROF) || !LJ_TARGET_LINUX || LJ_GC64 +#if defined(LUAJIT_DISABLE_SYSPROF) || defined(LJ_ARCH_NOSYSPROF) || !LJ_TARGET_LINUX #define LJ_HASSYSPROF 0 #else #define LJ_HASSYSPROF 1 diff --git a/src/lj_obj.h b/src/lj_obj.h index d1451c3a..45507e0d 100644 --- a/src/lj_obj.h +++ b/src/lj_obj.h @@ -674,7 +674,9 @@ typedef struct global_State { MRef jit_base; /* Current JIT code L->base or NULL. */ MRef ctype_state; /* Pointer to C type state. */ GCRef gcroot[GCROOT_MAX]; /* GC roots. */ +#ifdef LJ_HASSYSPROF TValue *top_frame; /* Top frame for sysprof. */ +#endif } global_State; #define mainthread(g) (&gcref(g->mainthref)->th) diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc index 974047d3..59f117ba 100644 --- a/src/vm_x64.dasc +++ b/src/vm_x64.dasc @@ -345,6 +345,22 @@ | mov dword [DISPATCH+DISPATCH_GL(vmstate)], ~LJ_VMST_..st |.endmacro | +|// Stash interpreter's internal base and set current VM state. +|// XXX: Each time profiler sees LFUNC, CFUNC or FFUNC state, it tries +|// to dump Lua calling stack, so it needs a stack base pointer. +|// If the sampling signal arriving during the execution of the VM code, +|// base pointer stored in the current lua_State can be irrelevant, as +|// it syncs with the BASE register only when the control is passed to +|// user code. So we need to sync the BASE on each vmstate change to +|// keep it consistent. +|.macro set_vmstate_sync_base, st +|.if LJ_HASSYSPROF +| set_vmstate INTERP // Guard for non-atomic VM context restoration +| mov qword [DISPATCH+DISPATCH_GL(top_frame)], BASE +|.endif +| set_vmstate st +|.endmacro +| |// Uses TMPRd (r10d). |.macro save_vmstate |.if not WIN @@ -356,6 +372,12 @@ |// Uses r10d. |.macro restore_vmstate |.if not WIN +|.if LJ_HASSYSPROF +| set_vmstate INTERP +| mov TMPR, SAVE_L +| mov TMPR, L:TMPR->base +| mov qword [DISPATCH+DISPATCH_GL(top_frame)], TMPR +|.endif | mov TMPRd, SAVE_VMSTATE | mov dword [DISPATCH+DISPATCH_GL(vmstate)], TMPRd |.endif // WIN @@ -435,7 +457,7 @@ static void build_subroutines(BuildCtx *ctx) | jnz ->vm_returnp | | // Return to C. - | set_vmstate CFUNC + | set_vmstate_sync_base CFUNC | and PC, -8 | sub PC, BASE | neg PC // Previous base = BASE - delta. @@ -725,7 +747,7 @@ static void build_subroutines(BuildCtx *ctx) | cleartp LFUNC:KBASE | mov KBASE, LFUNC:KBASE->pc | mov KBASE, [KBASE+PC2PROTO(k)] - | set_vmstate LFUNC // LFUNC after KBASE restoration. + | set_vmstate_sync_base LFUNC // LFUNC after KBASE restoration. | // BASE = base, RC = result, RB = meta base | jmp RA // Jump to continuation. | @@ -1166,7 +1188,7 @@ static void build_subroutines(BuildCtx *ctx) | |.macro .ffunc, name |->ff_ .. name: - | set_vmstate FFUNC + | set_vmstate_sync_base FFUNC |.endmacro | |.macro .ffunc_1, name @@ -1748,7 +1770,7 @@ static void build_subroutines(BuildCtx *ctx) | movzx RAd, PC_RA | neg RA | lea BASE, [BASE+RA*8-16] // base = base - (RA+2)*8 - | set_vmstate LFUNC // LFUNC state after BASE restoration. + | set_vmstate_sync_base LFUNC // LFUNC state after BASE restoration. | ins_next | |6: // Fill up results with nil. @@ -2513,7 +2535,7 @@ static void build_subroutines(BuildCtx *ctx) | mov KBASE, [KBASE+PC2PROTO(k)] | mov L:RB->base, BASE | mov qword [DISPATCH+DISPATCH_GL(jit_base)], 0 - | set_vmstate LFUNC // LFUNC after BASE & KBASE restoration. + | set_vmstate_sync_base LFUNC // LFUNC after BASE & KBASE restoration. | // Modified copy of ins_next which handles function header dispatch, too. | mov RCd, [PC] | movzx RAd, RCH @@ -2730,7 +2752,7 @@ static void build_subroutines(BuildCtx *ctx) | call extern lj_ccallback_enter // (CTState *cts, void *cf) | // lua_State * returned in eax (RD). | mov BASE, L:RD->base - | set_vmstate LFUNC // LFUNC after BASE restoration. + | set_vmstate_sync_base LFUNC // LFUNC after BASE restoration. | mov RD, L:RD->top | sub RD, BASE | mov LFUNC:RB, [BASE-16] @@ -4299,7 +4321,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) | mov KBASE, LFUNC:KBASE->pc | mov KBASE, [KBASE+PC2PROTO(k)] | // LFUNC after the old BASE & KBASE is restored. - | set_vmstate LFUNC + | set_vmstate_sync_base LFUNC | ins_next | |6: // Fill up results with nil. @@ -4591,7 +4613,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) | ins_AD // BASE = new base, RA = framesize, RD = nargs+1 | mov KBASE, [PC-4+PC2PROTO(k)] | mov L:RB, SAVE_L - | set_vmstate LFUNC // LFUNC after KBASE restoration. + | set_vmstate_sync_base LFUNC // LFUNC after KBASE restoration. | lea RA, [BASE+RA*8] // Top of frame. | cmp RA, L:RB->maxstack | ja ->vm_growstack_f @@ -4629,7 +4651,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) | mov [RD-8], RB // Store delta + FRAME_VARG. | mov [RD-16], LFUNC:KBASE // Store copy of LFUNC. | mov L:RB, SAVE_L - | set_vmstate LFUNC // LFUNC after KBASE restoration. + | set_vmstate_sync_base LFUNC // LFUNC after KBASE restoration. | lea RA, [RD+RA*8] | cmp RA, L:RB->maxstack | ja ->vm_growstack_v // Need to grow stack. @@ -4685,7 +4707,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) | mov CARG1, L:RB // Caveat: CARG1 may be RA. } | ja ->vm_growstack_c // Need to grow stack. - | set_vmstate CFUNC // CFUNC before entering C function. + | set_vmstate_sync_base CFUNC // CFUNC before entering C function. if (op == BC_FUNCC) { | call KBASE // (lua_State *L) } else { diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc index 22f69edf..f7ffe5d2 100644 --- a/src/vm_x86.dasc +++ b/src/vm_x86.dasc @@ -452,8 +452,10 @@ |// user code. So we need to sync the BASE on each vmstate change to |// keep it consistent. |.macro set_vmstate_sync_base, st +|.if LJ_HASSYSPROF | set_vmstate INTERP // Guard for non-atomic VM context restoration | mov dword [DISPATCH+DISPATCH_GL(top_frame)], BASE +|.endif | set_vmstate st |.endmacro | @@ -475,19 +477,25 @@ |// Uses spilled ecx on x86 or XCHGd (r11d) on x64. |.macro restore_vmstate |.if not WIN +|.if LJ_HASSYSPROF | set_vmstate INTERP +|.endif |.if not X64 | mov SPILLECX, ecx +|.if LJ_HASSYSPROF | mov ecx, SAVE_L | mov ecx, L:ecx->base | mov dword [DISPATCH+DISPATCH_GL(top_frame)], ecx +|.endif | mov ecx, SAVE_VMSTATE | mov dword [DISPATCH+DISPATCH_GL(vmstate)], ecx | mov ecx, SPILLECX |.else // X64 +|.if LJ_HASSYSPROF | mov XCHGd, SAVE_L | mov XCHGd, L:XCHGd->base | mov dword [DISPATCH+DISPATCH_GL(top_frame)], XCHGd +|.endif | mov XCHGd, SAVE_VMSTATE | mov dword [DISPATCH+DISPATCH_GL(vmstate)], XCHGd |.endif // X64 diff --git a/test/tarantool-tests/misclib-sysprof-capi.test.lua b/test/tarantool-tests/misclib-sysprof-capi.test.lua index afb99cf2..dad0fe4a 100644 --- a/test/tarantool-tests/misclib-sysprof-capi.test.lua +++ b/test/tarantool-tests/misclib-sysprof-capi.test.lua @@ -1,8 +1,7 @@ -- Sysprof is implemented for x86 and x64 architectures only. -local ffi = require("ffi") -require("utils").skipcond( - jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux" - or ffi.abi("gc64"), +local utils = require("utils") +utils.skipcond( + jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux", jit.arch.." architecture or "..jit.os.. " OS is NIY for sysprof" ) diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua index dbff41b0..4bf10e8d 100644 --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua @@ -1,10 +1,7 @@ -- Sysprof is implemented for x86 and x64 architectures only. -local ffi = require("ffi") local utils = require("utils") - utils.skipcond( - jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux" - or ffi.abi("gc64"), + jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux", jit.arch.." architecture or "..jit.os.. " OS is NIY for sysprof" ) -- 2.38.0