From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Sergey Ostanevich <sergos@tarantool.org>, Igor Munkin <imun@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH luajit v2] Fix frame traversal for __gc handler frames. Date: Fri, 19 Nov 2021 19:41:57 +0300 [thread overview] Message-ID: <20211119164157.18344-1-skaplun@tarantool.org> (raw) From: Mike Pall <mike> Reported by Changochen. (cherry picked from 53f82e6e2e858a0a62fd1a2ff47e9866693382e6) A cframe unwinding is missed for a C protected frame during a search for an error function to handle a runtime error. It leads to undefined behaviour or crash, when raising a runtime error on stack with the CP frame before an error function handler (for example, an error in __gc handler). This patch adds missing unwinding for CP frame. Sergey Kaplun: * added the description and the test for the problem --- LuaJIT issue: https://github.com/LuaJIT/LuaJIT/issues/601 Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-fix-gc-finderrfunc Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-fix-gc-finderrfunc Changes in v2: * Create CP and C stack manually in LuaC src/lj_err.c | 1 + test/tarantool-tests/CMakeLists.txt | 1 + .../lj-601-fix-gc-finderrfunc.test.lua | 35 ++++++++++++ .../lj-601-fix-gc-finderrfunc/CMakeLists.txt | 1 + .../lj-601-fix-gc-finderrfunc/mixcframe.c | 55 +++++++++++++++++++ 5 files changed, 93 insertions(+) create mode 100644 test/tarantool-tests/lj-601-fix-gc-finderrfunc.test.lua create mode 100644 test/tarantool-tests/lj-601-fix-gc-finderrfunc/CMakeLists.txt create mode 100644 test/tarantool-tests/lj-601-fix-gc-finderrfunc/mixcframe.c diff --git a/src/lj_err.c b/src/lj_err.c index b6be357e..b520b3d3 100644 --- a/src/lj_err.c +++ b/src/lj_err.c @@ -585,6 +585,7 @@ static ptrdiff_t finderrfunc(lua_State *L) if (cframe_canyield(cf)) return 0; if (cframe_errfunc(cf) >= 0) return cframe_errfunc(cf); + cf = cframe_prev(cf); frame = frame_prevd(frame); break; case FRAME_PCALL: diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt index a872fa5e..b21500a0 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-601-fix-gc-finderrfunc) add_subdirectory(lj-flush-on-trace) add_subdirectory(misclib-getmetrics-capi) diff --git a/test/tarantool-tests/lj-601-fix-gc-finderrfunc.test.lua b/test/tarantool-tests/lj-601-fix-gc-finderrfunc.test.lua new file mode 100644 index 00000000..d4c44489 --- /dev/null +++ b/test/tarantool-tests/lj-601-fix-gc-finderrfunc.test.lua @@ -0,0 +1,35 @@ +local tap = require('tap') + +local mixcframe = require('libmixcframe') +local test = tap.test('lj-601-fix-gc-finderrfunc') +test:plan(1) + +-- Test file to demonstrate LuaJIT incorrect behaviour, when +-- throwing error in __gc finalizer. +-- See also, https://github.com/LuaJIT/LuaJIT/issues/601. + +-- Stop GC for now. +collectgarbage('stop') + +local a = newproxy(true) +getmetatable(a).__gc = function() + -- Function to raise error via `lj_err_run()` inside __gc. + error('raise error in __gc') +end +-- luacheck: no unused +a = nil + +-- We need to get the following Lua stack format when raise an +-- error: +-- + L->stack +-- | ... +-- | CP -- any C protected frame. +-- | ...[L/LP/V]... +-- | C -- any C frame. +-- | ...[L/LP/V]... +-- | CP (with inherited errfunc) -- __gc frame. +-- V +-- Enter in the C to call CP func. Call `lua_gc()` inside. +test:ok(mixcframe.test_handle_err(), 'error in __gc is successfully handled') + +os.exit(test:check() and 0 or 1) diff --git a/test/tarantool-tests/lj-601-fix-gc-finderrfunc/CMakeLists.txt b/test/tarantool-tests/lj-601-fix-gc-finderrfunc/CMakeLists.txt new file mode 100644 index 00000000..3fed6105 --- /dev/null +++ b/test/tarantool-tests/lj-601-fix-gc-finderrfunc/CMakeLists.txt @@ -0,0 +1 @@ +BuildTestCLib(libmixcframe mixcframe.c) diff --git a/test/tarantool-tests/lj-601-fix-gc-finderrfunc/mixcframe.c b/test/tarantool-tests/lj-601-fix-gc-finderrfunc/mixcframe.c new file mode 100644 index 00000000..730e30ae --- /dev/null +++ b/test/tarantool-tests/lj-601-fix-gc-finderrfunc/mixcframe.c @@ -0,0 +1,55 @@ +#include <lua.h> +#include <lauxlib.h> +#include <string.h> + +static void spoil_cframe(void) +{ + /* + * We need to map CFRAME_SIZE + lua_call frame size. + * 1 Kb is totally enough with overhead. + */ + char a[1024]; + /* + * XXX: Need a value >= 0 on C stack to be interpreted as + * its own errfunc, not inherited. Just put the big + * address (0x7f7f7f7f). + */ + memset(a, 0x7f, sizeof(a)); +} + +static int cframe_func(lua_State *L) +{ + lua_gc(L, LUA_GCCOLLECT, 0); + return 0; +} + +static int call_cframe_func(lua_State *L) +{ + lua_pushcfunction(L, cframe_func); + spoil_cframe(); + lua_call(L, 0, 0); + return 0; +} + +static int test_handle_err(lua_State *L) +{ + /* + * Not interested in the result, just want to be sure that + * unwinding in `finderrfunc()` works correctly. + */ + lua_cpcall(L, call_cframe_func, NULL); + lua_pushboolean(L, 1); + return 1; +} + +static const struct luaL_Reg mixcframe[] = { + {"test_handle_err", test_handle_err}, + {NULL, NULL} +}; + +LUA_API int luaopen_libmixcframe(lua_State *L) +{ + luaL_register(L, "mixcframe", mixcframe); + return 1; +} + -- 2.31.0
next reply other threads:[~2021-11-19 16:43 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-19 16:41 Sergey Kaplun via Tarantool-patches [this message] 2021-11-23 12:57 ` Igor Munkin via Tarantool-patches 2021-11-23 22:00 ` Igor Munkin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211119164157.18344-1-skaplun@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imun@tarantool.org \ --cc=sergos@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit v2] Fix frame traversal for __gc handler frames.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox