* [Tarantool-patches] [PATCH luajit 1/3] test: unify helpers for a custom allocator setting
2025-03-10 14:51 [Tarantool-patches] [PATCH luajit 0/3] Fixes for the concat metamethod Sergey Kaplun via Tarantool-patches
@ 2025-03-10 14:51 ` Sergey Kaplun via Tarantool-patches
2025-03-11 11:32 ` Sergey Bronnikov via Tarantool-patches
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM Sergey Kaplun via Tarantool-patches
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-03-10 14:51 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
This patch merges two Lua C libraries that contain custom Lua allocators
with the injection of returning `NULL` into 1 library
<utils/allocinject.c>. The new library contains 2 helpers:
* `enable_null_alloc()` -- returns `NULL` on every allocation (not
reallocation).
* `enable_null_doubling_realloc()` -- returns `NULL` on every
reallocation that doubles the size of the object. It can be useful for
testing corner cases, like OOM during IR/snapshot buffer reallocation
during trace recording.
The original allocator is returned via the `disable()` library function.
The unified library helps to avoid code duplication and can be reused
for future tests.
Needed for tarantool/tarantool#11055
---
test/tarantool-tests/CMakeLists.txt | 5 +-
.../lj-1166-error-stitch-oom-ir-buff.test.lua | 2 +-
...j-1166-error-stitch-oom-snap-buff.test.lua | 2 +-
...j-1247-fin-tab-rehashing-on-trace.test.lua | 4 +-
.../CMakeLists.txt | 2 -
.../lj_1247_allocinject.c | 49 -------------------
.../CMakeLists.txt | 1 +
.../allocinject.c | 34 +++++++++++--
8 files changed, 37 insertions(+), 62 deletions(-)
delete mode 100644 test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt
delete mode 100644 test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c
rename test/tarantool-tests/{lj-1166-error-stitch => utils}/CMakeLists.txt (77%)
rename test/tarantool-tests/{lj-1166-error-stitch => utils}/allocinject.c (50%)
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index a6d09dc7..682a883a 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -60,13 +60,14 @@ add_subdirectory(lj-802-panic-at-mcode-protfail)
add_subdirectory(lj-flush-on-trace)
add_subdirectory(lj-1004-oom-error-frame)
add_subdirectory(lj-1066-fix-cur_L-after-coroutine-resume)
-add_subdirectory(lj-1166-error-stitch)
-add_subdirectory(lj-1247-fin-tab-rehashing-on-trace)
add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/both)
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)
+# Module for alloc injections to return OOM for allocations.
+add_subdirectory(utils)
+
if(BUILDMODE STREQUAL "dynamic")
add_subdirectory(fix-argv-handling)
endif()
diff --git a/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
index 354f5975..dc21cfbf 100644
--- a/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
+++ b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
@@ -57,7 +57,7 @@ jparse.start('t')
jit.on()
jit.opt.start('hotloop=1', '-loop', '-fold')
-allocinject.enable()
+allocinject.enable_null_doubling_realloc()
tracef()
diff --git a/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua b/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
index c8e13760..b2387f23 100644
--- a/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
+++ b/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
@@ -59,7 +59,7 @@ jparse.start('t')
jit.opt.start('hotloop=1')
jit.on()
-allocinject.enable()
+allocinject.enable_null_doubling_realloc()
tracef()
diff --git a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua
index 308043a2..b9a49700 100644
--- a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua
+++ b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua
@@ -24,7 +24,7 @@ local test = tap.test('lj-1247-fin-tab-rehashing-on-trace'):skipcond({
-- omitted, we test only the first case.
test:plan(2)
-local allocinject = require('lj_1247_allocinject')
+local allocinject = require('allocinject')
local ffi = require('ffi')
ffi.cdef[[
@@ -115,7 +115,7 @@ crash_on_trace_unwind_gc_setup()
-- OOM on every allocation (i.e., on finalizer table rehashing
-- too).
-allocinject.enable()
+allocinject.enable_null_alloc()
local r, err = pcall(f, res_tab)
diff --git a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt
deleted file mode 100644
index 8c6fe7e4..00000000
--- a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-get_filename_component(test_name ${CMAKE_CURRENT_SOURCE_DIR} NAME)
-BuildTestCLib(lj_1247_allocinject lj_1247_allocinject.c ${test_name}.test.lua)
diff --git a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c
deleted file mode 100644
index 81aea60b..00000000
--- a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c
+++ /dev/null
@@ -1,49 +0,0 @@
-#include "lua.h"
-#include "lauxlib.h"
-
-#undef NDEBUG
-#include <assert.h>
-
-static lua_Alloc old_allocf = NULL;
-static void *old_alloc_state = NULL;
-
-/* Function to be used instead of the default allocator. */
-static void *allocf_with_injection(void *ud, void *ptr, size_t osize,
- size_t nsize)
-{
- /* Always OOM on allocation (not on realloc). */
- if (ptr == NULL)
- return NULL;
- else
- return old_allocf(ud, ptr, osize, nsize);
-}
-
-static int enable(lua_State *L)
-{
- assert(old_allocf == NULL);
- old_allocf = lua_getallocf(L, &old_alloc_state);
- lua_setallocf(L, allocf_with_injection, old_alloc_state);
- return 0;
-}
-
-static int disable(lua_State *L)
-{
- assert(old_allocf != NULL);
- assert(old_allocf != allocf_with_injection);
- lua_setallocf(L, old_allocf, old_alloc_state);
- old_allocf = NULL;
- old_alloc_state = NULL;
- return 0;
-}
-
-static const struct luaL_Reg allocinject[] = {
- {"enable", enable},
- {"disable", disable},
- {NULL, NULL}
-};
-
-LUA_API int luaopen_lj_1247_allocinject(lua_State *L)
-{
- luaL_register(L, "lj_1247_allocinject", allocinject);
- return 1;
-}
diff --git a/test/tarantool-tests/lj-1166-error-stitch/CMakeLists.txt b/test/tarantool-tests/utils/CMakeLists.txt
similarity index 77%
rename from test/tarantool-tests/lj-1166-error-stitch/CMakeLists.txt
rename to test/tarantool-tests/utils/CMakeLists.txt
index 658b3b8c..624cc933 100644
--- a/test/tarantool-tests/lj-1166-error-stitch/CMakeLists.txt
+++ b/test/tarantool-tests/utils/CMakeLists.txt
@@ -1,5 +1,6 @@
list(APPEND tests
lj-1166-error-stitch-oom-ir-buff.test.lua
lj-1166-error-stitch-oom-snap-buff.test.lua
+ lj-1247-fin-tab-rehashing-on-trace.test.lua
)
BuildTestCLib(allocinject allocinject.c "${tests}")
diff --git a/test/tarantool-tests/lj-1166-error-stitch/allocinject.c b/test/tarantool-tests/utils/allocinject.c
similarity index 50%
rename from test/tarantool-tests/lj-1166-error-stitch/allocinject.c
rename to test/tarantool-tests/utils/allocinject.c
index 88fc9138..21e1d611 100644
--- a/test/tarantool-tests/lj-1166-error-stitch/allocinject.c
+++ b/test/tarantool-tests/utils/allocinject.c
@@ -7,9 +7,22 @@
static lua_Alloc old_allocf = NULL;
static void *old_alloc_state = NULL;
-/* Function to be used instead of the default allocator. */
-static void *allocf_with_injection(void *ud, void *ptr, size_t osize,
+/* Functions to be used instead of the default allocator. */
+
+/* Always OOM on allocation (not on realloc). */
+static void *allocf_inj_null_alloc(void *ud, void *ptr, size_t osize,
size_t nsize)
+{
+ assert(old_allocf != NULL);
+ if (ptr == NULL)
+ return NULL;
+ else
+ return old_allocf(ud, ptr, osize, nsize);
+}
+
+/* Returns `NULL` on reallocations doubling used memory. */
+static void *allocf_inj_null_doubling_realloc(void *ud, void *ptr, size_t osize,
+ size_t nsize)
{
assert(old_allocf != NULL);
/*
@@ -21,7 +34,7 @@ static void *allocf_with_injection(void *ud, void *ptr, size_t osize,
return old_allocf(ud, ptr, osize, nsize);
}
-static int enable(lua_State *L)
+static int enable(lua_State *L, lua_Alloc allocf_with_injection)
{
assert(old_allocf == NULL);
old_allocf = lua_getallocf(L, &old_alloc_state);
@@ -29,10 +42,20 @@ static int enable(lua_State *L)
return 0;
}
+static int enable_null_alloc(lua_State *L)
+{
+ return enable(L, allocf_inj_null_alloc);
+}
+
+static int enable_null_doubling_realloc(lua_State *L)
+{
+ return enable(L, allocf_inj_null_doubling_realloc);
+}
+
+/* Restore the default allocator function. */
static int disable(lua_State *L)
{
assert(old_allocf != NULL);
- assert(old_allocf != allocf_with_injection);
lua_setallocf(L, old_allocf, old_alloc_state);
old_allocf = NULL;
old_alloc_state = NULL;
@@ -40,7 +63,8 @@ static int disable(lua_State *L)
}
static const struct luaL_Reg allocinject[] = {
- {"enable", enable},
+ {"enable_null_alloc", enable_null_alloc},
+ {"enable_null_doubling_realloc", enable_null_doubling_realloc},
{"disable", disable},
{NULL, NULL}
};
--
2.48.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Tarantool-patches] [PATCH luajit 1/3] test: unify helpers for a custom allocator setting
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 1/3] test: unify helpers for a custom allocator setting Sergey Kaplun via Tarantool-patches
@ 2025-03-11 11:32 ` Sergey Bronnikov via Tarantool-patches
2025-03-11 12:38 ` Sergey Kaplun via Tarantool-patches
0 siblings, 1 reply; 14+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-03-11 11:32 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 9908 bytes --]
Hi, Sergey!
Thanks for the patch! LGTM with two comments below.
Feel free to ignore.
Sergey
On 10.03.2025 17:51, Sergey Kaplun wrote:
> This patch merges two Lua C libraries that contain custom Lua allocators
> with the injection of returning `NULL` into 1 library
Minor: s/1/a single/
> <utils/allocinject.c>. The new library contains 2 helpers:
Minor: s/2/two/
> * `enable_null_alloc()` -- returns `NULL` on every allocation (not
> reallocation).
> * `enable_null_doubling_realloc()` -- returns `NULL` on every
> reallocation that doubles the size of the object. It can be useful for
> testing corner cases, like OOM during IR/snapshot buffer reallocation
> during trace recording.
> The original allocator is returned via the `disable()` library function.
>
> The unified library helps to avoid code duplication and can be reused
> for future tests.
>
> Needed for tarantool/tarantool#11055
> ---
> test/tarantool-tests/CMakeLists.txt | 5 +-
> .../lj-1166-error-stitch-oom-ir-buff.test.lua | 2 +-
> ...j-1166-error-stitch-oom-snap-buff.test.lua | 2 +-
> ...j-1247-fin-tab-rehashing-on-trace.test.lua | 4 +-
> .../CMakeLists.txt | 2 -
> .../lj_1247_allocinject.c | 49 -------------------
> .../CMakeLists.txt | 1 +
> .../allocinject.c | 34 +++++++++++--
> 8 files changed, 37 insertions(+), 62 deletions(-)
> delete mode 100644 test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt
> delete mode 100644 test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c
> rename test/tarantool-tests/{lj-1166-error-stitch => utils}/CMakeLists.txt (77%)
> rename test/tarantool-tests/{lj-1166-error-stitch => utils}/allocinject.c (50%)
>
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index a6d09dc7..682a883a 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -60,13 +60,14 @@ add_subdirectory(lj-802-panic-at-mcode-protfail)
> add_subdirectory(lj-flush-on-trace)
> add_subdirectory(lj-1004-oom-error-frame)
> add_subdirectory(lj-1066-fix-cur_L-after-coroutine-resume)
> -add_subdirectory(lj-1166-error-stitch)
> -add_subdirectory(lj-1247-fin-tab-rehashing-on-trace)
> add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/both)
> 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)
>
> +# Module for alloc injections to return OOM for allocations.
> +add_subdirectory(utils)
> +
> if(BUILDMODE STREQUAL "dynamic")
> add_subdirectory(fix-argv-handling)
> endif()
> diff --git a/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
> index 354f5975..dc21cfbf 100644
> --- a/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
> +++ b/test/tarantool-tests/lj-1166-error-stitch-oom-ir-buff.test.lua
> @@ -57,7 +57,7 @@ jparse.start('t')
> jit.on()
> jit.opt.start('hotloop=1', '-loop', '-fold')
>
> -allocinject.enable()
> +allocinject.enable_null_doubling_realloc()
>
> tracef()
>
> diff --git a/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua b/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
> index c8e13760..b2387f23 100644
> --- a/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
> +++ b/test/tarantool-tests/lj-1166-error-stitch-oom-snap-buff.test.lua
> @@ -59,7 +59,7 @@ jparse.start('t')
> jit.opt.start('hotloop=1')
> jit.on()
>
> -allocinject.enable()
> +allocinject.enable_null_doubling_realloc()
>
> tracef()
>
> diff --git a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua
> index 308043a2..b9a49700 100644
> --- a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua
> +++ b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace.test.lua
> @@ -24,7 +24,7 @@ local test = tap.test('lj-1247-fin-tab-rehashing-on-trace'):skipcond({
> -- omitted, we test only the first case.
> test:plan(2)
>
> -local allocinject = require('lj_1247_allocinject')
> +local allocinject = require('allocinject')
>
> local ffi = require('ffi')
> ffi.cdef[[
> @@ -115,7 +115,7 @@ crash_on_trace_unwind_gc_setup()
>
> -- OOM on every allocation (i.e., on finalizer table rehashing
> -- too).
> -allocinject.enable()
> +allocinject.enable_null_alloc()
>
> local r, err = pcall(f, res_tab)
>
> diff --git a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt
> deleted file mode 100644
> index 8c6fe7e4..00000000
> --- a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/CMakeLists.txt
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -get_filename_component(test_name ${CMAKE_CURRENT_SOURCE_DIR} NAME)
> -BuildTestCLib(lj_1247_allocinject lj_1247_allocinject.c ${test_name}.test.lua)
> diff --git a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c b/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c
> deleted file mode 100644
> index 81aea60b..00000000
> --- a/test/tarantool-tests/lj-1247-fin-tab-rehashing-on-trace/lj_1247_allocinject.c
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -#include "lua.h"
> -#include "lauxlib.h"
> -
> -#undef NDEBUG
> -#include <assert.h>
> -
> -static lua_Alloc old_allocf = NULL;
> -static void *old_alloc_state = NULL;
> -
> -/* Function to be used instead of the default allocator. */
> -static void *allocf_with_injection(void *ud, void *ptr, size_t osize,
> - size_t nsize)
> -{
> - /* Always OOM on allocation (not on realloc). */
> - if (ptr == NULL)
> - return NULL;
> - else
> - return old_allocf(ud, ptr, osize, nsize);
> -}
> -
> -static int enable(lua_State *L)
> -{
> - assert(old_allocf == NULL);
> - old_allocf = lua_getallocf(L, &old_alloc_state);
> - lua_setallocf(L, allocf_with_injection, old_alloc_state);
> - return 0;
> -}
> -
> -static int disable(lua_State *L)
> -{
> - assert(old_allocf != NULL);
> - assert(old_allocf != allocf_with_injection);
> - lua_setallocf(L, old_allocf, old_alloc_state);
> - old_allocf = NULL;
> - old_alloc_state = NULL;
> - return 0;
> -}
> -
> -static const struct luaL_Reg allocinject[] = {
> - {"enable", enable},
> - {"disable", disable},
> - {NULL, NULL}
> -};
> -
> -LUA_API int luaopen_lj_1247_allocinject(lua_State *L)
> -{
> - luaL_register(L, "lj_1247_allocinject", allocinject);
> - return 1;
> -}
> diff --git a/test/tarantool-tests/lj-1166-error-stitch/CMakeLists.txt b/test/tarantool-tests/utils/CMakeLists.txt
> similarity index 77%
> rename from test/tarantool-tests/lj-1166-error-stitch/CMakeLists.txt
> rename to test/tarantool-tests/utils/CMakeLists.txt
> index 658b3b8c..624cc933 100644
> --- a/test/tarantool-tests/lj-1166-error-stitch/CMakeLists.txt
> +++ b/test/tarantool-tests/utils/CMakeLists.txt
> @@ -1,5 +1,6 @@
> list(APPEND tests
> lj-1166-error-stitch-oom-ir-buff.test.lua
> lj-1166-error-stitch-oom-snap-buff.test.lua
> + lj-1247-fin-tab-rehashing-on-trace.test.lua
> )
> BuildTestCLib(allocinject allocinject.c "${tests}")
> diff --git a/test/tarantool-tests/lj-1166-error-stitch/allocinject.c b/test/tarantool-tests/utils/allocinject.c
> similarity index 50%
> rename from test/tarantool-tests/lj-1166-error-stitch/allocinject.c
> rename to test/tarantool-tests/utils/allocinject.c
> index 88fc9138..21e1d611 100644
> --- a/test/tarantool-tests/lj-1166-error-stitch/allocinject.c
> +++ b/test/tarantool-tests/utils/allocinject.c
> @@ -7,9 +7,22 @@
> static lua_Alloc old_allocf = NULL;
> static void *old_alloc_state = NULL;
>
> -/* Function to be used instead of the default allocator. */
> -static void *allocf_with_injection(void *ud, void *ptr, size_t osize,
> +/* Functions to be used instead of the default allocator. */
> +
> +/* Always OOM on allocation (not on realloc). */
> +static void *allocf_inj_null_alloc(void *ud, void *ptr, size_t osize,
> size_t nsize)
> +{
> + assert(old_allocf != NULL);
> + if (ptr == NULL)
> + return NULL;
> + else
> + return old_allocf(ud, ptr, osize, nsize);
> +}
> +
> +/* Returns `NULL` on reallocations doubling used memory. */
> +static void *allocf_inj_null_doubling_realloc(void *ud, void *ptr, size_t osize,
> + size_t nsize)
> {
> assert(old_allocf != NULL);
> /*
> @@ -21,7 +34,7 @@ static void *allocf_with_injection(void *ud, void *ptr, size_t osize,
> return old_allocf(ud, ptr, osize, nsize);
> }
>
> -static int enable(lua_State *L)
> +static int enable(lua_State *L, lua_Alloc allocf_with_injection)
> {
> assert(old_allocf == NULL);
> old_allocf = lua_getallocf(L, &old_alloc_state);
> @@ -29,10 +42,20 @@ static int enable(lua_State *L)
> return 0;
> }
>
> +static int enable_null_alloc(lua_State *L)
> +{
> + return enable(L, allocf_inj_null_alloc);
> +}
> +
> +static int enable_null_doubling_realloc(lua_State *L)
> +{
> + return enable(L, allocf_inj_null_doubling_realloc);
> +}
> +
> +/* Restore the default allocator function. */
> static int disable(lua_State *L)
> {
> assert(old_allocf != NULL);
> - assert(old_allocf != allocf_with_injection);
> lua_setallocf(L, old_allocf, old_alloc_state);
> old_allocf = NULL;
> old_alloc_state = NULL;
> @@ -40,7 +63,8 @@ static int disable(lua_State *L)
> }
>
> static const struct luaL_Reg allocinject[] = {
> - {"enable", enable},
> + {"enable_null_alloc", enable_null_alloc},
> + {"enable_null_doubling_realloc", enable_null_doubling_realloc},
> {"disable", disable},
> {NULL, NULL}
> };
[-- Attachment #2: Type: text/html, Size: 10310 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Tarantool-patches] [PATCH luajit 1/3] test: unify helpers for a custom allocator setting
2025-03-11 11:32 ` Sergey Bronnikov via Tarantool-patches
@ 2025-03-11 12:38 ` Sergey Kaplun via Tarantool-patches
2025-03-11 14:45 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 1 reply; 14+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-03-11 12:38 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
Hi, Sergey!
Thanks for the review!
Fixed your comments and force-pushed the branch.
On 11.03.25, Sergey Bronnikov wrote:
> Hi, Sergey!
>
> Thanks for the patch! LGTM with two comments below.
>
> Feel free to ignore.
>
> Sergey
>
> On 10.03.2025 17:51, Sergey Kaplun wrote:
> > This patch merges two Lua C libraries that contain custom Lua allocators
> > with the injection of returning `NULL` into 1 library
> Minor: s/1/a single/
Fixed.
> > <utils/allocinject.c>. The new library contains 2 helpers:
> Minor: s/2/two/
Fixed.
> > * `enable_null_alloc()` -- returns `NULL` on every allocation (not
> > reallocation).
> > * `enable_null_doubling_realloc()` -- returns `NULL` on every
> > reallocation that doubles the size of the object. It can be useful for
> > testing corner cases, like OOM during IR/snapshot buffer reallocation
> > during trace recording.
> > The original allocator is returned via the `disable()` library function.
> >
> > The unified library helps to avoid code duplication and can be reused
> > for future tests.
> >
> > Needed for tarantool/tarantool#11055
> > ---
<snipped>
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM.
2025-03-10 14:51 [Tarantool-patches] [PATCH luajit 0/3] Fixes for the concat metamethod Sergey Kaplun via Tarantool-patches
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 1/3] test: unify helpers for a custom allocator setting Sergey Kaplun via Tarantool-patches
@ 2025-03-10 14:51 ` Sergey Kaplun via Tarantool-patches
2025-03-11 12:01 ` Sergey Bronnikov via Tarantool-patches
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 3/3] Fix state restore when recording __concat metamethod Sergey Kaplun via Tarantool-patches
2025-03-26 8:55 ` [Tarantool-patches] [PATCH luajit 0/3] Fixes for the concat metamethod Sergey Kaplun via Tarantool-patches
3 siblings, 1 reply; 14+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-03-10 14:51 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
From: Mike Pall <mike>
Reported by Sergey Kaplun.
(cherry picked from commit 19878ec05c239ccaf5f3d17af27670a963e25b8b)
Before the patch, the Lua stack isn't restored if the OOM related to
string allocation happens during fold optimization inside `rec_cat()`.
This patch moves most of the `rec_cat()` logic to the protected
`rec_mm_concat_cp()` to be able to restore the stack in case of OOM.
Unfortunately, after this patch, the test
<lj-839-concat-recording.test.lua> fails since the incorrect adjusting
of the topslot. It will be fixed in the next patch.
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#11055
---
src/lj_record.c | 51 +++++++++++-------
.../lj-1298-oom-on-concat-recording.test.lua | 53 +++++++++++++++++++
test/tarantool-tests/utils/CMakeLists.txt | 1 +
test/tarantool-tests/utils/allocinject.c | 21 ++++++++
4 files changed, 108 insertions(+), 18 deletions(-)
create mode 100644 test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
diff --git a/src/lj_record.c b/src/lj_record.c
index 5345fa63..7a481a51 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -1943,25 +1943,19 @@ static TRef rec_tnew(jit_State *J, uint32_t ah)
typedef struct RecCatDataCP {
jit_State *J;
- RecordIndex *ix;
+ BCReg baseslot, topslot;
+ TRef tr;
} RecCatDataCP;
static TValue *rec_mm_concat_cp(lua_State *L, lua_CFunction dummy, void *ud)
{
RecCatDataCP *rcd = (RecCatDataCP *)ud;
- UNUSED(L); UNUSED(dummy);
- rec_mm_arith(rcd->J, rcd->ix, MM_concat); /* Call __concat metamethod. */
- return NULL;
-}
-
-static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
-{
+ jit_State *J = rcd->J;
+ BCReg baseslot = rcd->baseslot, topslot = rcd->topslot;
TRef *top = &J->base[topslot];
- TValue savetv[5+LJ_FR2];
BCReg s;
RecordIndex ix;
- RecCatDataCP rcd;
- int errcode;
+ UNUSED(L); UNUSED(dummy);
lj_assertJ(baseslot < topslot, "bad CAT arg");
for (s = baseslot; s <= topslot; s++)
(void)getslot(J, s); /* Ensure all arguments have a reference. */
@@ -1983,7 +1977,10 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
} while (trp <= top);
tr = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr);
J->maxslot = (BCReg)(xbase - J->base);
- if (xbase == base) return tr; /* Return simple concatenation result. */
+ if (xbase == base) {
+ rcd->tr = tr; /* Return simple concatenation result. */
+ return NULL;
+ }
/* Pass partial result. */
topslot = J->maxslot--;
*xbase = tr;
@@ -1996,13 +1993,31 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
copyTV(J->L, &ix.tabv, &J->L->base[topslot-1]);
ix.tab = top[-1];
ix.key = top[0];
- memcpy(savetv, &J->L->base[topslot-1], sizeof(savetv)); /* Save slots. */
+ rec_mm_arith(J, &ix, MM_concat); /* Call __concat metamethod. */
+ rcd->tr = 0; /* No result yet. */
+ return NULL;
+}
+
+static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
+{
+ lua_State *L = J->L;
+ ptrdiff_t delta = L->top - L->base;
+ TValue savetv[5+LJ_FR2], errobj;
+ RecCatDataCP rcd;
+ int errcode;
rcd.J = J;
- rcd.ix = &ix;
- errcode = lj_vm_cpcall(J->L, NULL, &rcd, rec_mm_concat_cp);
- memcpy(&J->L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */
- if (errcode) return (TRef)(-errcode);
- return 0; /* No result yet. */
+ rcd.baseslot = baseslot;
+ rcd.topslot = topslot;
+ memcpy(savetv, &L->base[topslot-1], sizeof(savetv)); /* Save slots. */
+ errcode = lj_vm_cpcall(L, NULL, &rcd, rec_mm_concat_cp);
+ if (errcode) copyTV(L, &errobj, L->top-1);
+ memcpy(&L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */
+ if (errcode) {
+ L->top = L->base + delta;
+ copyTV(L, L->top++, &errobj);
+ return (TRef)(-errcode);
+ }
+ return rcd.tr;
}
/* -- Record bytecode ops ------------------------------------------------- */
diff --git a/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
new file mode 100644
index 00000000..961df798
--- /dev/null
+++ b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
@@ -0,0 +1,53 @@
+local tap = require('tap')
+
+-- Test file to demonstrate unbalanced Lua stack after instruction
+-- recording due to throwing an OOM error at the moment of
+-- recording without restoring the Lua stack back.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1298.
+
+local test = tap.test('lj-1298-oom-on-concat-recording'):skipcond({
+ ['Test requires JIT enabled'] = not jit.status(),
+})
+
+local allocinject = require('allocinject')
+
+test:plan(2)
+
+jit.opt.start('hotloop=1')
+
+-- Allocation limit to return `NULL`.
+local ALLOC_LIMIT = 2048
+
+local bigstr = string.rep('x', ALLOC_LIMIT)
+local __concat = function()
+ return 'concated'
+end
+
+-- Need to use metamethod call in the concat recording.
+-- We may use any object with a metamethod, but let's use a table
+-- as the most common one.
+local concatable_t = setmetatable({}, {
+ __concat = __concat,
+})
+
+local function concat_with_err()
+ local counter = 0
+ local _
+ while counter < 1 do
+ counter = counter + 1
+ _ = bigstr .. concatable_t .. ''
+ end
+ assert(false, 'unreachable, should raise an error before')
+end
+
+-- Returns `NULL` on any allocation beyond the given limit.
+allocinject.enable_null_limited_alloc(ALLOC_LIMIT)
+
+local status, errmsg = pcall(concat_with_err)
+
+allocinject.disable()
+
+test:ok(not status, 'no assertion failure during recording, correct status')
+test:like(errmsg, 'not enough memory', 'correct error message')
+
+test:done(true)
diff --git a/test/tarantool-tests/utils/CMakeLists.txt b/test/tarantool-tests/utils/CMakeLists.txt
index 624cc933..15871934 100644
--- a/test/tarantool-tests/utils/CMakeLists.txt
+++ b/test/tarantool-tests/utils/CMakeLists.txt
@@ -2,5 +2,6 @@ list(APPEND tests
lj-1166-error-stitch-oom-ir-buff.test.lua
lj-1166-error-stitch-oom-snap-buff.test.lua
lj-1247-fin-tab-rehashing-on-trace.test.lua
+ lj-1298-oom-on-concat-recording.test.lua
)
BuildTestCLib(allocinject allocinject.c "${tests}")
diff --git a/test/tarantool-tests/utils/allocinject.c b/test/tarantool-tests/utils/allocinject.c
index 21e1d611..7ac60625 100644
--- a/test/tarantool-tests/utils/allocinject.c
+++ b/test/tarantool-tests/utils/allocinject.c
@@ -34,6 +34,19 @@ static void *allocf_inj_null_doubling_realloc(void *ud, void *ptr, size_t osize,
return old_allocf(ud, ptr, osize, nsize);
}
+static size_t limit = 0;
+/* Returns `NULL` on allocations beyond the given limit. */
+static void *allocf_inj_null_limited_alloc(void *ud, void *ptr, size_t osize,
+ size_t nsize)
+{
+ assert(old_allocf != NULL);
+ assert(limit != 0);
+ /* Check the specific allocation. */
+ if (osize == 0 && nsize > limit)
+ return NULL;
+ return old_allocf(ud, ptr, osize, nsize);
+}
+
static int enable(lua_State *L, lua_Alloc allocf_with_injection)
{
assert(old_allocf == NULL);
@@ -52,6 +65,13 @@ static int enable_null_doubling_realloc(lua_State *L)
return enable(L, allocf_inj_null_doubling_realloc);
}
+static int enable_null_limited_alloc(lua_State *L)
+{
+ limit = lua_tointeger(L, 1);
+ assert(limit != 0);
+ return enable(L, allocf_inj_null_limited_alloc);
+}
+
/* Restore the default allocator function. */
static int disable(lua_State *L)
{
@@ -65,6 +85,7 @@ static int disable(lua_State *L)
static const struct luaL_Reg allocinject[] = {
{"enable_null_alloc", enable_null_alloc},
{"enable_null_doubling_realloc", enable_null_doubling_realloc},
+ {"enable_null_limited_alloc", enable_null_limited_alloc},
{"disable", disable},
{NULL, NULL}
};
--
2.48.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM.
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM Sergey Kaplun via Tarantool-patches
@ 2025-03-11 12:01 ` Sergey Bronnikov via Tarantool-patches
2025-03-11 12:37 ` Sergey Kaplun via Tarantool-patches
0 siblings, 1 reply; 14+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-03-11 12:01 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]
Hi, Sergey,
thanks for the patch! LGTM with a minor comment below.
Sergey
On 10.03.2025 17:51, Sergey Kaplun wrote:
<snipped>
> diff --git a/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
> new file mode 100644
> index 00000000..961df798
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
> @@ -0,0 +1,53 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate unbalanced Lua stack after instruction
> +-- recording due to throwing an OOM error at the moment of
> +-- recording without restoring the Lua stack back.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1298.
> +
> +local test = tap.test('lj-1298-oom-on-concat-recording'):skipcond({
> + ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +local allocinject = require('allocinject')
> +
> +test:plan(2)
> +
> +jit.opt.start('hotloop=1')
> +
> +-- Allocation limit to return `NULL`.
> +local ALLOC_LIMIT = 2048
> +
> +local bigstr = string.rep('x', ALLOC_LIMIT)
> +local __concat = function()
> + return 'concated'
s/concated/concatenated/
<snipped>
[-- Attachment #2: Type: text/html, Size: 1776 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM.
2025-03-11 12:01 ` Sergey Bronnikov via Tarantool-patches
@ 2025-03-11 12:37 ` Sergey Kaplun via Tarantool-patches
2025-03-11 14:46 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 1 reply; 14+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-03-11 12:37 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
Hi, Sergey!
Thanks for the review!
Fixed your comment and force-pushed the branch.
On 11.03.25, Sergey Bronnikov wrote:
> Hi, Sergey,
>
> thanks for the patch! LGTM with a minor comment below.
>
> Sergey
>
> On 10.03.2025 17:51, Sergey Kaplun wrote:
<snipped>
> > +local bigstr = string.rep('x', ALLOC_LIMIT)
> > +local __concat = function()
> > + return 'concated'
> s/concated/concatenated/
Fixed, thanks:
===================================================================
diff --git a/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
index 961df798..20d93c43 100644
--- a/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
+++ b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
@@ -20,7 +20,7 @@ local ALLOC_LIMIT = 2048
local bigstr = string.rep('x', ALLOC_LIMIT)
local __concat = function()
- return 'concated'
+ return 'concatenated'
end
-- Need to use metamethod call in the concat recording.
===================================================================
I've also checked that this is not ruined the testcase.
>
>
> <snipped>
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM.
2025-03-11 12:37 ` Sergey Kaplun via Tarantool-patches
@ 2025-03-11 14:46 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 0 replies; 14+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-03-11 14:46 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]
Thanks! LGTM
On 11.03.2025 15:37, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed your comment and force-pushed the branch.
>
> On 11.03.25, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for the patch! LGTM with a minor comment below.
>>
>> Sergey
>>
>> On 10.03.2025 17:51, Sergey Kaplun wrote:
> <snipped>
>
>>> +local bigstr = string.rep('x', ALLOC_LIMIT)
>>> +local __concat = function()
>>> + return 'concated'
>> s/concated/concatenated/
> Fixed, thanks:
>
> ===================================================================
> diff --git a/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
> index 961df798..20d93c43 100644
> --- a/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
> +++ b/test/tarantool-tests/lj-1298-oom-on-concat-recording.test.lua
> @@ -20,7 +20,7 @@ local ALLOC_LIMIT = 2048
>
> local bigstr = string.rep('x', ALLOC_LIMIT)
> local __concat = function()
> - return 'concated'
> + return 'concatenated'
> end
>
> -- Need to use metamethod call in the concat recording.
> ===================================================================
>
> I've also checked that this is not ruined the testcase.
>
>>
>> <snipped>
[-- Attachment #2: Type: text/html, Size: 2182 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Tarantool-patches] [PATCH luajit 3/3] Fix state restore when recording __concat metamethod.
2025-03-10 14:51 [Tarantool-patches] [PATCH luajit 0/3] Fixes for the concat metamethod Sergey Kaplun via Tarantool-patches
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 1/3] test: unify helpers for a custom allocator setting Sergey Kaplun via Tarantool-patches
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 2/3] Restore state when recording __concat metamethod throws OOM Sergey Kaplun via Tarantool-patches
@ 2025-03-10 14:51 ` Sergey Kaplun via Tarantool-patches
2025-03-12 7:53 ` Sergey Bronnikov via Tarantool-patches
2025-03-26 8:55 ` [Tarantool-patches] [PATCH luajit 0/3] Fixes for the concat metamethod Sergey Kaplun via Tarantool-patches
3 siblings, 1 reply; 14+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-03-10 14:51 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
From: Mike Pall <mike>
Reported by Sergey Kaplun.
(cherry picked from commit eee16efa77b542e99c8e546a3d52fc023925c7bc)
This commit is a follow-up to the previous one. It fixes the case when
the `topslot` is adjusting for simple concatenation results. This patch
adds the update of the corresponding Lua stack slots to be restored.
This fixes back the <lj-839-concat-recording.test.lua> test.
Sergey Kaplun:
* added the description and the test for the problem
Part of tarantool/tarantool#11055
---
src/lj_record.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/lj_record.c b/src/lj_record.c
index 7a481a51..92cf55e4 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -1942,6 +1942,7 @@ static TRef rec_tnew(jit_State *J, uint32_t ah)
/* -- Concatenation ------------------------------------------------------- */
typedef struct RecCatDataCP {
+ TValue savetv[5+LJ_FR2];
jit_State *J;
BCReg baseslot, topslot;
TRef tr;
@@ -1982,7 +1983,9 @@ static TValue *rec_mm_concat_cp(lua_State *L, lua_CFunction dummy, void *ud)
return NULL;
}
/* Pass partial result. */
- topslot = J->maxslot--;
+ rcd->topslot = topslot = J->maxslot--;
+ /* Save updated range of slots. */
+ memcpy(rcd->savetv, &L->base[topslot-1], sizeof(rcd->savetv));
*xbase = tr;
top = xbase;
setstrV(J->L, &ix.keyv, &J2G(J)->strempty); /* Simulate string result. */
@@ -2002,16 +2005,18 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
{
lua_State *L = J->L;
ptrdiff_t delta = L->top - L->base;
- TValue savetv[5+LJ_FR2], errobj;
+ TValue errobj;
RecCatDataCP rcd;
int errcode;
rcd.J = J;
rcd.baseslot = baseslot;
rcd.topslot = topslot;
- memcpy(savetv, &L->base[topslot-1], sizeof(savetv)); /* Save slots. */
+ /* Save slots. */
+ memcpy(rcd.savetv, &L->base[topslot-1], sizeof(rcd.savetv));
errcode = lj_vm_cpcall(L, NULL, &rcd, rec_mm_concat_cp);
if (errcode) copyTV(L, &errobj, L->top-1);
- memcpy(&L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */
+ /* Restore slots. */
+ memcpy(&L->base[rcd.topslot-1], rcd.savetv, sizeof(rcd.savetv));
if (errcode) {
L->top = L->base + delta;
copyTV(L, L->top++, &errobj);
--
2.48.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Tarantool-patches] [PATCH luajit 3/3] Fix state restore when recording __concat metamethod.
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 3/3] Fix state restore when recording __concat metamethod Sergey Kaplun via Tarantool-patches
@ 2025-03-12 7:53 ` Sergey Bronnikov via Tarantool-patches
2025-03-13 10:28 ` Sergey Kaplun via Tarantool-patches
0 siblings, 1 reply; 14+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-03-12 7:53 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 2585 bytes --]
Hi, Sergey,
thanks for the patch! LGTM wit ha minor comment.
Sergey
On 10.03.2025 17:51, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Sergey Kaplun.
>
> (cherry picked from commit eee16efa77b542e99c8e546a3d52fc023925c7bc)
>
> This commit is a follow-up to the previous one. It fixes the case when
> the `topslot` is adjusting for simple concatenation results. This patch
> adds the update of the corresponding Lua stack slots to be restored.
>
> This fixes back the <lj-839-concat-recording.test.lua> test.
>
> Sergey Kaplun:
> * added the description and the test for the problem
It is partially true actually :)
s/and the test//
>
> Part of tarantool/tarantool#11055
> ---
> src/lj_record.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/src/lj_record.c b/src/lj_record.c
> index 7a481a51..92cf55e4 100644
> --- a/src/lj_record.c
> +++ b/src/lj_record.c
> @@ -1942,6 +1942,7 @@ static TRef rec_tnew(jit_State *J, uint32_t ah)
> /* -- Concatenation ------------------------------------------------------- */
>
> typedef struct RecCatDataCP {
> + TValue savetv[5+LJ_FR2];
> jit_State *J;
> BCReg baseslot, topslot;
> TRef tr;
> @@ -1982,7 +1983,9 @@ static TValue *rec_mm_concat_cp(lua_State *L, lua_CFunction dummy, void *ud)
> return NULL;
> }
> /* Pass partial result. */
> - topslot = J->maxslot--;
> + rcd->topslot = topslot = J->maxslot--;
> + /* Save updated range of slots. */
> + memcpy(rcd->savetv, &L->base[topslot-1], sizeof(rcd->savetv));
> *xbase = tr;
> top = xbase;
> setstrV(J->L, &ix.keyv, &J2G(J)->strempty); /* Simulate string result. */
> @@ -2002,16 +2005,18 @@ static TRef rec_cat(jit_State *J, BCReg baseslot, BCReg topslot)
> {
> lua_State *L = J->L;
> ptrdiff_t delta = L->top - L->base;
> - TValue savetv[5+LJ_FR2], errobj;
> + TValue errobj;
> RecCatDataCP rcd;
> int errcode;
> rcd.J = J;
> rcd.baseslot = baseslot;
> rcd.topslot = topslot;
> - memcpy(savetv, &L->base[topslot-1], sizeof(savetv)); /* Save slots. */
> + /* Save slots. */
> + memcpy(rcd.savetv, &L->base[topslot-1], sizeof(rcd.savetv));
> errcode = lj_vm_cpcall(L, NULL, &rcd, rec_mm_concat_cp);
> if (errcode) copyTV(L, &errobj, L->top-1);
> - memcpy(&L->base[topslot-1], savetv, sizeof(savetv)); /* Restore slots. */
> + /* Restore slots. */
> + memcpy(&L->base[rcd.topslot-1], rcd.savetv, sizeof(rcd.savetv));
> if (errcode) {
> L->top = L->base + delta;
> copyTV(L, L->top++, &errobj);
[-- Attachment #2: Type: text/html, Size: 3261 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Tarantool-patches] [PATCH luajit 3/3] Fix state restore when recording __concat metamethod.
2025-03-12 7:53 ` Sergey Bronnikov via Tarantool-patches
@ 2025-03-13 10:28 ` Sergey Kaplun via Tarantool-patches
2025-03-14 10:53 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 1 reply; 14+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-03-13 10:28 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
Hi, Sergey!
Thanks for the review!
Fixed your comment and force-pushed the branch.
On 12.03.25, Sergey Bronnikov wrote:
> Hi, Sergey,
>
> thanks for the patch! LGTM wit ha minor comment.
>
> Sergey
>
> On 10.03.2025 17:51, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Reported by Sergey Kaplun.
> >
> > (cherry picked from commit eee16efa77b542e99c8e546a3d52fc023925c7bc)
> >
> > This commit is a follow-up to the previous one. It fixes the case when
> > the `topslot` is adjusting for simple concatenation results. This patch
> > adds the update of the corresponding Lua stack slots to be restored.
> >
> > This fixes back the <lj-839-concat-recording.test.lua> test.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
>
> It is partially true actually :)
>
> s/and the test//
Yes, the force of the habit (and copy-paste ;))
>
> >
> > Part of tarantool/tarantool#11055
> > ---
<snipped>
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 3/3] Fix state restore when recording __concat metamethod.
2025-03-13 10:28 ` Sergey Kaplun via Tarantool-patches
@ 2025-03-14 10:53 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 0 replies; 14+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-03-14 10:53 UTC (permalink / raw)
To: Sergey Kaplun; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]
Hi, Sergey,
LGTM
On 13.03.2025 13:28, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed your comment and force-pushed the branch.
>
> On 12.03.25, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for the patch! LGTM wit ha minor comment.
>>
>> Sergey
>>
>> On 10.03.2025 17:51, Sergey Kaplun wrote:
>>> From: Mike Pall <mike>
>>>
>>> Reported by Sergey Kaplun.
>>>
>>> (cherry picked from commit eee16efa77b542e99c8e546a3d52fc023925c7bc)
>>>
>>> This commit is a follow-up to the previous one. It fixes the case when
>>> the `topslot` is adjusting for simple concatenation results. This patch
>>> adds the update of the corresponding Lua stack slots to be restored.
>>>
>>> This fixes back the <lj-839-concat-recording.test.lua> test.
>>>
>>> Sergey Kaplun:
>>> * added the description and the test for the problem
>> It is partially true actually :)
>>
>> s/and the test//
> Yes, the force of the habit (and copy-paste ;))
>
>>> Part of tarantool/tarantool#11055
>>> ---
> <snipped>
>
[-- Attachment #2: Type: text/html, Size: 1932 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 0/3] Fixes for the concat metamethod.
2025-03-10 14:51 [Tarantool-patches] [PATCH luajit 0/3] Fixes for the concat metamethod Sergey Kaplun via Tarantool-patches
` (2 preceding siblings ...)
2025-03-10 14:51 ` [Tarantool-patches] [PATCH luajit 3/3] Fix state restore when recording __concat metamethod Sergey Kaplun via Tarantool-patches
@ 2025-03-26 8:55 ` Sergey Kaplun via Tarantool-patches
3 siblings, 0 replies; 14+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-03-26 8:55 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: tarantool-patches
I've applied the patch-set into all long-term branches in
tarantool/luajit and bumped a new version in master [1],
release/3.3 [2], release/3.2 [3] and release/2.11 [4].
[1]: https://github.com/tarantool/tarantool/pull/11281
[2]: https://github.com/tarantool/tarantool/pull/11282
[3]: https://github.com/tarantool/tarantool/pull/11283
[4]: https://github.com/tarantool/tarantool/pull/11284
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 14+ messages in thread