Issue with bitfield FBC 0.91

[SARG]

Hi all,

I got this message when testing the 0.91 version provided by MOD :
"'test.bas:16:1: error: size of array '__$fb_structsizecheck' is negative". Only with -gen gcc.

I guess there is a problem with the calculated size of the structure..... Below the complete message, the bas and C codes.

OFF TOPIC
@dkl Is it possible that you upload somewhere a 64bit exe with data debugging (dwarf 2) based on one of my test code ? That will be very helpfull for studying debugging under 64bit version.

Code: Select all

'C:\TEMP\dev\freebasic\fbc -v  -g -s console -gen gcc  -Wc -gdwarf-2 -R -RR "test.bas"
'FreeBASIC Compiler - Version 0.91.0 (11-10-2013) for win32
'Copyright (C) 2004-2013 The FreeBASIC development team.
'standalone
'target:       win32, 486, 32bit
'compiling:    test.bas -o test.c (main module)
'compiling C:  C:\TEMP\dev\freebasic\bin\win32\gcc.exe -m32 -march=i486 -S -nostdlib -nostdinc -Wall -Wno-unused-label -Wno-unused-function -Wno-unused-variable -Wno-unused-but-set-variable -Wno-main -Werror-implicit-function-declaration -O0 -fno-strict-aliasing -frounding-math -fno-math-errno -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -g -masm=intel "test.c" -o "test.asm" -gdwarf-2 
'test.bas:16:1: error: size of array '__$fb_structsizecheck' is negative
'compiling C failed: 'C:\TEMP\dev\freebasic\bin\win32\gcc.exe' terminated with exit code 1

Code: Select all

type tbitfield
    flag_0 : 1 as byte
    flag_1 : 1 as integer
    flag_2 : 1 as integer
    flag_3 : 1 as integer
    flag_4 : 1 as integer
    flag_5 : 1 as integer
    flag_6 : 1 as integer
    flag_7 : 1 as integer
    flag_8 : 1 as integer
    flag_9 : 1 as integer
    inter as integer
    flag_A : 3 as byte
end Type

Dim vbf As tbitfield
vbf.flag_1=1:vbf.flag_9=1:vbf.inter=12345:vbf.flag_A=5

C code generated

Code: Select all

struct $9TBITFIELD {
	int8 FLAG_0;
	int32 FLAG_8;
	int32 INTER;
	int8 FLAG_A;
};
#define __FB_STATIC_ASSERT( expr ) extern int __$fb_structsizecheck[(expr) ? 1 : -1]
#line 16 "test.bas"
__FB_STATIC_ASSERT( sizeof( struct $9TBITFIELD ) == 12 );

[counting_pine]

From a quick test (in -gen gas, assigning values to the flags and checking the memory), it looks like FB is combining Flag_0 and Flag_1..Flag_9 all into one Integer, while the C code it emits is splitting them up into an Byte and an Integer.

[SARG]

@counting_pine

I had already noticed the way the bitfield is managed.
But as the assert is triggered there may be an issue.

On the other hand the type I use is only in the purpose of test and not really a normal way to do....

[counting_pine]

There is an issue: the Assert is triggered because internally FB thinks the two fields are together and gives a size of 12, while the C code it generates separates the fields, and so the size there is 16.
I can't test at the minute, but it might be interesting to see what C code is generated when the fields are accessed.

[marcov]

From a quick test (in -gen gas, assigning values to the flags and checking the memory), it looks like FB is combining Flag_0 and Flag_1..Flag_9 all into one Integer, while the C code it emits is splitting them up into an Byte and an Integer.

(if you wonder why, my guess is that must be searched in endianess. Flags are differently arranged in memory, and accessing as byte or int matters)

[dkl]

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]

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.