[BUG FBC] Bitfields not properly packed

Jattenalle · Post by **Jattenalle** » Aug 21, 2024 1:58

Issue:
Bitfield packing depends on the order of member type sizes and does not properly and consistently pack bitfields.

Affects:
All: FBC32, FBC64, GEN GAS, GEN GCC

Description:
A bitfield with different access type sizes where members are ordered in descending order based on the bitsize is properly padded and behaves as expected.
A bitfield with different access type sizes where members are ordered in ascending order based on the bitsize is not packed at all, instead acting as a regular type non bitfield. Which is inconsistent and unexpected behaviour.

In addition, the documentation states that bitfields are packed:
(Via: https://www.freebasic.net/wiki/KeyPgType)

Bitfield members in a type are packed together, unless the next member is a non-bitfield (nested union is considered a non-bitfield).

It makes no mention of bitfield members having to all be the same access type in order to actually be packed and work -as expected/-consistently. Nor having to order them in descending size if you want different access types.

Consequence:
Makes porting several headers and libraries all but impossible as they rely on packed bitfields of different access types being passed back and forth.

Replicate:

Code: Select all

#include once "crt/mem.bi"
scope
	print "With different members"
	type DescendingBits field = 1
		Reserved:31 as ulong
		AutoDealloc:1 as ubyte
	end type
	type AscendingBits field = 1
		AutoDealloc:1 as ubyte
		Reserved:31 as ulong
	end type
	
	dim as DescendingBits descbits
	dim as AscendingBits ascbits
	dim as any ptr descptr = @descbits
	dim as any ptr ascptr = @ascbits
	
	descbits.Reserved = &h7fffffff
	descbits.AutoDealloc = 1
	ascbits.Reserved = &h7fffffff
	ascbits.AutoDealloc = 1
	
	print "	Bitfields:"
	print "		Desc: "& bin(*cptr(ulong ptr, descptr), 32)
	print "		Asc:32: "& bin(*cptr(ulong ptr, ascptr), 32)
	print "		Asc:40: "& bin(*cptr(ulongint ptr, ascptr), 40) '// Not out of bounds since we only access the first 5 bytes, which is the size of AscendingBits
	
	'// Save all the flags we've set, we assume they fit in a 32bit because the flags are 32bits
	dim as ulong descflags	=	*cptr(ulong ptr, descptr)
	dim as ulong ascflags	=	*cptr(ulong ptr, ascptr)
	
	'// Actually save the ascflags using the actual size and unwanted padding
	dim as any ptr ascflagsfull = callocate(5) '// Actual size is 40bits, 5 bytes
	memcpy(ascflagsfull, ascptr, 5)
	
	'// Clear all
	descbits.Reserved = 0
	descbits.AutoDealloc = 0
	ascbits.Reserved = 0
	ascbits.AutoDealloc = 0
	
	'// Load them back in from the 32bit saved value which should fit them all
	*cptr(ulong ptr, descptr)	=	descflags
	*cptr(ulong ptr, ascptr)	=	ascflags
	
	print "	Desc"
	print "		SizeOf: "& sizeof(descbits)
	print "		Bits:32: "& bin(*cptr(ulong ptr, descptr), 32)
	print "		Flags: "& descflags
	print "			:31: "& hex(descbits.Reserved)
	print "			:1: "& descbits.AutoDealloc
	print "	Asc (Note incorrect size and incorrect value of :31)"
	print "		SizeOf: "& sizeof(ascbits)
	print "		Bits:32: "& bin(*cptr(ulong ptr, ascptr), 32)
	print "		Bits:40: "& bin(*cptr(ulongint ptr, ascptr), 40)
	print "		Flags: "& ascflags
	print "			:31: "& hex(ascbits.Reserved)
	print "			:1: "& ascbits.AutoDealloc
	
	'// Clear ascbits
	ascbits.Reserved = 0
	ascbits.AutoDealloc = 0
	
	'// Load in "full"
	memcpy(ascptr, ascflagsfull, 5)
	deallocate ascflagsfull
	print "	Asc (40bit memcpy)"
	print "		SizeOf: "& sizeof(ascbits)
	print "		Bits:32: "& bin(*cptr(ulong ptr, ascptr), 32)
	print "		Bits:40: "& bin(*cptr(ulongint ptr, ascptr), 40)
	print "		Flags: "& ascflags
	print "			:31: "& hex(ascbits.Reserved)
	print "			:1: "& ascbits.AutoDealloc
end scope

scope
	print "With both members as ulong"
	'// However, if both members are set to ulong it works as expected for both cases:
	type DescendingBits field = 1
		Reserved:31 as ulong
		AutoDealloc:1 as ulong
	end type
	type AscendingBits field = 1
		AutoDealloc:1 as ulong
		Reserved:31 as ulong
	end type
	
	dim as DescendingBits descbits
	dim as AscendingBits ascbits
	dim as any ptr descptr = @descbits
	dim as any ptr ascptr = @ascbits
	
	descbits.Reserved = &h7fffffff
	descbits.AutoDealloc = 1
	ascbits.Reserved = &h7fffffff
	ascbits.AutoDealloc = 1
	
	print "	Bitfields:"
	print "		Desc: "& bin(*cptr(ulong ptr, descptr), 32)
	print "		Asc: "& bin(*cptr(ulong ptr, ascptr), 32)
	
	'// Save all the flags we've set, and now they both actually fit in 32bits
	dim as ulong descflags	=	*cptr(ulong ptr, descptr)
	dim as ulong ascflags	=	*cptr(ulong ptr, ascptr)
	
	'// Clear all
	descbits.Reserved = 0
	descbits.AutoDealloc = 0
	ascbits.Reserved = 0
	ascbits.AutoDealloc = 0
	
	'// Load them back in from the 32bit saved value which should fit them all
	*cptr(ulong ptr, descptr)	=	descflags
	*cptr(ulong ptr, ascptr)	=	ascflags
	
	print "	Desc"
	print "		SizeOf: "& sizeof(descbits)
	print "		Bits: "& bin(*cptr(ulong ptr, descptr), 32)
	print "		Flags: "& descflags
	print "			:31: "& hex(descbits.Reserved)
	print "			:1: "& descbits.AutoDealloc
	print "	Asc"
	print "		SizeOf: "& sizeof(ascbits)
	print "		Bits: "& bin(*cptr(ulong ptr, ascptr), 32)
	print "		Flags: "& ascflags
	print "			:31: "& hex(ascbits.Reserved)
	print "			:1: "& ascbits.AutoDealloc
end scope

Output:

Code: Select all

With different members
        Bitfields:
                Desc: 11111111111111111111111111111111
                Asc:32: 11111111111111111111111100000001
                Asc:40: 0111111111111111111111111111111100000001
        Desc
                SizeOf: 4
                Bits:32: 11111111111111111111111111111111
                Flags: 4294967295
                        :31: 7FFFFFFF
                        :1: 1
        Asc (Note incorrect size and incorrect value of :31)
                SizeOf: 5
                Bits:32: 11111111111111111111111100000001
                Bits:40: 0000000011111111111111111111111100000001
                Flags: 4294967041
                        :31: FFFFFF
                        :1: 1
        Asc (40bit memcpy)
                SizeOf: 5
                Bits:32: 11111111111111111111111100000001
                Bits:40: 0111111111111111111111111111111100000001
                Flags: 4294967041
                        :31: 7FFFFFFF
                        :1: 1
With both members as ulong
        Bitfields:
                Desc: 11111111111111111111111111111111
                Asc: 11111111111111111111111111111111
        Desc
                SizeOf: 4
                Bits: 11111111111111111111111111111111
                Flags: 4294967295
                        :31: 7FFFFFFF
                        :1: 1
        Asc
                SizeOf: 4
                Bits: 11111111111111111111111111111111
                Flags: 4294967295
                        :31: 7FFFFFFF
                        :1: 1

Post by **fxm** » Aug 21, 2024 7:46

Yes.

But if all bitfields are declared "As Ulong", the packaging seems to work better:

Code: Select all

Type UDT1 Field = 1
    bf1:31 As Ulong
    bf2: 1 As Ubyte
End Type

Type UDT2 Field = 1
    bf1: 1 As Ubyte
    bf2:31 As Ulong
End Type

Type UDT3 Field = 1
    bf1:31 As Ulong
    bf2: 1 As Ulong
End Type

Type UDT4 Field = 1
    bf1: 1 As Ulong
    bf2:31 As Ulong
End Type

Sub PrintMemory(Byref s As String, Byval addr As Any Ptr, Byval size As Uinteger)
    Print size & " bytes used for " & s
    For I As Integer = 0 to size - 1
        Print Bin(Cptr(Ubyte Ptr, addr)[I], 8)
    Next I
End Sub

Dim As UDT1 u1
u1.bf1 = &h7FFFFFFF : u1.bf2 = &h0
PrintMemory("UDT1 (first bit field = &h7FFFFFFF, second bit fiels = &h0)", @u1, Sizeof(u1))
u1.bf1 = &h0 : u1.bf2 = &h1
PrintMemory("UDT1 (first bit field = &h0, second bit fiels = &h1)", @u1, Sizeof(u1))
Print

Dim As UDT2 u2
u2.bf1 = &h1 : u2.bf2 = &h0
PrintMemory("UDT2 (first bit field = &h1, second bit fiels = &h0)", @u2, Sizeof(u2))
u2.bf1 = &h0 : u2.bf2 = &h7FFFFFFF
PrintMemory("UDT1 (first bit field = &h0, second bit fiels = &h7FFFFFFF)", @u2, Sizeof(u2))
Print

Dim As UDT3 u3
u3.bf1 = &h7FFFFFFF : u3.bf2 = &h0
PrintMemory("UDT3 (first bit field = &h7FFFFFFF, second bit fiels = &h0)", @u3, Sizeof(u3))
u3.bf1 = &h0 : u3.bf2 = &h1
PrintMemory("UDT3 (first bit field = &h0, second bit fiels = &h1)", @u3, Sizeof(u3))
Print

Dim As UDT4 u4
u4.bf1 = &h1 : u4.bf2 = &h0
PrintMemory("UDT4 (first bit field = &h1, second bit fiels = &h0)", @u4, Sizeof(u4))
u4.bf1 = &h0 : u4.bf2 = &h7FFFFFFF
PrintMemory("UDT4 (first bit field = &h0, second bit fiels = &h7FFFFFFF)", @u4, Sizeof(u4))
Print

Sleep

Output:

Code: Select all

4 bytes used for UDT1 (first bit field = &h7FFFFFFF, second bit fiels = &h0)
11111111
11111111
11111111
01111111
4 bytes used for UDT1 (first bit field = &h0, second bit fiels = &h1)
00000000
00000000
00000000
10000000

5 bytes used for UDT2 (first bit field = &h1, second bit fiels = &h0)
00000001
00000000
00000000
00000000
00000000
5 bytes used for UDT1 (first bit field = &h0, second bit fiels = &h7FFFFFFF)
00000000
11111111
11111111
11111111
01111111

4 bytes used for UDT3 (first bit field = &h7FFFFFFF, second bit fiels = &h0)
11111111
11111111
11111111
01111111
4 bytes used for UDT3 (first bit field = &h0, second bit fiels = &h1)
00000000
00000000
00000000
10000000

4 bytes used for UDT4 (first bit field = &h1, second bit fiels = &h0)
00000001
00000000
00000000
00000000
4 bytes used for UDT4 (first bit field = &h0, second bit fiels = &h7FFFFFFF)
11111110
11111111
11111111
11111111

Bit-filling from low bit to high bit and from low byte to high byte.

Post by **fxm** » Aug 21, 2024 9:02

If we view the bytes with the least significant bit on the left rather than the right, we get a more harmonious bit-filling sequence on the screen:

Code: Select all

Type UDT1 Field = 1
    bf1:31 As Ulong
    bf2: 1 As Ubyte
End Type

Type UDT2 Field = 1
    bf1: 1 As Ubyte
    bf2:31 As Ulong
End Type

Type UDT3 Field = 1
    bf1:31 As Ulong
    bf2: 1 As Ulong
End Type

Type UDT4 Field = 1
    bf1: 1 As Ulong
    bf2:31 As Ulong
End Type

Sub PrintMemory(Byref s As String, Byval addr As Any Ptr, Byval size As Uinteger)
    Print size & " bytes used for " & s
    For I As Integer = 0 to size - 1
        Dim As String bs = Bin(Cptr(Ubyte Ptr, addr)[I], 8)
        For J As Integer = 8 To 1 Step -1
            Print Mid(bs, J, 1);
        Next J
        Print
    Next I
End Sub

Dim As UDT1 u1
u1.bf1 = &h7FFFFFFF : u1.bf2 = &h0
PrintMemory("UDT1 (first bit field = &h7FFFFFFF, second bit fiels = &h0)", @u1, Sizeof(u1))
u1.bf1 = &h0 : u1.bf2 = &h1
PrintMemory("UDT1 (first bit field = &h0, second bit fiels = &h1)", @u1, Sizeof(u1))
Print

Dim As UDT2 u2
u2.bf1 = &h1 : u2.bf2 = &h0
PrintMemory("UDT2 (first bit field = &h1, second bit fiels = &h0)", @u2, Sizeof(u2))
u2.bf1 = &h0 : u2.bf2 = &h7FFFFFFF
PrintMemory("UDT1 (first bit field = &h0, second bit fiels = &h7FFFFFFF)", @u2, Sizeof(u2))
Print

Dim As UDT3 u3
u3.bf1 = &h7FFFFFFF : u3.bf2 = &h0
PrintMemory("UDT3 (first bit field = &h7FFFFFFF, second bit fiels = &h0)", @u3, Sizeof(u3))
u3.bf1 = &h0 : u3.bf2 = &h1
PrintMemory("UDT3 (first bit field = &h0, second bit fiels = &h1)", @u3, Sizeof(u3))
Print

Dim As UDT4 u4
u4.bf1 = &h1 : u4.bf2 = &h0
PrintMemory("UDT4 (first bit field = &h1, second bit fiels = &h0)", @u4, Sizeof(u4))
u4.bf1 = &h0 : u4.bf2 = &h7FFFFFFF
PrintMemory("UDT4 (first bit field = &h0, second bit fiels = &h7FFFFFFF)", @u4, Sizeof(u4))
Print

Sleep

Output:

Code: Select all

4 bytes used for UDT1 (first bit field = &h7FFFFFFF, second bit fiels = &h0)
11111111
11111111
11111111
11111110
4 bytes used for UDT1 (first bit field = &h0, second bit fiels = &h1)
00000000
00000000
00000000
00000001

5 bytes used for UDT2 (first bit field = &h1, second bit fiels = &h0)
10000000
00000000
00000000
00000000
00000000
5 bytes used for UDT1 (first bit field = &h0, second bit fiels = &h7FFFFFFF)
00000000
11111111
11111111
11111111
11111110

4 bytes used for UDT3 (first bit field = &h7FFFFFFF, second bit fiels = &h0)
11111111
11111111
11111111
11111110
4 bytes used for UDT3 (first bit field = &h0, second bit fiels = &h1)
00000000
00000000
00000000
00000001

4 bytes used for UDT4 (first bit field = &h1, second bit fiels = &h0)
10000000
00000000
00000000
00000000
4 bytes used for UDT4 (first bit field = &h0, second bit fiels = &h7FFFFFFF)
01111111
11111111
11111111
11111111

Bit-filling from left to right (instead of right to left) and from top to bottom on the screen.

Post by **fxm** » Aug 21, 2024 11:26

Some posts from dkl in chronological order:

dkl wrote: ↑Aug 24, 2012 20:36 if it involves bitfields, that could be the problem - bitfield declarations are not yet emitted correctly under -gen gcc, for example, "as integer i : 1" is emitted as if it were "as integer i", causing the UDT to have wrong size and all following [bit]fields to be misaligned...

You can preserve the generated .c file by passing the -R option on the fbc command line.

dkl wrote: ↑Nov 28, 2013 19:09 There are two issues:

1. fbc's bitfield layout differs from gcc's (i.e. it's not ABI compatible, I think in both cases, Linux bitfields and Microsoft bitfields). I've been trying to fix this but it's difficult because I'm having a hard time figuring out gcc's rules for bitfield layout/packing, and what I know so far is different enough from fbc's layout code that I'm having trouble implementing it. It looks like bitfield access code needs updating too to prevent buffer overflow problems with the new bitfield layout.

2. The C backend only emits container fields, not individual bitfields, and apparently not even that is working correctly. Getting structure sizes right in the C backend is very important, because field and array accesses are emitted as *(&struct + offset) just like the ASM backend would do it, and of course the offsets are calculated on the FB side. Thus, if the structures on the C side are too small, we'll get buffer overflows; if they're too big we'll get misaligned array elements which breaks in combination with global variable initializers.

All in all we need to fix fbc's bitfield layout/packing to match gcc's exactly, and let the C backend emit the original bitfields. That way we can be fairly sure that everything is correct, and the static assertions emitted into the C code will help testing.

Besides that, emitting the original bitfields into the C code is the only way to get proper debugging data for bitfields, so that's a good reason to go this way. For the LLVM backend at least the debugging data is separate, so there we can just emit an array of bytes in place of the bitfields.

dkl wrote: ↑Jul 16, 2014 19:48 For now, I've finally allowed myself to add this patch: C backend: Emit structs containing bitfields as byte arrays

It will hopefully only be a temporary solution. I did make some progress on figuring out gcc's bitfield layout rules (most importantly, the basic rules are written down in the i386 SysV ABI specification -- just need to read that, and then rewrite fbc's symbAddField() and some more to implement it), and I've got a half-finished patch here, but I'm not sure when I can finish it.

In the last post, he refers to this commit: C backend: Emit structs containing bitfields as byte arrays

adeyblue · Post by **adeyblue** » Aug 21, 2024 19:34

Looks like he never got around to finishing that patch, since it still works like that (at least in some circumstances), and even that doesn't always work. Hence the first bug in FB Generates Wrongly Sized Struct which is also caused by a bitfield

dafhi · Post by **dafhi** » Aug 24, 2024 11:05

i had a hard-to-isolate bug in my bit-aligned r/w.

it was solved by using all signed. it would only show up when using 64 bit int

here's an illustration, which may relate

Code: Select all

#define min( a, b)    iif( (a)<(b), (a), (b) )
#define max( a, b)    iif( (a)>(b), (a), (b) )

dim as longint i

'showcasing corrected max()

#if 1
dim as longint c = 1 ' change to ulongint to produce unexpected result
dim as longint clip

clip = max( 0, i + c - 2 )

#else
dim as ulongint c = 1 ' 2-line work-around for 'clip'
dim as longint clip

clip = i + c - 2 ' c can be ulongint but must use 2-liner
clip = max( 0, clip )

#endif


? clip; " "; iif( clip=0, "(expected)", "expected: 0" )
sleep

srvaldez · Post by **srvaldez** » Aug 24, 2024 11:58

hi dafhi

what result do you get from your code ?
I get 0 regardless whether I use #if 1 or #if 0
I am using the latest work-in-progress fbc, 64-bit on Windows

dafhi · Post by **dafhi** » Aug 24, 2024 12:43

my bad. so in the first choice change c = 1 to ulongint.

2nd choice demonstrates unsighed workaround . i'll update comments

[BUG FBC] Bitfields not properly packed

[BUG FBC] Bitfields not properly packed

Re: [BUG FBC] Bitfields not properly packed

Re: [BUG FBC] Bitfields not properly packed

Re: [BUG FBC] Bitfields not properly packed

Re: [BUG FBC] Bitfields not properly packed

Re: [BUG FBC] Bitfields not properly packed

Re: [BUG FBC] Bitfields not properly packed

Re: [BUG FBC] Bitfields not properly packed