Fix the layout of bitfields
Francois Gouget
fgouget at free.fr
Thu Aug 19 11:40:13 CDT 2004
gcc and msvc deal with bitfields very differently. For instance,
assuming a packing of 1, let's consider:
typedef struct
{
int field1:1;
int field2:1;
int field3;
} almostempty;
MSVC will generate an 8 bytes structure: 4 bytes for the first int (2
bits of which are actually used) and 4 bytes for field3.
However gcc will generate a 5 bytes structure: 1 byte for the first two
fields (since we only use 2 bits they fit into a single byte), and 4
bytes, packed, for field3.
Which compiler is wrong? Well, neither. Paraphrasing Bjarne Stroustrup's
"The C++ Programming Language" reference book:
* the layout of bitfields is implementation dependent
* bitfields may be allocated left to right or right to left depending
on implementation
* individual bitfields may or may not overlap word boundaries depending
on implementation
* the alignment of bitfields is implementation dependent
* '&' is not allowed on bitfields. Pointers and references to bitfields
don't exist.
* a nameless 0 bit field can be used to force the next bitfield to be
allocated on a word boundary
IOW, if you use bitfields, forget about portability!
To better understand the problems, check out the attached file. It
compiles with MSVC and the type size and field offsets will match the
expected values. However if you compile it with gcc you will get
discrepencies:
$ gcc -o bitfield -I ~/wine/include bitfield.c
$ ./bitfield
sizeof(singlebyte) == 1, expected 1
sizeof(byteoverflow) == 2, expected 3
sizeof(sizechange) == 1, expected 5
sizeof(almostempty) == 5, expected 8
offsetof(almostempty) == 1, expected 4
sizeof(packing1) == 5, expected 5
offsetof(packing1) == 1, expected 1
sizeof(packing4) == 8, expected 8
offsetof(packing4) == 4, expected 4
The fix is to use the nameless 0 bit fields mentionned by Bjarne
Stroustrup. Uncomment the ':0' lines in the attached file, recompile
with gcc and things will work. Note that this works with MSVC too.
This bitfield issue impacts CABINETSTATE, SHELLFLAGSTATE and SHELLSTATE.
This patch fixes this bitfield layout issue, as well as a shlobj.h
packing problem.
It also modifies winapi_test so it computes the bitfield layout as MSVC
does it since that's what we try to achieve, ands adds support for the
nameless 0 bit fields. It also removes testing the offset of bitfields
since MSVC refuses to take the address of a bitfield in accordance with
what's stated in "The C++ Programming Language". This way running
./tools/winapi/winapi_test will generate tests that compile with MSVC
(which is the goal after all).
Finally the patch adds some support for DECLSPEC_ALIGN().
Changelog:
* include/shlobj.h
dlls/shell32/tests/generated.c
tools/winapi/c_parser.pm
tools/winapi/c_type.pm
tools/winapi/winapi_test
shlobj.h: Fix packing bug.
shlobj.h: Fix the declaration of bitfields so their layout matches
what MSVC generates.
Modify winapi_test to compute the bitfields size/alignment like MSVC
does.
This moves the processing of the bitfields and arrays from
winapi_test to to c_type.pm and adds a 'find_count' callback to deal
with macros in the array size.
winapi_test: Add support for unnamed 0 bit bitfields so we support
the updated Wine headers.
winapi_test: Add some support for DECLSPEC_ALIGN().
winapi_test: Don't test the offset of bitfields since MSVC refuses to
take their address.
Update the impacted generated.c files.
Index: include/shlobj.h
===================================================================
RCS file: /var/cvs/wine/include/shlobj.h,v
retrieving revision 1.89
diff -u -r1.89 shlobj.h
--- include/shlobj.h 12 Aug 2004 03:33:30 -0000 1.89
+++ include/shlobj.h 18 Aug 2004 18:15:29 -0000
@@ -28,6 +28,7 @@
extern "C" {
#endif /* defined(__cplusplus) */
+/* Except for specific structs, this header is byte packed */
#include <pshpack1.h>
#include <shtypes.h>
@@ -210,6 +211,8 @@
*/
typedef INT (CALLBACK *BFFCALLBACK)(HWND hwnd, UINT uMsg, LPARAM lParam, LPARAM lpData);
+#include <pshpack8.h>
+
typedef struct tagBROWSEINFOA {
HWND hwndOwner;
LPCITEMIDLIST pidlRoot;
@@ -236,6 +239,8 @@
#define PBROWSEINFO WINELIB_NAME_AW(PBROWSEINFO)
#define LPBROWSEINFO WINELIB_NAME_AW(LPBROWSEINFO)
+#include <poppack.h>
+
/* Browsing for directory. */
#define BIF_RETURNONLYFSDIRS 0x0001
#define BIF_DONTGOBELOWDOMAIN 0x0002
@@ -299,11 +304,15 @@
#define SHDID_COMPUTER_AUDIO 19
#define SHDID_COMPUTER_SHAREDDOCS 20
+#include <pshpack8.h>
+
typedef struct _SHDESCRIPTIONID
{ DWORD dwDescriptionId;
CLSID clsid;
} SHDESCRIPTIONID, *LPSHDESCRIPTIONID;
+#include <poppack.h>
+
HRESULT WINAPI SHGetDataFromIDListA(LPSHELLFOLDER psf, LPCITEMIDLIST pidl, int nFormat, LPVOID pv, int cb);
HRESULT WINAPI SHGetDataFromIDListW(LPSHELLFOLDER psf, LPCITEMIDLIST pidl, int nFormat, LPVOID pv, int cb);
#define SHGetDataFromIDList WINELIB_NAME_AW(SHGetDataFromIDList)
@@ -375,6 +348,7 @@
BOOL fShowSuperHidden : 1;
BOOL fNoNetCrawling : 1;
+ DWORD :0; /* Required for proper binary layout with gcc */
DWORD dwWin95Unused;
UINT uWin95Unused;
LONG lParamSort;
@@ -385,6 +359,7 @@
BOOL fStartPanelOn: 1;
BOOL fShowStartPage: 1;
UINT fSpareFlags : 13;
+ UINT :0; /* Required for proper binary layout with gcc */
} SHELLSTATE, *LPSHELLSTATE;
/**********************************************************************
@@ -408,6 +383,7 @@
BOOL fHideIcons : 1;
UINT fRestFlags : 3;
+ UINT :0; /* Required for proper binary layout with gcc */
} SHELLFLAGSTATE, * LPSHELLFLAGSTATE;
VOID WINAPI SHGetSettings(LPSHELLFLAGSTATE lpsfs, DWORD dwMask);
@@ -888,6 +864,7 @@
BOOL fDontPrettyNames:1;
BOOL fAdminsCreateCommonGroups:1;
UINT fUnusedFlags:7;
+ UINT :0; /* Required for proper binary layout with gcc */
UINT fMenuEnumFilter;
} CABINETSTATE, *LPCABINETSTATE;
@@ -901,8 +916,6 @@
*/
VOID WINAPI PathGetShortPath(LPWSTR pszPath);
-#include <poppack.h>
-
/****************************************************************************
* Drag And Drop Routines
*/
@@ -1017,6 +1030,8 @@
BOOL WINAPI ILIsParent(LPCITEMIDLIST,LPCITEMIDLIST,BOOL);
BOOL WINAPI ILRemoveLastID(LPITEMIDLIST);
+#include <poppack.h>
+
#ifdef __cplusplus
} /* extern "C" */
#endif /* defined(__cplusplus) */
Index: dlls/shell32/tests/generated.c
===================================================================
RCS file: /var/cvs/wine/dlls/shell32/tests/generated.c,v
retrieving revision 1.6
diff -u -r1.6 generated.c
--- dlls/shell32/tests/generated.c 16 Aug 2004 19:46:09 -0000 1.6
+++ dlls/shell32/tests/generated.c 19 Aug 2004 14:15:36 -0000
@@ -791,10 +814,10 @@
static void test_pack_CABINETSTATE(void)
{
/* CABINETSTATE (pack 1) */
- TEST_TYPE(CABINETSTATE, 10, 1);
+ TEST_TYPE(CABINETSTATE, 12, 1);
TEST_FIELD(CABINETSTATE, WORD, cLength, 0, 2, 1);
TEST_FIELD(CABINETSTATE, WORD, nVersion, 2, 2, 1);
- TEST_FIELD(CABINETSTATE, UINT, fMenuEnumFilter, 6, 4, 1);
+ TEST_FIELD(CABINETSTATE, UINT, fMenuEnumFilter, 8, 4, 1);
}
static void test_pack_CIDA(void)
@@ -889,7 +912,7 @@
{
/* LPCABINETSTATE */
TEST_TYPE(LPCABINETSTATE, 4, 4);
- TEST_TYPE_POINTER(LPCABINETSTATE, 10, 1);
+ TEST_TYPE_POINTER(LPCABINETSTATE, 12, 1);
}
static void test_pack_LPDROPFILES(void)
@@ -771,13 +758,13 @@
static void test_pack_AUTO_SCROLL_DATA(void)
{
- /* AUTO_SCROLL_DATA (pack 4) */
- TEST_TYPE(AUTO_SCROLL_DATA, 48, 4);
- TEST_FIELD(AUTO_SCROLL_DATA, int, iNextSample, 0, 4, 4);
- TEST_FIELD(AUTO_SCROLL_DATA, DWORD, dwLastScroll, 4, 4, 4);
- TEST_FIELD(AUTO_SCROLL_DATA, BOOL, bFull, 8, 4, 4);
- TEST_FIELD(AUTO_SCROLL_DATA, POINT[NUM_POINTS], pts, 12, 24, 4);
- TEST_FIELD(AUTO_SCROLL_DATA, DWORD[NUM_POINTS], dwTimes, 36, 12, 4);
+ /* AUTO_SCROLL_DATA (pack 1) */
+ TEST_TYPE(AUTO_SCROLL_DATA, 48, 1);
+ TEST_FIELD(AUTO_SCROLL_DATA, int, iNextSample, 0, 4, 1);
+ TEST_FIELD(AUTO_SCROLL_DATA, DWORD, dwLastScroll, 4, 4, 1);
+ TEST_FIELD(AUTO_SCROLL_DATA, BOOL, bFull, 8, 4, 1);
+ TEST_FIELD(AUTO_SCROLL_DATA, POINT[NUM_POINTS], pts, 12, 24, 1);
+ TEST_FIELD(AUTO_SCROLL_DATA, DWORD[NUM_POINTS], dwTimes, 36, 12, 1);
}
static void test_pack_BFFCALLBACK(void)
@@ -952,14 +975,14 @@
{
/* LPSHELLFLAGSTATE */
TEST_TYPE(LPSHELLFLAGSTATE, 4, 4);
- TEST_TYPE_POINTER(LPSHELLFLAGSTATE, 2, 1);
+ TEST_TYPE_POINTER(LPSHELLFLAGSTATE, 4, 1);
}
static void test_pack_LPSHELLSTATE(void)
{
/* LPSHELLSTATE */
TEST_TYPE(LPSHELLSTATE, 4, 4);
- TEST_TYPE_POINTER(LPSHELLSTATE, 29, 1);
+ TEST_TYPE_POINTER(LPSHELLSTATE, 32, 1);
}
static void test_pack_SHChangeDWORDAsIDList(void)
@@ -995,19 +1021,19 @@
static void test_pack_SHELLFLAGSTATE(void)
{
/* SHELLFLAGSTATE (pack 1) */
- TEST_TYPE(SHELLFLAGSTATE, 2, 1);
+ TEST_TYPE(SHELLFLAGSTATE, 4, 1);
}
static void test_pack_SHELLSTATE(void)
{
/* SHELLSTATE (pack 1) */
- TEST_TYPE(SHELLSTATE, 29, 1);
- TEST_FIELD(SHELLSTATE, DWORD, dwWin95Unused, 3, 4, 1);
- TEST_FIELD(SHELLSTATE, UINT, uWin95Unused, 7, 4, 1);
- TEST_FIELD(SHELLSTATE, LONG, lParamSort, 11, 4, 1);
- TEST_FIELD(SHELLSTATE, int, iSortDirection, 15, 4, 1);
- TEST_FIELD(SHELLSTATE, UINT, version, 19, 4, 1);
- TEST_FIELD(SHELLSTATE, UINT, uNotUsed, 23, 4, 1);
+ TEST_TYPE(SHELLSTATE, 32, 1);
+ TEST_FIELD(SHELLSTATE, DWORD, dwWin95Unused, 4, 4, 1);
+ TEST_FIELD(SHELLSTATE, UINT, uWin95Unused, 8, 4, 1);
+ TEST_FIELD(SHELLSTATE, LONG, lParamSort, 12, 4, 1);
+ TEST_FIELD(SHELLSTATE, int, iSortDirection, 16, 4, 1);
+ TEST_FIELD(SHELLSTATE, UINT, version, 20, 4, 1);
+ TEST_FIELD(SHELLSTATE, UINT, uNotUsed, 24, 4, 1);
}
static void test_pack_SHELLVIEWID(void)
Index: tools/winapi/c_parser.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi/c_parser.pm,v
retrieving revision 1.11
diff -u -r1.11 c_parser.pm
--- tools/winapi/c_parser.pm 12 Nov 2002 01:05:17 -0000 1.11
+++ tools/winapi/c_parser.pm 19 Aug 2004 12:03:48 -0000
@@ -1979,7 +1979,7 @@
}
$finished = 1;
- } elsif(s/^((?:enum\s+|struct\s+|union\s+)?\w+\b(?:\s*\*)*)\s*(\w+)\s*(\[.*?\]$|:\s*(\d+)$|\{)?//s) {
+ } elsif(s/^((?:enum\s+|struct\s+|union\s+)?\w+\b(?:\s+DECLSPEC_ALIGN\(.*?\)|\s*\*)*)\s*(\w+)\s*(\[.*?\]$|:\s*(\d+)$|\{)?//s) {
$type = "$sign$1";
$name = $2;
@@ -1998,6 +1998,12 @@
$type = $self->_format_c_type($type);
$finished = 1;
+ } elsif(s/^((?:enum\s+|struct\s+|union\s+)?\w+\b(?:\s*\*)*)\s*:\s*(\d+)$//s) {
+ $type = "$sign$1:$2";
+ $name = "";
+ $type = $self->_format_c_type($type);
+
+ $finished = 1;
} elsif(s/^((?:enum\s+|struct\s+|union\s+)?\w+\b(?:\s*\*)*\s*\((?:\s*CALLBACK|\s*NTAPI|\s*WINAPI)?(?:\s*\*)*)\s*(\w+)\s*(\)\s*\(.*?\))$//s) {
$type = $self->_format_c_type("$sign$1$3");
$name = $2;
Index: tools/winapi/c_type.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi/c_type.pm,v
retrieving revision 1.9
diff -u -r1.9 c_type.pm
--- tools/winapi/c_type.pm 4 May 2004 00:38:27 -0000 1.9
+++ tools/winapi/c_type.pm 19 Aug 2004 12:00:00 -0000
@@ -64,6 +64,17 @@
$$find_size = shift;
}
+########################################################################
+# set_find_count_callback
+#
+sub set_find_count_callback {
+ my $self = shift;
+
+ my $find_count = \${$self->{FIND_COUNT}};
+
+ $$find_count = shift;
+}
+
sub kind {
my $self = shift;
my $kind = \${$self->{KIND}};
@@ -236,6 +247,7 @@
my $find_align = \${$self->{FIND_ALIGN}};
my $find_kind = \${$self->{FIND_KIND}};
my $find_size = \${$self->{FIND_SIZE}};
+ my $find_count = \${$self->{FIND_COUNT}};
my $align = \${$self->{ALIGN}};
my $kind = \${$self->{KIND}};
@@ -251,70 +263,100 @@
my $max_field_align = 0;
my $offset = 0;
- my $offset_bits = 0;
+ my $bitfield_size = 0;
+ my $bitfield_bits = 0;
my $n = 0;
foreach my $field ($self->fields) {
my $type_name = $field->type_name;
- my $type_size = &$$find_size($type_name);
-
- my $base_type_name = $type_name;
- if ($base_type_name =~ s/^(.*?)\s*(?:\[\s*(.*?)\s*\]|:(\d+))?$/$1/) {
- my $count = $2;
- my $bits = $3;
- }
- my $base_size = &$$find_size($base_type_name);
- $$align = &$$find_align($base_type_name);
- if (defined($$align)) {
- $$align = $pack if $$align > $pack;
- $max_field_align = $$align if $$align > $max_field_align;
-
- if ($offset % $$align != 0) {
- $offset = (int($offset / $$align) + 1) * $$align;
- }
+ my $bits;
+ my $count;
+ if ($type_name =~ s/^(.*?)\s*(?:\[\s*(.*?)\s*\]|:(\d+))?$/$1/)
+ {
+ $count = $2;
+ $bits = $3;
}
-
- if ($$kind !~ /^(?:struct|union)$/) {
- $$kind = &$$find_kind($type_name) || "";
- }
-
- if (!defined($type_size)) {
- $$align = undef;
- $$size = undef;
- return;
- } elsif ($type_size >= 0) {
- if ($offset_bits) {
- $offset += $pack * int(($offset_bits + 8 * $pack - 1 ) / (8 * $pack));
- $offset_bits = 0;
- }
-
- $$$field_aligns[$n] = $$align;
- $$$field_base_sizes[$n] = $base_size;
- $$$field_offsets[$n] = $offset;
- $$$field_sizes[$n] = $type_size;
-
- $offset += $type_size;
- } else {
- $$$field_aligns[$n] = $$align;
- $$$field_base_sizes[$n] = $base_size;
- $$$field_offsets[$n] = $offset;
- $$$field_sizes[$n] = $type_size;
-
- $offset_bits += -$type_size;
- }
-
+ my $declspec_align;
+ if ($type_name =~ s/\s+DECLSPEC_ALIGN\((\d+)\)//)
+ {
+ $declspec_align=$1;
+ }
+ my $base_size = &$$find_size($type_name);
+ my $type_size=$base_size;
+ if (defined $count)
+ {
+ $count=&$$find_count($count) if ($count !~ /^\d+$/);
+ if (!defined $count)
+ {
+ $type_size=undef;
+ }
+ else
+ {
+ $type_size *= int($count);
+ }
+ }
+ if ($bitfield_size != 0)
+ {
+ if (($type_name eq "" and defined $bits and $bits == 0) or
+ (defined $type_size and $bitfield_size != $type_size) or
+ !defined $bits or
+ $bitfield_bits + $bits > 8 * $bitfield_size)
+ {
+ # This marks the end of the previous bitfield
+ $bitfield_size=0;
+ $bitfield_bits=0;
+ }
+ else
+ {
+ $bitfield_bits+=$bits;
+ $n++;
+ next;
+ }
+ }
+
+ $$align = &$$find_align($type_name);
+ $$align=$declspec_align if (defined $declspec_align);
+
+ if (defined $$align)
+ {
+ $$align = $pack if $$align > $pack;
+ $max_field_align = $$align if $$align > $max_field_align;
+
+ if ($offset % $$align != 0) {
+ $offset = (int($offset / $$align) + 1) * $$align;
+ }
+ }
+
+ if ($$kind !~ /^(?:struct|union)$/)
+ {
+ $$kind = &$$find_kind($type_name) || "";
+ }
+
+ if (!$type_size)
+ {
+ $$align = undef;
+ $$size = undef;
+ return;
+ }
+
+ $$$field_aligns[$n] = $$align;
+ $$$field_base_sizes[$n] = $base_size;
+ $$$field_offsets[$n] = $offset;
+ $$$field_sizes[$n] = $type_size;
+ $offset += $type_size;
+
+ if ($bits)
+ {
+ $bitfield_size=$type_size;
+ $bitfield_bits=$bits;
+ }
$n++;
}
$$align = $pack;
$$align = $max_field_align if $max_field_align < $pack;
- if ($offset_bits) {
- $offset += $pack * int(($offset_bits + 8 * $pack - 1 ) / (8 * $pack));
- $offset_bits = 0;
- }
-
$$size = $offset;
if ($$kind =~ /^(?:struct|union)$/) {
if ($$size % $$align != 0) {
Index: tools/winapi/winapi_test
===================================================================
RCS file: /var/cvs/wine/tools/winapi/winapi_test,v
retrieving revision 1.16
diff -u -r1.16 winapi_test
--- tools/winapi/winapi_test 16 Aug 2004 19:46:09 -0000 1.16
+++ tools/winapi/winapi_test 19 Aug 2004 12:07:50 -0000
@@ -177,13 +177,6 @@
local $_ = $type_name;
- my $count;
- my $bits;
- if (s/^(.*?)\s*(?:\[\s*(.*?)\s*\]|:(\d+))?$/$1/) {
- $count = $2;
- $bits = $3;
- }
-
my $align;
my $kind;
my $size;
@@ -313,21 +306,6 @@
$output->write("$type_name: type needn't be kludged\n");
}
- if (!defined($size)) {
- # $output->write("$type_name: can't find type\n");
- } elsif (defined($count)) {
- if ($count =~ /^\d+$/) {
- $size *= int($count);
- } elsif (defined(my $count2 = $defines{$count})) {
- $size *= int($count2);
- } else {
- $output->write("$type_name: can't parse type ('$_') ('$count')\n");
- $size = undef;
- }
- } elsif (defined($bits)) {
- $size = -$bits;
- }
-
return ($align, $kind, $size);
}
@@ -350,6 +328,11 @@
return $size;
}
+sub find_count {
+ my $count = shift;
+ return $defines{$count};
+}
+
foreach my $file (@files) {
$progress_current++;
@@ -426,6 +409,7 @@
$type->set_find_align_callback(\&find_align);
$type->set_find_kind_callback(\&find_kind);
$type->set_find_size_callback(\&find_size);
+ $type->set_find_count_callback(\&find_count);
my $pack = $packs[$#packs];
if (!defined($type->pack) && $type->kind =~ /^(?:struct|union)$/) {
@@ -728,6 +712,8 @@
my $field_align = $field->align;
next if $field_name eq "" || (defined($field_size) && $field_size < 0);
+ # We cannot take the address of a bitfield with MSVC
+ next if ($field_type_name =~ /:/);
if ($$optional_fields{$field_name}) {
# Nothing
--
Francois Gouget fgouget at free.fr http://fgouget.free.fr/
If it stinks, it's chemistry. If it moves, it's biology.
If it does not work, It's computer science.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bitfield.c
Type: text/x-csrc
Size: 1447 bytes
Desc: bitfield.c
Url : http://www.winehq.org/pipermail/wine-patches/attachments/20040819/7a730477/bitfield.c
More information about the wine-patches
mailing list