[Tarantool-patches] [PATCH luajit v1] Fix BC_UCLO insertion for returns.
Sergey Bronnikov
sergeyb at tarantool.org
Thu Jul 13 12:55:51 MSK 2023
Hi, Sergey!
On 7/13/23 10:57, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the fixes!
>
> Still some thoughts about the `pcall()`.
>
> On 10.07.23, Sergey Bronnikov wrote:
>> Hi, Sergey!
<snipped>
>> @@ -110,6 +114,7 @@ f()
>> f = missing_uclo()
>> local _
>> _, res = pcall(f)
>> -test:ok(type(res) == 'function', 'consistency on compilation: type of
>> returned value is correct')
>> +test:ok(type(res) == 'function',
>> + 'consistency on compilation: type of returned value is correct')
>>
>> os.exit(test:check() and 0 or 1)
>>
>>> Do we need pcall here?
>>
>> I would use it to avoid breaking test due to assert.
>>
>> Without a pcall:
>>
>>
>> TAP version 13
>> 1..2
>> not ok - VM consistency: type of returned value is correct
>> filename: eval
>> line: -1
>> frame #1
>> line: 0
>> source: @test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> filename: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> what: main
>> namewhat:
>> src: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> frame #2
>> line: -1
>> source: =[C]
>> filename: eval
>> what: C
>> namewhat:
>> src: [C]
>> luajit:
>> /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_record.c:135:
>> rec_check_slots: Assertion `((((((tr))>>24) & IRT_TYPE) -
>> (TRef)(IRT_NUM) <= (TRef)(IRT_INT-IRT_NUM)))' failed.
>> Aborted (core dumped)
>>
>> With pcall:
>>
>> TAP version 13
>> 1..2
>> not ok - VM consistency: type of returned value is correct
>>
>> filename: eval
>> line: -1
>> frame #1
>> line: 0
>> source: @test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> filename: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> what: main
>> namewhat:
>> src: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> frame #2
>> line: -1
>> source: =[C]
>> filename: eval
>> what: C
>> namewhat:
>> src: [C]
>> not ok - consistency on compilation: type of returned value is correct
>> filename: eval
>> line: -1
>> frame #1
>> line: 0
>>
>> <snipped>
>>
>>
>> I like second output more.
> Yes, but there is no trace related to the `f()` only for
> `test:check()`:
>
> | ---- TRACE 1 start tap.lua:33
> | ---- TRACE 2 start 1/stitch tap.lua:34
> | ---- TRACE 3 start tap.lua:16
> | ---- TRACE 3 start tap.lua:80
>
> So, with this `pcall()` we lose the JIT testing.
With patch below JIT assertion is triggered:
--- a/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
+++ b/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
@@ -101,10 +101,10 @@ local function missing_uclo()
end
local f = missing_uclo()
-local res = f()
+local res_interp = f()
-- Without a patch, we don't get here a function, because upvalue
-- isn't closed as desirable.
-test:ok(type(res) == 'function',
+test:ok(type(res_interp) == 'function',
'VM consistency: type of returned value is correct')
jit.opt.start('hotloop=1')
@@ -112,9 +112,8 @@ jit.opt.start('hotloop=1')
f = missing_uclo()
f()
f = missing_uclo()
-local _
-_, res = pcall(f)
-test:ok(type(res) == 'function',
+local _, res_jit = pcall(f)
+test:ok(type(res_jit) == 'function',
'consistency on compilation: type of returned value is correct')
os.exit(test:check() and 0 or 1)
In a 1:1 conversation we agreed that it is ok. I added patch to the
branch and force-pushed.
More information about the Tarantool-patches
mailing list