pcall() memory corruption #1516

rtsisyk · 2016-06-03T13:50:27Z

1.7.0-1024-g5fd0a7d

local fiber = require('fiber')

box.cfg {
    slab_alloc_arena  = 0.1,
    rows_per_wal      = 1000000,
}

local s1 = box.schema.space.create('s1', { engine = 'phia' })
s1:create_index('pk')

local function insert(key)
    s1:insert(key)
end

--jit.off(insert)

local function t1()
    for i = 1, 100000 do
        pcall(insert, {i})
    end
end;

fiber.create(t1)
fiber.create(t1)

fiber.sleep(10)
os.exit()

I added "regression" tag because 1.6 doesn't crash.
We realized that our pcall_wrapper() impedes JIT compilation and, therefore, prevents crash in this test case.

The text was updated successfully, but these errors were encountered:

mejedi · 2016-06-07T12:00:15Z

Patch prepared and sent to LuaJIT mailing list; awaiting response.

mejedi · 2016-06-08T13:29:02Z

Applied patch to Tarantool LuaJIT mirror.

The old code flow was the following: 1) `struct port_lua` given to `port_lua_do_dump()` has Lua stack with arguments to encode to MessagePack 2) The main coroutine `tarantool_L` is used with `lua_cpcall()` to call `encode_lua_call_16()` or `encode_lua_call()` 3) Objects on minor coroutine are encoded via `luamp_encode_call16()` or `luamp_encode()`. This encoding may raise an error on unprotected `port->L` coroutine. This coroutine has no protected frame on it and this call should fail in pure Lua. It is forbidden to call anything on unprotected coroutine [1] (Lua 5.1 sets protection only for specific lua_State [2] and calls a panic function if we raise an error on unprotected lua_State [3]). Netherless, there is no panic at now due to two facts. The first one is LuaJIT's support of C++ exception handling [4] that allows to raise an error in Lua and catch it in C++ or vice versa. But documentation still doesn't permit errors on unprotected coroutines (at least we must set try-catch block). The second one is double monkey-patching of LuaJIT to restore currently executed coroutine, when C function or fast function raises an error [5][6] (see related issue here [7][8]). For these reasons, when an error occurs, the unwinder searches and finds the C-protected stack frame from the `lua_cpcall()` for the `tarantool_L` coroutine and unwinds until that point (without aforementioned patches LuaJIT just calls a panic function and exit). 4) If an error is raised, and `lua_cpcall()` returns not `LUA_OK`, then the error from `port->L` coroutine is converted into a Tarantool error and a diagnostic is set. The auxiliary usage of `tarantool_L` coroutine is redundant and doesn't respect Lua idiomatic of usage. So this patch drops it and uses only minor coroutine instead with `lua_pcall()`. Functions to encode are saved as entrance in the `LUA_REGISTRY` table to reduce GC pressure, like it is done for other handlers [9]. [1]: https://www.lua.org/manual/5.2/manual.html#4.6 > If an error happens outside any protected environment, Lua calls a > panic function [2]: https://www.lua.org/source/5.1/lstate.h.html#lua_State [3]: https://www.lua.org/source/5.1/ldo.c.html#luaD_throw [4]: https://luajit.org/extensions.html#exceptions [5]: tarantool/luajit@ed412cd [6]: tarantool/luajit@97699d9 [7]: #1516 [8]: https://www.freelists.org/post/luajit/Issue-with-PCALL-in-21 [9]: e88c0d2

The old code flow was the following: 1) `struct port_lua` given to `port_lua_do_dump()` has Lua stack with arguments to encode to MessagePack. 2) The main coroutine `tarantool_L` is used to call `encode_lua_call()` or `encode_lua_call_16`() via `lua_cpcall()`. 3) Objects on port coroutine are encoded via `luamp_encode()` or `luamp_encode_call16()`. 4) This encoding may raise an error on unprotected `port->L` coroutine. This coroutine has no protected frame on it and this call should fail in pure Lua. Calling anything on unprotected coroutine is not allowed in Lua [1]: | If an error happens outside any protected environment, Lua calls a | panic function Lua 5.1 sets protection only for specific lua_State [2] and calls a panic function if we raise an error on unprotected lua_State [3]. Nevertheless, no panic occurs now due to two facts: * The first one is LuaJIT's support of C++ exception handling [4] that allows to raise an error in Lua and catch it in C++ or vice versa. But documentation still doesn't allow raising errors on unprotected coroutines (at least we must use try-catch block). * The second one is the patch made in LuaJIT to restore currently executed coroutine, when C function or fast function raises an error [5][6] (see the related issue here [7][8]). For these reasons, when an error occurs, the unwinder searches and finds the C-protected stack frame from the `lua_cpcall()` for `tarantool_L` coroutine and unwinds until that point (without aforementioned patches LuaJIT just calls a panic function and exit). If an error is raised, and `lua_cpcall()` returns not `LUA_OK`, then the error from `port->L` coroutine is converted into a Tarantool error and a diagnostic is set. The auxiliary usage of `tarantool_L` coroutine doesn't respect Lua idiomatic of usage. Internal unwinder used on M1 is not such flexible, so such misuse leads to panic call. Also the `tarantool_L` usage is redundant. So this patch drops it and uses only minor coroutine instead with `lua_pcall()`. Functions to encode are saved as entrance in the `LUA_REGISTRY` table to reduce GC pressure, like it is done for other handlers [9]. [1]: https://www.lua.org/manual/5.2/manual.html#4.6 [2]: https://www.lua.org/source/5.1/lstate.h.html#lua_State [3]: https://www.lua.org/source/5.1/ldo.c.html#luaD_throw [4]: https://luajit.org/extensions.html#exceptions [5]: tarantool/luajit@ed412cd [6]: tarantool/luajit@97699d9 [7]: #1516 [8]: https://www.freelists.org/post/luajit/Issue-with-PCALL-in-21 [9]: e88c0d2 Closes #6248 Closes #4617

The old code flow was the following: 1) `struct port_lua` given to `port_lua_do_dump()` has Lua stack with arguments to encode to MessagePack. 2) The main coroutine `tarantool_L` is used to call `encode_lua_call()` or `encode_lua_call_16`() via `lua_cpcall()`. 3) Objects on port coroutine are encoded via `luamp_encode()` or `luamp_encode_call16()`. 4) This encoding may raise an error on unprotected `port->L` coroutine. This coroutine has no protected frame on it and this call should fail in pure Lua. Calling anything on unprotected coroutine is not allowed in Lua [1]: | If an error happens outside any protected environment, Lua calls a | panic function Lua 5.1 sets protection only for specific lua_State [2] and calls a panic function if we raise an error on unprotected lua_State [3]. Nevertheless, no panic occurs now due to two facts: * The first one is LuaJIT's support of C++ exception handling [4] that allows to raise an error in Lua and catch it in C++ or vice versa. But documentation still doesn't allow raising errors on unprotected coroutines (at least we must use try-catch block). * The second one is the patch made in LuaJIT to restore currently executed coroutine, when C function or fast function raises an error [5][6] (see the related issue here [7][8]). For these reasons, when an error occurs, the unwinder searches and finds the C-protected stack frame from the `lua_cpcall()` for `tarantool_L` coroutine and unwinds until that point (without aforementioned patches LuaJIT just calls a panic function and exits). If an error is raised, and `lua_cpcall()` returns not `LUA_OK`, then the error from `port->L` coroutine is converted into a Tarantool error and a diagnostic is set. Such auxiliary usage of `tarantool_L` is not idiomatic for Lua. Internal unwinder used on M1 is not such flexible, so such misuse leads to panic call. Also the `tarantool_L` usage is redundant. So this patch drops it and uses only port coroutine instead with `lua_pcall()`. Functions to encode are saved to the `LUA_REGISTRY` table to reduce GC pressure, like it is done for other handlers [9]. [1]: https://www.lua.org/manual/5.2/manual.html#4.6 [2]: https://www.lua.org/source/5.1/lstate.h.html#lua_State [3]: https://www.lua.org/source/5.1/ldo.c.html#luaD_throw [4]: https://luajit.org/extensions.html#exceptions [5]: tarantool/luajit@ed412cd [6]: tarantool/luajit@97699d9 [7]: #1516 [8]: https://www.freelists.org/post/luajit/Issue-with-PCALL-in-21 [9]: e88c0d2 Closes #6248 Closes #4617

(cherry picked from commit 434d31b) The old code flow was the following: 1) `struct port_lua` given to `port_lua_do_dump()` has Lua stack with arguments to encode to MessagePack. 2) The main coroutine `tarantool_L` is used to call `encode_lua_call()` or `encode_lua_call_16`() via `lua_cpcall()`. 3) Objects on port coroutine are encoded via `luamp_encode()` or `luamp_encode_call16()`. 4) This encoding may raise an error on unprotected `port->L` coroutine. This coroutine has no protected frame on it and this call should fail in pure Lua. Calling anything on unprotected coroutine is not allowed in Lua [1]: | If an error happens outside any protected environment, Lua calls a | panic function Lua 5.1 sets protection only for specific lua_State [2] and calls a panic function if we raise an error on unprotected lua_State [3]. Nevertheless, no panic occurs now due to two facts: * The first one is LuaJIT's support of C++ exception handling [4] that allows to raise an error in Lua and catch it in C++ or vice versa. But documentation still doesn't allow raising errors on unprotected coroutines (at least we must use try-catch block). * The second one is the patch made in LuaJIT to restore currently executed coroutine, when C function or fast function raises an error [5][6] (see the related issue here [7][8]). For these reasons, when an error occurs, the unwinder searches and finds the C-protected stack frame from the `lua_cpcall()` for `tarantool_L` coroutine and unwinds until that point (without aforementioned patches LuaJIT just calls a panic function and exits). If an error is raised, and `lua_cpcall()` returns not `LUA_OK`, then the error from `port->L` coroutine is converted into a Tarantool error and a diagnostic is set. Such auxiliary usage of `tarantool_L` is not idiomatic for Lua. Internal unwinder used on M1 is not such flexible, so such misuse leads to panic call. Also the `tarantool_L` usage is redundant. So this patch drops it and uses only port coroutine instead with `lua_pcall()`. Functions to encode are saved to the `LUA_REGISTRY` table to reduce GC pressure, like it is done for other handlers [9]. [1]: https://www.lua.org/manual/5.2/manual.html#4.6 [2]: https://www.lua.org/source/5.1/lstate.h.html#lua_State [3]: https://www.lua.org/source/5.1/ldo.c.html#luaD_throw [4]: https://luajit.org/extensions.html#exceptions [5]: tarantool/luajit@ed412cd [6]: tarantool/luajit@97699d9 [7]: #1516 [8]: https://www.freelists.org/post/luajit/Issue-with-PCALL-in-21 [9]: e88c0d2 Closes #6248 Closes #4617

The old code flow was the following: 1) `struct port_lua` given to `port_lua_do_dump()` has Lua stack with arguments to encode to MessagePack. 2) The main coroutine `tarantool_L` is used to call `encode_lua_call()` or `encode_lua_call_16`() via `lua_cpcall()`. 3) Objects on port coroutine are encoded via `luamp_encode()` or `luamp_encode_call16()`. 4) This encoding may raise an error on unprotected `port->L` coroutine. This coroutine has no protected frame on it and this call should fail in pure Lua. Calling anything on unprotected coroutine is not allowed in Lua [1]: | If an error happens outside any protected environment, Lua calls a | panic function Lua 5.1 sets protection only for specific lua_State [2] and calls a panic function if we raise an error on unprotected lua_State [3]. Nevertheless, no panic occurs now due to two facts: * The first one is LuaJIT's support of C++ exception handling [4] that allows to raise an error in Lua and catch it in C++ or vice versa. But documentation still doesn't allow raising errors on unprotected coroutines (at least we must use try-catch block). * The second one is the patch made in LuaJIT to restore currently executed coroutine, when C function or fast function raises an error [5][6] (see the related issue here [7][8]). For these reasons, when an error occurs, the unwinder searches and finds the C-protected stack frame from the `lua_cpcall()` for `tarantool_L` coroutine and unwinds until that point (without aforementioned patches LuaJIT just calls a panic function and exits). If an error is raised, and `lua_cpcall()` returns not `LUA_OK`, then the error from `port->L` coroutine is converted into a Tarantool error and a diagnostic is set. Such auxiliary usage of `tarantool_L` is not idiomatic for Lua. Internal unwinder used on M1 is not such flexible, so such misuse leads to panic call. Also the `tarantool_L` usage is redundant. So this patch drops it and uses only port coroutine instead with `lua_pcall()`. Functions to encode are saved to the `LUA_REGISTRY` table to reduce GC pressure, like it is done for other handlers [9]. [1]: https://www.lua.org/manual/5.2/manual.html#4.6 [2]: https://www.lua.org/source/5.1/lstate.h.html#lua_State [3]: https://www.lua.org/source/5.1/ldo.c.html#luaD_throw [4]: https://luajit.org/extensions.html#exceptions [5]: tarantool/luajit@ed412cd [6]: tarantool/luajit@97699d9 [7]: #1516 [8]: https://www.freelists.org/post/luajit/Issue-with-PCALL-in-21 [9]: e88c0d2 Closes #6248 Closes #4617 Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Reviewed-by: Igor Munkin <imun@tarantool.org> Signed-off-by: Igor Munkin <imun@tarantool.org> (cherry picked from commit 027775f)

The old code flow was the following: 1) `struct port_lua` given to `port_lua_do_dump()` has Lua stack with arguments to encode to MessagePack. 2) The main coroutine `tarantool_L` is used to call `encode_lua_call()` or `encode_lua_call_16`() via `lua_cpcall()`. 3) Objects on port coroutine are encoded via `luamp_encode()` or `luamp_encode_call16()`. 4) This encoding may raise an error on unprotected `port->L` coroutine. This coroutine has no protected frame on it and this call should fail in pure Lua. Calling anything on unprotected coroutine is not allowed in Lua [1]: | If an error happens outside any protected environment, Lua calls a | panic function Lua 5.1 sets protection only for specific lua_State [2] and calls a panic function if we raise an error on unprotected lua_State [3]. Nevertheless, no panic occurs now due to two facts: * The first one is LuaJIT's support of C++ exception handling [4] that allows to raise an error in Lua and catch it in C++ or vice versa. But documentation still doesn't allow raising errors on unprotected coroutines (at least we must use try-catch block). * The second one is the patch made in LuaJIT to restore currently executed coroutine, when C function or fast function raises an error [5][6] (see the related issue here [7][8]). For these reasons, when an error occurs, the unwinder searches and finds the C-protected stack frame from the `lua_cpcall()` for `tarantool_L` coroutine and unwinds until that point (without aforementioned patches LuaJIT just calls a panic function and exits). If an error is raised, and `lua_cpcall()` returns not `LUA_OK`, then the error from `port->L` coroutine is converted into a Tarantool error and a diagnostic is set. Such auxiliary usage of `tarantool_L` is not idiomatic for Lua. Internal unwinder used on M1 is not such flexible, so such misuse leads to panic call. Also the `tarantool_L` usage is redundant. So this patch drops it and uses only port coroutine instead with `lua_pcall()`. Functions to encode are saved to the `LUA_REGISTRY` table to reduce GC pressure, like it is done for other handlers [9]. [1]: https://www.lua.org/manual/5.2/manual.html#4.6 [2]: https://www.lua.org/source/5.1/lstate.h.html#lua_State [3]: https://www.lua.org/source/5.1/ldo.c.html#luaD_throw [4]: https://luajit.org/extensions.html#exceptions [5]: tarantool/luajit@ed412cd [6]: tarantool/luajit@97699d9 [7]: #1516 [8]: https://www.freelists.org/post/luajit/Issue-with-PCALL-in-21 [9]: e88c0d2 Closes #6248 Closes #4617 Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Reviewed-by: Igor Munkin <imun@tarantool.org> Signed-off-by: Igor Munkin <imun@tarantool.org>

This change is a kind of revertion 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` within `lj_err_throw()` to the given lua_State, where the error is raised, since this is the only coroutine that can proceed later. Also, it removes unnecessary restorations of `cur_L` at C and FF exception handlers in the VM. 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 within the corresponding assert. Resolves tarantool/tarantool#6189 Relates to tarantool/tarantool#6323 Follows up tarantool/tarantool#1516

This change is a kind of 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 within the corresponding assert in `lj_err_throw()`. Resolves tarantool/tarantool#6189 Relates to tarantool/tarantool#6323 Follows up tarantool/tarantool#1516

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

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 Reviewed-by: Igor Munkin <imun@tarantool.org> Reviewed-by: Kirill Yukhin <kyukhin@tarantool.org> Signed-off-by: Igor Munkin <imun@tarantool.org>

* ARM64: Fix exit stub patching. * arm64: fix cur_L restoration on error throw Closes #6098 Closes #6189 Part of #5629 Relates to #6323 Follows up #1516

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 Reviewed-by: Igor Munkin <imun@tarantool.org> Reviewed-by: Kirill Yukhin <kyukhin@tarantool.org> Signed-off-by: Igor Munkin <imun@tarantool.org>

rtsisyk added bug Something isn't working regression labels Jun 3, 2016

rtsisyk added this to the 1.7.0 milestone Jun 3, 2016

rtsisyk self-assigned this Jun 3, 2016

mejedi self-assigned this Jun 7, 2016

mejedi closed this as completed Jun 8, 2016

Buristan mentioned this issue Jul 1, 2021

Investigate restoring of cur_L on exceptional path rationale #6189

Closed

igormunkin added a commit that referenced this issue Aug 19, 2021

luajit: bump new version

847b096

* ARM64: Fix exit stub patching. * arm64: fix cur_L restoration on error throw Closes #6098 Closes #6189 Part of #5629 Relates to #6323 Follows up #1516

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pcall() memory corruption #1516

pcall() memory corruption #1516

rtsisyk commented Jun 3, 2016

mejedi commented Jun 7, 2016

mejedi commented Jun 8, 2016

pcall() memory corruption #1516

pcall() memory corruption #1516

Comments

rtsisyk commented Jun 3, 2016

mejedi commented Jun 7, 2016

mejedi commented Jun 8, 2016