Re: [msvcrt] properly align struct _stati64's st_size

[Dmitry Timoshkov]

"Damjan Jovanovic"  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.

[msvcrt] properly align struct _stati64's st_size

[Hans Leidekker]

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

[msvcrt] properly align struct _stati64's st_size

[Hans Leidekker]

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

[msvcrt] properly align struct _stati64's st_size

[Dmitry Timoshkov]

"Hans Leidekker"  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 */

[msvcrt] properly align struct _stati64's st_size

[Hans Leidekker]

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

[msvcrt] properly align struct _stati64's st_size

[Damjan Jovanovic]

On 2/3/07, Dmitry Timoshkov  wrote:
> "Damjan Jovanovic"  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

[msvcrt] properly align struct _stati64's st_size

[Damjan Jovanovic]

On 2/5/07, Damjan Jovanovic  wrote:
> On 2/3/07, Dmitry Timoshkov  wrote:
> > "Damjan Jovanovic"  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