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 A4C644695D1; Wed, 24 May 2023 09:12:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A4C644695D1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1684908740; bh=td3NHuxcEX3f7QsLZxNqejI4OUGIXOBVQqk9eFXd8QE=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=GAhPaJBa87TyZyxLgXNpijOzK5y/W1qzeUYJ6gOVhc3uPzO9ppv1QVHMmtFYBIdWN 7mZHwU+ZpBM17sEJ3kWGSgjDAJTKXv6cQ8TQ/sP0yWT1aa1IuaLqrIFmLpDYC1t9mm RNMFQKhQagowyQop7U0lWV8kdVfMmh51KeX3JQq4= Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [95.163.41.77]) (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 BF3777031B for ; Wed, 24 May 2023 09:12:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BF3777031B Received: by smtp36.i.mail.ru with esmtpa (envelope-from ) id 1q1hjN-009XQn-Cr; Wed, 24 May 2023 09:12:17 +0300 Date: Wed, 24 May 2023 06:09:32 +0000 To: Maksim Kokryashkin Message-ID: References: <20230515091622.30232-1-max.kokryashkin@gmail.com> <20230515091622.30232-2-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20230515091622.30232-2-max.kokryashkin@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9318AAE2601AA39B80A486A77AB14FCA136339E4D20D451E900894C459B0CD1B9C1F46ACF253E317C19A0AE7D371B0616EACB1E0530CB486C2B3B62D25A8A63BE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7AC4684DF4EC4B256EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063725FA9CD6081C82518638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8FDC6F5DB250D37E81A43A216836D112B117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC6CE7A4E25BAF3B969FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C32152B39F416BD1B5117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CFA510ABF3883D7472BA3038C0950A5D36C8A9BA7A39EFB766EC990983EF5C0329BA3038C0950A5D36D5E8D9A59859A8B667CA0F70D1FF63E976E601842F6C81A1F004C906525384303E02D724532EE2C3F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CA6E571FFFD2F2B6C43847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5C8D1F416FD041FF04687CADBBCFE232936E8290E62CB4BB3F87CCE6106E1FC07E67D4AC08A07B9B0A6C7FFFE744CA7FBCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF86C6C616FC7558951CCBA30E3294E89E0481C4BC1771655F708F6864C7795EAE760962882471415BD5191E2618EE498CEE51F04F990769714B4561429A1F336954CE6006BCE07E73 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj7RK+RruQK1dscDte9+Lz4Q== X-Mailru-Sender: 2FEBA92C8E508479FE7B9A1DF348D531C15EC91AC9D6FC8FA1910C7524A32392D2AAB7317867626B2326FE6F2A341ACE0FB9F97486540B4CD9E8847AB8CFED4D9ABF8A61C016C2CFB0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v4 1/4] Handle on-trace OOM errors from helper functions. 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: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Max, Thanks for the patch! LGTM with some minor comments. Minor: All the changes for vm_{mips,mips64,ppc} are quite cryptic for me, so I'd rather drop them. However, I believe that Sergey has already verified them, so let's leave them for consistency. On 15.05.23, Maksim Kokryashkin wrote: > From: Mike Pall > > (cherry-picked from commit 4bba29e697d00df5f020e76c2003bb9ce51c5d38) > > This patch introduces handling of errors from internal helper > functions on traces. FFI C++ exception interoperability is > not yet implemented. > > For each throwing trace, its mcode entry is augmented with a > DWARF2 frame description entry and a common information entry. > After that, a dynamic DWARF2 frame info is registered based on > that entry with `__register_frame()`[1][2], which is just an > adapter to `__unw_add_dynamic_fde`[3] on OSX. Because the ARM32 > architecture lacks the `__register_frame`, unwinding is not > supported on it. > > It is important to notice, that both the CIE and FDE are > used on Linux, and only the FDE is used on OSX. The > CIE is unnecessary on OSX, which can be clearly seen in > the sources[3] of Apple's libunwind: there is an attempt > to parse it, however its data is unused. In the same time, > the CIE is required on Linux[4] to perfrom dynamic frame > registration. > > For each throwing function call, a snapshot is allocated. > When we have a parent trace, our side trace head requires > an additional snapshot allocation, so the additional > `asm_snap_prev()` call is added. > > The `lj_err_trace()` is introduced to use instead > `lj_err_run()` for throwing the error on trace. > > The following fields were added to the ASMState structure: > * `snapalloc` -- flag showing whether the current snapshot > needs allocation. > * `mctoporig` -- holds the pointer to the top of the > generated mcode, including the DWARF entries, if present. > > And the following fields were added to the SnapShot structure: > * `mcofs` -- offset into machine code in MCode units, > needed to skip the DWARF entries, if present. > * `exitcode` -- exit code from unwound trace. > > The following registers were chosen to act as EHRAREG > (Exception Handler Return Address Register) on each platform: > * X86 `eip` (8) > * X64 `rip` (16) > * ARM `lr` (14) > * ARM64 `lr` (30) maps to x30 > * PPC `lr` (65) maps to SPR8 > * MIPS `$ra` (31) maps to $31 > > Also, introduction of `lj_err_trace` changes the semantics of > `lj-603-err-snap-restore.test.lua`, since now those errors are handled > on trace. The test was modified corresponding to the updates. > > Maxim Kokryashkin: > * added the description and the test for the problem > > Part of tarantool/tarantool#7745 > Part of tarantool/tarantool#8069 > > [1]: https://github.com/gcc-mirror/gcc/blob/ce83c3e492c2fa5a08c15b5f4619d58f42a5dcd0/libgcc/unwind-dw2-fde.c#L149 > [2]: https://opensource.apple.com/source/libunwind/libunwind-201/libunwind/src/UnwindLevel1-gcc-ext.c.auto.html > [3]: https://opensource.apple.com/source/libunwind/libunwind-201/libunwind/src/libunwind.cpp.auto.html > [4]: https://github.com/gcc-mirror/gcc/blob/ce83c3e492c2fa5a08c15b5f4619d58f42a5dcd0/libgcc/unwind-dw2-fde.c#L711 > --- > CMakeLists.txt | 6 + > doc/status.html | 7 - > src/Makefile.original | 3 + > src/lj_arch.h | 12 + > src/lj_asm.c | 77 ++++- > src/lj_dispatch.h | 4 +- > src/lj_err.c | 274 +++++++++++++++++- > src/lj_err.h | 19 +- > src/lj_ffrecord.c | 2 + > src/lj_jit.h | 2 + > src/lj_mcode.c | 5 +- > src/lj_opt_loop.c | 1 + > src/lj_record.c | 3 +- > src/lj_snap.c | 1 + > src/lj_state.c | 1 + > src/lj_target_x86.h | 2 + > src/lj_trace.c | 61 +++- > src/lj_trace.h | 3 + > src/lj_vm.h | 3 + > src/vm_arm.dasc | 3 +- > src/vm_arm64.dasc | 4 +- > src/vm_mips.dasc | 9 +- > src/vm_mips64.dasc | 10 +- > src/vm_ppc.dasc | 3 +- > src/vm_x64.dasc | 6 +- > src/vm_x86.dasc | 4 +- > .../gh-7745-oom-on-trace.test.lua | 41 +++ > .../lj-603-err-snap-restore.test.lua | 77 +++-- > 28 files changed, 553 insertions(+), 90 deletions(-) > create mode 100644 test/tarantool-tests/gh-7745-oom-on-trace.test.lua > > diff --git a/src/lj_asm.c b/src/lj_asm.c > index fd31cd04..f7c40fea 100644 > --- a/src/lj_asm.c > +++ b/src/lj_asm.c > @@ -944,6 +946,14 @@ static void asm_snap_alloc(ASMState *as) > if (!irref_isk(ref)) { > asm_snap_alloc1(as, ref); > if (LJ_SOFTFP && (sn & SNAP_SOFTFPNUM)) { > + /* > + ** FIXME: The following assert was replaced with > + ** the conventional `lua_assert`. > + ** > + ** lj_assertA(irt_type(IR(ref+1)->t) == IRT_SOFTFP, > + ** "snap %d[%d] points to bad SOFTFP IR %04d", > + ** snapno, n, ref - REF_BIAS); Typo: Invalid indent. > + */ > lua_assert(irt_type(IR(ref+1)->t) == IRT_SOFTFP); > asm_snap_alloc1(as, ref+1); > } > diff --git a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua > new file mode 100644 > index 00000000..0c495f5c > --- /dev/null > +++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua Typo: Mixed intent levels, let's adjust everything to 2 spaces. > @@ -0,0 +1,41 @@ > diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua > index 6eb53dfd..13b8e646 100644 > --- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua > +++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua Typo: AFAIR, we use indent of 2 spaces instead of 4. > @@ -6,6 +6,41 @@ local test = tap.test('lj-603-err-snap-restore') > -- > 2.39.2 (Apple Git-143) > -- Best regards, IM