Re: [Tarantool-patches] [PATCH luajit] FFI: Shrink container of packed bitfield.

[Evgeniy Temirgaleev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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

Hi, Sergey!
Please, consider the idea about marking the tests with special build options.

> 
> From: Sergey Kaplun <skaplun@tarantool.org>
> To: Evgeniy Temirgaleev <e.temirgaleev@tarantool.org>
> Cc: tarantool-patches@dev.tarantool.org, Sergey Bronnikov <sergeyb@tarantool.org
> >
> Date: Wednesday, July 29, 2026 11:44 AM +03:00
> Hi, Evgeniy!
> Thanks for the review!
> Please, see my answers below.
> 
> On 28.07.26, Evgeniy Temirgaleev wrote:
> > Hi, Sergey! Thanks for the patch!
> >
> > LGTM after updating the test. I tried the test and it passed without the
> fix.
> 
> You need to build LuaJIT with ASAN support, see the comment below.
> 

I thinks it will be better to include such information to the test’s code in a some way.
For example, we can add comments to the code, which we expect to fail with ASAN checks. Also, we can add mark to the corresponding test condition messages with ‘ASAN’ to reflect this in the test reports.

> 
> 
> <snipped>
> 
> The test fails if run under ASAN. Build LuaJIT with its support like the
> following:
> 
> | cmake -DLUAJIT_ENABLE_GC64=ON -DLUAJIT_USE_SYSMALLOC=ON
> -DLUAJIT_USE_ASAN=ON -DLUAJIT_USE_UBSAN=ON -DCMAKE_BUILD_TYPE=Debug
> -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON . && make -j
> 
> But I like the idea to check memory layout of the given structure. See
> the iterative patch below:
> 
> ===================================================================
> diff --git a/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> b/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> index 07e53f76..76483b0d 100644
> --- a/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> +++ b/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> @@ -10,13 +10,21 @@ local ffi = require('ffi')
> 
> ffi.cdef[[
> #pragma pack(push, 2)
> +
> typedef struct {
> unsigned int bitfield:1;
> } packed_struct;
> +
> +typedef struct {
> + unsigned int bitfield0:1;
> + unsigned int bitfield15:15;
> + unsigned int bitfield16:1;
> +} packed_struct2;
> +
> #pragma pack(pop)
> ]]
> 
> -test:plan(2)
> +test:plan(5)
> 
> local packed = ffi.new('packed_struct')
> 
> @@ -25,4 +33,10 @@ test:is(packed.bitfield, 0, 'correct 0-initialization')
> 
> 

test:is(packed.bitfield, 0, 'ASAN: correct 0-initialization') -- ASAN: heap overflow on read

> 
> packed.bitfield = 1
> 

> 
> test:is(packed.bitfield, 1, 'bitfield set correctly')
> 

packed.bitfield = 1 -- ASAN: heap overflow on write
test:is(packed.bitfield, 1, 'ASAN: bitfield set correctly')

> 
> 
> +-- Check correct structure layout.
> +local byteoffset, bitpos, bitsize = ffi.offsetof('packed_struct2',
> 'bitfield16')
> +test:is(byteoffset, 2, 'byteoffset is correct')
> +test:is(bitpos, 0, 'bitpos is correct')
> +test:is(bitsize, 1, 'bitsize is correct')
> +
> test:done(true)
> ===================================================================
> 
> Branch is force-pushed.
> >
> > --
> > Best regards,
> > Evgeniy Temirgaleev
> >
> > >
> 
> <snipped>
> 
> > > b/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> > > new file mode 100644
> > > index 00000000..07e53f76
> > > --- /dev/null
> > > +++ b/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> > > @@ -0,0 +1,28 @@
> > > +local tap = require('tap')
> > > +
> > > +-- Test file to demonstrate LuaJIT's incorrect behaviour of the
> > > +-- `#pragma` pack directive for bitfields in structures.
> > > +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1451.
> > > +
> > > +local test = tap.test('lj-1451-ffi-packed-bitfield')
> > > +
> > > +local ffi = require('ffi')
> > > +
> > > +ffi.cdef[[
> > > +#pragma pack(push, 2)
> > > +typedef struct {
> > > + unsigned int bitfield:1;
> > > +} packed_struct;
> > > +#pragma pack(pop)
> > > +]]
> > > +
> > > +test:plan(2)
> > > +
> > > +local packed = ffi.new('packed_struct')
> > > +
> > > +test:is(packed.bitfield, 0, 'correct 0-initialization')
> > > +
> > > +packed.bitfield = 1
> > > +test:is(packed.bitfield, 1, 'bitfield set correctly')
> > > +
> > > +test:done(true)
> > > --
> > > 2.55.0
> > >
> 
> --
> Best regards,
> Sergey Kaplun
> 

--
Best regards,
Evgeniy Temirgaleev

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