[Tarantool-patches] [PATCH luajit 3/3] Avoid conflict between 64 bit lightuserdata and ITERN key.
sergos
sergos at tarantool.org
Wed Sep 15 18:31:12 MSK 2021
Hi! Thanks for the patch!
Just some test comment.
Regards,
Sergos
> On 9 Sep 2021, at 10:03, Sergey Kaplun <skaplun at tarantool.org> wrote:
>
> From: Mike Pall <mike>
>
> Reported by XmiliaH.
>
> (cherry picked from commit 16d38a4b214e8b8a20be554be77bd20286072365)
>
> This patch fixes the regression introduced in scope of
> fa8e7ffefb715abf55dc5b0c708c63251868 ('Add support for full-range
> 64 bit lightuserdata.').
>
> The maximum available number of lightuserdata segment is 255. So the
> high bits of this lightuserdata TValue are 0xfffe7fff. The same high
> bits are set for special control variable on the stack for ITERN/ITERC
> bytecodes via ISNEXT bytecode. When ITERN bytecode is despecialize to
> ITERC bytecode and a table has the lightuserdata with the maximum
> available segment number as a key, the special control variable is
> considered as this key and iteration is broken.
>
> This patch forbids to use more than 254 lightuserdata segments to
> avoid clashing with the aforementioned control variable. In case when
> user tries to create lightuserdata with 255th segment number an error
> "bad light userdata pointer" is raised.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#5629
> ---
> src/lj_udata.c | 3 +-
> test/tarantool-tests/CMakeLists.txt | 1 +
> .../lj-727-lightuserdata-itern.test.lua | 48 ++++++++++++++
> .../lj-727-lightuserdata-itern/CMakeLists.txt | 1 +
> .../lightuserdata.c | 63 +++++++++++++++++++
> 5 files changed, 115 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/lj-727-lightuserdata-itern.test.lua
> create mode 100644 test/tarantool-tests/lj-727-lightuserdata-itern/CMakeLists.txt
> create mode 100644 test/tarantool-tests/lj-727-lightuserdata-itern/lightuserdata.c
>
> diff --git a/src/lj_udata.c b/src/lj_udata.c
> index 6808b1bc..1b7841fa 100644
> --- a/src/lj_udata.c
> +++ b/src/lj_udata.c
> @@ -49,9 +49,10 @@ void *lj_lightud_intern(lua_State *L, void *p)
> if (segmap[seg] == up) /* Fast path. */
> return (void *)(((uint64_t)seg << LJ_LIGHTUD_BITS_LO) | lightudlo(u));
> segnum++;
> + /* Leave last segment unused to avoid clash with ITERN key. */
> + if (segnum >= (1 << LJ_LIGHTUD_BITS_SEG)-1) lj_err_msg(L, LJ_ERR_BADLU);
> }
> if (!((segnum-1) & segnum) && segnum != 1) {
> - if (segnum >= (1 << LJ_LIGHTUD_BITS_SEG)) lj_err_msg(L, LJ_ERR_BADLU);
> lj_mem_reallocvec(L, segmap, segnum, segnum ? 2*segnum : 2u, uint32_t);
> setmref(g->gc.lightudseg, segmap);
> }
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index a872fa5e..c933cc3f 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -60,6 +60,7 @@ add_subdirectory(gh-4427-ffi-sandwich)
> add_subdirectory(gh-6098-fix-side-exit-patching-on-arm64)
> add_subdirectory(gh-6189-cur_L)
> add_subdirectory(lj-49-bad-lightuserdata)
> +add_subdirectory(lj-727-lightuserdata-itern)
> add_subdirectory(lj-flush-on-trace)
> add_subdirectory(misclib-getmetrics-capi)
>
> diff --git a/test/tarantool-tests/lj-727-lightuserdata-itern.test.lua b/test/tarantool-tests/lj-727-lightuserdata-itern.test.lua
> new file mode 100644
> index 00000000..ebe885bf
> --- /dev/null
> +++ b/test/tarantool-tests/lj-727-lightuserdata-itern.test.lua
> @@ -0,0 +1,48 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate next FF incorrect behaviour on LJ_64.
> +-- See also, https://github.com/LuaJIT/LuaJIT/issues/727.
> +
> +local test = tap.test('lj-727-lightuserdata-itern')
> +test:plan(1)
> +
> +local ud = require('lightuserdata').craft_ptr_wp()
> +
> +-- We now have the tagged lightuuserdata pointer
> +-- 0xFFFE7FFF00000002 in the up before this patch (after the patch
> +-- the maximum available lightuserdata segment is 0xffe).
Shall we end the test here with just an expectation of an error?
I believe you can make a way simpler test: pcall(craft_ptr()) should work
successfully 254 times and error on an 255th one, isn’t it?
> +
> +-- These pointers are special in for loop keys as they are used in
> +-- the INTERN control variable to denote the current index in the
> +-- array.
> +-- If the ITERN is then patched to ITERC because of
> +-- despecialization via the ISNEXT bytecode, the control variable
> +-- is considered as the existing key in the table and some
> +-- elements are skipped during iteration.
> +
> +local real_next = next
> +local next = next
> +
> +local function itern_test(t, f)
> + for k, v in next, t do
> + f(k, v)
> + end
> +end
> +
> +local visited = {}
> +local t = {1, [ud] = 2345}
> +itern_test(t, function(k, v)
> + visited[k] = v
> + if next == real_next then
> + next = function(tab, key)
> + return real_next(tab, key)
> + end
> + -- Despecialize bytecode.
> + itern_test({}, function() end)
> + end
> +end)
> +
> +test.strict = true
> +test:is_deeply(visited, t, 'userdata node is visited')
> +
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/tarantool-tests/lj-727-lightuserdata-itern/CMakeLists.txt b/test/tarantool-tests/lj-727-lightuserdata-itern/CMakeLists.txt
> new file mode 100644
> index 00000000..564b688b
> --- /dev/null
> +++ b/test/tarantool-tests/lj-727-lightuserdata-itern/CMakeLists.txt
> @@ -0,0 +1 @@
> +BuildTestCLib(lightuserdata lightuserdata.c)
> diff --git a/test/tarantool-tests/lj-727-lightuserdata-itern/lightuserdata.c b/test/tarantool-tests/lj-727-lightuserdata-itern/lightuserdata.c
> new file mode 100644
> index 00000000..0fe6441c
> --- /dev/null
> +++ b/test/tarantool-tests/lj-727-lightuserdata-itern/lightuserdata.c
> @@ -0,0 +1,63 @@
> +#include <lua.h>
> +#include <lauxlib.h>
> +
> +#undef NDEBUG
> +#include <assert.h>
> +
> +/* To stay within 47 bits, lightuserdata is segmented. */
> +#define LJ_LIGHTUD_BITS_SEG 8
> +#define NSEGMENTS ((1 << LJ_LIGHTUD_BITS_SEG)-1)
> +
> +/*
> + * The function to wrap: get a number to form lightuserdata to
> + * return with the 0xXXXXXfff00000002 format.
> + * It may raise an error, when the available lightuserdata
> + * segments are run out.
> + */
> +static int craft_ptr(lua_State *L)
> +{
> + const unsigned long long i = lua_tonumber(L, 1);
> + lua_pushlightuserdata(L, (void *)((i << 44) + 0xfff00000002));
> + return 1;
> +}
> +
> +/*
> + * The function to generate bunch of lightuserdata of the
> + * 0xXXXXXfff00000002 format and push the last one on the stack.
> + */
> +static int craft_ptr_wp(lua_State *L)
> +{
> + void *ptr = NULL;
> + /*
> + * There are only 255 available lightuserdata segments.
> + * Generate a bunch of pointers to take them all.
> + * XXX: After this patch the last userdata segment is
> + * reserved for ISNEXT/ITERC/ITERN control variable, so
> + * `craft_ptr()` function will raise an error at the last
> + * iteration.
> + */
> + unsigned long long i = 0;
> + for (; i < NSEGMENTS; i++) {
> + lua_pushcfunction(L, craft_ptr);
> + lua_pushnumber(L, i);
> + if (lua_pcall(L, 1, 1, 0) == LUA_OK)
> + ptr = (void *)lua_topointer(L, -1);
> + else
> + break;
> + }
> + assert(ptr != NULL);
> + /* Overwrite possible error message. */
> + lua_pushlightuserdata(L, ptr);
> + return 1;
> +}
> +
> +static const struct luaL_Reg lightuserdata[] = {
> + {"craft_ptr_wp", craft_ptr_wp},
> + {NULL, NULL}
> +};
> +
> +LUA_API int luaopen_lightuserdata(lua_State *L)
> +{
> + luaL_register(L, "lightuserdata", lightuserdata);
> + return 1;
> +}
> --
> 2.31.0
>
More information about the Tarantool-patches
mailing list