Hello, Max
thanks for the patch!
proposed test is passed after reverting a fix
On 11/22/23 17:35, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
>
> Thanks to Nicolas Lebedenco.
>
> (cherry-picked from commit 8135de2a0204e6acd92b231131c4a1e0be03ac1c)
>
> The `lj_state_growstack` doesn't account for a potential error
> handler invocation by `xpcall`, which may lead to the second
> error while handling a stack overflow, resulting in a misleading
> error "error in error handling", while the real issue is a stack
> overflow. This patch addresses this issue by fixing the condition
> at which stack overflow errors are thrown. Now it's thrown if the
> stack size is at least at the limit, instead of when it is over
> the limit.
>
> This commit also disables the second test from
> `lj-603-err-snap-restore`, since after this patch and the two
> follow-ups for it, there is no such amount of stack slots with
> which the test works the way it should.
>
> Lastly, this patch adds an alternative to `luacmd` to the
> `utils.exec` module, which is called `luabin`. That function is
> similar to the `luacmd`, with the only difference of returning
> only the executable itself without any additional CLI options
> passed.
>
> Maxim Kokryashkin:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9145
> ---
> src/lj_state.c | 2 +-
> .../lj-603-err-snap-restore.test.lua | 1 +
> .../lj-962-stack-overflow-report.test.lua | 10 ++++++++++
> .../lj-962-stack-overflow-report/script.lua | 3 +++
> test/tarantool-tests/utils/exec.lua | 15 ++++++++++++---
> 5 files changed, 27 insertions(+), 4 deletions(-)
> create mode 100644 test/tarantool-tests/lj-962-stack-overflow-report.test.lua
> create mode 100644 test/tarantool-tests/lj-962-stack-overflow-report/script.lua
>
> diff --git a/src/lj_state.c b/src/lj_state.c
> index 684336d5..76153bad 100644
> --- a/src/lj_state.c
> +++ b/src/lj_state.c
> @@ -132,7 +132,7 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
> n = LJ_STACK_MAX;
> }
> resizestack(L, n);
> - if (L->stacksize > LJ_STACK_MAXEX)
> + if (L->stacksize >= LJ_STACK_MAXEX)
> lj_err_msg(L, LJ_ERR_STKOV);
> }
>
> diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> index 96ebf92c..f5c8474f 100644
> --- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> +++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> @@ -36,6 +36,7 @@ local function do_test()
> -- Tarantool at start, so just skip test for it.
> -- luacheck: no global
> ['Disable test for Tarantool'] = _TARANTOOL,
> + ['Stack overflow is now handled differently'] = true,
> })
>
> test:ok(not handler_is_called)
> diff --git a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
> new file mode 100644
> index 00000000..45a888f4
> --- /dev/null
> +++ b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
> @@ -0,0 +1,10 @@
> +local tap = require('tap')
> +-- The test reproduces the problem only for GC64 mode with enabled JIT.
> +local test = tap.test('lj-962-stack-overflow-report')
> +test:plan(1)
> +
> +local LUABIN = require('utils').exec.luabin(arg)
> +local SCRIPT = arg[0]:gsub('%.test%.lua$', '/script.lua')
> +local output = io.popen(LUABIN .. ' 2>&1 ' .. SCRIPT, 'r'):read('*all')
> +test:like(output, 'stack overflow', 'stack overflow reported correctly')
> +test:done(true)
> diff --git a/test/tarantool-tests/lj-962-stack-overflow-report/script.lua b/test/tarantool-tests/lj-962-stack-overflow-report/script.lua
> new file mode 100644
> index 00000000..31c5ca33
> --- /dev/null
> +++ b/test/tarantool-tests/lj-962-stack-overflow-report/script.lua
> @@ -0,0 +1,3 @@
> +-- XXX: Function `f` is global to avoid using an additional stack slot.
> +-- luacheck: no global
> +f = function() f() end; f()
> diff --git a/test/tarantool-tests/utils/exec.lua b/test/tarantool-tests/utils/exec.lua
> index a56ca2dc..48a08ba5 100644
> --- a/test/tarantool-tests/utils/exec.lua
> +++ b/test/tarantool-tests/utils/exec.lua
> @@ -1,14 +1,23 @@
> local M = {}
>
> -function M.luacmd(args)
> +local function executable_idx(args)
> -- arg[-1] is guaranteed to be not nil.
> local idx = -2
> while args[idx] do
> assert(type(args[idx]) == 'string', 'Command part have to be a string')
> idx = idx - 1
> end
> - -- return the full command with flags.
> - return table.concat(args, ' ', idx + 1, -1)
> + return idx + 1
> +end
> +
> +function M.luabin(args)
> + -- Return only the executable.
> + return args[executable_idx(args)]
> +end
> +
> +function M.luacmd(args)
> + -- Return the full command with flags.
> + return table.concat(args, ' ', executable_idx(args), -1)
> end
>
> local function makeenv(tabenv)