* [Tarantool-patches] [PATCH luajit 1/4] Revert "Fix cur_L tracking on exceptional path"
2023-09-28 17:08 [Tarantool-patches] [PATCH luajit 0/4] Fix error-throwing on an incorrect coroutine Maxim Kokryashkin via Tarantool-patches
@ 2023-09-28 17:08 ` Maxim Kokryashkin via Tarantool-patches
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 2/4] Revert "Update cur_L " Maxim Kokryashkin via Tarantool-patches
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-09-28 17:08 UTC (permalink / raw)
To: tarantool-patches, skaplun, sergeyb
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. 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
|
--
2.42.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Tarantool-patches] [PATCH luajit 2/4] Revert "Update cur_L on exceptional path"
2023-09-28 17:08 [Tarantool-patches] [PATCH luajit 0/4] Fix error-throwing on an incorrect coroutine Maxim Kokryashkin via Tarantool-patches
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 1/4] Revert "Fix cur_L tracking on exceptional path" Maxim Kokryashkin via Tarantool-patches
@ 2023-09-28 17:08 ` Maxim Kokryashkin via Tarantool-patches
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 3/4] Revert "Update cur_L on exceptional path (arm)" Maxim Kokryashkin via Tarantool-patches
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 4/4] Restore cur_L for specific Lua/C API use case Maxim Kokryashkin via Tarantool-patches
3 siblings, 0 replies; 5+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-09-28 17:08 UTC (permalink / raw)
To: tarantool-patches, skaplun, sergeyb
This reverts commit ed412cd9f55fe87fd32a69c86e1732690fc5c1b0.
As was mentioned in tarantool/tarantool#6189, throwing an error
not on the currently executed coroutine is a violation of the
Lua/C API. 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 | 1 -
src/vm_x86.dasc | 2 --
2 files changed, 3 deletions(-)
diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
index 116716ac..399dfcbf 100644
--- a/src/vm_x64.dasc
+++ b/src/vm_x64.dasc
@@ -535,7 +535,6 @@ static void build_subroutines(BuildCtx *ctx)
|->vm_unwind_c_eh: // Landing pad for external unwinder.
| 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, GL:RB // Setup pointer to dispatch table.
| add DISPATCH, GG_G2DISP
diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
index e3fbf751..9fa9a3f7 100644
--- a/src/vm_x86.dasc
+++ b/src/vm_x86.dasc
@@ -683,7 +683,6 @@ static void build_subroutines(BuildCtx *ctx)
|->vm_unwind_c_eh: // Landing pad for external unwinder.
| 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, GL:RB // Setup pointer to dispatch table.
| add DISPATCH, GG_G2DISP
@@ -719,7 +718,6 @@ static void build_subroutines(BuildCtx *ctx)
| add DISPATCH, GG_G2DISP
| mov PC, [BASE-4] // Fetch PC of previous frame.
| mov dword [BASE-4], LJ_TFALSE // Prepend false to error message.
- | mov [DISPATCH+DISPATCH_GL(cur_L)], L:RB
| // INTERP until jump to BC_RET* or return to C.
| set_vmstate INTERP
| jmp ->vm_returnc // Increments RD/MULTRES and returns.
--
2.42.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Tarantool-patches] [PATCH luajit 3/4] Revert "Update cur_L on exceptional path (arm)"
2023-09-28 17:08 [Tarantool-patches] [PATCH luajit 0/4] Fix error-throwing on an incorrect coroutine Maxim Kokryashkin via Tarantool-patches
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 1/4] Revert "Fix cur_L tracking on exceptional path" Maxim Kokryashkin via Tarantool-patches
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 2/4] Revert "Update cur_L " Maxim Kokryashkin via Tarantool-patches
@ 2023-09-28 17:08 ` Maxim Kokryashkin via Tarantool-patches
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 4/4] Restore cur_L for specific Lua/C API use case Maxim Kokryashkin via Tarantool-patches
3 siblings, 0 replies; 5+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-09-28 17:08 UTC (permalink / raw)
To: tarantool-patches, skaplun, sergeyb
This reverts commit 5ccd25d740476a37d414733b5192d5be0ef06173.
As was mentioned in tarantool/tarantool#6189, throwing an error
not on the currently executed coroutine is a violation of the
Lua/C API. 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_arm.dasc | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
index 767d31f9..7095e660 100644
--- a/src/vm_arm.dasc
+++ b/src/vm_arm.dasc
@@ -351,7 +351,6 @@ static void build_subroutines(BuildCtx *ctx)
| mv_vmstate CARG4, CFUNC
| ldr GL:CARG3, L->glref
| str CARG4, GL:CARG3->vmstate
- | str L, GL:CARG3->cur_L
| b ->vm_leave_unw
|
|->vm_unwind_ff: // Unwind C stack, return from ff pcall.
@@ -372,7 +371,6 @@ static void build_subroutines(BuildCtx *ctx)
| mv_vmstate CARG2, INTERP
| str CARG1, [BASE, #-4] // Prepend false to error message.
| st_vmstate CARG2
- | str L, [DISPATCH, #DISPATCH_GL(cur_L)]
| b ->vm_returnc
|
|->vm_unwind_ext: // Complete external unwind.
--
2.42.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Tarantool-patches] [PATCH luajit 4/4] Restore cur_L for specific Lua/C API use case.
2023-09-28 17:08 [Tarantool-patches] [PATCH luajit 0/4] Fix error-throwing on an incorrect coroutine Maxim Kokryashkin via Tarantool-patches
` (2 preceding siblings ...)
2023-09-28 17:08 ` [Tarantool-patches] [PATCH luajit 3/4] Revert "Update cur_L on exceptional path (arm)" Maxim Kokryashkin via Tarantool-patches
@ 2023-09-28 17:08 ` Maxim Kokryashkin via Tarantool-patches
3 siblings, 0 replies; 5+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-09-28 17:08 UTC (permalink / raw)
To: tarantool-patches, skaplun, sergeyb
From: Mike Pall <mike>
Thanks to Peter Cawley.
(cherry-picked from commit e86990f7f24a94b0897061f25a84547fe1108bed)
Consider the following Lua C API function:
```
static int error_after_coroutine_return(lua_State *L)
{
lua_State *innerL = lua_newthread(L);
luaL_loadstring(innerL, "print('inner coro')");
lua_pcall(innerL, 0, 0, 0);
luaL_error(L, "my fancy error");
return 0;
}
```
And the following Lua script:
```
local libcur_L = require('libcur_L')
local function onesnap_f(var)
if var then
return 1
else
return 0
end
end
-- Compile function to trace with snapshot.
if jit then jit.opt.start('hotloop=1') end
onesnap_f(true)
onesnap_f(true)
local r, s = pcall(libcur_L.error_after_coroutine_return)
onesnap_f(false)
```
This is the only case when `cur_L` is not restored, according to
the analysis done in https://github.com/LuaJIT/LuaJIT/issues/1066.
This patch changes the error-catching routine, so now the patch
sets the actual cur_L there.
Now it is possible to throw errors on non-executing coroutines,
which is a violation of the Lua C API. So, even though it is now
possible, that behavior should be avoided anyway.
Maxim Kokryashkin:
* added the description for the problem
Resolves tarantool/tarantool#6323
---
src/lj_err.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/lj_err.c b/src/lj_err.c
index 46fb81ee..1a9a2f2b 100644
--- a/src/lj_err.c
+++ b/src/lj_err.c
@@ -174,12 +174,15 @@ static void *err_unwind(lua_State *L, void *stopcf, int errcode)
case FRAME_PCALL: /* FF pcall() frame. */
case FRAME_PCALLH: /* FF pcall() frame inside hook. */
if (errcode) {
+ global_State *g;
if (errcode == LUA_YIELD) {
frame = frame_prevd(frame);
break;
}
+ g = G(L);
+ setgcref(g->cur_L, obj2gco(L));
if (frame_typep(frame) == FRAME_PCALL)
- hook_leave(G(L));
+ hook_leave(g);
L->base = frame_prevd(frame) + 1;
L->cframe = cf;
unwindstack(L, L->base);
--
2.42.0
^ permalink raw reply [flat|nested] 5+ messages in thread