From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 5760C6EC40; Mon, 16 Aug 2021 13:21:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5760C6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629109270; bh=9W3UGyjweIvQZzB2MRm+Ionxutt/69ADqvQzSSEdeo4=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=ZAZWGR8vIkXF+cYfUVheIm02/Zl+JoBUUJjOtASgEN9ukYdja3CCfg6FFP8ozli2J wHbVEDaRa2xr2zdIs0rTB/TfapshUXzRKnlEb2h6ZcOXV17bY0WbMPWfdVQB6fDAqh XyyACDi+Ddu8UA+Pv5GBX5XjNYx/mNrLIvkvaf+k= Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id F28AE6EC40 for ; Mon, 16 Aug 2021 13:21:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F28AE6EC40 Received: by smtp38.i.mail.ru with esmtpa (envelope-from ) id 1mFZjv-0003fU-Nn; Mon, 16 Aug 2021 13:21:08 +0300 To: Igor Munkin , Kirill Yukhin Date: Mon, 16 Aug 2021 13:19:49 +0300 Message-Id: <20210816101949.25035-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.31.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD972FF4A7D76DB5E242D14FEF1BD8BF4AC182A05F5380850406988AE85933BA7DBCB470AE47BE4FB84803B09BF0B27DE6901689F9475F5D62E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE727FD6E7FC3A8F857EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637E78B284398E2029E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8ECF1CEEE2C37E2D298D534A9A65AF212117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A45692FFBBD75A6A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B5054B1BDF0EDAD5CA97A4880E8351FDA621 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C4C7A0BC55FA0FE5FCD9B3D14C4F9F8C22DF455A853C5FA124816CF6AB210F9D19B1881A6453793CE9C32612AADDFBE06181DF583BEE9BDE689510FB958DCE06DB6ED91DBE5ABE359AA347268583507A9B23D4379F09C64C7393EDB24507CE13387DFF0A840B692CF8 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34FDC9529E4578B99C9C80F673EBFF3A7B496449143BBE9E26B935A329F578AAAA990C80E45F1A06691D7E09C32AA3244C6C172C4C553346C8FBB68E675B21DCB64DBEAD0ED6C55A80927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXht+2/rSLgackml19bDwWv2 X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB437CA13A18079EF5418B8CDEC8A249B59D72A5496D47674B1F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] core: fix cur_L restoration on error throw X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This change is a kind of revertion of commits ed412cd9f55fe87fd32a69c86e1732690fc5c1b0 ('Update cur_L on exceptional path') and 97699d9ee2467389b6aea21a098e38aff3469b5f ('Fix cur_L tracking on exceptional path'). When an error is thrown on the coroutine that is not the one being currently executed, `cur_L` is not set up. Hence, when the running trace exits at assertion guard right after the error is caught, Lua state is restored from the incorrect `cur_L`. As a result the resulting stack is inconsistent and the crash occurs. Aforementioned patches fix the behaviour only for x86/x64 architectures. This patch updates the `cur_L` within `lj_err_throw()` to the given lua_State, where the error is raised, since this is the only coroutine that can proceed later. Also, it removes unnecessary restorations of `cur_L` at C and FF exception handlers in the VM. Nevertheless, throwing an error at non-currently executed coroutine is a violation of Lua/C API. So, in the nearest possible future this patch should be replaced within the corresponding assert. Resolves tarantool/tarantool#6189 Relates to tarantool/tarantool#6323 Follows up tarantool/tarantool#1516 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-6189-curL Issues: * https://github.com/tarantool/tarantool/issues/6189 * https://github.com/tarantool/tarantool/issues/6323 * https://github.com/tarantool/tarantool/issues/1516 Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-6189-curL CI is red on FreeBSD, so the patch is probably not ready to merge. I've tried to reproduce it on sh8, but I've failed. sh4 is still not working and is used for running our CI jobs. Stay tuned! src/lj_err.c | 6 ++++ src/vm_x64.dasc | 2 -- src/vm_x86.dasc | 2 -- test/tarantool-tests/CMakeLists.txt | 1 + test/tarantool-tests/gh-6189-cur_L.test.lua | 25 +++++++++++++ .../gh-6189-cur_L/CMakeLists.txt | 1 + test/tarantool-tests/gh-6189-cur_L/libcur_L.c | 36 +++++++++++++++++++ 7 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 test/tarantool-tests/gh-6189-cur_L.test.lua create mode 100644 test/tarantool-tests/gh-6189-cur_L/CMakeLists.txt create mode 100644 test/tarantool-tests/gh-6189-cur_L/libcur_L.c diff --git a/src/lj_err.c b/src/lj_err.c index b6be357e..df9755a2 100644 --- a/src/lj_err.c +++ b/src/lj_err.c @@ -509,6 +509,12 @@ LJ_NOINLINE void LJ_FASTCALL lj_err_throw(lua_State *L, int errcode) global_State *g = G(L); lj_trace_abort(g); setmref(g->jit_base, NULL); + /* + ** FIXME: Throwing error not on the currently executing coroutine is + ** a violation of Lua/C API. The next line should be changed to assert. + ** Set the running lua_State as the one where the error can be handled. + */ + setgcref(g->cur_L, obj2gco(L)); L->status = LUA_OK; #if LJ_UNWIND_EXT err_raise_ext(errcode); diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc index 974047d3..c65e5625 100644 --- a/src/vm_x64.dasc +++ b/src/vm_x64.dasc @@ -513,7 +513,6 @@ static void build_subroutines(BuildCtx *ctx) |->vm_unwind_c_eh: // Landing pad for external unwinder. | mov L:DISPATCH, SAVE_L | mov GL:RB, L:DISPATCH->glref - | mov GL:RB->cur_L, L:DISPATCH | mov dword GL:RB->vmstate, ~LJ_VMST_CFUNC | mov DISPATCH, L:DISPATCH->glref // Setup pointer to dispatch table. | add DISPATCH, GG_G2DISP @@ -539,7 +538,6 @@ static void build_subroutines(BuildCtx *ctx) | add DISPATCH, GG_G2DISP | mov PC, [BASE-8] // Fetch PC of previous frame. | mov_false RA - | mov [DISPATCH+DISPATCH_GL(cur_L)], L:RB | mov RB, [BASE] | mov [BASE-16], RA // Prepend false to error message. | mov [BASE-8], RB diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc index ab8e6f27..bdd5c95e 100644 --- a/src/vm_x86.dasc +++ b/src/vm_x86.dasc @@ -654,7 +654,6 @@ static void build_subroutines(BuildCtx *ctx) |->vm_unwind_c_eh: // Landing pad for external unwinder. | mov L:DISPATCH, SAVE_L | mov GL:RB, L:DISPATCH->glref - | mov dword GL:RB->cur_L, L:DISPATCH | mov dword GL:RB->vmstate, ~LJ_VMST_CFUNC | mov DISPATCH, L:DISPATCH->glref // Setup pointer to dispatch table. | add DISPATCH, GG_G2DISP @@ -690,7 +689,6 @@ static void build_subroutines(BuildCtx *ctx) | add DISPATCH, GG_G2DISP | mov PC, [BASE-4] // Fetch PC of previous frame. | mov dword [BASE-4], LJ_TFALSE // Prepend false to error message. - | mov [DISPATCH+DISPATCH_GL(cur_L)], L:RB | // INTERP until jump to BC_RET* or return to C. | set_vmstate INTERP | jmp ->vm_returnc // Increments RD/MULTRES and returns. diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt index 2fdb4d1f..df74a277 100644 --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -57,6 +57,7 @@ macro(BuildTestCLib lib sources) endmacro() add_subdirectory(gh-4427-ffi-sandwich) +add_subdirectory(gh-6189-cur_L) add_subdirectory(lj-flush-on-trace) add_subdirectory(misclib-getmetrics-capi) diff --git a/test/tarantool-tests/gh-6189-cur_L.test.lua b/test/tarantool-tests/gh-6189-cur_L.test.lua new file mode 100644 index 00000000..8521af9a --- /dev/null +++ b/test/tarantool-tests/gh-6189-cur_L.test.lua @@ -0,0 +1,25 @@ +local libcur_L = require('libcur_L') +local tap = require('tap') + +local test = tap.test('gh-6189-cur_L') +test:plan(1) + +local function cbool(cond) + if cond then + return 1 + else + return 0 + end +end + +-- Compile function to trace with snapshot. +jit.opt.start('hotloop=1') +cbool(true) +cbool(true) + +pcall(libcur_L.error_from_other_thread) +-- Call with restoration from a snapshot with wrong cur_L. +cbool(false) + +test:ok(true) +os.exit(test:check() and 0 or 1) diff --git a/test/tarantool-tests/gh-6189-cur_L/CMakeLists.txt b/test/tarantool-tests/gh-6189-cur_L/CMakeLists.txt new file mode 100644 index 00000000..1e58e560 --- /dev/null +++ b/test/tarantool-tests/gh-6189-cur_L/CMakeLists.txt @@ -0,0 +1 @@ +BuildTestCLib(libcur_L libcur_L.c) diff --git a/test/tarantool-tests/gh-6189-cur_L/libcur_L.c b/test/tarantool-tests/gh-6189-cur_L/libcur_L.c new file mode 100644 index 00000000..2d58d2e7 --- /dev/null +++ b/test/tarantool-tests/gh-6189-cur_L/libcur_L.c @@ -0,0 +1,36 @@ +#include +#include + +static lua_State *old_L = NULL; + +int throw_error_at_old_thread(lua_State *cur_L) +{ + lua_error(old_L); + /* Unreachable. */ + return 0; +} + +static int error_from_other_thread(lua_State *L) +{ + lua_State *next_cur_L = lua_newthread(L); + old_L = L; + /* Remove thread. */ + lua_pop(L, 1); + /* Do not show frame slot as return result after error. */ + lua_pushnil(L); + lua_pushcfunction(next_cur_L, throw_error_at_old_thread); + lua_call(next_cur_L, 0, 0); + /* Unreachable. */ + return 0; +} + +static const struct luaL_Reg libcur_L[] = { + {"error_from_other_thread", error_from_other_thread}, + {NULL, NULL} +}; + +LUA_API int luaopen_libcur_L(lua_State *L) +{ + luaL_register(L, "libcur_L", libcur_L); + return 1; +} -- 2.31.0