Понедельник, 11 сентября 2023, 11:25 +03:00 от Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
Hi, Maxim!
Thanks for the patch!
LGTM, with a few nits below.
On 05.09.23, Maxim Kokryashkin wrote:
> From: Mike Pall <mike>
>
> Thanks to Maxim Kokryashkin.
>
> (cherry-picked from commit d5bbf9cdb4c5eddc404a90bd44f077cfb3a57a90)
>
> This patch fixes the same issue with frame, as the previous
Typo: s/frame,/the frame/
> one, but now for the table overflow error in the `err_msgv`
> function. The test for the problem uses the table of GC
> finalizers, although they are not required to reproduce the
> issue. They only used to make the test as simple as possible.
Typo: s/They only used/They are only used/
Minor: Feel free to add the similar comment to the test itself.
>
> Resolves tarantool/tarantool#562
> Part of tarantool/tarantool#8825
> ---
> src/lj_err.c | 4 +++
> .../lj-1034-tabov-error-frame.test.lua | 27 +++++++++++++++++++
> 2 files changed, 31 insertions(+)
> create mode 100644 test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
>
> diff --git a/src/lj_err.c b/src/lj_err.c
> index 763746e6..46fb81ee 100644
> --- a/src/lj_err.c
> +++ b/src/lj_err.c
<snipped>
> diff --git a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> new file mode 100644
> index 00000000..b7520d92
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> @@ -0,0 +1,27 @@
> +local tap = require('tap')
> +local ffi = require('ffi')
> +local test = tap.test('lj-1034-tabov-error-frame'):skipcond({
> + ['Test requires JIT enabled'] = not jit.status(),
> + ['Test requires GC64 mode enabled'] = not ffi.abi('gc64'),
> + ['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
> +})
> +
> +test:plan(2)
> +
Side note: The test requires ~6Gb of memory to see the error.
Feel free to add the corresponding comment or ignore.
> +-- luacheck: no unused
> +local anchor = {}
> +local function on_gc(t) end
> +
> +local function test_finalizers()
> + local i = 1
> + while true do
> + anchor[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
> + i = i + 1
> + end
> +end
Minor: Please use 2 spaces for indentation instead.
> +
> +local st, err = pcall(test_finalizers)
> +st, err = pcall(test_finalizers)
> +test:ok(st == false, 'error handled successfully')
> +test:like(err, '^.+table overflow', 'error is table overflow')
> +test:done(true)
> --
> 2.41.0
>
--
Best regards,
Sergey Kaplun