[Tarantool-patches] [PATCH luajit v4 1/4] Handle on-trace OOM errors from helper functions.
Igor Munkin
imun at tarantool.org
Wed May 24 09:09:32 MSK 2023
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 <mike>
>
> (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
>
<snipped>
> 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
<snipped>
> @@ -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);
> }
<snipped>
> 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 @@
<snipped>
> 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')
<snipped>
> --
> 2.39.2 (Apple Git-143)
>
--
Best regards,
IM
More information about the Tarantool-patches
mailing list