[Tarantool-patches] [PATCH luajit v4 8/8] OSX/ARM64: Fix external unwinding.

sergos sergos at tarantool.org
Tue Dec 6 08:58:10 MSK 2022


Hi!

Thanks for the patch!

LGTM.

Sergos

> On 28 Oct 2022, at 12:26, Maksim Kokryashkin <max.kokryashkin at 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 at GOTPCREL\n"
> +	"\t.long _lj_err_unwind_dwarf at 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)
> 



More information about the Tarantool-patches mailing list