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: 6366 bytes --]

Hi, Sergey! Thanks for the patch!

LGTM after updating the test. I tried the test and it passed without the fix.

To check container bit size calculation we can add a field at bit 16. It’s byte offset with invalid calculation will be 0 and it will be 2 with the fixed calculation.

The output of the test with check printing:

Without the fix:

TAP version 13
1..2
XX-R: 4
ok - correct 0-initialization
XX: 4
XX-R: 4
ok - bitfield set correctly
# align=2 sz=4
# bitfield ofs=0 bitofs=0 bitsize=1
# bitfield15 ofs=0 bitofs=1 bitsize=15
# bitfield16 ofs=0 bitofs=16 bitsize=1

With the fix:

TAP version 13
1..2
XX-R: 2
ok - correct 0-initialization
XX: 2
XX-R: 2
ok - bitfield set correctly
# align=2 sz=4
# bitfield ofs=0 bitofs=0 bitsize=1
# bitfield15 ofs=0 bitofs=1 bitsize=15
# bitfield16 ofs=2 bitofs=0 bitsize=1

The update for the code with check prints:

diff --git a/src/lj_cconv.c b/src/lj_cconv.c
index 94ca93bb..26597aa5 100644
--- a/src/lj_cconv.c
+++ b/src/lj_cconv.c
@@ -429,6 +429,7 @@ int lj_cconv_tv_bf(CTState *cts, CType *s, TValue *o, uint8_t *sp)
uint32_t val;
lj_assertCTS(ctype_isbitfield(info), "bitfield expected");
/* NYI: packed bitfields may cause misaligned reads. */
+  printf("XX-R: %u\n", ctype_bitcsz(info));
switch (ctype_bitcsz(info)) {
case 4: val = *(uint32_t *)sp; break;
case 2: val = *(uint16_t *)sp; break;
@@ -666,6 +667,7 @@ void lj_cconv_bf_tv(CTState *cts, CType *d, uint8_t *dp, TValue *o)
mask = ((1u << bsz) - 1u) << pos;
val = (val << pos) & mask;
/* NYI: packed bitfields may cause misaligned reads/writes. */
+  printf("XX: %u\n", ctype_bitcsz(info));
switch (ctype_bitcsz(info)) {
case 4: *(uint32_t *)dp = (*(uint32_t *)dp & ~mask) | (uint32_t)val; break;
case 2: *(uint16_t *)dp = (*(uint16_t *)dp & ~mask) | (uint16_t)val; break;
diff --git a/src/lj_cparse.c b/src/lj_cparse.c
index 1b3ce7ec..44c5e16d 100644
--- a/src/lj_cparse.c
+++ b/src/lj_cparse.c
@@ -1341,8 +1341,8 @@ static void cp_struct_layout(CPState *cp, CTypeID sid, CTInfo sattr)
CTALIGN(lj_fls(sz));
ct->size = (bofs >> 3);  /* Store field offset. */
} else {
-         if (csz > amask+1 && bsz <= amask+1)
-           csz = amask+1;  /* Shrink container of packed bitfield. */
+       //   if (csz > amask+1 && bsz <= amask+1)
+       //     csz = amask+1;  /* Shrink container of packed bitfield. */
ct->info = CTINFO(CT_BITFIELD,
(info & (CTF_QUAL|CTF_UNSIGNED|CTF_BOOL)) +
(csz << (CTSHIFT_BITCSZ-3)) + (bsz << CTSHIFT_BITBSZ));
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..bdd5c69e 100644
--- a/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
+++ b/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
@@ -12,6 +12,8 @@ ffi.cdef[[
#pragma pack(push, 2)
typedef struct {
unsigned int bitfield:1;
+  unsigned int bitfield15:15;
+  unsigned int bitfield16:1;
} packed_struct;
#pragma pack(pop)
]]
@@ -25,4 +27,16 @@ test:is(packed.bitfield, 0, 'correct 0-initialization')
packed.bitfield = 1
test:is(packed.bitfield, 1, 'bitfield set correctly')

+local ps = ffi.typeof('packed_struct')
+test:diag('align=%d sz=%d', ffi.alignof(ps), ffi.sizeof(ps))
+local o, bo, bs
+o, bo, bs = ffi.offsetof(ps, 'bitfield')
+test:diag('bitfield ofs=%d bitofs=%d bitsize=%d', o, bo, bs)
+o, bo, bs = ffi.offsetof(ps, 'bitfield15')
+test:diag('bitfield15 ofs=%d bitofs=%d bitsize=%d', o, bo, bs)
+o, bo, bs = ffi.offsetof(ps, 'bitfield16')
+test:diag('bitfield16 ofs=%d bitofs=%d bitsize=%d', o, bo, bs)
+
+test:fail()
+
test:done(true)

--
Best regards,
Evgeniy Temirgaleev

> 
> From: Sergey Kaplun <skaplun@tarantool.org>
> To: Sergey Bronnikov <sergeyb@tarantool.org>, Evgeniy Temirgaleev <e.temirgaleev@tarantool.org
> >
> Cc: tarantool-patches@dev.tarantool.org, Sergey Kaplun <skaplun@tarantool.org
> >
> Date: Monday, July 20, 2026 3:59 PM +03:00
> From: Mike Pall <mike>
> 
> Reported by Huang Haiyang.
> 
> (cherry picked from commit e4c7d8b38040518d42599eef8ddb5e67aa967a9c)
> 
> The bitfield of packed structure uses the original type size, which
> leads to heap-buffer-overflow access on conversions.
> 
> This patch fixes it by adjusting the size of the bitfield container when
> necessary.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#12880
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1451-ffi-packed-bitfield
> 
> Related issues:
> * https://github.com/LuaJIT/LuaJIT/issues/1451
> * https://github.com/tarantool/tarantool/issues/12880
> 
> src/lj_cparse.c | 2 ++
> .../lj-1451-ffi-packed-bitfield.test.lua | 28 +++++++++++++++++++
> 2 files changed, 30 insertions(+)
> create mode 100644
> test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> 
> diff --git a/src/lj_cparse.c b/src/lj_cparse.c
> index ff23b44b..1b3ce7ec 100644
> --- a/src/lj_cparse.c
> +++ b/src/lj_cparse.c
> @@ -1341,6 +1341,8 @@ static void cp_struct_layout(CPState *cp, CTypeID
> sid, CTInfo sattr)
> CTALIGN(lj_fls(sz));
> ct->size = (bofs >> 3); /* Store field offset. */
> } else {
> + if (csz > amask+1 && bsz <= amask+1)
> + csz = amask+1; /* Shrink container of packed bitfield. */
> ct->info = CTINFO(CT_BITFIELD,
> (info & (CTF_QUAL|CTF_UNSIGNED|CTF_BOOL)) +
> (csz << (CTSHIFT_BITCSZ-3)) + (bsz << CTSHIFT_BITBSZ));
> diff --git a/test/tarantool-tests/lj-1451-ffi-packed-bitfield.test.lua
> 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
>

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