Re: [Tarantool-patches] [PATCH luajit] Avoid assertion in case of stack overflow from stitched trace.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Max!

thanks for the patch! LGTM after addressing comments by Sergey.

Sergey

> won't be as long as the mentioned error is raised. Because of
> that, we get the incorrect bytecode instruction in
> `debug_framepc`. This patch fixes this behavior, so the
> `debug_framepc` now returns `NO_BCPOS` for this case.
>
> Maxim Kokryashkin:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9145
> ---
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/lj-913-avoid-assertion-stkov-from-stitched-trace
> PR: https://github.com/tarantool/tarantool/pull/9484
> Issues: https://github.com/tarantool/tarantool/issues/9145
> https://github.com/LuaJIT/LuaJIT/issues/913
>
>   src/lj_debug.c                                |  9 +++++---
>   ...-913-stackoverflow-stitched-trace.test.lua | 23 +++++++++++++++++++
>   2 files changed, 29 insertions(+), 3 deletions(-)
>   create mode 100644 test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua
>
> diff --git a/src/lj_debug.c b/src/lj_debug.c
> index 46c442c6..107f464c 100644
> --- a/src/lj_debug.c
> +++ b/src/lj_debug.c
> @@ -101,9 +101,12 @@ static BCPos debug_framepc(lua_State *L, GCfunc *fn, cTValue *nextframe)
>     pos = proto_bcpos(pt, ins) - 1;
>   #if LJ_HASJIT
>     if (pos > pt->sizebc) {  /* Undo the effects of lj_trace_exit for JLOOP. */
> -    GCtrace *T = (GCtrace *)((char *)(ins-1) - offsetof(GCtrace, startins));
> -    lj_assertL(bc_isret(bc_op(ins[-1])), "return bytecode expected");
> -    pos = proto_bcpos(pt, mref(T->startpc, const BCIns));
> +    if (bc_isret(bc_op(ins[-1]))) {
> +      GCtrace *T = (GCtrace *)((char *)(ins-1) - offsetof(GCtrace, startins));
> +      pos = proto_bcpos(pt, mref(T->startpc, const BCIns));
> +    } else {
> +      pos = NO_BCPOS;  /* Punt in case of stack overflow for stitched trace. */
> +    }
>     }
>   #endif
>     return pos;
> diff --git a/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua b/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua
> new file mode 100644
> index 00000000..3c12f0d9
> --- /dev/null
> +++ b/test/tarantool-tests/lj-913-stackoverflow-stitched-trace.test.lua
> @@ -0,0 +1,23 @@
> +local tap = require('tap')
> +-- Test to demonstrate the incorrect LuaJIT behavior when exiting
> +-- from a snapshot for stitched trace.
> +local test = tap.test('lj-913-stackoverflow-stitched-trace'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(3)
> +
> +-- Recursion to cause stack overflow.
> +local function callee()
> +  -- `math.fmod()` is NYI, so trace will be stitched here.
> +  local _ = math.fmod(42, 42)
> +  callee()
> +end
> +
> +local st, err = pcall(callee)
> +
> +test:ok(true, 'assertion is not triggered')
> +test:ok(not st, 'error happened')
> +test:like(err, 'stack overflow', 'stack overflow happened')
> +
> +test:done(true)
> --
> 2.43.0
>