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 B4A306EC40; Wed, 18 Aug 2021 11:51:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B4A306EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629276674; bh=SKqYuOGE+hmo/dGNdhedHM5DMMl6fB4K8e5N2yxKqkE=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=vGkKdyXklU3O2oHHAt3HBihF031NEOATOAEQ+Sd9TzNsgSrXRIsXuozQgn/8qAbVX CVR+zQtxz4I1sz73rftm9Q3Hm47sI4NSijDPzwI2U3HyRy8MxZGQPI+zUFwpjD8sq3 r7tCQ3Spp+qF+rHvNdZx5XAe0ufJouX7DMgtLFOQ= Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (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 8203F6EC40 for ; Wed, 18 Aug 2021 11:51:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8203F6EC40 Received: by smtp17.mail.ru with esmtpa (envelope-from ) id 1mGHI0-0003Wm-9U; Wed, 18 Aug 2021 11:51:12 +0300 Date: Wed, 18 Aug 2021 11:49:55 +0300 To: Igor Munkin , Kirill Yukhin Cc: tarantool-patches@dev.tarantool.org Message-ID: References: <20210816101949.25035-1-skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210816101949.25035-1-skaplun@tarantool.org> X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9F9A2272A1D086A28553D1D5C4B4124EF182A05F538085040ABD9246CAD1E47437FF521DC90FD6B0293D944A4EEB929C32DFC87A7DEDCE683 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE763424119D34F5CBFEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379C6642364E0E74208638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D81C31AD93D69F034F30CD3C1A0721B093117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735209ECD01F8117BC8BEA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CB24F08513AFFAC7943847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C4C7A0BC55FA0FE5FCC2634D85D6462F2EC535EBDDDFFB829795D5BA6A4E0B8646B1881A6453793CE9C32612AADDFBE061C61BE10805914D3804EBA3D8E7E5B87ABF8C51168CD8EBDB317C7E487E00003ADC48ACC2A39D04F89CDFB48F4795C241BDAD6C7F3747799A X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D343C45ADCD169245FA1307B0542FF164DEAAEF6E0E25E2684CD1DB80AF830C6820586064C18C14A52F1D7E09C32AA3244CD949FE2E3151BF05D2565959C6172979408A6A02710B7304FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojuRQ/H5n28tqAXZCAk31RIw== X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB4526FCC84B8DBCCBDDBEC7E5A1F20A4E2BB06D3D5DEE6E277F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit v2] 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 Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Implement cur_L restoration only for arm64 architecture, due to FreeBSD issue. Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-6189-curL-v2 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-v2 Enable test-run tests on arm64, Odroid with bump to show their coverage. P.S. this problem is JIT-related, however, when I turn on `jit.dump()` in CI [1], it is disappeared :(. Also, can't reproduce it inside sh4/sh8 VM, test fails only in the CI. Red test-run.py suite due to fiber.top issue, see also [2]. I suppose it would be nice to have a FreeBSD test machine like we have for M1 and Odroid. It may be helpful to research the console issue [3] too. =================================================================== commit 0f555bf79fefa1016849577500aec52719378ca5 Author: Sergey Kaplun Date: Sun Aug 15 15:47:13 2021 +0300 arm64: fix cur_L restoration on error throw This change is a kind of follow-up 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` for arm64 architecture too. 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 in `lj_err_throw()`. Resolves tarantool/tarantool#6189 Relates to tarantool/tarantool#6323 Follows up tarantool/tarantool#1516 diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc index 6e298255..2abf17fc 100644 --- a/src/vm_arm64.dasc +++ b/src/vm_arm64.dasc @@ -394,6 +394,7 @@ static void build_subroutines(BuildCtx *ctx) | mv_vmstate TMP0w, CFUNC | ldr GL, L->glref | st_vmstate TMP0w + | str L, GL->cur_L | b ->vm_leave_unw | |->vm_unwind_ff: // Unwind C stack, return from ff pcall. @@ -409,6 +410,7 @@ static void build_subroutines(BuildCtx *ctx) | ldr GL, L->glref // Setup pointer to global state. | mov_false TMP0 | sub RA, BASE, #8 // Results start at BASE-8. + | str L, GL->cur_L | ldr PC, [BASE, FRAME_PC] // Fetch PC of previous frame. | str TMP0, [BASE, #-8] // Prepend false to error message. | st_vmstate ST_INTERP 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; +} =================================================================== [1]: https://github.com/tarantool/tarantool/runs/3349429293#step:5:4569 [2]: https://github.com/tarantool/tarantool/pull/6303 [3]: https://github.com/tarantool/tarantool/issues/6231 -- Best regards, Sergey Kaplun