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 DF751E8D9D9; Mon, 27 Jan 2025 13:18:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DF751E8D9D9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1737973139; bh=wBlm9/ffJ4NrEmCsNAfCgiGu/4UG+T1RDNQEYnysmqg=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=jnIcRLTX7mfCxBupAXn7PA0FyXUQHpM8uVoOTZecH+bovOrW/DcpG4b5ag2LJzFJ8 YFkBJ0AslLXdJBrNLs5cShjwslFU/sNUN00HSLT6Z8tIEoC7mt1hDSiFRyjnrhW+wg 004sOXwLu5Kuj6txbtItaLU0GOCx44nRGlRPhsSA= Received: from send35.i.mail.ru (send35.i.mail.ru [89.221.237.130]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 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 3BDCE512ECE for ; Mon, 27 Jan 2025 13:18:57 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3BDCE512ECE Received: by exim-smtp-5bf6658466-z972b with esmtpa (envelope-from ) id 1tcMCm-00000000ALl-0owU; Mon, 27 Jan 2025 13:18:56 +0300 To: Sergey Bronnikov Date: Mon, 27 Jan 2025 13:18:14 +0300 Message-ID: <20250127101814.23617-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.47.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD948575D5A188668289760662F93C09369D033A68A439925CA182A05F53808504058C1784A143E9B3C3DE06ABAFEAF6705DF09A4E9052CFE0ED568772B823B3C89880CFDCFA9C15CC8 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E50EC9128971FD6EEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006377F497DA9C8A5F4D48638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8DE73A087E1ED4038BF6CD62543987012C6863A408DD9F6F0CC7F00164DA146DAFE8445B8C89999728AA50765F7900637BA939FD1B3BAB99B389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8D23BF7408B3F9022F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE33AC447995A7AD182BEBFE083D3B9BA73A03B725D353964B062BEEFFB5F8EA3E35872C767BF85DA227C277FBC8AE2E8B9BFB91CAEB05C77775ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A53839B998986D56805002B1117B3ED696A5A6088F5819ACD292212597CCBD6D77823CB91A9FED034534781492E4B8EEAD09BC2A87FC68FC48C79554A2A72441328621D336A7BC284946AD531847A6065A535571D14F44ED41 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF1BCEB71DAB08CC40E948CBAF843B4010A10E76A81D23175E7A64BA1E041B46FFCCDF2CE9FE05F2E7477492209842FC2BE7A540486ACC6DA13605FAFAD0136C786DB1E5B7CDBB8A2C5F4332CA8FE04980913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXjbEsAXAHjYdcFCexfXYXdS X-DA7885C5: B25D84CCBA7F628EF255D290C0D534F9365DCBC47E3FF8E9BF1B1A1631B70F82B006C22A78EC5A8F5B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F739381B31377CF4CA2191B469EF426D0AB91B8D1FFE808D819FFCBE88052DE701A89E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: [Tarantool-patches] [PATCH v3 luajit] Fix command-line argv handling. 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 (cherry-picked from commit 9ebebc9b588dc1516c988b46d829445f505fdc1f) Before the patch, there was a situation where `luaL_newstate()` could fail in `main()` and the `argv[0]` could be used as a progname in `l_message()`. However, `argv[0]` is not guaranteed to be non-NULL, so the segmentation fault could occur. This patch fixes the issue by using the predefined name in that case. Moreover, it refactors the `l_message()`, so now there is no need to pass the program name everywhere. The patch is tested with the help of the mocking of `luaL_newstate` by providing an error-injected implementation of it and preloading it. For preload to work, the LuaJIT must be built with dynamic build mode enabled. The corresponding flavor is added to the CI only for x86_64 Debug build to avoid CI jobs outgrowing. The inspects internal symbols from the LuaJIT static library, so it can't be linked for shared build. The test is disabled for the dynamic build mode. Since the Linux kernel 5.18-rc1 release, `argv` is forced to a single empty string if it is empty [1], so the issue is not reproducible on new kernels. You may inspect the `dmesg` log for the corresponding warning entrance. Maxim Kokryashkin: * added the description and the test for the problem [1]: https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/ Part of tarantool/tarantool#10709 --- Changes in v3: * Skip only one test from tarantool-c-tests, not the whole suite. * Skip the added test not inside the main loop, but via adding the environment variable. * Rebased on the current tarantool/master. * Fix typos. Branch: https://github.com/tarantool/luajit/tree/fckxorg/fix-argv-handling Issue: https://github.com/tarantool/tarantool/issues/10709 .github/workflows/exotic-builds-testing.yml | 10 ++- src/luajit.c | 24 +++---- test/tarantool-c-tests/CMakeLists.txt | 8 +++ test/tarantool-tests/CMakeLists.txt | 10 +++ .../fix-argv-handling.test.lua | 26 +++++++ .../fix-argv-handling/CMakeLists.txt | 3 + .../fix-argv-handling/execlib.c | 67 +++++++++++++++++++ .../fix-argv-handling/mynewstate.c | 9 +++ 8 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 test/tarantool-tests/fix-argv-handling.test.lua create mode 100644 test/tarantool-tests/fix-argv-handling/CMakeLists.txt create mode 100644 test/tarantool-tests/fix-argv-handling/execlib.c create mode 100644 test/tarantool-tests/fix-argv-handling/mynewstate.c diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml index 70096439..374c879b 100644 --- a/.github/workflows/exotic-builds-testing.yml +++ b/.github/workflows/exotic-builds-testing.yml @@ -34,7 +34,7 @@ jobs: BUILDTYPE: [Debug, Release] ARCH: [ARM64, x86_64] GC64: [ON, OFF] - FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind, tablebump] + FLAVOR: [checkhook, dualnum, dynamic, gdbjit, nojit, nounwind, tablebump] include: - BUILDTYPE: Debug CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON @@ -42,6 +42,8 @@ jobs: CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo - FLAVOR: dualnum FLAVORFLAGS: -DLUAJIT_NUMMODE=2 + - FLAVOR: dynamic + FLAVORFLAGS: -DBUILDMODE=dynamic - FLAVOR: checkhook FLAVORFLAGS: -DLUAJIT_ENABLE_CHECKHOOK=ON - FLAVOR: nojit @@ -58,6 +60,12 @@ jobs: # DUALNUM is default for ARM64, no need for additional testing. - FLAVOR: dualnum ARCH: ARM64 + # There is no need to test any scenarios except the most + # common one for the shared library build. + - FLAVOR: dynamic + ARCH: ARM64 + - FLAVOR: dynamic + BUILDTYPE: Release # With table bump optimization enabled (and due to our modification # related to metrics), some offsets in GG_State stop fitting in 12bit # immediate. Hence, the build failed due to the DASM error diff --git a/src/luajit.c b/src/luajit.c index e04a5a30..d9d85361 100644 --- a/src/luajit.c +++ b/src/luajit.c @@ -39,6 +39,7 @@ static lua_State *globalL = NULL; static const char *progname = LUA_PROGNAME; +static char *empty_argv[2] = { NULL, NULL }; #if !LJ_TARGET_CONSOLE static void lstop(lua_State *L, lua_Debug *ar) @@ -90,9 +91,9 @@ static void print_tools_usage(void) fflush(stderr); } -static void l_message(const char *pname, const char *msg) +static void l_message(const char *msg) { - if (pname) { fputs(pname, stderr); fputc(':', stderr); fputc(' ', stderr); } + if (progname) { fputs(progname, stderr); fputc(':', stderr); fputc(' ', stderr); } fputs(msg, stderr); fputc('\n', stderr); fflush(stderr); } @@ -102,7 +103,7 @@ static int report(lua_State *L, int status) if (status && !lua_isnil(L, -1)) { const char *msg = lua_tostring(L, -1); if (msg == NULL) msg = "(error object is not a string)"; - l_message(progname, msg); + l_message(msg); lua_pop(L, 1); } return status; @@ -268,9 +269,8 @@ static void dotty(lua_State *L) lua_getglobal(L, "print"); lua_insert(L, 1); if (lua_pcall(L, lua_gettop(L)-1, 0, 0) != 0) - l_message(progname, - lua_pushfstring(L, "error calling " LUA_QL("print") " (%s)", - lua_tostring(L, -1))); + l_message(lua_pushfstring(L, "error calling " LUA_QL("print") " (%s)", + lua_tostring(L, -1))); } } lua_settop(L, 0); /* clear stack */ @@ -322,8 +322,7 @@ static int loadjitmodule(lua_State *L) lua_getfield(L, -1, "start"); if (lua_isnil(L, -1)) { nomodule: - l_message(progname, - "unknown luaJIT command or jit.* modules not installed"); + l_message("unknown luaJIT command or jit.* modules not installed"); return 1; } lua_remove(L, -2); /* Drop module table. */ @@ -383,7 +382,7 @@ static int runtoolcmd(lua_State *L, const char *tool_name) if (msg) { if (!strncmp(msg, "module ", 7)) msg = "unknown luaJIT command or tools not installed"; - l_message(progname, msg); + l_message(msg); } return 1; } @@ -567,7 +566,6 @@ static int pmain(lua_State *L) int argn; int flags = 0; globalL = L; - if (argv[0] && argv[0][0]) progname = argv[0]; LUAJIT_VERSION_SYM(); /* Linker-enforced version check. */ @@ -623,9 +621,11 @@ static int pmain(lua_State *L) int main(int argc, char **argv) { int status; - lua_State *L = lua_open(); + lua_State *L; + if (!argv[0]) argv = empty_argv; else if (argv[0][0]) progname = argv[0]; + L = lua_open(); /* create state */ if (L == NULL) { - l_message(argv[0], "cannot create state: not enough memory"); + l_message("cannot create state: not enough memory"); return EXIT_FAILURE; } smain.argc = argc; diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt index c4a402d0..c84c651d 100644 --- a/test/tarantool-c-tests/CMakeLists.txt +++ b/test/tarantool-c-tests/CMakeLists.txt @@ -41,6 +41,14 @@ file(GLOB tests "${CMAKE_CURRENT_SOURCE_DIR}/*${CTEST_SRC_SUFFIX}") foreach(test_source ${tests}) # Get test name without suffix. Needed to set OUTPUT_NAME. get_filename_component(exe ${test_source} NAME_WE) + + # Test requires static build, since it inspects internal + # symbols. + if(exe STREQUAL "gh-8594-sysprof-ffunc-crash" + AND BUILDMODE STREQUAL "dynamic") + continue() + endif() + add_executable(${exe} EXCLUDE_FROM_ALL ${test_source}) target_include_directories(${exe} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt index c43b3d48..9bacac88 100644 --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -67,6 +67,10 @@ add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/gnuhash) add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/hash) add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/stripped) +if(BUILDMODE STREQUAL "dynamic") + add_subdirectory(fix-argv-handling) +endif() + # JIT, profiler, and bytecode toolchains are located in the # directory, is the autogenerated file also # located in the directory, but in the scope of the binary @@ -99,6 +103,12 @@ if(LUAJIT_USE_VALGRIND) list(APPEND LUA_TEST_ENV_MORE LUAJIT_TEST_USE_VALGRIND=1) endif() +# Needed to skip the for a +# non-dynamic build. +if(BUILDMODE STREQUAL "dynamic") + list(APPEND LUA_TEST_ENV_MORE LUAJIT_BUILDMODE=dynamic) +endif() + set(TEST_SUITE_NAME "tarantool-tests") # XXX: The call produces both test and target diff --git a/test/tarantool-tests/fix-argv-handling.test.lua b/test/tarantool-tests/fix-argv-handling.test.lua new file mode 100644 index 00000000..84b626c3 --- /dev/null +++ b/test/tarantool-tests/fix-argv-handling.test.lua @@ -0,0 +1,26 @@ +local tap = require('tap') +local test = tap.test('fix-argv-handling'):skipcond({ + ['DYLD_INSERT_LIBRARIES does not work on macOS'] = jit.os == 'OSX', + ['Skipped for static build'] = os.getenv('LUAJIT_BUILDMODE') ~= 'dynamic', +}) + +test:plan(1) + +-- XXX: Since the Linux kernel 5.18-rc1 release, `argv` is forced +-- to a single empty string if it is empty [1], so the issue is +-- not reproducible on new kernels. +-- +-- [1]: https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/ + +local utils = require('utils') +local execlib = require('execlib') +local cmd = utils.exec.luabin(arg) + +-- Start the LuaJIT with an empty argv array and mocked +-- `luaL_newstate`. +local output = execlib.empty_argv_exec(cmd) + +-- Without the patch, the test fails with a segmentation fault +-- instead of returning an error. +test:like(output, 'cannot create state', 'correct argv handling') +test:done(true) diff --git a/test/tarantool-tests/fix-argv-handling/CMakeLists.txt b/test/tarantool-tests/fix-argv-handling/CMakeLists.txt new file mode 100644 index 00000000..6b9bb4a6 --- /dev/null +++ b/test/tarantool-tests/fix-argv-handling/CMakeLists.txt @@ -0,0 +1,3 @@ +get_filename_component(test_name ${CMAKE_CURRENT_SOURCE_DIR} NAME) +BuildTestCLib(mynewstate mynewstate.c ${test_name}.test.lua) +BuildTestCLib(execlib execlib.c ${test_name}.test.lua) diff --git a/test/tarantool-tests/fix-argv-handling/execlib.c b/test/tarantool-tests/fix-argv-handling/execlib.c new file mode 100644 index 00000000..a8d5f16c --- /dev/null +++ b/test/tarantool-tests/fix-argv-handling/execlib.c @@ -0,0 +1,67 @@ +#define _GNU_SOURCE +#include "lua.h" +#include "lauxlib.h" + +#include +#include +#include +#include +#include + +/* 1Kb should be enough. */ +#define BUF_SIZE 1024 +#define CHECKED(call) \ +do { \ + if ((call) == -1) { \ + perror(#call); \ + exit(1); \ + } \ +} while(0) + +static int empty_argv_exec(struct lua_State *L) +{ + const char *path = luaL_checkstring(L, -1); + int pipefds[2] = {}; + char *const argv[] = {NULL}; + char buf[BUF_SIZE]; + + CHECKED(pipe2(pipefds, O_CLOEXEC)); + + pid_t pid = fork(); + CHECKED(pid); + + if (pid == 0) { + /* + * Mock the `luaL_newstate` with an error-injected + * version. + */ + setenv("LD_PRELOAD", "mynewstate.so", 1); + CHECKED(dup2(pipefds[1], STDOUT_FILENO)); + CHECKED(dup2(pipefds[1], STDERR_FILENO)); + /* + * Pipes are closed on the exec call because of + * the O_CLOEXEC flag. + */ + CHECKED(execvp(path, argv)); + } + + close(pipefds[1]); + CHECKED(waitpid(pid, NULL, 0)); + + CHECKED(read(pipefds[0], buf, BUF_SIZE)); + close(pipefds[0]); + + lua_pushstring(L, buf); + return 1; +} + +static const struct luaL_Reg execlib[] = { + {"empty_argv_exec", empty_argv_exec}, + {NULL, NULL} +}; + +LUA_API int luaopen_execlib(lua_State *L) +{ + luaL_register(L, "execlib", execlib); + return 1; +} diff --git a/test/tarantool-tests/fix-argv-handling/mynewstate.c b/test/tarantool-tests/fix-argv-handling/mynewstate.c new file mode 100644 index 00000000..cf4a67e7 --- /dev/null +++ b/test/tarantool-tests/fix-argv-handling/mynewstate.c @@ -0,0 +1,9 @@ +#include + +struct lua_State; + +/* Error-injected mock. */ +struct lua_State *luaL_newstate(void) +{ + return NULL; +} -- 2.47.1