Re: [Tarantool-patches] [PATCH luajit] FFI: Fix various issues in recff_cdata_arith.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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

Thanks! LGTM

On 12.09.2024 13:16, Sergey Kaplun wrote:
> On 10.09.24, Sergey Kaplun via Tarantool-patches wrote:
>> Hi, Sergey!
>> Thanks for the review!
>>
>> On 10.09.24, Sergey Bronnikov wrote:
>>> Hi, Sergey,
>>>
>>> On 09.09.2024 18:37, Sergey Bronnikov via Tarantool-patches wrote:
>>>> Hi, Sergey,
>>>>
>>>> thanks for the patch! see my comments below.
>>>>
>>>> On 21.08.2024 19:52, Sergey Kaplun wrote:
>>>>> From: Mike Pall <mike>
>>>>>
>> <snipped>
>>
>>>>> ---
>>>>>
>>>>> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1224-fix-jit-cdata-arith
>>>>> Related issues:
>>>>> *https://github.com/tarantool/tarantool/issues/10199
>>>>> *https://github.com/LuaJIT/LuaJIT/issues/1224
>>>>>
>>>>>    src/lj_crecord.c                              | 10 ++--
>>>>>    .../lj-1224-fix-cdata-arith-ptr.test.lua      | 48 +++++++++++++++++++
>>>> This test does not fail without fix (but repro from the issue does):
>>>>
>>>> [0] ~/sources/MRG/tarantool/third_party/luajit $
>>>> ./build/gc64/src/luajit -Ohotloop=1 -e "
>>>> repeat
>>>>    r = 1LL + nil
>>>> until true
>>>> "
>>>> LuaJIT ASSERT
>>>> /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_ctype.c:185:
>>>> lj_ctype_intern: uninitialized cts->L
>>>> Aborted
>>>> [0] ~/sources/MRG/tarantool/third_party/luajit $
>>>> ./build/gc64/src/luajit
>>>> test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua
>>>> TAP version 13
>>>> 1..2
>>>>      # cdata arithmetic with nil
>>>> 1..2
>>>>      ok - correct recording error with bad cdata arithmetic
>>>>      ok - correct error message
>>>>      # cdata arithmetic with nil: end
>>>> ok - cdata arithmetic with nil
>>>>      # cdata arithmetic with string
>>>>      1..2
>>>>      ok - correct recording error with bad cdata arithmetic
>>>>      ok - correct error message
>>>>      # cdata arithmetic with string: end
>>>> ok - cdata arithmetic with string
>>>> [0] ~/sources/MRG/tarantool/third_party/luajit $
>>>>
>>> With GC64 only (LUAJIT_ENABLE_GC64).
>> It don't fail with GC64?
>>
>> Should I add the comment?
> I've added the following comment to avoid the confusion.
> Branch is force-pushed.
>
> ===================================================================
> diff --git a/test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua b/test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua
> index 9de9586d..dd7e23ec 100644
> --- a/test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua
> +++ b/test/tarantool-tests/lj-1224-fix-cdata-arith-ptr.test.lua
> @@ -11,6 +11,8 @@ local test = tap.test('lj-1224-fix-cdata-arith-ptr'):skipcond({
>   
>   test:plan(2)
>   
> +-- XXX: This test leads to the assertion failure before the patch
> +-- only for non-GC64 build.
>   local function arith_nil_protected()
>     local i = 1
>     while i < 3 do
> ===================================================================
>
>>> <snipped>
>> -- 
>> Best regards,
>> Sergey Kaplun

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