Fix command-line argv handling.

(cherry-picked from commit 9ebebc9)

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 `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. The corresponding flavor is added to the CI.

The <gh-8594-sysprof-ffunc-crash.test.c> 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/[email protected]/

Part of tarantool/tarantool#10709

.github/workflows/exotic-builds-testing.yml

@@ -34,14 +34,16 @@ jobs:
         BUILDTYPE: [Debug, Release]
         ARCH: [ARM64, x86_64]
         GC64: [ON, OFF]
-        FLAVOR: [checkhook, dualnum, gdbjit, nojit, nounwind, tablebump]
+        FLAVOR: [checkhook, dualnum, dynamic_build, gdbjit, nojit, nounwind, tablebump]
         include:
           - BUILDTYPE: Debug
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
           - FLAVOR: dualnum
             FLAVORFLAGS: -DLUAJIT_NUMMODE=2
+          - FLAVOR: dynamic_build
+            FLAVORFLAGS: -DBUILDMODE=dynamic
           - FLAVOR: checkhook
             FLAVORFLAGS: -DLUAJIT_ENABLE_CHECKHOOK=ON
           - FLAVOR: nojit

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;

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}

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 <src/>
 # directory, <jit/vmdef.lua> is the autogenerated file also
 # located in the <src/> directory, but in the scope of the binary
@@ -111,6 +115,12 @@ add_test_suite_target(tarantool-tests
 file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
 foreach(test_path ${tests})
   file(RELATIVE_PATH test_name ${CMAKE_CURRENT_SOURCE_DIR} ${test_path})
+
+  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}

test/tarantool-tests/fix-argv-handling.test.lua

@@ -0,0 +1,25 @@
+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)
+
+-- 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/[email protected]/
+
+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)

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)

test/tarantool-tests/fix-argv-handling/execlib.c

@@ -0,0 +1,67 @@
+#define _GNU_SOURCE
+#include "lua.h"
+#include "lauxlib.h"
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+/* 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;
+}

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;
+}