Hi, Sergey,

thanks for the patch! LGTM with a minor comment. See below.

On 21.08.2024 11:23, Sergey Kaplun wrote:


<snipped>

> diff --git a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua
> new file mode 100644
> index 00000000..9abaeba5
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua
> @@ -0,0 +1,43 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate the crash during the concat recording
> +-- if it throws an error.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1234.
> +
> +local test = tap.test('lj-1234-err-in-record-concat'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(2)
> +
> +jit.opt.start('hotloop=1')
> +
> +local __concat = function(v1, v2)
> +    return tostring(v1) .. tostring(v2)
> +end
> +
> +-- Need to use metamethod call in the concat recording.
> +debug.setmetatable(nil, {
I propose to add a comment that explain why `nil` is used here.
> +  __concat = __concat,
> +})
> +
> +local function test_concat_p()
> +  local counter = 0
> +  while counter < 1 do
> +    counter = counter + 1
> +    -- The first result is placed on the Lua stack before the
> +    -- error is raised. When the error is raised, it is handled by
> +    -- the trace recorder, but since neither `rec_cat()` nor
> +    -- `lj_record_ret()` restore the Lua stack (before the patch),
> +    -- it becomes unbalanced after the instruction recording
> +    -- attempt.
> +    local _ = {} .. (nil .. nil)
> +  end
> +end
> +
> +local result, errmsg = pcall(test_concat_p)
> +
> +test:ok(not result, 'the error is raised')
> +test:like(errmsg, 'attempt to concatenate a table value', 'correct error')
> +
> +test:done(true)