From: Sergey Kaplun 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: Thu, 24 Nov 2022 16:10:42 +0300 [thread overview]
Message-ID: <Y39tUjirIH14Gzg0@root> (raw)
In-Reply-To: <20221028092638.11506-9-max.kokryashkin@gmail.com>
Hi, Maksim!
Thanks for the patch!
LGTM, but I have a bunch of questions to clarify it.
On 28.10.22, Maksim Kokryashkin 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.
Is there any link to documentation or source code to inspect this
behaviour?
> ELF unwind info has
> been updated to also use fp+16 rather than sp+CFRAME_SIZE.
Also, I see that `.byte` notation is replaced with `.uleb128` or
`.sleb128`. What is the reason?
Same question for removing names of entries ("_lj_vm_ffi_call.eh:" for
example).
>
> Offset to pointer to personality routine specified as @GOT-. rather
> than @GOTPCREL.
Does it mean that we use incorrect encoded offset (I see encoding for
offset is still the same) for our personality routine?
If so, maybe the other changes are just refactoring?
>
> 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
<snipped>
> diff --git a/src/Makefile.original b/src/Makefile.original
> index 2d014e43..813d9c12 100644
> --- a/src/Makefile.original
> +++ b/src/Makefile.original
<snipped>
> 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
<snipped>
> @@ -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.
Why can't we just use the following?
| str RC, SAVE_CFRAME
| str sp, 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.
Ditto.
> | 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)
<snipped>
> @@ -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
<snipped>
> --
> 2.37.0 (Apple Git-136)
>
--
Best regards,
Sergey Kaplun
next prev parent reply other threads:[~2022-11-24 13:13 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-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-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-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-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-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
[not found] ` <20221028092638.11506-9-max.kokryashkin@gmail.com>
2022-11-24 13:10 ` Sergey Kaplun via Tarantool-patches [this message]
2022-11-30 13:21 ` [Tarantool-patches] [PATCH luajit v4 8/8] OSX/ARM64: Fix external unwinding 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
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=Y39tUjirIH14Gzg0@root \
--to=tarantool-patches@dev.tarantool.org \
--cc=max.kokryashkin@gmail.com \
--cc=skaplun@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