[Tarantool-patches] [PATCH luajit v3 0/3] Improve error reporting on stack overflow.

[Tarantool-patches] [PATCH luajit v3 0/3] Improve error reporting on stack overflow.

[Maksim Kokryashkin via Tarantool-patches]

This patchset fixes error reporting on stack overflow for situation
when an xpcall error handler can use a bit more space.

Issues: https://github.com/luajit/luajit/issues/962
https://github.com/tarantool/tarantool/issues/9145

PR: https://github.com/tarantool/tarantool/pull/9380
Branch: https://github.com/tarantool/luajit/tree/fckxorg/lj-962-error-reporting-on-stack-overflow

Changes in v3:
- Fixed comments as per review by Sergey.

Mike Pall (3):
  Improve error reporting on stack overflow.
  Cleanup stack overflow handling.
  Follow-up fix for stack overflow handling cleanup.

 src/lj_state.c                                | 15 +++-
 .../lj-962-premature-stack-overflow.test.c    | 86 +++++++++++++++++++
 .../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 +++-
 6 files changed, 123 insertions(+), 7 deletions(-)
 create mode 100644 test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
 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

--
2.39.3 (Apple Git-145)

[Tarantool-patches] [PATCH luajit v3 1/3] Improve error reporting on stack overflow.

[Maksim Kokryashkin via Tarantool-patches]

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)

Re: [Tarantool-patches] [PATCH luajit v3 1/3] Improve error reporting on stack overflow.

[Sergey Bronnikov via Tarantool-patches]

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)

Re: [Tarantool-patches] [PATCH luajit v3 1/3] Improve error reporting on stack overflow.

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 7895 bytes --]


Hi, Sergey!
Thanks for the review!
The test case indeed fails only for the GC64 mode before the patch.
I've tried to make it fail for non-GC64 mode, but the same issue arised —
an arbitrary amount of slots needed, and that amount needs to be modified
each time there is a minor change in the Lua stack behavior. With that in
mind, I’ve decided to leave it as it is. Added a comment and modified the
commit message. The new commit message:
===
Improve error reporting on stack overflow.
 
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.
 
The test case is very fragile, and the issue is hard to reproduce
in a robust way. The provided reproducer failed before the patch
for GC64 mode only. Non-GC64 mode requires an arbitrary number
of stack slots to be filled, which is even more fragile.
 
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
===
 
And the diff for the comment:
===
diff --git a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
index 45a888f4..e5f9d9a4 100644
--- a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
+++ b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
@@ -3,6 +3,12 @@ local tap = require('tap')
 local test = tap.test('lj-962-stack-overflow-report')
 test:plan(1)
 
+-- XXX: The test case is very fragile, and the issue is hard to
+-- reproduce in a robust way. The provided reproducer failed
+-- before the patch for GC64 mode only. Non-GC64 mode requires
+-- an arbitrary number of stack slots to be filled, which is
+-- even more fragile.
+
 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')
 
===
--
Best regards,
Maxim Kokryashkin
 
  
>Пятница, 24 ноября 2023, 15:22 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>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)
 

[-- Attachment #2: Type: text/html, Size: 9867 bytes --]

Re: [Tarantool-patches] [PATCH luajit v3 1/3] Improve error reporting on stack overflow.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 8797 bytes --]

Hi, Max


the problem is reproduced, thanks!

LGTM


On 12/6/23 18:07, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the review!
> The test case indeed fails only for the GC64 mode before the patch.
> I've tried to make it fail for non-GC64 mode, but the same issue arised —
> an arbitrary amount of slots needed, and that amount needs to be modified
> each time there is a minor change in the Lua stack behavior. With that in
> mind, I’ve decided to leave it as it is. Added a comment and modified the
> commit message. The new commit message:
> ===
> Improve error reporting on stack overflow.
> 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.
> The test case is very fragile, and the issue is hard to reproduce
> in a robust way. The provided reproducer failed before the patch
> for GC64 mode only. Non-GC64 mode requires an arbitrary number
> of stack slots to be filled, which is even more fragile.
> 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
> ===
> And the diff for the comment:
> ===
> diff --git 
> a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua 
> b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
> index 45a888f4..e5f9d9a4 100644
> --- a/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
> +++ b/test/tarantool-tests/lj-962-stack-overflow-report.test.lua
> @@ -3,6 +3,12 @@ local tap = require('tap')
>  local test = tap.test('lj-962-stack-overflow-report')
>  test:plan(1)
> +-- XXX: The test case is very fragile, and the issue is hard to
> +-- reproduce in a robust way. The provided reproducer failed
> +-- before the patch for GC64 mode only. Non-GC64 mode requires
> +-- an arbitrary number of stack slots to be filled, which is
> +-- even more fragile.
> +
>  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')
> ===
> --
> Best regards,
> Maxim Kokryashkin
>
>     Пятница, 24 ноября 2023, 15:22 +03:00 от Sergey Bronnikov
>     <sergeyb@tarantool.org>:
>     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)
>

[-- Attachment #2: Type: text/html, Size: 15101 bytes --]

[Tarantool-patches] [PATCH luajit v3 2/3] Cleanup stack overflow handling.

[Maksim Kokryashkin via Tarantool-patches]

From: Mike Pall <mike>

Reported by Peter Cawley.

(cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12)

After the previous patch, it is possible to trigger the
`stack overflow` error prematurely. Consider the following
situation: there are already 33000 slots allocated on a Lua
stack, and then there are 30 additional slots needed. In this
case, the actual allocated amount would be twice the already
allocated size, shrunk to the `LJ_STACK_MAXEX` size, which
would lead to the stack overflow error, despite the fact there
is plenty of unused space. This patch completely reworks the
logic of error handling during stack growth to address the issue.

Another important thing to notice is that the `LJ_ERR_STKOV` is
thrown only if the `L->status` is `LUA_OK` and that status is set
to `LUA_ERRRUN` just before throwing the error. The status is set
to `LUA_ERRRUN` to avoid the second stack overflow during the
`err_msgv` execution.

Maxim Kokryashkin:
* added the description and the test for the problem

Part of tarantool/tarantool#9145
---
 src/lj_state.c                                | 15 +++--
 .../lj-962-premature-stack-overflow.test.c    | 63 +++++++++++++++++++
 2 files changed, 74 insertions(+), 4 deletions(-)
 create mode 100644 test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c

diff --git a/src/lj_state.c b/src/lj_state.c
index 76153bad..d8a5134c 100644
--- a/src/lj_state.c
+++ b/src/lj_state.c
@@ -121,8 +121,17 @@ void lj_state_shrinkstack(lua_State *L, MSize used)
 void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
 {
   MSize n;
-  if (L->stacksize > LJ_STACK_MAXEX)  /* Overflow while handling overflow? */
-    lj_err_throw(L, LUA_ERRERR);
+  if (L->stacksize >= LJ_STACK_MAXEX) {
+    /* 4. Throw 'error in error handling' when we are _over_ the limit. */
+    if (L->stacksize > LJ_STACK_MAXEX)
+      lj_err_throw(L, LUA_ERRERR);  /* Does not invoke an error handler. */
+    /* 1. We are _at_ the limit after the last growth. */
+    if (!L->status) {  /* 2. Throw 'stack overflow'. */
+      L->status = LUA_ERRRUN;  /* Prevent ending here again for pushed msg. */
+      lj_err_msg(L, LJ_ERR_STKOV);  /* May invoke an error handler. */
+    }
+    /* 3. Add space (over the limit) for pushed message and error handler. */
+  }
   n = L->stacksize + need;
   if (n > LJ_STACK_MAX) {
     n += 2*LUA_MINSTACK;
@@ -132,8 +141,6 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
       n = LJ_STACK_MAX;
   }
   resizestack(L, n);
-  if (L->stacksize >= LJ_STACK_MAXEX)
-    lj_err_msg(L, LJ_ERR_STKOV);
 }
 
 void LJ_FASTCALL lj_state_growstack1(lua_State *L)
diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
new file mode 100644
index 00000000..12cb9004
--- /dev/null
+++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
@@ -0,0 +1,63 @@
+#include "lua.h"
+#include "lauxlib.h"
+
+#include "test.h"
+#include "utils.h"
+
+/*
+ * XXX: The "lj_obj.h" header is included to calculate the
+ * number of stack slots used from the bottom of the stack.
+ */
+#include "lj_obj.h"
+
+static int cur_slots = -1;
+
+static int fill_stack(lua_State *L)
+{
+	cur_slots = L->base - tvref(L->stack);
+
+	while(lua_gettop(L) < LUAI_MAXSTACK) {
+		cur_slots += 1;
+		lua_pushinteger(L, 42);
+	}
+
+	return 0;
+}
+
+static int premature_stackoverflow(void *test_state)
+{
+	lua_State *L = test_state;
+	lua_cpcall(L, fill_stack, NULL);
+	assert_true(cur_slots == LUAI_MAXSTACK - 1);
+	return TEST_EXIT_SUCCESS;
+}
+
+/*
+ * XXX: This test should fail neither before the patch
+ * nor after it.
+ */
+static int stackoverflow_during_stackoverflow(void *test_state)
+{
+	lua_State *L = test_state;
+	/*
+	 * XXX: `fill_stack` acts here as its own error handler,
+	 * causing the second stack overflow.
+	 */
+	lua_pushcfunction(L, fill_stack);
+	lua_pushcfunction(L, fill_stack);
+	int status = lua_pcall(L, 0, 0, -2);
+	assert_true(status == LUA_ERRERR);
+	return TEST_EXIT_SUCCESS;
+}
+
+int main(void)
+{
+	lua_State *L = utils_lua_init();
+	const struct test_unit tgroup[] = {
+		test_unit_def(premature_stackoverflow),
+		test_unit_def(stackoverflow_during_stackoverflow),
+	};
+	const int test_result = test_run_group(tgroup, L);
+	utils_lua_close(L);
+	return test_result;
+}
-- 
2.39.3 (Apple Git-145)

Re: [Tarantool-patches] [PATCH luajit v3 2/3] Cleanup stack overflow handling.

[Sergey Bronnikov via Tarantool-patches]

Hello, Max

thanks for the patch!

See a couple of minor comments below

Sergey

On 11/22/23 17:35, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
>
> Reported by Peter Cawley.
>
> (cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12)
>
> After the previous patch, it is possible to trigger the
> `stack overflow` error prematurely. Consider the following
> situation: there are already 33000 slots allocated on a Lua
> stack, and then there are 30 additional slots needed. In this
> case, the actual allocated amount would be twice the already
> allocated size, shrunk to the `LJ_STACK_MAXEX` size, which
> would lead to the stack overflow error, despite the fact there
> is plenty of unused space. This patch completely reworks the
> logic of error handling during stack growth to address the issue.
>
> Another important thing to notice is that the `LJ_ERR_STKOV` is
> thrown only if the `L->status` is `LUA_OK` and that status is set
> to `LUA_ERRRUN` just before throwing the error. The status is set
> to `LUA_ERRRUN` to avoid the second stack overflow during the
> `err_msgv` execution.
>
> Maxim Kokryashkin:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9145
> ---
>   src/lj_state.c                                | 15 +++--
>   .../lj-962-premature-stack-overflow.test.c    | 63 +++++++++++++++++++
>   2 files changed, 74 insertions(+), 4 deletions(-)
>   create mode 100644 test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>
> diff --git a/src/lj_state.c b/src/lj_state.c
> index 76153bad..d8a5134c 100644
> --- a/src/lj_state.c
> +++ b/src/lj_state.c
> @@ -121,8 +121,17 @@ void lj_state_shrinkstack(lua_State *L, MSize used)
>   void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>   {
>     MSize n;
> -  if (L->stacksize > LJ_STACK_MAXEX)  /* Overflow while handling overflow? */
> -    lj_err_throw(L, LUA_ERRERR);
> +  if (L->stacksize >= LJ_STACK_MAXEX) {
> +    /* 4. Throw 'error in error handling' when we are _over_ the limit. */
> +    if (L->stacksize > LJ_STACK_MAXEX)
> +      lj_err_throw(L, LUA_ERRERR);  /* Does not invoke an error handler. */
> +    /* 1. We are _at_ the limit after the last growth. */
> +    if (!L->status) {  /* 2. Throw 'stack overflow'. */
> +      L->status = LUA_ERRRUN;  /* Prevent ending here again for pushed msg. */
> +      lj_err_msg(L, LJ_ERR_STKOV);  /* May invoke an error handler. */
> +    }
> +    /* 3. Add space (over the limit) for pushed message and error handler. */
> +  }
>     n = L->stacksize + need;
>     if (n > LJ_STACK_MAX) {
>       n += 2*LUA_MINSTACK;
> @@ -132,8 +141,6 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>         n = LJ_STACK_MAX;
>     }
>     resizestack(L, n);
> -  if (L->stacksize >= LJ_STACK_MAXEX)
> -    lj_err_msg(L, LJ_ERR_STKOV);
>   }
>   
>   void LJ_FASTCALL lj_state_growstack1(lua_State *L)
> diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
> new file mode 100644
> index 00000000..12cb9004
> --- /dev/null
> +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
> @@ -0,0 +1,63 @@
> +#include "lua.h"
> +#include "lauxlib.h"
> +
> +#include "test.h"
> +#include "utils.h"
> +
> +/*
> + * XXX: The "lj_obj.h" header is included to calculate the
> + * number of stack slots used from the bottom of the stack.
> + */
> +#include "lj_obj.h"
> +
> +static int cur_slots = -1;
> +
> +static int fill_stack(lua_State *L)
> +{
> +	cur_slots = L->base - tvref(L->stack);
> +
> +	while(lua_gettop(L) < LUAI_MAXSTACK) {
> +		cur_slots += 1;
> +		lua_pushinteger(L, 42);
> +	}
> +
> +	return 0;
> +}
> +
> +static int premature_stackoverflow(void *test_state)
> +{
> +	lua_State *L = test_state;
> +	lua_cpcall(L, fill_stack, NULL);
> +	assert_true(cur_slots == LUAI_MAXSTACK - 1);
> +	return TEST_EXIT_SUCCESS;
> +}
> +
this testcase should fail with reverted patch, right? but it is not
> +/*
> + * XXX: This test should fail neither before the patch
> + * nor after it.

I propose to say about it in commit message.

We have a rule that test must fail without backported patch, so passed 
test is unexpected here.

> + */
> +static int stackoverflow_during_stackoverflow(void *test_state)
> +{
> +	lua_State *L = test_state;
> +	/*
> +	 * XXX: `fill_stack` acts here as its own error handler,
> +	 * causing the second stack overflow.
> +	 */
> +	lua_pushcfunction(L, fill_stack);
> +	lua_pushcfunction(L, fill_stack);
> +	int status = lua_pcall(L, 0, 0, -2);
> +	assert_true(status == LUA_ERRERR);
> +	return TEST_EXIT_SUCCESS;
> +}
> +
> +int main(void)
> +{
> +	lua_State *L = utils_lua_init();
> +	const struct test_unit tgroup[] = {
> +		test_unit_def(premature_stackoverflow),
> +		test_unit_def(stackoverflow_during_stackoverflow),
> +	};
> +	const int test_result = test_run_group(tgroup, L);
> +	utils_lua_close(L);
> +	return test_result;
> +}

Re: [Tarantool-patches] [PATCH luajit v3 2/3] Cleanup stack overflow handling.

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 6465 bytes --]


Hi, Sergey!
Thanks for the review!
  
>Пятница, 24 ноября 2023, 15:30 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>Hello, Max
>
>thanks for the patch!
>
>See a couple of minor comments below
>
>Sergey
>
>On 11/22/23 17:35, Maksim Kokryashkin wrote:
>> From: Mike Pall <mike>
>>
>> Reported by Peter Cawley.
>>
>> (cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12)
>>
>> After the previous patch, it is possible to trigger the
>> `stack overflow` error prematurely. Consider the following
>> situation: there are already 33000 slots allocated on a Lua
>> stack, and then there are 30 additional slots needed. In this
>> case, the actual allocated amount would be twice the already
>> allocated size, shrunk to the `LJ_STACK_MAXEX` size, which
>> would lead to the stack overflow error, despite the fact there
>> is plenty of unused space. This patch completely reworks the
>> logic of error handling during stack growth to address the issue.
>>
>> Another important thing to notice is that the `LJ_ERR_STKOV` is
>> thrown only if the `L->status` is `LUA_OK` and that status is set
>> to `LUA_ERRRUN` just before throwing the error. The status is set
>> to `LUA_ERRRUN` to avoid the second stack overflow during the
>> `err_msgv` execution.
>>
>> Maxim Kokryashkin:
>> * added the description and the test for the problem
>>
>> Part of tarantool/tarantool#9145
>> ---
>> src/lj_state.c | 15 +++--
>> .../lj-962-premature-stack-overflow.test.c | 63 +++++++++++++++++++
>> 2 files changed, 74 insertions(+), 4 deletions(-)
>> create mode 100644 test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>>
>> diff --git a/src/lj_state.c b/src/lj_state.c
>> index 76153bad..d8a5134c 100644
>> --- a/src/lj_state.c
>> +++ b/src/lj_state.c
>> @@ -121,8 +121,17 @@ void lj_state_shrinkstack(lua_State *L, MSize used)
>> void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>> {
>> MSize n;
>> - if (L->stacksize > LJ_STACK_MAXEX) /* Overflow while handling overflow? */
>> - lj_err_throw(L, LUA_ERRERR);
>> + if (L->stacksize >= LJ_STACK_MAXEX) {
>> + /* 4. Throw 'error in error handling' when we are _over_ the limit. */
>> + if (L->stacksize > LJ_STACK_MAXEX)
>> + lj_err_throw(L, LUA_ERRERR); /* Does not invoke an error handler. */
>> + /* 1. We are _at_ the limit after the last growth. */
>> + if (!L->status) { /* 2. Throw 'stack overflow'. */
>> + L->status = LUA_ERRRUN; /* Prevent ending here again for pushed msg. */
>> + lj_err_msg(L, LJ_ERR_STKOV); /* May invoke an error handler. */
>> + }
>> + /* 3. Add space (over the limit) for pushed message and error handler. */
>> + }
>> n = L->stacksize + need;
>> if (n > LJ_STACK_MAX) {
>> n += 2*LUA_MINSTACK;
>> @@ -132,8 +141,6 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>> n = LJ_STACK_MAX;
>> }
>> resizestack(L, n);
>> - if (L->stacksize >= LJ_STACK_MAXEX)
>> - lj_err_msg(L, LJ_ERR_STKOV);
>> }
>>
>> void LJ_FASTCALL lj_state_growstack1(lua_State *L)
>> diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>> new file mode 100644
>> index 00000000..12cb9004
>> --- /dev/null
>> +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>> @@ -0,0 +1,63 @@
>> +#include "lua.h"
>> +#include "lauxlib.h"
>> +
>> +#include "test.h"
>> +#include "utils.h"
>> +
>> +/*
>> + * XXX: The "lj_obj.h" header is included to calculate the
>> + * number of stack slots used from the bottom of the stack.
>> + */
>> +#include "lj_obj.h"
>> +
>> +static int cur_slots = -1;
>> +
>> +static int fill_stack(lua_State *L)
>> +{
>> + cur_slots = L->base - tvref(L->stack);
>> +
>> + while(lua_gettop(L) < LUAI_MAXSTACK) {
>> + cur_slots += 1;
>> + lua_pushinteger(L, 42);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int premature_stackoverflow(void *test_state)
>> +{
>> + lua_State *L = test_state;
>> + lua_cpcall(L, fill_stack, NULL);
>> + assert_true(cur_slots == LUAI_MAXSTACK - 1);
>> + return TEST_EXIT_SUCCESS;
>> +}
>> +
>this testcase should fail with reverted patch, right? but it is not
And it does fail. Tested on GC64/non-GC64 builds on Linux/MacOS.
>> +/*
>> + * XXX: This test should fail neither before the patch
>> + * nor after it.
>
>I propose to say about it in commit message.
Fixed, the branch is force-pushed. New commit message:
====
Cleanup stack overflow handling.
 
Reported by Peter Cawley.
 
(cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12)
 
After the previous patch, it is possible to trigger the
`stack overflow` error prematurely. Consider the following
situation: there are already 33000 slots allocated on a Lua
stack, and then there are 30 additional slots needed. In this
case, the actual allocated amount would be twice the already
allocated size, shrunk to the `LJ_STACK_MAXEX` size, which
would lead to the stack overflow error, despite the fact there
is plenty of unused space. This patch completely reworks the
logic of error handling during stack growth to address the issue.
 
Another important thing to notice is that the `LJ_ERR_STKOV` is
thrown only if the `L->status` is `LUA_OK` and that status is set
to `LUA_ERRRUN` just before throwing the error. The status is set
to `LUA_ERRRUN` to avoid the second stack overflow during the
`err_msgv` execution.
 
The `stackoverflow_during_stackoverflow` should fail neither
before the patch nor after and is added for the test to be
exhaustive.
 
Maxim Kokryashkin:
* added the description and the test for the problem
 
Part of tarantool/tarantool#9145
====
>
>We have a rule that test must fail without backported patch, so passed
>test is unexpected here.
>
>> + */
>> +static int stackoverflow_during_stackoverflow(void *test_state)
>> +{
>> + lua_State *L = test_state;
>> + /*
>> + * XXX: `fill_stack` acts here as its own error handler,
>> + * causing the second stack overflow.
>> + */
>> + lua_pushcfunction(L, fill_stack);
>> + lua_pushcfunction(L, fill_stack);
>> + int status = lua_pcall(L, 0, 0, -2);
>> + assert_true(status == LUA_ERRERR);
>> + return TEST_EXIT_SUCCESS;
>> +}
>> +
>> +int main(void)
>> +{
>> + lua_State *L = utils_lua_init();
>> + const struct test_unit tgroup[] = {
>> + test_unit_def(premature_stackoverflow),
>> + test_unit_def(stackoverflow_during_stackoverflow),
>> + };
>> + const int test_result = test_run_group(tgroup, L);
>> + utils_lua_close(L);
>> + return test_result;
>> +}
 

[-- Attachment #2: Type: text/html, Size: 8517 bytes --]

Re: [Tarantool-patches] [PATCH luajit v3 2/3] Cleanup stack overflow handling.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 7357 bytes --]

Thanks for changes! LGTM

On 12/6/23 18:02, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the review!
>
>     Пятница, 24 ноября 2023, 15:30 +03:00 от Sergey Bronnikov
>     <sergeyb@tarantool.org>:
>     Hello, Max
>
>     thanks for the patch!
>
>     See a couple of minor comments below
>
>     Sergey
>
>     On 11/22/23 17:35, Maksim Kokryashkin wrote:
>     > From: Mike Pall <mike>
>     >
>     > Reported by Peter Cawley.
>     >
>     > (cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12)
>     >
>     > After the previous patch, it is possible to trigger the
>     > `stack overflow` error prematurely. Consider the following
>     > situation: there are already 33000 slots allocated on a Lua
>     > stack, and then there are 30 additional slots needed. In this
>     > case, the actual allocated amount would be twice the already
>     > allocated size, shrunk to the `LJ_STACK_MAXEX` size, which
>     > would lead to the stack overflow error, despite the fact there
>     > is plenty of unused space. This patch completely reworks the
>     > logic of error handling during stack growth to address the issue.
>     >
>     > Another important thing to notice is that the `LJ_ERR_STKOV` is
>     > thrown only if the `L->status` is `LUA_OK` and that status is set
>     > to `LUA_ERRRUN` just before throwing the error. The status is set
>     > to `LUA_ERRRUN` to avoid the second stack overflow during the
>     > `err_msgv` execution.
>     >
>     > Maxim Kokryashkin:
>     > * added the description and the test for the problem
>     >
>     > Part of tarantool/tarantool#9145
>     > ---
>     > src/lj_state.c | 15 +++--
>     > .../lj-962-premature-stack-overflow.test.c | 63 +++++++++++++++++++
>     > 2 files changed, 74 insertions(+), 4 deletions(-)
>     > create mode 100644
>     test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     >
>     > diff --git a/src/lj_state.c b/src/lj_state.c
>     > index 76153bad..d8a5134c 100644
>     > --- a/src/lj_state.c
>     > +++ b/src/lj_state.c
>     > @@ -121,8 +121,17 @@ void lj_state_shrinkstack(lua_State *L,
>     MSize used)
>     > void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>     > {
>     > MSize n;
>     > - if (L->stacksize > LJ_STACK_MAXEX) /* Overflow while handling
>     overflow? */
>     > - lj_err_throw(L, LUA_ERRERR);
>     > + if (L->stacksize >= LJ_STACK_MAXEX) {
>     > + /* 4. Throw 'error in error handling' when we are _over_ the
>     limit. */
>     > + if (L->stacksize > LJ_STACK_MAXEX)
>     > + lj_err_throw(L, LUA_ERRERR); /* Does not invoke an error
>     handler. */
>     > + /* 1. We are _at_ the limit after the last growth. */
>     > + if (!L->status) { /* 2. Throw 'stack overflow'. */
>     > + L->status = LUA_ERRRUN; /* Prevent ending here again for
>     pushed msg. */
>     > + lj_err_msg(L, LJ_ERR_STKOV); /* May invoke an error handler. */
>     > + }
>     > + /* 3. Add space (over the limit) for pushed message and error
>     handler. */
>     > + }
>     > n = L->stacksize + need;
>     > if (n > LJ_STACK_MAX) {
>     > n += 2*LUA_MINSTACK;
>     > @@ -132,8 +141,6 @@ void LJ_FASTCALL
>     lj_state_growstack(lua_State *L, MSize need)
>     > n = LJ_STACK_MAX;
>     > }
>     > resizestack(L, n);
>     > - if (L->stacksize >= LJ_STACK_MAXEX)
>     > - lj_err_msg(L, LJ_ERR_STKOV);
>     > }
>     >
>     > void LJ_FASTCALL lj_state_growstack1(lua_State *L)
>     > diff --git
>     a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     > new file mode 100644
>     > index 00000000..12cb9004
>     > --- /dev/null
>     > +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     > @@ -0,0 +1,63 @@
>     > +#include "lua.h"
>     > +#include "lauxlib.h"
>     > +
>     > +#include "test.h"
>     > +#include "utils.h"
>     > +
>     > +/*
>     > + * XXX: The "lj_obj.h" header is included to calculate the
>     > + * number of stack slots used from the bottom of the stack.
>     > + */
>     > +#include "lj_obj.h"
>     > +
>     > +static int cur_slots = -1;
>     > +
>     > +static int fill_stack(lua_State *L)
>     > +{
>     > + cur_slots = L->base - tvref(L->stack);
>     > +
>     > + while(lua_gettop(L) < LUAI_MAXSTACK) {
>     > + cur_slots += 1;
>     > + lua_pushinteger(L, 42);
>     > + }
>     > +
>     > + return 0;
>     > +}
>     > +
>     > +static int premature_stackoverflow(void *test_state)
>     > +{
>     > + lua_State *L = test_state;
>     > + lua_cpcall(L, fill_stack, NULL);
>     > + assert_true(cur_slots == LUAI_MAXSTACK - 1);
>     > + return TEST_EXIT_SUCCESS;
>     > +}
>     > +
>     this testcase should fail with reverted patch, right? but it is not
>
> And it does fail. Tested on GC64/non-GC64 builds on Linux/MacOS.
>
>     > +/*
>     > + * XXX: This test should fail neither before the patch
>     > + * nor after it.
>
>     I propose to say about it in commit message.
>
> Fixed, the branch is force-pushed. New commit message:
> ====
> Cleanup stack overflow handling.
> Reported by Peter Cawley.
> (cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12)
> After the previous patch, it is possible to trigger the
> `stack overflow` error prematurely. Consider the following
> situation: there are already 33000 slots allocated on a Lua
> stack, and then there are 30 additional slots needed. In this
> case, the actual allocated amount would be twice the already
> allocated size, shrunk to the `LJ_STACK_MAXEX` size, which
> would lead to the stack overflow error, despite the fact there
> is plenty of unused space. This patch completely reworks the
> logic of error handling during stack growth to address the issue.
> Another important thing to notice is that the `LJ_ERR_STKOV` is
> thrown only if the `L->status` is `LUA_OK` and that status is set
> to `LUA_ERRRUN` just before throwing the error. The status is set
> to `LUA_ERRRUN` to avoid the second stack overflow during the
> `err_msgv` execution.
> The `stackoverflow_during_stackoverflow` should fail neither
> before the patch nor after and is added for the test to be
> exhaustive.
> Maxim Kokryashkin:
> * added the description and the test for the problem
> Part of tarantool/tarantool#9145
> ====
>
>
>     We have a rule that test must fail without backported patch, so passed
>     test is unexpected here.
>
>     > + */
>     > +static int stackoverflow_during_stackoverflow(void *test_state)
>     > +{
>     > + lua_State *L = test_state;
>     > + /*
>     > + * XXX: `fill_stack` acts here as its own error handler,
>     > + * causing the second stack overflow.
>     > + */
>     > + lua_pushcfunction(L, fill_stack);
>     > + lua_pushcfunction(L, fill_stack);
>     > + int status = lua_pcall(L, 0, 0, -2);
>     > + assert_true(status == LUA_ERRERR);
>     > + return TEST_EXIT_SUCCESS;
>     > +}
>     > +
>     > +int main(void)
>     > +{
>     > + lua_State *L = utils_lua_init();
>     > + const struct test_unit tgroup[] = {
>     > + test_unit_def(premature_stackoverflow),
>     > + test_unit_def(stackoverflow_during_stackoverflow),
>     > + };
>     > + const int test_result = test_run_group(tgroup, L);
>     > + utils_lua_close(L);
>     > + return test_result;
>     > +}
>

[-- Attachment #2: Type: text/html, Size: 13131 bytes --]

[Tarantool-patches] [PATCH luajit v3 3/3] Follow-up fix for stack overflow handling cleanup.

[Maksim Kokryashkin via Tarantool-patches]

From: Mike Pall <mike>

(cherry-picked from commit aa6b15c1a8922848bd6f596ba384824ca3fe0f5f)

The stack overflow error is thrown in `lj_state_growstack` only
if the coroutine status is `OK`, however, stack overflow can
happen on a yielded coroutine too. This patch fixes the condition
for status, so now the error thrown on yielded coroutines too.

Maxim Kokryashkin:
* added the description and the test for the patch

Part of tarantool/tarantool#9145
---
 src/lj_state.c                                |  2 +-
 .../lj-962-premature-stack-overflow.test.c    | 23 +++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/lj_state.c b/src/lj_state.c
index d8a5134c..01d4901a 100644
--- a/src/lj_state.c
+++ b/src/lj_state.c
@@ -126,7 +126,7 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
     if (L->stacksize > LJ_STACK_MAXEX)
       lj_err_throw(L, LUA_ERRERR);  /* Does not invoke an error handler. */
     /* 1. We are _at_ the limit after the last growth. */
-    if (!L->status) {  /* 2. Throw 'stack overflow'. */
+    if (L->status < LUA_ERRRUN) {  /* 2. Throw 'stack overflow'. */
       L->status = LUA_ERRRUN;  /* Prevent ending here again for pushed msg. */
       lj_err_msg(L, LJ_ERR_STKOV);  /* May invoke an error handler. */
     }
diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
index 12cb9004..461e0ccc 100644
--- a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
+++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
@@ -24,6 +24,20 @@ static int fill_stack(lua_State *L)
 	return 0;
 }
 
+static int immediate_yield(lua_State *L)
+{
+	return lua_yield(L, 0);
+}
+
+static int overflow_suspended_coro(lua_State *L)
+{
+	lua_State *newL = lua_newthread(L);
+	lua_pushcfunction(newL, immediate_yield);
+	lua_resume(newL, 0);
+	fill_stack(newL);
+	return 0;
+}
+
 static int premature_stackoverflow(void *test_state)
 {
 	lua_State *L = test_state;
@@ -50,12 +64,21 @@ static int stackoverflow_during_stackoverflow(void *test_state)
 	return TEST_EXIT_SUCCESS;
 }
 
+static int stackoverflow_on_suspended_coro(void *test_state)
+{
+	lua_State *L = test_state;
+	int status = lua_cpcall(L, overflow_suspended_coro, NULL);
+	assert_true(status == LUA_ERRRUN);
+	return TEST_EXIT_SUCCESS;
+}
+
 int main(void)
 {
 	lua_State *L = utils_lua_init();
 	const struct test_unit tgroup[] = {
 		test_unit_def(premature_stackoverflow),
 		test_unit_def(stackoverflow_during_stackoverflow),
+		test_unit_def(stackoverflow_on_suspended_coro),
 	};
 	const int test_result = test_run_group(tgroup, L);
 	utils_lua_close(L);
-- 
2.39.3 (Apple Git-145)

Re: [Tarantool-patches] [PATCH luajit v3 3/3] Follow-up fix for stack overflow handling cleanup.

[Sergey Bronnikov via Tarantool-patches]

Hello, Max

added testcase is passed with reverted patch

Sergey

On 11/22/23 17:35, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
>
> (cherry-picked from commit aa6b15c1a8922848bd6f596ba384824ca3fe0f5f)
>
> The stack overflow error is thrown in `lj_state_growstack` only
> if the coroutine status is `OK`, however, stack overflow can
> happen on a yielded coroutine too. This patch fixes the condition
> for status, so now the error thrown on yielded coroutines too.
>
> Maxim Kokryashkin:
> * added the description and the test for the patch
>
> Part of tarantool/tarantool#9145
> ---
>   src/lj_state.c                                |  2 +-
>   .../lj-962-premature-stack-overflow.test.c    | 23 +++++++++++++++++++
>   2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/src/lj_state.c b/src/lj_state.c
> index d8a5134c..01d4901a 100644
> --- a/src/lj_state.c
> +++ b/src/lj_state.c
> @@ -126,7 +126,7 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>       if (L->stacksize > LJ_STACK_MAXEX)
>         lj_err_throw(L, LUA_ERRERR);  /* Does not invoke an error handler. */
>       /* 1. We are _at_ the limit after the last growth. */
> -    if (!L->status) {  /* 2. Throw 'stack overflow'. */
> +    if (L->status < LUA_ERRRUN) {  /* 2. Throw 'stack overflow'. */
>         L->status = LUA_ERRRUN;  /* Prevent ending here again for pushed msg. */
>         lj_err_msg(L, LJ_ERR_STKOV);  /* May invoke an error handler. */
>       }
> diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
> index 12cb9004..461e0ccc 100644
> --- a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
> +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
> @@ -24,6 +24,20 @@ static int fill_stack(lua_State *L)
>   	return 0;
>   }
>   
> +static int immediate_yield(lua_State *L)
> +{
> +	return lua_yield(L, 0);
> +}
> +
> +static int overflow_suspended_coro(lua_State *L)
> +{
> +	lua_State *newL = lua_newthread(L);
> +	lua_pushcfunction(newL, immediate_yield);
> +	lua_resume(newL, 0);
> +	fill_stack(newL);
> +	return 0;
> +}
> +
>   static int premature_stackoverflow(void *test_state)
>   {
>   	lua_State *L = test_state;
> @@ -50,12 +64,21 @@ static int stackoverflow_during_stackoverflow(void *test_state)
>   	return TEST_EXIT_SUCCESS;
>   }
>   
> +static int stackoverflow_on_suspended_coro(void *test_state)
> +{
> +	lua_State *L = test_state;
> +	int status = lua_cpcall(L, overflow_suspended_coro, NULL);
> +	assert_true(status == LUA_ERRRUN);
> +	return TEST_EXIT_SUCCESS;
> +}
> +
>   int main(void)
>   {
>   	lua_State *L = utils_lua_init();
>   	const struct test_unit tgroup[] = {
>   		test_unit_def(premature_stackoverflow),
>   		test_unit_def(stackoverflow_during_stackoverflow),
> +		test_unit_def(stackoverflow_on_suspended_coro),
>   	};
>   	const int test_result = test_run_group(tgroup, L);
>   	utils_lua_close(L);

Re: [Tarantool-patches] [PATCH luajit v3 3/3] Follow-up fix for stack overflow handling cleanup.

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 3250 bytes --]


Hi, Sergey!
Thanks for the review!
Tested on GC64 and non-GC64, and Linux/MacOS builds — test fails
before the patch everywhere.
Tested with API check and assertions enabled.
--
Best regards,
Maxim Kokryashkin
 
  
>Пятница, 24 ноября 2023, 15:34 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>Hello, Max
>
>added testcase is passed with reverted patch
>
>Sergey
>
>On 11/22/23 17:35, Maksim Kokryashkin wrote:
>> From: Mike Pall <mike>
>>
>> (cherry-picked from commit aa6b15c1a8922848bd6f596ba384824ca3fe0f5f)
>>
>> The stack overflow error is thrown in `lj_state_growstack` only
>> if the coroutine status is `OK`, however, stack overflow can
>> happen on a yielded coroutine too. This patch fixes the condition
>> for status, so now the error thrown on yielded coroutines too.
>>
>> Maxim Kokryashkin:
>> * added the description and the test for the patch
>>
>> Part of tarantool/tarantool#9145
>> ---
>> src/lj_state.c | 2 +-
>> .../lj-962-premature-stack-overflow.test.c | 23 +++++++++++++++++++
>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/lj_state.c b/src/lj_state.c
>> index d8a5134c..01d4901a 100644
>> --- a/src/lj_state.c
>> +++ b/src/lj_state.c
>> @@ -126,7 +126,7 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need)
>> if (L->stacksize > LJ_STACK_MAXEX)
>> lj_err_throw(L, LUA_ERRERR); /* Does not invoke an error handler. */
>> /* 1. We are _at_ the limit after the last growth. */
>> - if (!L->status) { /* 2. Throw 'stack overflow'. */
>> + if (L->status < LUA_ERRRUN) { /* 2. Throw 'stack overflow'. */
>> L->status = LUA_ERRRUN; /* Prevent ending here again for pushed msg. */
>> lj_err_msg(L, LJ_ERR_STKOV); /* May invoke an error handler. */
>> }
>> diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>> index 12cb9004..461e0ccc 100644
>> --- a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>> +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>> @@ -24,6 +24,20 @@ static int fill_stack(lua_State *L)
>> return 0;
>> }
>>
>> +static int immediate_yield(lua_State *L)
>> +{
>> + return lua_yield(L, 0);
>> +}
>> +
>> +static int overflow_suspended_coro(lua_State *L)
>> +{
>> + lua_State *newL = lua_newthread(L);
>> + lua_pushcfunction(newL, immediate_yield);
>> + lua_resume(newL, 0);
>> + fill_stack(newL);
>> + return 0;
>> +}
>> +
>> static int premature_stackoverflow(void *test_state)
>> {
>> lua_State *L = test_state;
>> @@ -50,12 +64,21 @@ static int stackoverflow_during_stackoverflow(void *test_state)
>> return TEST_EXIT_SUCCESS;
>> }
>>
>> +static int stackoverflow_on_suspended_coro(void *test_state)
>> +{
>> + lua_State *L = test_state;
>> + int status = lua_cpcall(L, overflow_suspended_coro, NULL);
>> + assert_true(status == LUA_ERRRUN);
>> + return TEST_EXIT_SUCCESS;
>> +}
>> +
>> int main(void)
>> {
>> lua_State *L = utils_lua_init();
>> const struct test_unit tgroup[] = {
>> test_unit_def(premature_stackoverflow),
>> test_unit_def(stackoverflow_during_stackoverflow),
>> + test_unit_def(stackoverflow_on_suspended_coro),
>> };
>> const int test_result = test_run_group(tgroup, L);
>> utils_lua_close(L);
 

[-- Attachment #2: Type: text/html, Size: 4170 bytes --]

Re: [Tarantool-patches] [PATCH luajit v3 3/3] Follow-up fix for stack overflow handling cleanup.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 3827 bytes --]

Hi, Max


my bad, test triggers a fixed problem.

Thanks for the patch! LGTM

On 12/6/23 17:47, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the review!
> Tested on GC64 and non-GC64, and Linux/MacOS builds — test fails
> before the patch everywhere.
> Tested with API check and assertions enabled.
> --
> Best regards,
> Maxim Kokryashkin
>
>     Пятница, 24 ноября 2023, 15:34 +03:00 от Sergey Bronnikov
>     <sergeyb@tarantool.org>:
>     Hello, Max
>
>     added testcase is passed with reverted patch
>
>     Sergey
>
>     On 11/22/23 17:35, Maksim Kokryashkin wrote:
>     > From: Mike Pall <mike>
>     >
>     > (cherry-picked from commit aa6b15c1a8922848bd6f596ba384824ca3fe0f5f)
>     >
>     > The stack overflow error is thrown in `lj_state_growstack` only
>     > if the coroutine status is `OK`, however, stack overflow can
>     > happen on a yielded coroutine too. This patch fixes the condition
>     > for status, so now the error thrown on yielded coroutines too.
>     >
>     > Maxim Kokryashkin:
>     > * added the description and the test for the patch
>     >
>     > Part of tarantool/tarantool#9145
>     > ---
>     > src/lj_state.c | 2 +-
>     > .../lj-962-premature-stack-overflow.test.c | 23 +++++++++++++++++++
>     > 2 files changed, 24 insertions(+), 1 deletion(-)
>     >
>     > diff --git a/src/lj_state.c b/src/lj_state.c
>     > index d8a5134c..01d4901a 100644
>     > --- a/src/lj_state.c
>     > +++ b/src/lj_state.c
>     > @@ -126,7 +126,7 @@ void LJ_FASTCALL
>     lj_state_growstack(lua_State *L, MSize need)
>     > if (L->stacksize > LJ_STACK_MAXEX)
>     > lj_err_throw(L, LUA_ERRERR); /* Does not invoke an error handler. */
>     > /* 1. We are _at_ the limit after the last growth. */
>     > - if (!L->status) { /* 2. Throw 'stack overflow'. */
>     > + if (L->status < LUA_ERRRUN) { /* 2. Throw 'stack overflow'. */
>     > L->status = LUA_ERRRUN; /* Prevent ending here again for pushed
>     msg. */
>     > lj_err_msg(L, LJ_ERR_STKOV); /* May invoke an error handler. */
>     > }
>     > diff --git
>     a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     > index 12cb9004..461e0ccc 100644
>     > --- a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     > +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c
>     > @@ -24,6 +24,20 @@ static int fill_stack(lua_State *L)
>     > return 0;
>     > }
>     >
>     > +static int immediate_yield(lua_State *L)
>     > +{
>     > + return lua_yield(L, 0);
>     > +}
>     > +
>     > +static int overflow_suspended_coro(lua_State *L)
>     > +{
>     > + lua_State *newL = lua_newthread(L);
>     > + lua_pushcfunction(newL, immediate_yield);
>     > + lua_resume(newL, 0);
>     > + fill_stack(newL);
>     > + return 0;
>     > +}
>     > +
>     > static int premature_stackoverflow(void *test_state)
>     > {
>     > lua_State *L = test_state;
>     > @@ -50,12 +64,21 @@ static int
>     stackoverflow_during_stackoverflow(void *test_state)
>     > return TEST_EXIT_SUCCESS;
>     > }
>     >
>     > +static int stackoverflow_on_suspended_coro(void *test_state)
>     > +{
>     > + lua_State *L = test_state;
>     > + int status = lua_cpcall(L, overflow_suspended_coro, NULL);
>     > + assert_true(status == LUA_ERRRUN);
>     > + return TEST_EXIT_SUCCESS;
>     > +}
>     > +
>     > int main(void)
>     > {
>     > lua_State *L = utils_lua_init();
>     > const struct test_unit tgroup[] = {
>     > test_unit_def(premature_stackoverflow),
>     > test_unit_def(stackoverflow_during_stackoverflow),
>     > + test_unit_def(stackoverflow_on_suspended_coro),
>     > };
>     > const int test_result = test_run_group(tgroup, L);
>     > utils_lua_close(L);
>

[-- Attachment #2: Type: text/html, Size: 7042 bytes --]

Re: [Tarantool-patches] [PATCH luajit v3 0/3] Improve error reporting on stack overflow.

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!
Thanks for the fixes!
The whole patchset LGTM!

-- 
Best regards,
Sergey Kaplun