Skip to content

Commit

Permalink
arm64: fix cur_L restoration on error throw
Browse files Browse the repository at this point in the history
This change is the follow-up of commits
ed412cd ('Update cur_L on exceptional
path') and 97699d9 ('Fix cur_L tracking
on exceptional path').

When an error is thrown on the coroutine that is not the one being
currently executed, `cur_L` is not set up. Hence, when the running trace
exits at assertion guard right after the error is caught, Lua state is
restored from the incorrect `cur_L`. As a result the resulting stack is
inconsistent and the crash occurs.

Aforementioned patches fix the behaviour only for x86/x64 architectures.
This patch updates the `cur_L` for arm64 architecture too.

Nevertheless, throwing an error at non-currently executed coroutine is a
violation of Lua/C API. So, in the nearest possible future this patch
should be replaced with the corresponding assert in `lj_err_throw()`.

Resolves tarantool/tarantool#6189
Relates to tarantool/tarantool#6323
Follows up tarantool/tarantool#1516
  • Loading branch information
Buristan committed Aug 18, 2021
1 parent 66c6d16 commit 7232c70
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/vm_arm64.dasc
Expand Up @@ -394,6 +394,7 @@ static void build_subroutines(BuildCtx *ctx)
| mv_vmstate TMP0w, CFUNC
| ldr GL, L->glref
| st_vmstate TMP0w
| str L, GL->cur_L
| b ->vm_leave_unw
|
|->vm_unwind_ff: // Unwind C stack, return from ff pcall.
Expand All @@ -409,6 +410,7 @@ static void build_subroutines(BuildCtx *ctx)
| ldr GL, L->glref // Setup pointer to global state.
| mov_false TMP0
| sub RA, BASE, #8 // Results start at BASE-8.
| str L, GL->cur_L
| ldr PC, [BASE, FRAME_PC] // Fetch PC of previous frame.
| str TMP0, [BASE, #-8] // Prepend false to error message.
| st_vmstate ST_INTERP
Expand Down
1 change: 1 addition & 0 deletions test/tarantool-tests/CMakeLists.txt
Expand Up @@ -57,6 +57,7 @@ macro(BuildTestCLib lib sources)
endmacro()

add_subdirectory(gh-4427-ffi-sandwich)
add_subdirectory(gh-6189-cur_L)
add_subdirectory(lj-49-bad-lightuserdata)
add_subdirectory(lj-flush-on-trace)
add_subdirectory(misclib-getmetrics-capi)
Expand Down
28 changes: 28 additions & 0 deletions test/tarantool-tests/gh-6189-cur_L.test.lua
@@ -0,0 +1,28 @@
local libcur_L = require('libcur_L')
local tap = require('tap')

local test = tap.test('gh-6189-cur_L')
test:plan(1)

local function cbool(cond)
if cond then
return 1
else
return 0
end
end

-- Compile function to trace with snapshot.
jit.opt.start('hotloop=1')
-- First call makes `cbool()` hot enough to be recorded next time.
cbool(true)
-- Second call records `cbool()` body (i.e. `if` branch). This is
-- a root trace for `cbool()`.
cbool(true)

assert(pcall(libcur_L.error_from_other_thread) == false, "return from error")
-- Call with restoration from a snapshot with wrong cur_L.
cbool(false)

test:ok(true)
os.exit(test:check() and 0 or 1)
1 change: 1 addition & 0 deletions test/tarantool-tests/gh-6189-cur_L/CMakeLists.txt
@@ -0,0 +1 @@
BuildTestCLib(libcur_L libcur_L.c)
40 changes: 40 additions & 0 deletions test/tarantool-tests/gh-6189-cur_L/libcur_L.c
@@ -0,0 +1,40 @@
#include <lua.h>
#include <lauxlib.h>

#undef NDEBUG
#include <assert.h>

static lua_State *old_L = NULL;

int throw_error_at_old_thread(lua_State *cur_L)
{
lua_error(old_L);
/* Unreachable. */
return 0;
}

static int error_from_other_thread(lua_State *L)
{
lua_State *next_cur_L = lua_newthread(L);
old_L = L;
/* Remove thread. */
lua_pop(L, 1);
/* Do not show frame slot as return result after error. */
lua_pushnil(L);
lua_pushcfunction(next_cur_L, throw_error_at_old_thread);
lua_call(next_cur_L, 0, 0);
/* Unreachable. */
assert(0);
return 0;
}

static const struct luaL_Reg libcur_L[] = {
{"error_from_other_thread", error_from_other_thread},
{NULL, NULL}
};

LUA_API int luaopen_libcur_L(lua_State *L)
{
luaL_register(L, "libcur_L", libcur_L);
return 1;
}

0 comments on commit 7232c70

Please sign in to comment.