Re: [Tarantool-patches] [PATCH luajit v2 1/5] Revert "Fix cur_L tracking on exceptional path"

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Max

please see my comment below


On 9/29/23 09:20, Maxim Kokryashkin wrote:
> This reverts commit 97699d9ee2467389b6aea21a098e38aff3469b5f.
>
> As was mentioned in tarantool/tarantool#6189, throwing an error
> not on the currently executed coroutine is a violation of the
> Lua/C API.

Could you add a source of this statement?

There is a section "3.6 – Error Handling in C" in Lua 5.1 Reference Manual,

but there is no such statement there [1]. Could you clarify?

1. https://www.lua.org/manual/5.1/manual.html

> This patch is a part of the patchset that supports
> this violation and is reverted because of it.
>
> Part of tarantool/tarantool#6323
> ---
>   src/vm_x64.dasc | 9 ++++-----
>   src/vm_x86.dasc | 8 ++++----
>   2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
> index 09bf67e5..116716ac 100644
> --- a/src/vm_x64.dasc
> +++ b/src/vm_x64.dasc
> @@ -533,11 +533,11 @@ static void build_subroutines(BuildCtx *ctx)
>     |  mov eax, CARG2d			// Error return status for vm_pcall.
>     |  mov rsp, CARG1
>     |->vm_unwind_c_eh:			// Landing pad for external unwinder.
> -  |  mov L:DISPATCH, SAVE_L
> -  |  mov GL:RB, L:DISPATCH->glref
> -  |  mov GL:RB->cur_L, L:DISPATCH
> +  |  mov L:RB, SAVE_L
> +  |  mov GL:RB, L:RB->glref
> +  |  mov [GL:RB->cur_L], L:RB
>     |  mov dword GL:RB->vmstate, ~LJ_VMST_CFUNC
> -  |  mov DISPATCH, L:DISPATCH->glref	// Setup pointer to dispatch table.
> +  |  mov DISPATCH, GL:RB   // Setup pointer to dispatch table.
>     |  add DISPATCH, GG_G2DISP
>     |  jmp ->vm_leave_unw
>     |
> @@ -561,7 +561,6 @@ static void build_subroutines(BuildCtx *ctx)
>     |  add DISPATCH, GG_G2DISP
>     |  mov PC, [BASE-8]			// Fetch PC of previous frame.
>     |  mov_false RA
> -  |  mov [DISPATCH+DISPATCH_GL(cur_L)], L:RB
>     |  mov RB, [BASE]
>     |  mov [BASE-16], RA			// Prepend false to error message.
>     |  mov [BASE-8], RB
> diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
> index f16ade1a..e3fbf751 100644
> --- a/src/vm_x86.dasc
> +++ b/src/vm_x86.dasc
> @@ -681,11 +681,11 @@ static void build_subroutines(BuildCtx *ctx)
>     |.endif
>     |.endif
>     |->vm_unwind_c_eh:			// Landing pad for external unwinder.
> -  |  mov L:DISPATCH, SAVE_L
> -  |  mov GL:RB, L:DISPATCH->glref
> -  |  mov dword GL:RB->cur_L, L:DISPATCH
> +  |  mov L:RB, SAVE_L
> +  |  mov GL:RB, L:RB->glref
> +  |  mov dword GL:RB->cur_L, L:RB
>     |  mov dword GL:RB->vmstate, ~LJ_VMST_CFUNC
> -  |  mov DISPATCH, L:DISPATCH->glref	// Setup pointer to dispatch table.
> +  |  mov DISPATCH, GL:RB   // Setup pointer to dispatch table.
>     |  add DISPATCH, GG_G2DISP
>     |  jmp ->vm_leave_unw
>     |