Re: [msvcrt] properly align struct _stati64's st_size
"Damjan Jovanovic" <damjan.jov(a)gmail.com> wrote:
In wine's struct _stati64, st_size has an offset of 20 bytes from the beginning of the struct, unlike mingw's and Window's 24. This breaks Java 1.4.x pretty badly, it dies on startup complaining about a truncated class file (#2953). Using #include <pshpack8.h> and #include <pshpop8.h> around the struct didn't help, so a padding field was used instead.
Does the approach used in winbase.h in declaration of WIN32_STREAM_ID to align the Size field help? #include <pshpack8.h> typedef struct _WIN32_STREAM_ID { DWORD dwStreamId; DWORD dwStreamAttributes; LARGE_INTEGER DECLSPEC_ALIGN(8) Size; DWORD dwStreamNameSize; WCHAR cStreamName[ANYSIZE_ARRAY]; } WIN32_STREAM_ID, *LPWIN32_STREAM_ID; #include <poppack.h>
+static void test_stat( void ) +{ + int offset = offsetof(struct _stati64, st_size); + ok(offset == 24, "struct _stati64's st_size is misaligned, got %d, expected 24\n", offset); +}
This should be a part of auto generated header tests. Have a look at dlls/*/tests/generated.c files. -- Dmitry.
On Saturday 03 February 2007 16:15:11 Dmitry Timoshkov wrote:
Does the approach used in winbase.h in declaration of WIN32_STREAM_ID to align the Size field help?
That should fix this particular struct but __int64 is used elsewhere inside and outside of Wine, so maybe the typedef for __int64 itself should get the alignment attribute? -Hans
On Saturday 03 February 2007 18:25:05 Hans Leidekker wrote:
inside and outside of Wine, so maybe the typedef for __int64 itself should get the alignment attribute?
To answer my own question: __int64 is not a typedef when compiling with GCC, but a preprocessor define. This way it is possible to use signed/unsigned modifiers as you can on MSVC because it's a builtin type there. We can't add an alignment attribute to the preprocessor define because that will result in compiler errors when the type is used in function declarations. So it seems like we're stuck with fixing individual structs in Wine. Winelib apps will still be affected though. -Hans
"Hans Leidekker" <hans(a)it.vu.nl> wrote:
Does the approach used in winbase.h in declaration of WIN32_STREAM_ID to align the Size field help?
That should fix this particular struct but __int64 is used elsewhere inside and outside of Wine, so maybe the typedef for __int64 itself should get the alignment attribute?
To make sure that Wine headers are free of alignment bugs we have dlls/*/tests/generated.c auto generated by tools/winapi. We just need to add msvcrt types to tests.dat. It looks like a lot of the CRT headers in PSDK (including sys/stat.h) set alignment manually to 8, for example direct.h has the following comment: #ifdef _MSC_VER /* * Currently, all MS C compilers for Win32 platforms default to 8 byte * alignment. */ #pragma pack(push,8) #endif /* _MSC_VER */ -- Dmitry.
On Sunday 04 February 2007 06:58:58 Dmitry Timoshkov wrote:
To make sure that Wine headers are free of alignment bugs we have dlls/*/tests/generated.c auto generated by tools/winapi. We just need to add msvcrt types to tests.dat.
Sure, that will help fixing Wine alignment bugs. But an MSVC source that makes assumptions about the alignment of __int64, when recompiled as a Winelib app with GCC could still prove to be very hard to debug. So it's important to find a way to make all 64 bit types align on 8 byte boundaries, like MinGW does as well. We have to do it selectively of course, because Unix libraries have different assumptions about the alignment of 64 bit types. -Hans
On 2/3/07, Dmitry Timoshkov <dmitry(a)codeweavers.com> wrote:
"Damjan Jovanovic" <damjan.jov(a)gmail.com> wrote:
In wine's struct _stati64, st_size has an offset of 20 bytes from the beginning of the struct, unlike mingw's and Window's 24. This breaks Java 1.4.x pretty badly, it dies on startup complaining about a truncated class file (#2953). Using #include <pshpack8.h> and #include <pshpop8.h> around the struct didn't help, so a padding field was used instead.
Does the approach used in winbase.h in declaration of WIN32_STREAM_ID to align the Size field help?
#include <pshpack8.h> typedef struct _WIN32_STREAM_ID { DWORD dwStreamId; DWORD dwStreamAttributes; LARGE_INTEGER DECLSPEC_ALIGN(8) Size; DWORD dwStreamNameSize; WCHAR cStreamName[ANYSIZE_ARRAY]; } WIN32_STREAM_ID, *LPWIN32_STREAM_ID; #include <poppack.h>
That doesn't help. I heard somewhere GCC up to version 4.1.12 or so didn't align some things properly (I'm using 4.0.3). Since mingw aligns it properly with the same declaration as wine and it uses GCC 3.4.4, maybe it's a GCC bug.
+static void test_stat( void ) +{ + int offset = offsetof(struct _stati64, st_size); + ok(offset == 24, "struct _stati64's st_size is misaligned, got %d, expected 24\n", offset); +}
This should be a part of auto generated header tests. Have a look at dlls/*/tests/generated.c files.
Thanks.
-- Dmitry.
Damjan
On 2/5/07, Damjan Jovanovic <damjan.jov(a)gmail.com> wrote:
On 2/3/07, Dmitry Timoshkov <dmitry(a)codeweavers.com> wrote:
"Damjan Jovanovic" <damjan.jov(a)gmail.com> wrote:
In wine's struct _stati64, st_size has an offset of 20 bytes from the beginning of the struct, unlike mingw's and Window's 24. This breaks Java 1.4.x pretty badly, it dies on startup complaining about a truncated class file (#2953). Using #include <pshpack8.h> and #include <pshpop8.h> around the struct didn't help, so a padding field was used instead.
Does the approach used in winbase.h in declaration of WIN32_STREAM_ID to align the Size field help?
#include <pshpack8.h> typedef struct _WIN32_STREAM_ID { DWORD dwStreamId; DWORD dwStreamAttributes; LARGE_INTEGER DECLSPEC_ALIGN(8) Size; DWORD dwStreamNameSize; WCHAR cStreamName[ANYSIZE_ARRAY]; } WIN32_STREAM_ID, *LPWIN32_STREAM_ID; #include <poppack.h>
That doesn't help.
Sorry, that (as in DECLSPEC_ALIGN(8)) DOES work, I didn't see it before. I'll send a patch that does that instead of using a padding field.
Dmitry.
Damjan
Damjan
participants (3)
-
Damjan Jovanovic -
Dmitry Timoshkov -
Hans Leidekker