[Tarantool-patches] [PATCH luajit 0/4] Fix error-throwing on an incorrect coroutine

[Tarantool-patches] [PATCH luajit 0/4] Fix error-throwing on an incorrect coroutine

[Maxim Kokryashkin via Tarantool-patches]

This patch set reverts three patches, that introduced cur_L update
on error throw, because this behavior is a violation of the Lua C
API. Then, a proper fix from the vanilla LuaJIT is applied.

This proper fix now lets us perform that Lua C API violation
though, so the old test for the issue left intact and no new
tests were added.

Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-6323-fix-curL
PR: https://github.com/tarantool/tarantool/pull/9168

Maxim Kokryashkin (3):
  Revert "Fix cur_L tracking on exceptional path"
  Revert "Update cur_L on exceptional path"
  Revert "Update cur_L on exceptional path (arm)"

Mike Pall (1):
  Restore cur_L for specific Lua/C API use case.

 src/lj_err.c    | 5 ++++-
 src/vm_arm.dasc | 2 --
 src/vm_x64.dasc | 8 +++-----
 src/vm_x86.dasc | 8 +++-----
 4 files changed, 10 insertions(+), 13 deletions(-)

-- 
2.42.0

[Tarantool-patches] [PATCH luajit 1/4] Revert "Fix cur_L tracking on exceptional path"

[Maxim Kokryashkin via Tarantool-patches]

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

[Tarantool-patches] [PATCH luajit 2/4] Revert "Update cur_L on exceptional path"

[Maxim Kokryashkin via Tarantool-patches]

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

[Tarantool-patches] [PATCH luajit 3/4] Revert "Update cur_L on exceptional path (arm)"

[Maxim Kokryashkin via Tarantool-patches]

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

[Tarantool-patches] [PATCH luajit 4/4] Restore cur_L for specific Lua/C API use case.

[Maxim Kokryashkin via Tarantool-patches]

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