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 ED10336B204; Wed, 5 Apr 2023 09:35:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ED10336B204 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1680676560; bh=RGWYnk/aGPN1vT7SlFaDRRFFQlQlf8F3h6kE/2SnipY=; 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=RAT7lamrhsI6JKUz6cbw6jw2yHPM19ugPkMmBO464CG90sQG4BoD8AdqqKt/90y68 ROj9oihDVMHv6i3Qm55zd85LWkpqpLjlJfceQBRZ/XYjek05k7JkkioAsieXVoAi95 il8yl1bMhRYOYSCUGEUdOrDej8xK2YMdI+BVYvzI= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 B351B369877 for ; Wed, 5 Apr 2023 09:35:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B351B369877 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1pjwkP-00065F-Ik; Wed, 05 Apr 2023 09:35:58 +0300 Date: Wed, 5 Apr 2023 09:32:07 +0300 To: Maksim Kokryashkin Message-ID: References: <20230328000317.33238-1-max.kokryashkin@gmail.com> <20230328000317.33238-2-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230328000317.33238-2-max.kokryashkin@gmail.com> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9487C1178585EEA15BF567FF7A07AC25CD88ACB5CC1FD1616182A05F538085040262D624DB96F673B822A493AEA8BD471760C70F0BA65A52659D9CC6A405194DF X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE745C0EDBD94D46193EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F1FEB73A813C0D3B8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D80731552F88F662534BD25C02C0543B3C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC6CE7A4E25BAF3B969FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3F1165EFABA1780A6117882F4460429728AD0CFFFB425014E868A13BD56FB6657E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637BC468E7E89D8C5D6EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A579D74D6C3CC55A3EFF2CCAF3AC3EB6457EEF1D64634DDCADF87CCE6106E1FC07E67D4AC08A07B9B0DB8A315C1FF4794DBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFB13925CEA8DEC5A5648116AF4C95BD1EEE887D7D81429ED718E5274599AB53526238D603D2F19CCFD93D275CAE0A69C46D3B96F216D56C2645B7D0F524998CCFF4E8A8FB6BF8EBF5 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojJlgUV9tWvkRQd93nGKW63Q== X-DA7885C5: 601E1A49D0795B7E8B422C1640E487A6EF659AEA3BE4E8395069446AAC9AB19B262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73933AF1F914F131DBF59DEB393E75A1897407F18CE7C807100C0FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v3 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: 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" Hi, Maxim! Thanks for the fixes! LGTM, after fixing the comments below. On 28.03.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 > provided for traces on Linux, and only the FDE on OSX. The This part is slightly misleading: JIT compiler emits CIE and FDE unconditionaly for OS. OSX just don't use CIE. > 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. Minor: linewidth is more than 72 symbols > * `mctoporig` -- holds the pointer to the top of the generated mcode, including Ditto. > 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 Ditto. > 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 `$31` (31) Minor: `$31` maps to $ra. > > 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 > --- > doc/status.html | 7 - > 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 | 22 ++ > .../lj-603-err-snap-restore.test.lua | 77 +++-- > 26 files changed, 525 insertions(+), 90 deletions(-) > create mode 100644 test/tarantool-tests/gh-7745-oom-on-trace.test.lua > > 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..e2cd0304 > --- /dev/null > +++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua > @@ -0,0 +1,22 @@ > +local tap = require('tap') > +local ffi = require('ffi') > + > +local test = tap.test('OOM on trace'):skipcond({ > + ['Broken unwiding in tarantool_panic_handler'] = jit.os == 'OSX', So, maybe we should change the condition to the following: `_TARANTOOL and (jit.os == 'OSX')` > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +test:plan(1) > + > +local function memory_payload() > + local t = {} Minor: Something wrong with offset. > + for i = 1, 1e10 do > + t[ffi.new("uint64_t")] = i Minor: Typo: s/"/'/g Also, please, add the comment that the non-GC64 build fails by OOM, while GC64 build fails by TABOV. > + end > + print(t) Looks like this print is excess. > +end Tests is very long even for disabled GC64, with the following patch non-GC64 mode runs much faster (its better to add the corresponding if, since we don't rely on OOM for GC64 mode): =================================================================== diff --git a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua index e2cd0304..ce3bccf1 100644 --- a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua +++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua @@ -13,10 +13,22 @@ local function memory_payload() for i = 1, 1e10 do t[ffi.new("uint64_t")] = i end - print(t) end +local anchor = {} +local function eatchunks(size) + while true do + anchor[ffi.new('char[?]', size)] = 1 + end +end +pcall(eatchunks, 64 * 1024 * 1024) + local res = pcall(memory_payload) + +-- Free memory for `test:ok()`. +anchor = nil +collectgarbage() + test:ok(res == false) os.exit(test:check() and 0 or 1) =================================================================== > + > +local res = pcall(memory_payload) > +test:ok(res == false) > + > +os.exit(test:check() and 0 or 1) > 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 > -- > 2.37.1 (Apple Git-137.1) > -- Best regards, Sergey Kaplun