From: sergos via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Maksim Kokryashkin <max.kokryashkin@gmail.com>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit v4 8/8] OSX/ARM64: Fix external unwinding.
Date: Tue, 6 Dec 2022 08:58:10 +0300 [thread overview]
Message-ID: <12FB15A0-DBFC-4385-A0D4-07A96A015C0B@tarantool.org> (raw)
In-Reply-To: <20221028092638.11506-9-max.kokryashkin@gmail.com>
Hi!
Thanks for the patch!
LGTM.
Sergos
> On 28 Oct 2022, at 12:26, Maksim Kokryashkin <max.kokryashkin@gmail.com> wrote:
>
> Contributed by Edmund Kapusniak. For more info,
> see #698 and #757.
>
> (cherry picked from commit c38747b626b978555324504ec29a110f6b04902f)
>
> To allow compiler generate compact unwind info generation
> for Mach-O, fp must point to the saved fp, and the frame
> must be specified relative to fp+16. ELF unwind info has
> been updated to also use fp+16 rather than sp+CFRAME_SIZE.
>
> Offset to pointer to personality routine specified as @GOT-. rather
> than @GOTPCREL.
>
> Re-enabled LUAJIT_UNWIND_EXTERNAL by default on OSX.
>
> Maxim Kokryashkin:
> * added the description for the issue and the test
>
> Resolves tarantool/tarantool#6096
> Part of tarantool/tarantool#7230
> ---
> cmake/SetTargetFlags.cmake | 4 +-
> src/Makefile.original | 5 +-
> src/vm_arm64.dasc | 89 ++++++++-----------
> ...-6096-external-unwinding-on-arm64.test.lua | 13 +++
> 4 files changed, 54 insertions(+), 57 deletions(-)
> create mode 100644 test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua
>
> diff --git a/cmake/SetTargetFlags.cmake b/cmake/SetTargetFlags.cmake
> index 8abb6288..7cf26be1 100644
> --- a/cmake/SetTargetFlags.cmake
> +++ b/cmake/SetTargetFlags.cmake
> @@ -16,9 +16,7 @@ LuaJITTestArch(TESTARCH "${TARGET_C_FLAGS}")
> LuaJITArch(LUAJIT_ARCH "${TESTARCH}")
>
> if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
> - if(NOT LUAJIT_ARCH STREQUAL "arm64")
> - AppendFlags(TARGET_C_FLAGS -DLUAJIT_UNWIND_EXTERNAL)
> - endif()
> + AppendFlags(TARGET_C_FLAGS -DLUAJIT_UNWIND_EXTERNAL)
> else()
> string(FIND ${TARGET_C_FLAGS} "LJ_NO_UNWIND 1" UNWIND_POS)
> if(UNWIND_POS EQUAL -1)
> diff --git a/src/Makefile.original b/src/Makefile.original
> index 2d014e43..813d9c12 100644
> --- a/src/Makefile.original
> +++ b/src/Makefile.original
> @@ -325,10 +325,7 @@ ifeq (Darwin,$(TARGET_SYS))
> export MACOSX_DEPLOYMENT_TARGET=10.4
> endif
> TARGET_STRIP+= -x
> - # Ext. unwinding is broken on OSX/ARM64 until someone finds a fix. See #698.
> - ifneq (arm64,$(TARGET_LJARCH))
> - TARGET_XCFLAGS+= -DLUAJIT_UNWIND_EXTERNAL
> - endif
> + TARGET_XCFLAGS+= -DLUAJIT_UNWIND_EXTERNAL
> TARGET_XSHLDFLAGS= -dynamiclib -single_module -undefined dynamic_lookup -fPIC
> TARGET_DYNXLDOPTS=
> TARGET_XSHLDFLAGS+= -install_name $(TARGET_DYLIBPATH) -compatibility_version $(MAJVER).$(MINVER) -current_version $(MAJVER).$(MINVER).$(RELVER)
> diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> index ccfa72bd..f517a808 100644
> --- a/src/vm_arm64.dasc
> +++ b/src/vm_arm64.dasc
> @@ -81,8 +81,7 @@
> |
> |.define CFRAME_SPACE, 208
> |//----- 16 byte aligned, <-- sp entering interpreter
> -|.define SAVE_LR, [sp, #200]
> -|.define SAVE_FP, [sp, #192]
> +|.define SAVE_FP_LR_, 192
> |.define SAVE_GPR_, 112 // 112+10*8: 64 bit GPR saves
> |.define SAVE_FPR_, 48 // 48+8*8: 64 bit FPR saves
> |// Unused [sp, #44] // 32 bit values
> @@ -108,8 +107,8 @@
> |
> |.macro saveregs
> | sub sp, sp, # CFRAME_SPACE
> -| stp fp, lr, SAVE_FP
> -| add fp, sp, #0
> +| stp fp, lr, [sp, # SAVE_FP_LR_]
> +| add fp, sp, # SAVE_FP_LR_
> | stp x20, x19, [sp, # SAVE_GPR_+(27-19)*8]
> | save_ 21, 22, 8, 9
> | save_ 23, 24, 10, 11
> @@ -122,7 +121,7 @@
> | rest_ 23, 24, 10, 11
> | rest_ 25, 26, 12, 13
> | rest_ 27, 28, 14, 15
> -| ldp fp, lr, SAVE_FP
> +| ldp fp, lr, [sp, # SAVE_FP_LR_]
> | add sp, sp, # CFRAME_SPACE
> |.endmacro
> |
> @@ -504,8 +503,9 @@ static void build_subroutines(BuildCtx *ctx)
> | ldr GL, L->glref // Setup pointer to global state.
> | mov BASE, CARG2
> | str CARG1, SAVE_PC // Any value outside of bytecode is ok.
> - | str RC, SAVE_CFRAME
> - | str fp, L->cframe // Add our C frame to cframe chain.
> + | add TMP0, sp, #0
> + | str RC, SAVE_CFRAME
> + | str TMP0, L->cframe // Add our C frame to cframe chain.
> |
> |3: // Entry point for vm_cpcall/vm_resume (BASE = base, PC = ftype).
> | str L, GL->cur_L
> @@ -540,8 +540,9 @@ static void build_subroutines(BuildCtx *ctx)
> | sub RA, RA, RB // Compute -savestack(L, L->top).
> | str RAw, SAVE_NRES // Neg. delta means cframe w/o frame.
> | str wzr, SAVE_ERRF // No error function.
> - | str RC, SAVE_CFRAME
> - | str fp, L->cframe // Add our C frame to cframe chain.
> + | add TMP0, sp, #0
> + | str RC, SAVE_CFRAME
> + | str TMP0, L->cframe // Add our C frame to cframe chain.
> | str L, GL->cur_L
> | blr CARG4 // (lua_State *L, lua_CFunction func, void *ud)
> | mov BASE, CRET1
> @@ -2129,14 +2130,14 @@ static void build_subroutines(BuildCtx *ctx)
> | .type CCSTATE, CCallState, x19
> | stp x20, CCSTATE, [sp, #-32]!
> | stp fp, lr, [sp, #16]
> - | add fp, sp, #0
> + | add fp, sp, #16
> | mov CCSTATE, x0
> | ldr TMP0w, CCSTATE:x0->spadj
> | ldrb TMP1w, CCSTATE->nsp
> | add TMP2, CCSTATE, #offsetof(CCallState, stack)
> | subs TMP1, TMP1, #1
> | ldr TMP3, CCSTATE->func
> - | sub sp, fp, TMP0
> + | sub sp, sp, TMP0
> | bmi >2
> |1: // Copy stack slots
> | ldr TMP0, [TMP2, TMP1, lsl #3]
> @@ -2154,7 +2155,7 @@ static void build_subroutines(BuildCtx *ctx)
> | ldp d6, d7, CCSTATE->fpr[6]
> | ldr x8, CCSTATE->retp
> | blr TMP3
> - | mov sp, fp
> + | sub sp, fp, #16
> | stp x0, x1, CCSTATE->gpr[0]
> | stp d0, d1, CCSTATE->fpr[0]
> | stp d2, d3, CCSTATE->fpr[2]
> @@ -3879,7 +3880,7 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.uleb128 0x1\n"
> "\t.sleb128 -8\n"
> "\t.byte 30\n" /* Return address is in lr. */
> - "\t.byte 0xc\n\t.uleb128 31\n\t.uleb128 0\n" /* def_cfa sp */
> + "\t.byte 0xc\n\t.uleb128 29\n\t.uleb128 16\n" /* def_cfa fp 16 */
> "\t.align 3\n"
> ".LECIE0:\n\n");
> fprintf(ctx->fp,
> @@ -3889,10 +3890,9 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long .Lframe0\n"
> "\t.quad .Lbegin\n"
> "\t.quad %d\n"
> - "\t.byte 0xe\n\t.uleb128 %d\n" /* def_cfa_offset */
> "\t.byte 0x9e\n\t.uleb128 1\n" /* offset lr */
> "\t.byte 0x9d\n\t.uleb128 2\n", /* offset fp */
> - fcofs, CFRAME_SIZE);
> + fcofs);
> for (i = 19; i <= 28; i++) /* offset x19-x28 */
> fprintf(ctx->fp, "\t.byte 0x%x\n\t.uleb128 %d\n", 0x80+i, i+(3-19));
> for (i = 8; i <= 15; i++) /* offset d8-d15 */
> @@ -3909,12 +3909,10 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long .Lframe0\n"
> "\t.quad lj_vm_ffi_call\n"
> "\t.quad %d\n"
> - "\t.byte 0xe\n\t.uleb128 32\n" /* def_cfa_offset */
> "\t.byte 0x9e\n\t.uleb128 1\n" /* offset lr */
> "\t.byte 0x9d\n\t.uleb128 2\n" /* offset fp */
> "\t.byte 0x93\n\t.uleb128 3\n" /* offset x19 */
> "\t.byte 0x94\n\t.uleb128 4\n" /* offset x20 */
> - "\t.byte 0xd\n\t.uleb128 0x1d\n" /* def_cfa_register fp */
> "\t.align 3\n"
> ".LEFDE1:\n\n", (int)ctx->codesz - fcofs);
> #endif
> @@ -3933,7 +3931,7 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.byte 0x1b\n" /* pcrel|sdata4 */
> "\t.long lj_err_unwind_dwarf-.\n"
> "\t.byte 0x1b\n" /* pcrel|sdata4 */
> - "\t.byte 0xc\n\t.uleb128 31\n\t.uleb128 0\n" /* def_cfa sp */
> + "\t.byte 0xc\n\t.uleb128 29\n\t.uleb128 16\n" /* def_cfa fp 16 */
> "\t.align 3\n"
> ".LECIE1:\n\n");
> fprintf(ctx->fp,
> @@ -3944,10 +3942,9 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long .Lbegin-.\n"
> "\t.long %d\n"
> "\t.uleb128 0\n" /* augmentation length */
> - "\t.byte 0xe\n\t.uleb128 %d\n" /* def_cfa_offset */
> "\t.byte 0x9e\n\t.uleb128 1\n" /* offset lr */
> "\t.byte 0x9d\n\t.uleb128 2\n", /* offset fp */
> - fcofs, CFRAME_SIZE);
> + fcofs);
> for (i = 19; i <= 28; i++) /* offset x19-x28 */
> fprintf(ctx->fp, "\t.byte 0x%x\n\t.uleb128 %d\n", 0x80+i, i+(3-19));
> for (i = 8; i <= 15; i++) /* offset d8-d15 */
> @@ -3969,7 +3966,7 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.byte 30\n" /* Return address is in lr. */
> "\t.uleb128 1\n" /* augmentation length */
> "\t.byte 0x1b\n" /* pcrel|sdata4 */
> - "\t.byte 0xc\n\t.uleb128 31\n\t.uleb128 0\n" /* def_cfa sp */
> + "\t.byte 0xc\n\t.uleb128 29\n\t.uleb128 16\n" /* def_cfa fp 16 */
> "\t.align 3\n"
> ".LECIE2:\n\n");
> fprintf(ctx->fp,
> @@ -3980,18 +3977,15 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long lj_vm_ffi_call-.\n"
> "\t.long %d\n"
> "\t.uleb128 0\n" /* augmentation length */
> - "\t.byte 0xe\n\t.uleb128 32\n" /* def_cfa_offset */
> "\t.byte 0x9e\n\t.uleb128 1\n" /* offset lr */
> "\t.byte 0x9d\n\t.uleb128 2\n" /* offset fp */
> "\t.byte 0x93\n\t.uleb128 3\n" /* offset x19 */
> "\t.byte 0x94\n\t.uleb128 4\n" /* offset x20 */
> - "\t.byte 0xd\n\t.uleb128 0x1d\n" /* def_cfa_register fp */
> "\t.align 3\n"
> ".LEFDE3:\n\n", (int)ctx->codesz - fcofs);
> #endif
> break;
> - /* Disabled until someone finds a fix. See #698. */
> -#if !LJ_NO_UNWIND && 0
> +#if !LJ_NO_UNWIND
> case BUILD_machasm: {
> #if LJ_HASFFI
> int fcsize = 0;
> @@ -4006,14 +4000,14 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long 0\n"
> "\t.byte 0x1\n"
> "\t.ascii \"zPR\\0\"\n"
> - "\t.byte 0x1\n"
> - "\t.byte 128-8\n"
> + "\t.uleb128 0x1\n"
> + "\t.sleb128 -8\n"
> "\t.byte 30\n" /* Return address is in lr. */
> - "\t.byte 6\n" /* augmentation length */
> + "\t.uleb128 6\n" /* augmentation length */
> "\t.byte 0x9b\n" /* indirect|pcrel|sdata4 */
> - "\t.long _lj_err_unwind_dwarf@GOTPCREL\n"
> + "\t.long _lj_err_unwind_dwarf@GOT-.\n"
> "\t.byte 0x1b\n" /* pcrel|sdata4 */
> - "\t.byte 0xc\n\t.byte 31\n\t.byte 0\n" /* def_cfa sp */
> + "\t.byte 0xc\n\t.uleb128 29\n\t.uleb128 16\n" /* def_cfa fp 16 */
> "\t.align 3\n"
> "LECIEX:\n\n");
> for (j = 0; j < ctx->nsym; j++) {
> @@ -4024,7 +4018,6 @@ static void emit_asm_debug(BuildCtx *ctx)
> if (!strcmp(name, "_lj_vm_ffi_call")) { fcsize = size; continue; }
> #endif
> fprintf(ctx->fp,
> - "%s.eh:\n"
> "LSFDE%d:\n"
> "\t.set L$set$%d,LEFDE%d-LASFDE%d\n"
> "\t.long L$set$%d\n"
> @@ -4032,15 +4025,14 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long LASFDE%d-EH_frame1\n"
> "\t.long %s-.\n"
> "\t.long %d\n"
> - "\t.byte 0\n" /* augmentation length */
> - "\t.byte 0xe\n\t.byte %d\n\t.byte 1\n" /* def_cfa_offset */
> - "\t.byte 0x9e\n\t.byte 1\n" /* offset lr */
> - "\t.byte 0x9d\n\t.byte 2\n", /* offset fp */
> - name, j, j, j, j, j, j, j, name, size, CFRAME_SIZE);
> + "\t.uleb128 0\n" /* augmentation length */
> + "\t.byte 0x9e\n\t.uleb128 1\n" /* offset lr */
> + "\t.byte 0x9d\n\t.uleb128 2\n", /* offset fp */
> + j, j, j, j, j, j, j, name, size);
> for (i = 19; i <= 28; i++) /* offset x19-x28 */
> - fprintf(ctx->fp, "\t.byte 0x%x\n\t.byte %d\n", 0x80+i, i+(3-19));
> + fprintf(ctx->fp, "\t.byte 0x%x\n\t.uleb128 %d\n", 0x80+i, i+(3-19));
> for (i = 8; i <= 15; i++) /* offset d8-d15 */
> - fprintf(ctx->fp, "\t.byte 5\n\t.byte 0x%x\n\t.byte %d\n",
> + fprintf(ctx->fp, "\t.byte 5\n\t.uleb128 0x%x\n\t.uleb128 %d\n",
> 64+i, i+(3+(28-19+1)-8));
> fprintf(ctx->fp,
> "\t.align 3\n"
> @@ -4056,16 +4048,15 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long 0\n"
> "\t.byte 0x1\n"
> "\t.ascii \"zR\\0\"\n"
> - "\t.byte 0x1\n"
> - "\t.byte 128-8\n"
> + "\t.uleb128 0x1\n"
> + "\t.sleb128 -8\n"
> "\t.byte 30\n" /* Return address is in lr. */
> - "\t.byte 1\n" /* augmentation length */
> + "\t.uleb128 1\n" /* augmentation length */
> "\t.byte 0x1b\n" /* pcrel|sdata4 */
> - "\t.byte 0xc\n\t.byte 31\n\t.byte 0\n" /* def_cfa sp */
> + "\t.byte 0xc\n\t.uleb128 29\n\t.uleb128 16\n" /* def_cfa fp 16 */
> "\t.align 3\n"
> "LECIEY:\n\n");
> fprintf(ctx->fp,
> - "_lj_vm_ffi_call.eh:\n"
> "LSFDEY:\n"
> "\t.set L$set$yy,LEFDEY-LASFDEY\n"
> "\t.long L$set$yy\n"
> @@ -4073,13 +4064,11 @@ static void emit_asm_debug(BuildCtx *ctx)
> "\t.long LASFDEY-EH_frame2\n"
> "\t.long _lj_vm_ffi_call-.\n"
> "\t.long %d\n"
> - "\t.byte 0\n" /* augmentation length */
> - "\t.byte 0xe\n\t.byte 32\n" /* def_cfa_offset */
> - "\t.byte 0x9e\n\t.byte 1\n" /* offset lr */
> - "\t.byte 0x9d\n\t.byte 2\n" /* offset fp */
> - "\t.byte 0x93\n\t.byte 3\n" /* offset x19 */
> - "\t.byte 0x94\n\t.byte 4\n" /* offset x20 */
> - "\t.byte 0xd\n\t.uleb128 0x1d\n" /* def_cfa_register fp */
> + "\t.uleb128 0\n" /* augmentation length */
> + "\t.byte 0x9e\n\t.uleb128 1\n" /* offset lr */
> + "\t.byte 0x9d\n\t.uleb128 2\n" /* offset fp */
> + "\t.byte 0x93\n\t.uleb128 3\n" /* offset x19 */
> + "\t.byte 0x94\n\t.uleb128 4\n" /* offset x20 */
> "\t.align 3\n"
> "LEFDEY:\n\n", fcsize);
> }
> diff --git a/test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua b/test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua
> new file mode 100644
> index 00000000..cdeea441
> --- /dev/null
> +++ b/test/tarantool-tests/gh-6096-external-unwinding-on-arm64.test.lua
> @@ -0,0 +1,13 @@
> +local tap = require('tap')
> +
> +-- Test file to check correctnes of external unwinding
> +-- in LuaJIT.
> +-- See also https://github.com/LuaJIT/LuaJIT/issues/698,
> +-- https://github.com/LuaJIT/LuaJIT/pull/757.
> +local test = tap.test('gh-6096-external-unwinding-on-arm64')
> +test:plan(1)
> +
> +local res = pcall(require, 'not-existing-module')
> +test:ok(res == false, 'successful unwinding in pcall')
> +
> +os.exit(test:check() and 0 or 1)
> --
> 2.37.0 (Apple Git-136)
>
next prev parent reply other threads:[~2022-12-06 5:58 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20221028092638.11506-1-max.kokryashkin@gmail.com>
[not found] ` <20221028092638.11506-6-max.kokryashkin@gmail.com>
2022-11-24 11:41 ` [Tarantool-patches] [PATCH luajit v4 5/8] OSX/ARM64: Disable unwind info Sergey Kaplun via Tarantool-patches
2022-11-30 13:05 ` Maxim Kokryashkin via Tarantool-patches
2022-12-05 21:43 ` sergos via Tarantool-patches
[not found] ` <20221028092638.11506-7-max.kokryashkin@gmail.com>
2022-11-24 11:49 ` [Tarantool-patches] [PATCH luajit v4 6/8] BSD: Fix build with BSD grep Sergey Kaplun via Tarantool-patches
2022-11-30 13:05 ` Maxim Kokryashkin via Tarantool-patches
2022-12-05 21:46 ` sergos via Tarantool-patches
[not found] ` <20221028092638.11506-9-max.kokryashkin@gmail.com>
2022-11-24 13:10 ` [Tarantool-patches] [PATCH luajit v4 8/8] OSX/ARM64: Fix external unwinding Sergey Kaplun via Tarantool-patches
2022-11-30 13:21 ` Maxim Kokryashkin via Tarantool-patches
2022-12-01 8:52 ` Sergey Kaplun via Tarantool-patches
2022-12-01 12:28 ` Sergey Kaplun via Tarantool-patches
2022-12-06 5:58 ` sergos via Tarantool-patches [this message]
[not found] ` <20221028092638.11506-2-max.kokryashkin@gmail.com>
2022-12-05 16:01 ` [Tarantool-patches] [PATCH luajit v4 1/8] Cleanup and enable external unwinding for more platforms sergos via Tarantool-patches
[not found] ` <20221028092638.11506-3-max.kokryashkin@gmail.com>
2022-12-05 16:06 ` [Tarantool-patches] [PATCH luajit v4 2/8] OSX: Fix build by hardcoding external frame unwinding sergos via Tarantool-patches
[not found] ` <20221028092638.11506-4-max.kokryashkin@gmail.com>
2022-12-05 16:11 ` [Tarantool-patches] [PATCH luajit v4 3/8] OSX/ARM64: Disable external unwinding for now sergos via Tarantool-patches
[not found] ` <20221028092638.11506-5-max.kokryashkin@gmail.com>
2022-11-24 11:37 ` [Tarantool-patches] [PATCH luajit v4 4/8] ARM64: Reorder interpreter stack frame and fix unwinding Sergey Kaplun via Tarantool-patches
2022-11-30 13:04 ` Maxim Kokryashkin via Tarantool-patches
2022-12-05 21:42 ` sergos via Tarantool-patches
[not found] ` <20221028092638.11506-8-max.kokryashkin@gmail.com>
2022-11-24 11:56 ` [Tarantool-patches] [PATCH luajit v4 7/8] Fix build with busybox grep Sergey Kaplun via Tarantool-patches
2022-11-30 13:06 ` Maxim Kokryashkin via Tarantool-patches
2022-12-05 21:51 ` sergos via Tarantool-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=12FB15A0-DBFC-4385-A0D4-07A96A015C0B@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=max.kokryashkin@gmail.com \
--cc=sergos@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH luajit v4 8/8] OSX/ARM64: Fix external unwinding.' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox