Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org,
	sergeyb@tarantool.org
Subject: [Tarantool-patches] [PATCH luajit] Avoid assertion in case of stack overflow from stitched trace.
Date: Thu, 14 Dec 2023 14:44:49 +0300	[thread overview]
Message-ID: <20231214114458.17929-1-m.kokryashkin@tarantool.org> (raw)

From: Mike Pall <mike>

Reported by Sergey Bronnikov. Fixed by Sergey Kaplun.

(cherry-picked from commit 1d75cd4d7be638babe6d4e47bf73ea05fc65d81c)

When we exit from a stitched trace due to the Lua stack overflow
error, the Lua and JIT stacks are not synchronized, and they
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


             reply	other threads:[~2023-12-14 11:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-14 11:44 Maxim Kokryashkin via Tarantool-patches [this message]
2023-12-14 12:20 ` Sergey Kaplun via Tarantool-patches
2023-12-21 12:01   ` Maxim Kokryashkin via Tarantool-patches
2024-01-09 13:03     ` Sergey Kaplun via Tarantool-patches
2023-12-19 11:57 ` Sergey Bronnikov via Tarantool-patches
2024-02-15 13:42 ` Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231214114458.17929-1-m.kokryashkin@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] Avoid assertion in case of stack overflow from stitched trace.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox