[Tarantool-patches] [PATCH luajit] Fix command-line argv handling.
Maxim Kokryashkin
max.kokryashkin at gmail.com
Thu Apr 11 02:29:29 MSK 2024
From: Mike Pall <mike>
(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 <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#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 <stddef.h>
+
+struct lua_State;
+
+/* Error-injected mock. */
+struct lua_State *luaL_newstate(void)
+{
+ return NULL;
+}
--
2.44.0
More information about the Tarantool-patches
mailing list