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 C335E9D674B; Thu, 11 Apr 2024 02:29:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C335E9D674B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1712791778; bh=uO3c+yT8pfoT1I8xjIqdD+x4XIBcy8CiOJUL/OMKCAo=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=trxY/zsnIqzhm3bBHMFtb08e56zmCXeYqAZEH75qR5GTLIbVTPcAMn+L+Vlh1thJ5 b9lbzOTvrjqBEMZMCOXue1MWAflY/zrEjH9mKeUeoFXMzMieRAHpRG5Sh/s66Zse6r Xe72PC+7pVymnz/braUME6JeCqc9vJkPp9nHFMOY= Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) (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 A7F3263F138 for ; Thu, 11 Apr 2024 02:29:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A7F3263F138 Received: by mail-lf1-f54.google.com with SMTP id 2adb3069b0e04-516dd07d373so3670668e87.3 for ; Wed, 10 Apr 2024 16:29:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712791776; x=1713396576; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=qsftMTSnpNt0FtgDiYKQdOPWX1sMiWgvHz8c4pw0cDk=; b=U3IYVUzMYLrNC2rxYkgkO6RyLXDvzf5fp4fZSvxPs2qeYiJ84ojzpyfzPG2DTeKHMH kGQsSoE4DfeF2FCqeB6wF8yklGeSThbh16gfFPGyKaMHsXuO4FFIz7f4JcURl3gCTcAF Vz4Z8q3PbGK6VK/ctsoc3/2Vy4g7IFHOdNpK36Lo0gCwXiRTzFJ24V9YwFnjBPQkpZfI eVrGh1MKYebgk0amNpBPoLKujnwznSVtZtEkyix9S1mgrGEvwraaMIJxjlvk9lQyxnVC Tx6w9wctHsbHN7cZxym4RXIJ+qvIVjjyjMC7UeEXKlYC12gbU4ZsRWvBNavbYvjuWTbz +28A== X-Gm-Message-State: AOJu0Yzrpvy4opFFDTdxA4Wnxngl8b+c9xFbNacBgkr7Oz4GriVb/CUt ZHRWLra0h59nofuTqY09TQV4YiYx8RxK5x+EfGxVjpvLmEnmBfInVYbqLr+KPgQ= X-Google-Smtp-Source: AGHT+IEoY1cHKGfXD8dXNsF5PuGTy98Tn4TSjNRkzoh93t4f4Q9SA6X7nd9Goh2YTTRvTi0X7J1YWQ== X-Received: by 2002:ac2:5bc4:0:b0:516:d2b9:20c3 with SMTP id u4-20020ac25bc4000000b00516d2b920c3mr2754691lfn.10.1712791776250; Wed, 10 Apr 2024 16:29:36 -0700 (PDT) Received: from localhost.localdomain (95-24-11-149.broadband.corbina.ru. [95.24.11.149]) by smtp.gmail.com with ESMTPSA id r16-20020ac252b0000000b00513c7c7879fsm42526lfm.259.2024.04.10.16.29.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Apr 2024 16:29:35 -0700 (PDT) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergeyb@tarantool.org Date: Thu, 11 Apr 2024 02:29:29 +0300 Message-ID: <20240410232933.969244-1-m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.44.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin 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 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 `pname` 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. Corresponding flavor is added to the CI. The tarantool-c-tests target cannot be linked with the LuaJIT library when it is built as shared. The test suite is disabled for the dynamic build mode. Part of tarantool/tarantool#9924 --- Branch: https://github.com/tarantool/luajit/tree/fckxorg/fix-argv-handling .github/workflows/exotic-builds-testing.yml | 4 +- src/luajit.c | 24 +++++----- test/tarantool-c-tests/CMakeLists.txt | 8 ++++ test/tarantool-tests/CMakeLists.txt | 9 ++++ .../fix-argv-handling.test.lua | 26 +++++++++++ .../fix-argv-handling/CMakeLists.txt | 2 + .../fix-argv-handling/empty_argv_exec.c | 45 +++++++++++++++++++ .../fix-argv-handling/mynewstate.c | 9 ++++ 8 files changed, 114 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/empty_argv_exec.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 859603bd..b3cc5ca1 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] + FLAVOR: [checkhook, dualnum, dynamic_build, gdbjit, nojit, nounwind] include: - BUILDTYPE: Debug CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON @@ -50,6 +50,8 @@ jobs: FLAVORFLAGS: -DLUAJIT_USE_GDBJIT=ON - FLAVOR: nounwind FLAVORFLAGS: -DLUAJIT_NO_UNWIND=ON + - FLAVOR: dynamic_build + FLAVORFLAGS: -DBUILDMODE=dynamic exclude: - ARCH: ARM64 GC64: OFF diff --git a/src/luajit.c b/src/luajit.c index b63c92d1..dc142684 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; @@ -267,9 +268,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 */ @@ -321,8 +321,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. */ @@ -382,7 +381,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; } @@ -566,7 +565,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. */ @@ -622,9 +620,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 30d174bb..7ae440e2 100644 --- a/test/tarantool-c-tests/CMakeLists.txt +++ b/test/tarantool-c-tests/CMakeLists.txt @@ -36,6 +36,14 @@ add_test_suite_target(tarantool-c-tests DEPENDS libluajit libtest tarantool-c-tests-build ) +# XXX: The tarantool-c-tests target cannot be linked with the LuaJIT +# library when it is built as shared. The test suite is disabled for +# the dynamic build mode. +if(BUILDMODE STREQUAL "dynamic") + message("Dynamic build mode, tarantool-c-tests suite is empty") + return() +endif() + set(CTEST_SRC_SUFFIX ".test.c") file(GLOB tests "${CMAKE_CURRENT_SOURCE_DIR}/*${CTEST_SRC_SUFFIX}") foreach(test_source ${tests}) diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt index 56660932..05deb534 100644 --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -40,6 +40,10 @@ add_subdirectory(lj-flush-on-trace) add_subdirectory(lj-1004-oom-error-frame) add_subdirectory(lj-1066-fix-cur_L-after-coroutine-resume) +if(BUILDMODE STREQUAL "dynamic") + add_subdirectory(fix-argv-handling) +endif() + # The part of the memory profiler toolchain is located in tools # directory, jit, profiler, and bytecode toolchains are located # in src/ directory, jit/vmdef.lua is autogenerated file also @@ -123,6 +127,11 @@ add_test_suite_target(tarantool-tests file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}") foreach(test_path ${tests}) get_filename_component(test_name ${test_path} NAME) + + if(test_name STREQUAL "fix-argv-handling.test.lua" AND NOT BUILDMODE STREQUAL "dynamic") + continue() + endif() + set(test_title "test/${TEST_SUITE_NAME}/${test_name}") add_test(NAME ${test_title} COMMAND ${LUAJIT_TEST_COMMAND} ${test_path} 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..ccb2f52e --- /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', +}) + +test:plan(1) + +local ffi = require('ffi') +local utils = require('utils') + +ffi.cdef[[ +const char *empty_argv_exec(const char *path); +void free(void *ptr); +]] +local execlib = ffi.load('emptyargvexec') +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) +ffi.gc(output, ffi.C.free) +local output_str = ffi.string(output) + +-- Without the patch, the test fails with a segmentation fault instead of +-- returning an error. +test:like(output_str, '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..431da2ad --- /dev/null +++ b/test/tarantool-tests/fix-argv-handling/CMakeLists.txt @@ -0,0 +1,2 @@ +BuildTestCLib(mynewstate mynewstate.c) +BuildTestCLib(libemptyargvexec empty_argv_exec.c) diff --git a/test/tarantool-tests/fix-argv-handling/empty_argv_exec.c b/test/tarantool-tests/fix-argv-handling/empty_argv_exec.c new file mode 100644 index 00000000..d1f06014 --- /dev/null +++ b/test/tarantool-tests/fix-argv-handling/empty_argv_exec.c @@ -0,0 +1,45 @@ +#define _GNU_SOURCE +#include +#include +#include +#include +#include + +#define BUF_SIZE 1024 +#define CHECKED(call) \ +do { \ + int status = 0; \ + status = (call); \ + if (status == -1) { \ + perror(#call); \ + exit(1); \ +} \ +} while(0) + +const char *empty_argv_exec(const char *path) +{ + int pipefds[2] = {}; + char* const argv[] = {NULL}; + CHECKED(pipe2(pipefds, O_CLOEXEC)); + + pid_t pid = fork(); + if (pid == 0) { + /* Mock the `luaL_newstate` with an error-injected version. */ + setenv("LD_PRELOAD", "mynewstate.so", 1); + CHECKED(dup2(pipefds[1], 1)); + CHECKED(dup2(pipefds[1], 2)); + CHECKED(execvp(path, argv)); + } + + close(pipefds[1]); + waitpid(pid, NULL, 0); + /* 1Kb should be enough. */ + char *buf = calloc(BUF_SIZE, sizeof(char)); + if (buf == NULL) { + perror("calloc"); + exit(1); + } + CHECKED(read(pipefds[0], buf, BUF_SIZE)); + return buf; +} + 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.44.0