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 C3F80464E96; Tue, 23 May 2023 18:27:55 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C3F80464E96 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1684855675; bh=js7Q8hObvSqoDNgrc0lgIR+eb2ngcX1NgJXaiwQM8jM=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=OEYysYospSBxUYCO7KT7NNcVFbF4iA93gT8EiFVDtX5vAE/RNt52do3vnencSUZNQ nuKY1+roGDBbJKAxWEB4u+vo4ipOP3JfUKGPXp1FFNYc6XW3tG4uALpCwdSlCzBaH1 dWWXrvN4znl4eQVs8/CTXIw0BpVmii1Z76oyHSao= Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) (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 86E922ACFC1 for ; Tue, 23 May 2023 18:27:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 86E922ACFC1 Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-4f3b314b1d7so4615640e87.1 for ; Tue, 23 May 2023 08:27:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684855673; x=1687447673; 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=2iDKqzErqiMOyyDisvR68D3/z3Vl/dypyhfCh3dJPnw=; b=J8Ga5oC5QuOnoxkAVYCfLr/4gRieV5i2UXByeyHa00wwHAU1XKIZJoE7HctadETGUo Uc/VSjD53ceg85hQR/ddxTdzSjHUalS58cjZ/t1yZftneB+hrXKLHq0wFRATCLqXPu31 PIZoLiZ58TxuBznR2gfolWtVmOXdgcZvXsKbTbtEb50GO/sMsvCbx7us5WMyNt3qItYN mzpzUH/2vqn1n45BBRnMRfZocGb6L/Pl04WMt+6ZQxxCr95vXN42Stce90hlDgK88duK gHWlA31y8LcqVGhnHIrNd8jt1YoxtDLOq7zXtG17+V1TVwh3S2qNL0vabk//H/RxusG0 GzXA== X-Gm-Message-State: AC+VfDwQPA+0vvgJ1Umhy1hmgxmj5wj5iyHmBsMfQnBy6no0E0tCu1gH dLlmHnrPSFeS5ArSXCwuNLGJ0PLZJIa1jQ== X-Google-Smtp-Source: ACHHUZ4VsK+Kk6PseSRoNnhuliq/CZu6ApLYbodX5w9TuEYaEUjC494HG5hUWWxo+s72ZqToIparnA== X-Received: by 2002:ac2:5a0b:0:b0:4ef:e87e:df88 with SMTP id q11-20020ac25a0b000000b004efe87edf88mr4419199lfn.64.1684855673133; Tue, 23 May 2023 08:27:53 -0700 (PDT) Received: from fckxorg.mail.msk ([2a00:1148:b0ba:16:a3e8:bdc1:dbed:dbc8]) by smtp.gmail.com with ESMTPSA id y15-20020ac2446f000000b004f140788184sm1373692lfl.289.2023.05.23.08.27.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 May 2023 08:27:52 -0700 (PDT) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergos@tarantool.org Date: Tue, 23 May 2023 18:27:48 +0300 Message-Id: <20230523152748.203212-1-m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit] sysprof: move symtab update into profile hook 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" Before the patch, the symtab update was done in the signal handler. That update requires memory allocation, which can't be done safely in a signal handler. This patch reuses LuaJIT's HOOK_PROFILE for symtab update routine execution, so it is now possible to update it safely after the signal handler exit. Resolves tarantool/tarantool#8140 --- Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-8140-sysprof-allocator-crash Issue: https://github.com/tarantool/tarantool/issues/8140 PR: https://github.com/tarantool/tarantool/pull/8691 src/lj_dispatch.c | 10 ++++ src/lj_sysprof.c | 34 +++++++++++++- src/lj_sysprof.h | 4 ++ test/tarantool-tests/CMakeLists.txt | 1 + .../gh-8140-sysprof-allocator-crash.test.lua | 46 +++++++++++++++++++ .../CMakeLists.txt | 9 ++++ .../sysprofalloc.c | 17 +++++++ 7 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua create mode 100644 test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt create mode 100644 test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c index ee735450..65948a0c 100644 --- a/src/lj_dispatch.c +++ b/src/lj_dispatch.c @@ -30,6 +30,9 @@ #if LJ_HASPROFILE #include "lj_profile.h" #endif +#if LJ_HASSYSPROF +#include "lj_sysprof.h" +#endif #include "lj_vm.h" #include "luajit.h" @@ -552,7 +555,14 @@ void LJ_FASTCALL lj_dispatch_profile(lua_State *L, const BCIns *pc) global_State *g; setcframe_pc(cf, pc); L->top = L->base + cur_topslot(pt, pc, cframe_multres_n(cf)); +#if LJ_HASSYSPROF + if (lj_symtab_update_requested()) + lj_symtab_update_hook(L); + else + lj_profile_interpreter(L); +#else lj_profile_interpreter(L); +#endif setcframe_pc(cf, oldpc); g = G(L); setgcref(g->cur_L, obj2gco(L)); diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c index 2e9ed9b3..5b5cbe2a 100644 --- a/src/lj_sysprof.c +++ b/src/lj_sysprof.c @@ -76,6 +76,7 @@ struct sysprof { lj_profile_timer timer; /* Profiling timer. */ int saved_errno; /* Saved errno when profiler failed. */ uint32_t lib_adds; /* Number of libs loaded. Monotonic. */ + volatile sig_atomic_t symtab_update_needed; /* Symtab update request flag. */ }; /* ** XXX: Only one VM can be profiled at a time. @@ -88,6 +89,36 @@ static struct sysprof sysprof = {0}; static const uint8_t ljp_header[] = {'l', 'j', 'p', LJP_FORMAT_VERSION, 0x0, 0x0, 0x0}; +void lj_symtab_update_hook(lua_State *L) { + struct sysprof *sp = &sysprof; + global_State *g = G(L); + uint8_t mask; + mask = (g->hookmask & ~HOOK_PROFILE); + sp->symtab_update_needed = 0; + if (!(mask & HOOK_VMEVENT)) { + g->hookmask = HOOK_VMEVENT; + lj_dispatch_update(g); + lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_CFUNC_EVENT, L); + } + g->hookmask = mask; + lj_dispatch_update(g); +} + +int lj_symtab_update_requested() { + struct sysprof *sp = &sysprof; + return sp->symtab_update_needed; +} + +static void setup_symtab_update_hook(struct sysprof *sp) { + global_State *g = sp->g; + uint8_t mask = g->hookmask; + if (!(mask & (HOOK_PROFILE|HOOK_VMEVENT|HOOK_GC))) { + sp->symtab_update_needed = 1; + g->hookmask = (mask | HOOK_PROFILE); + lj_dispatch_update(g); + } +} + static int stream_is_needed(struct sysprof *sp) { return sp->opt.mode != LUAM_SYSPROF_DEFAULT; @@ -240,8 +271,7 @@ static void stream_guest(struct sysprof *sp, uint32_t vmstate) static void stream_host(struct sysprof *sp, uint32_t vmstate) { - struct lua_State *L = gco2th(gcref(sp->g->cur_L)); - lj_symtab_dump_newc(&sp->lib_adds, &sp->out, LJP_SYMTAB_CFUNC_EVENT, L); + setup_symtab_update_hook(sp); lj_wbuf_addbyte(&sp->out, (uint8_t)vmstate); stream_backtrace_host(sp); } diff --git a/src/lj_sysprof.h b/src/lj_sysprof.h index 7e8c2e6e..456ca76f 100644 --- a/src/lj_sysprof.h +++ b/src/lj_sysprof.h @@ -108,4 +108,8 @@ void lj_sysprof_add_proto(const struct GCproto *pt); void lj_sysprof_add_trace(const struct GCtrace *tr); #endif /* LJ_HASJIT */ +void lj_symtab_update_hook(lua_State *L); + +int lj_symtab_update_requested(); + #endif diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt index a428d009..25244733 100644 --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -61,6 +61,7 @@ add_subdirectory(gh-5813-resolving-of-c-symbols/gnuhash) add_subdirectory(gh-5813-resolving-of-c-symbols/stripped) add_subdirectory(gh-6098-fix-side-exit-patching-on-arm64) add_subdirectory(gh-6189-cur_L) +add_subdirectory(gh-8140-sysprof-allocator-crash) add_subdirectory(lj-49-bad-lightuserdata) add_subdirectory(lj-416-xor-before-jcc) add_subdirectory(lj-601-fix-gc-finderrfunc) diff --git a/test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua b/test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua new file mode 100644 index 00000000..afce83e2 --- /dev/null +++ b/test/tarantool-tests/gh-8140-sysprof-allocator-crash.test.lua @@ -0,0 +1,46 @@ +local tap = require('tap') +local test = tap.test('gh-8140-sysprof-allocator-crash'):skipcond({ + ["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and + jit.arch ~= "x64", + ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux", +}) +test:plan(2) + +local profilename = require("utils").profilename +local profile = require('jit.profile') + +local TMP_BINFILE = profilename("sysprofdata.tmp.bin") +local callback_called = false + +local function payload() + local r = 0 + for i = 1, 1e8 do + r = r + i + end + return r +end + +local function callback(_, _, _) + callback_called = true +end + +profile.start('f', callback) +payload() +profile.stop() + +test:ok(callback_called, 'LuaJIT profiler callback was not called.') + +jit.off() +misc.sysprof.start{mode='C', interval=1, path=TMP_BINFILE} +for _ = 1, 1e4 do + require('sysprofalloc').get_string() + -- Make sure that C library is collected, so it will be loaded + -- again on the next iteration. + collectgarbage() +end +misc.sysprof.stop() + +test:ok(true, 'Sysprof has crashed.') + +os.remove(TMP_BINFILE) +os.exit(test:check() and 0 or 1) diff --git a/test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt new file mode 100644 index 00000000..dbe60219 --- /dev/null +++ b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/CMakeLists.txt @@ -0,0 +1,9 @@ +if (NOT(CMAKE_SYSTEM_NAME STREQUAL "Darwin")) + BuildTestCLib(sysprofalloc sysprofalloc.c) + # Unfortunately, command is introduced + # since CMake 3.13, so we can't use it now considering ancient + # distros support. Just build linker flags by hands. + set(CMAKE_SHARED_LINKER_FLAGS + "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--hash-style=both" + ) +endif() diff --git a/test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c new file mode 100644 index 00000000..d3c41d42 --- /dev/null +++ b/test/tarantool-tests/gh-8140-sysprof-allocator-crash/sysprofalloc.c @@ -0,0 +1,17 @@ +#include +#include + +int get_string(lua_State *L) { + lua_pushstring(L, "test string"); + return 1; +} + +static const struct luaL_Reg sysprofalloc [] = { + {"get_string", get_string}, + {NULL, NULL} +}; + +int luaopen_sysprofalloc(lua_State *L) { + luaL_register(L, "sysprofalloc", sysprofalloc); + return 1; +} -- 2.40.1