Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maksim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, sergeyb@tarantool.org,
	skaplun@tarantool.org, m.kokryashkin@tarantool.org
Subject: [Tarantool-patches] [PATCH luajit v3 1/3] Improve error reporting on stack overflow.
Date: Wed, 22 Nov 2023 17:35:32 +0300	[thread overview]
Message-ID: <20231122143534.11330-2-max.kokryashkin@gmail.com> (raw)
In-Reply-To: <20231122143534.11330-1-max.kokryashkin@gmail.com>

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)
-- 
2.39.3 (Apple Git-145)


  reply	other threads:[~2023-11-22 14:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-22 14:35 [Tarantool-patches] [PATCH luajit v3 0/3] " Maksim Kokryashkin via Tarantool-patches
2023-11-22 14:35 ` Maksim Kokryashkin via Tarantool-patches [this message]
2023-11-24 12:22   ` [Tarantool-patches] [PATCH luajit v3 1/3] " Sergey Bronnikov via Tarantool-patches
2023-12-06 15:07     ` Maxim Kokryashkin via Tarantool-patches
2024-01-18 13:18       ` Sergey Bronnikov via Tarantool-patches
2023-11-22 14:35 ` [Tarantool-patches] [PATCH luajit v3 2/3] Cleanup stack overflow handling Maksim Kokryashkin via Tarantool-patches
2023-11-24 12:30   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 15:02     ` Maxim Kokryashkin via Tarantool-patches
2024-01-18 13:02       ` Sergey Bronnikov via Tarantool-patches
2023-11-22 14:35 ` [Tarantool-patches] [PATCH luajit v3 3/3] Follow-up fix for stack overflow handling cleanup Maksim Kokryashkin via Tarantool-patches
2023-11-24 12:34   ` Sergey Bronnikov via Tarantool-patches
2023-12-06 14:47     ` Maxim Kokryashkin via Tarantool-patches
2024-01-18 13:02       ` Sergey Bronnikov via Tarantool-patches
2023-11-23  6:25 ` [Tarantool-patches] [PATCH luajit v3 0/3] Improve error reporting on stack overflow Sergey Kaplun 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=20231122143534.11330-2-max.kokryashkin@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit v3 1/3] Improve error reporting on stack overflow.' \
    /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