Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling.
@ 2024-04-11 13:00 Maxim Kokryashkin via Tarantool-patches
  2024-04-11 13:26 ` Sergey Kaplun via Tarantool-patches
  2024-04-11 16:30 ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 2 replies; 5+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2024-04-11 13:00 UTC (permalink / raw)
  To: tarantool-patches, skaplun, sergeyb

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 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 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.

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/

Part of tarantool/tarantool#9924
---
Branch: https://github.com/tarantool/luajit/tree/fckxorg/fix-argv-handling

Changes in v2:
- Fixed comments as per review by Sergey Kaplun

 .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                | 25 +++++++
 .../fix-argv-handling/CMakeLists.txt          |  2 +
 .../fix-argv-handling/execlib.c               | 68 +++++++++++++++++++
 .../fix-argv-handling/mynewstate.c            |  9 +++
 8 files changed, 136 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 859603bd..ae54de2e 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
@@ -42,6 +42,8 @@ jobs:
             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
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..57e5f169
--- /dev/null
+++ b/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/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..c37bded7
--- /dev/null
+++ b/test/tarantool-tests/fix-argv-handling/CMakeLists.txt
@@ -0,0 +1,2 @@
+BuildTestCLib(mynewstate mynewstate.c)
+BuildTestCLib(execlib execlib.c)
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..ef8217d4
--- /dev/null
+++ b/test/tarantool-tests/fix-argv-handling/execlib.c
@@ -0,0 +1,68 @@
+#include "lua.h"
+#include "lauxlib.h"
+
+#define _GNU_SOURCE
+#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], 1));
+		CHECKED(dup2(pipefds[1], 2));
+		/*
+		 * 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 <stddef.h>
+
+struct lua_State;
+
+/* Error-injected mock. */
+struct lua_State *luaL_newstate(void)
+{
+	return NULL;
+}
--
2.44.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling.
  2024-04-11 13:00 [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling Maxim Kokryashkin via Tarantool-patches
@ 2024-04-11 13:26 ` Sergey Kaplun via Tarantool-patches
  2024-04-11 14:45   ` Maxim Kokryashkin via Tarantool-patches
  2024-04-11 16:30 ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 5+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-04-11 13:26 UTC (permalink / raw)
  To: Maxim Kokryashkin; +Cc: tarantool-patches

Hi, Maxim!
Thanks for the fixes and clarifications.
LGTM, except a few nits below.

On 11.04.24, Maxim Kokryashkin wrote:
> From: Mike Pall <mike>
> 
> (cherry-picked from commit 9ebebc9b588dc1516c988b46d829445f505fdc1f)
> 

<snipped>

> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/fix-argv-handling
> 
> Changes in v2:
> - Fixed comments as per review by Sergey Kaplun
> 
>  .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                | 25 +++++++
>  .../fix-argv-handling/CMakeLists.txt          |  2 +
>  .../fix-argv-handling/execlib.c               | 68 +++++++++++++++++++
>  .../fix-argv-handling/mynewstate.c            |  9 +++
>  8 files changed, 136 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 859603bd..ae54de2e 100644
> --- a/.github/workflows/exotic-builds-testing.yml
> +++ b/.github/workflows/exotic-builds-testing.yml

<snipped>

> diff --git a/src/luajit.c b/src/luajit.c
> index b63c92d1..dc142684 100644
> --- a/src/luajit.c
> +++ b/src/luajit.c

<snipped>

> 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.

Nit: comment width is more than 66 symbols.

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

<snipped>

> @@ -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")

Nit: Line width is more than 80 symbols.

Side note: Such skip looks clumsy, but for now we can leave it as the
most robust solution.

> +    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..57e5f169
> --- /dev/null
> +++ b/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

Nit: Comment line looks underfilled

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

Nit: comment line width is more than 66 symbols.

> +-- returning an error.
> +test:like(output, 'cannot create state', 'correct argv handling')
> +test:done(true)

<snipped>

> 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..ef8217d4
> --- /dev/null
> +++ b/test/tarantool-tests/fix-argv-handling/execlib.c
> @@ -0,0 +1,68 @@
> +#include "lua.h"
> +#include "lauxlib.h"

Now I got the following warning during the build.

| /home/burii/reviews/luajit/argv-handling/test/tarantool-tests/fix-argv-handling/execlib.c:28:17:
| warning: implicit declaration of function 'pipe2'; did you mean 'pipe'? [-Wimplicit-function-declaration]
|    28 |         CHECKED(pipe2(pipefds, O_CLOEXEC));
|       |                 ^~~~~
| /home/burii/reviews/luajit/argv-handling/test/tarantool-tests/fix-argv-handling/execlib.c:15:14: note: in definition of macro 'CHECKED'
|    15 |         if ((call) == -1) { \
|       |              ^~

I suppose _GNU_SOURCE should be defined before any onther includes.

> +
> +#define _GNU_SOURCE
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/wait.h>
> +#include <unistd.h>

<snipped>

> +static int empty_argv_exec(struct lua_State *L)
> +{
> +	const char *path = luaL_checkstring(L, -1);
> +	int pipefds[2] = {};
> +	char *const  argv[] = {NULL};

Typo: s/  / /

> +	char buf[BUF_SIZE];
> +
> +	CHECKED(pipe2(pipefds, O_CLOEXEC));
> +
> +	pid_t pid = fork();
> +	CHECKED(pid);
> +
> +	if (pid == 0) {
> +		/*
> +		 * Mock the `luaL_newstate` with

Nit: Comment line looks underfilled.

> +		 * an error-injected version.
> +		 */
> +		setenv("LD_PRELOAD", "mynewstate.so", 1);
> +		CHECKED(dup2(pipefds[1], 1));
> +		CHECKED(dup2(pipefds[1], 2));

Why don't use `STDOUT_FILENO` and `STDERR_FILENO`?

> +		/*
> +		 * Pipes are closed on the exec
> +		 * call because of the O_CLOEXEC flag.

Side note: Thanks for the clarification. PEBKAC.

> +		 */
> +		CHECKED(execvp(path, argv));
> +	}
> +
> +	close(pipefds[1]);
> +	CHECKED(waitpid(pid, NULL, 0));
> +

Nit: Excess empty line.

> +
> +	CHECKED(read(pipefds[0], buf, BUF_SIZE));

<snipped>

> 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

<snipped>

> 2.44.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling.
  2024-04-11 13:26 ` Sergey Kaplun via Tarantool-patches
@ 2024-04-11 14:45   ` Maxim Kokryashkin via Tarantool-patches
  2024-04-11 14:57     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 5+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2024-04-11 14:45 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: Maxim Kokryashkin, tarantool-patches

Hi, Sergey!
Thanks for the patch!
Fixed, branch is force-pushed:
===
diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
index 7ae440e2..5f789ad8 100644
--- a/test/tarantool-c-tests/CMakeLists.txt
+++ b/test/tarantool-c-tests/CMakeLists.txt
@@ -36,9 +36,9 @@ 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.
+# 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()
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 05deb534..0d5f3774 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -128,7 +128,8 @@ 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")
+  if(test_name STREQUAL "fix-argv-handling.test.lua"
+      AND NOT BUILDMODE STREQUAL "dynamic")
     continue()
   endif()

diff --git a/test/tarantool-tests/fix-argv-handling.test.lua b/test/tarantool-tests/fix-argv-handling.test.lua
index 57e5f169..fd558705 100644
--- a/test/tarantool-tests/fix-argv-handling.test.lua
+++ b/test/tarantool-tests/fix-argv-handling.test.lua
@@ -5,9 +5,9 @@ local test = tap.test('fix-argv-handling'):skipcond({

 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.
+-- 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/

@@ -19,7 +19,7 @@ local cmd = utils.exec.luabin(arg)
 -- `luaL_newstate`.
 local output = execlib.empty_argv_exec(cmd)

--- Without the patch, the test fails with a segmentation fault instead of
--- returning an error.
+-- 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/execlib.c b/test/tarantool-tests/fix-argv-handling/execlib.c
index ef8217d4..a8d5f16c 100644
--- a/test/tarantool-tests/fix-argv-handling/execlib.c
+++ b/test/tarantool-tests/fix-argv-handling/execlib.c
@@ -1,7 +1,7 @@
+#define _GNU_SOURCE
 #include "lua.h"
 #include "lauxlib.h"

-#define _GNU_SOURCE
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -22,7 +22,7 @@ static int empty_argv_exec(struct lua_State *L)
 {
 	const char *path = luaL_checkstring(L, -1);
 	int pipefds[2] = {};
-	char *const  argv[] = {NULL};
+	char *const argv[] = {NULL};
 	char buf[BUF_SIZE];

 	CHECKED(pipe2(pipefds, O_CLOEXEC));
@@ -32,15 +32,15 @@ static int empty_argv_exec(struct lua_State *L)

 	if (pid == 0) {
 		/*
-		 * Mock the `luaL_newstate` with
-		 * an error-injected version.
+		 * 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(dup2(pipefds[1], STDOUT_FILENO));
+		CHECKED(dup2(pipefds[1], STDERR_FILENO));
 		/*
-		 * Pipes are closed on the exec
-		 * call because of the O_CLOEXEC flag.
+		 * Pipes are closed on the exec call because of
+		 * the O_CLOEXEC flag.
 		 */
 		CHECKED(execvp(path, argv));
 	}
@@ -48,7 +48,6 @@ static int empty_argv_exec(struct lua_State *L)
 	close(pipefds[1]);
 	CHECKED(waitpid(pid, NULL, 0));

-
 	CHECKED(read(pipefds[0], buf, BUF_SIZE));
 	close(pipefds[0]);

===

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling.
  2024-04-11 14:45   ` Maxim Kokryashkin via Tarantool-patches
@ 2024-04-11 14:57     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 5+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-04-11 14:57 UTC (permalink / raw)
  To: Maxim Kokryashkin; +Cc: Maxim Kokryashkin, tarantool-patches

Hi, Maxim!
Thanks for the fixes!
LGTM!

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling.
  2024-04-11 13:00 [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling Maxim Kokryashkin via Tarantool-patches
  2024-04-11 13:26 ` Sergey Kaplun via Tarantool-patches
@ 2024-04-11 16:30 ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 0 replies; 5+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-04-11 16:30 UTC (permalink / raw)
  To: Maxim Kokryashkin, tarantool-patches, skaplun

[-- Attachment #1: Type: text/plain, Size: 15901 bytes --]

Hi, Maxim

thanks for the patch. See my comments below.


Sergey

On 11.04.2024 16:00, Maxim Kokryashkin wrote:
> 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

Nit: main and progname are the function and variable in code,

so I would wrap them by backticks.

> 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 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.

This wording is confusing because target cannot be linked with anything.

I would rephrase to something like: tests in "tarantool-c-tests" cannot 
be linked

with LuaJIT library built as shared library. Same for the message in 
CMakeLists.txt.

> 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/
>
> Part of tarantool/tarantool#9924
> ---
> Branch:https://github.com/tarantool/luajit/tree/fckxorg/fix-argv-handling
>
> Changes in v2:
> - Fixed comments as per review by Sergey Kaplun
>
>   .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                | 25 +++++++
>   .../fix-argv-handling/CMakeLists.txt          |  2 +
>   .../fix-argv-handling/execlib.c               | 68 +++++++++++++++++++
>   .../fix-argv-handling/mynewstate.c            |  9 +++
>   8 files changed, 136 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 859603bd..ae54de2e 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]

Nit: "Dynamic build" can be replaced with "shared_lib" which is more 
clear for my taste.

Feel free to ignore.

>           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_build
> +            FLAVORFLAGS: -DBUILDMODE=dynamic
>             - FLAVOR: checkhook
>               FLAVORFLAGS: -DLUAJIT_ENABLE_CHECKHOOK=ON
>             - FLAVOR: nojit
> 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()

This is confusing ("Add tarantool-c-tests..." and then "test suite is 
empty"):

[1] ~/sources/MRG/tarantool/third_party/luajit$ cmake -S . -B build 
-DBUILDMODE=dynamic
-- [SetVersion] Reading version from VCS: v2.1.0-beta3-528-g964e26a2
-- [SetBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is 8
-- Add test suite LuaJIT-tests
-- Add test suite PUC-Rio-Lua-5.1-tests
-- Add test suite lua-Harness-tests
-- Add test suite tarantool-c-tests
Dynamic build mode, tarantool-c-tests suite is empty
-- Add test suite tarantool-tests
-- Configuring done (0.1s)
-- Generating done (0.1s)


I propose to add an empty target and put condition 
before add_test_suite_target:

diff --git a/test/tarantool-c-tests/CMakeLists.txt 
b/test/tarantool-c-tests/CMakeLists.txt
index 5f789ad8..63525682 100644
--- a/test/tarantool-c-tests/CMakeLists.txt
+++ b/test/tarantool-c-tests/CMakeLists.txt
@@ -21,6 +21,17 @@ AppendFlags(TESTS_C_FLAGS 
"-D__LJ_TEST_DIR__='\"${CMAKE_CURRENT_SOURCE_DIR}\"'")

  set(TEST_SUITE_NAME "tarantool-c-tests")

+# 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")
+  set(MSG "Dynamic build mode, ${TEST_SUITE_NAME} suite is empty")
+  add_custom_target(${TEST_SUITE_NAME}
+    COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${MSG}
+  )
+  add_custom_target(${TEST_SUITE_NAME}-deps)
+  return()
+endif()
+

+
  # The proxy CMake target with all targets that build C tests.
  # This is needed because targets for each C test are generated
  # at the same time as CMake tests, and all prerequisites must
@@ -36,14 +47,6 @@ 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})

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


I would not touch this loop. After adding a number quirks for tests like 
above

the loop body will become unreadable. I propose to add such quirks right 
after the loop

and mark such tests as disabled using test properties. It is sad, but 
CTest cannot mark test as skipped.


--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -127,12 +127,6 @@ 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}
@@ -147,3 +141,12 @@ foreach(test_path ${tests})
      DEPENDS tarantool-tests-deps
    )
  endforeach()
+
+# Test's properties.
+
+if (NOT BUILDMODE STREQUAL "dynamic")
+  set(test_title "test/tarantool-tests/fix-argv-handling.test.lua")
+  set_tests_properties(${test_title} PROPERTIES
+    DISABLED True
+  )
+endif()

> 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..57e5f169
> --- /dev/null
> +++ b/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/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..c37bded7
> --- /dev/null
> +++ b/test/tarantool-tests/fix-argv-handling/CMakeLists.txt
> @@ -0,0 +1,2 @@
> +BuildTestCLib(mynewstate mynewstate.c)
> +BuildTestCLib(execlib execlib.c)
> 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..ef8217d4
> --- /dev/null
> +++ b/test/tarantool-tests/fix-argv-handling/execlib.c
> @@ -0,0 +1,68 @@
> +#include "lua.h"
> +#include "lauxlib.h"
> +
> +#define _GNU_SOURCE
> +#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], 1));
> +		CHECKED(dup2(pipefds[1], 2));
> +		/*
> +		 * 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 <stddef.h>
> +
> +struct lua_State;
> +
> +/* Error-injected mock. */
> +struct lua_State *luaL_newstate(void)
> +{
> +	return NULL;
> +}
> --
> 2.44.0
>

[-- Attachment #2: Type: text/html, Size: 18173 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-04-11 16:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-11 13:00 [Tarantool-patches] [PATCH luajit v2] Fix command-line argv handling Maxim Kokryashkin via Tarantool-patches
2024-04-11 13:26 ` Sergey Kaplun via Tarantool-patches
2024-04-11 14:45   ` Maxim Kokryashkin via Tarantool-patches
2024-04-11 14:57     ` Sergey Kaplun via Tarantool-patches
2024-04-11 16:30 ` Sergey Bronnikov via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox