[PATCH 0/3] MR7909: include: Fix IMAGE_OS2_HEADER structure
This merge request contains fixes for the Windows New Executable structure IMAGE_OS2_HEADER defined in include file winnt.h and used by wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909
From: Pali Rohár <pali(a)kernel.org> On-disk NE structure contains at offset 38 field which contains the offset to the fastload/gangload area and at offset 3a field which contains the length of the fastload/gangload area. Fields ne_pretthunks and ne_psegrefbytes were used only for the in-memory structure of loaded NE modules on Windows 1.x and 2.x systems. IMAGE_OS2_HEADER describe the on-disk structure, not the loaded in-memory structure. Fix the IMAGE_OS2_HEADER structure and also its all incorrect usage. This was verified against the Windows 3.1 DDK file NEWEXE.INC and also against the Andrew Schulman's book Undocumented Windows. Both contains description of NE structures and uses ne_gang_start and ne_gang_length names. But for the consistency with other fields in IMAGE_OS2_HEADER, use just one underscore in names: ne_gangstart and ne_ganglength. To have include file winnt.h compatible with Windows NT SDK, define both old incorrect field name and new correct field name via union. Same trick is used in struct from the Undocumented Windows book (Chapter 5: KERNEL). Signed-off-by: Pali Rohár <pali(a)kernel.org> --- dlls/ntdll/tests/generated.c | 24 ++++++++++++------------ include/winnt.h | 18 ++++++++++++++++-- tools/sfnt2fon/sfnt2fon.c | 4 ++-- tools/winebuild/spec16.c | 8 ++++---- tools/winedump/ne.c | 4 ++-- 5 files changed, 36 insertions(+), 22 deletions(-) diff --git a/dlls/ntdll/tests/generated.c b/dlls/ntdll/tests/generated.c index 2c3b462011e..191d1974d72 100644 --- a/dlls/ntdll/tests/generated.c +++ b/dlls/ntdll/tests/generated.c @@ -1311,12 +1311,12 @@ static void test_pack_IMAGE_OS2_HEADER(void) TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_flagsothers, 1) TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_flagsothers, 1) TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_flagsothers, 55) - TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_pretthunks, 2) - TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_pretthunks, 2) - TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_pretthunks, 56) - TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_psegrefbytes, 2) - TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_psegrefbytes, 2) - TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_psegrefbytes, 58) + TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_gangstart, 2) + TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_gangstart, 2) + TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_gangstart, 56) + TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_ganglength, 2) + TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_ganglength, 2) + TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_ganglength, 58) TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_swaparea, 2) TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_swaparea, 2) TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_swaparea, 60) @@ -4739,12 +4739,12 @@ static void test_pack_IMAGE_OS2_HEADER(void) TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_flagsothers, 1) TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_flagsothers, 1) TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_flagsothers, 55) - TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_pretthunks, 2) - TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_pretthunks, 2) - TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_pretthunks, 56) - TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_psegrefbytes, 2) - TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_psegrefbytes, 2) - TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_psegrefbytes, 58) + TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_gangstart, 2) + TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_gangstart, 2) + TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_gangstart, 56) + TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_ganglength, 2) + TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_ganglength, 2) + TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_ganglength, 58) TEST_FIELD_SIZE (IMAGE_OS2_HEADER, ne_swaparea, 2) TEST_FIELD_ALIGN (IMAGE_OS2_HEADER, ne_swaparea, 2) TEST_FIELD_OFFSET(IMAGE_OS2_HEADER, ne_swaparea, 60) diff --git a/include/winnt.h b/include/winnt.h index 722d2c3a542..080044e6ad6 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -2629,8 +2629,22 @@ typedef struct WORD ne_cres; /* 34 # of resource segments */ BYTE ne_exetyp; /* 36 Flags indicating target OS */ BYTE ne_flagsothers; /* 37 Additional information flags */ - WORD ne_pretthunks; /* 38 Offset to return thunks */ - WORD ne_psegrefbytes; /* 3a Offset to segment ref. bytes */ + union { + /* ne_pretthunks is the incorrect name specified in Windows NT SDK. + Offset to return thunks was used only for the in-memory structure + of loaded NE modules on Windows 1.x and 2.x. On-disk structure always + contains offset to gangload area (if gangload area is present). */ + WORD ne_pretthunks; + WORD ne_gangstart; /* 38 Offset to gangload area */ + }; + union { + /* ne_psegrefbytes is the incorrect name specified in Windows NT SDK. + Offset to segment ref. bytes was used only for the in-memory structure + of loaded NE modules on Windows 1.x and 2.x. On-disk structure always + contains length of gangload area (if gangload area is present). */ + WORD ne_psegrefbytes; + WORD ne_ganglength; /* 3a Length of gangload area */ + }; WORD ne_swaparea; /* 3c Reserved by Microsoft */ WORD ne_expver; /* 3e Expected Windows version number */ } IMAGE_OS2_HEADER, *PIMAGE_OS2_HEADER; diff --git a/tools/sfnt2fon/sfnt2fon.c b/tools/sfnt2fon/sfnt2fon.c index bf31124aa25..24d19ed1dee 100644 --- a/tools/sfnt2fon/sfnt2fon.c +++ b/tools/sfnt2fon/sfnt2fon.c @@ -956,8 +956,8 @@ int main(int argc, char **argv) put_word( 0 ); /* ne_cres */ put_byte( 2 ); /* ne_exetyp = NE_OSFLAGS_WINDOWS */ put_byte( 0 ); /* ne_flagsothers */ - put_word( 0 ); /* ne_pretthunks */ - put_word( 0 ); /* ne_psegrefbytes */ + put_word( 0 ); /* ne_gangstart */ + put_word( 0 ); /* ne_ganglength */ put_word( 0 ); /* ne_swaparea */ put_word( 0x400 ); /* ne_expver */ diff --git a/tools/winebuild/spec16.c b/tools/winebuild/spec16.c index 630dc16b69d..15cba69f215 100644 --- a/tools/winebuild/spec16.c +++ b/tools/winebuild/spec16.c @@ -598,8 +598,8 @@ static void output_module16( DLLSPEC *spec ) output( "\t.short 0\n" ); /* ne_cres */ output( "\t.byte 0x02\n" ); /* ne_exetyp = NE_OSFLAGS_WINDOWS */ output( "\t.byte 0x08\n" ); /* ne_flagsothers = NE_AFLAGS_FASTLOAD */ - output( "\t.short 0\n" ); /* ne_pretthunks */ - output( "\t.short 0\n" ); /* ne_psegrefbytes */ + output( "\t.short 0\n" ); /* ne_gangstart */ + output( "\t.short 0\n" ); /* ne_ganglength */ output( "\t.short 0\n" ); /* ne_swaparea */ output( "\t.short 0\n" ); /* ne_expver */ @@ -915,8 +915,8 @@ void output_fake_module16( DLLSPEC *spec ) put_word( 0 ); /* ne_cres */ put_byte( 2 /*NE_OSFLAGS_WINDOWS*/ ); /* ne_exetyp */ put_byte( 8 /*NE_AFLAGS_FASTLOAD*/ ); /* ne_flagsothers */ - put_word( 0 ); /* ne_pretthunks */ - put_word( 0 ); /* ne_psegrefbytes */ + put_word( 0 ); /* ne_gangstart */ + put_word( 0 ); /* ne_ganglength */ put_word( 0 ); /* ne_swaparea */ put_word( 0 ); /* ne_expver */ diff --git a/tools/winedump/ne.c b/tools/winedump/ne.c index c27d7d6a9b7..e718ca3bf00 100644 --- a/tools/winedump/ne.c +++ b/tools/winedump/ne.c @@ -130,8 +130,8 @@ static void dump_ne_header( const IMAGE_OS2_HEADER *ne ) printf( "Non-resident table: %x\n", (UINT)ne->ne_nrestab ); printf( "Exe type: %x\n", ne->ne_exetyp ); printf( "Other flags: %x\n", ne->ne_flagsothers ); - printf( "Fast load area: %x-%x\n", ne->ne_pretthunks << ne->ne_align, - (ne->ne_pretthunks+ne->ne_psegrefbytes) << ne->ne_align ); + printf( "Fast load area: %x-%x\n", ne->ne_gangstart << ne->ne_align, + (ne->ne_gangstart+ne->ne_ganglength) << ne->ne_align ); printf( "Expected version: %d.%d\n", HIBYTE(ne->ne_expver), LOBYTE(ne->ne_expver) ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7909
From: Pali Rohár <pali(a)kernel.org> Signed-off-by: Pali Rohár <pali(a)kernel.org> --- include/winnt.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/winnt.h b/include/winnt.h index 080044e6ad6..bd3cd6dbb1f 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -2645,7 +2645,7 @@ typedef struct WORD ne_psegrefbytes; WORD ne_ganglength; /* 3a Length of gangload area */ }; - WORD ne_swaparea; /* 3c Reserved by Microsoft */ + WORD ne_swaparea; /* 3c Minimum code swap area size */ WORD ne_expver; /* 3e Expected Windows version number */ } IMAGE_OS2_HEADER, *PIMAGE_OS2_HEADER; #include <poppack.h> -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7909
From: Pali Rohár <pali(a)kernel.org> Field ne_ver contains the NE linker major version. This value represents the layout of the NE binary, which means which fields are available in the IMAGE_OS2_HEADER structure. And hence it specifies the version of NE binary. NE binaries with version less than 4 have last field at offset 2c. NE binaries with version less than 5 have last field at offset 32. And all non-Windows NE binaries have the maximal possible field at offset 3a. After the last field of the NE header of particular version is the NE segment table. This was verified against more NE ver4 binaries. Sometimes there is a padding between end of NE header and NE segment table but in more cases there is not. And so the IMAGE_OS2_HEADER's ne_cres field for NE ver4 binaries points to the segment table. Signed-off-by: Pali Rohár <pali(a)kernel.org> --- include/winnt.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/winnt.h b/include/winnt.h index bd3cd6dbb1f..960d644eb11 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -2624,8 +2624,14 @@ typedef struct WORD ne_modtab; /* 28 Offset to module reference table */ WORD ne_imptab; /* 2a Offset to imported name table */ DWORD ne_nrestab; /* 2c Offset to nonresident-name table */ + + /* end of the ne_ver < 4 structure */ + WORD ne_cmovent; /* 30 # of movable entry points */ WORD ne_align; /* 32 Logical sector alignment shift count */ + + /* end of the ne_ver < 5 structure */ + WORD ne_cres; /* 34 # of resource segments */ BYTE ne_exetyp; /* 36 Flags indicating target OS */ BYTE ne_flagsothers; /* 37 Additional information flags */ @@ -2645,6 +2651,9 @@ typedef struct WORD ne_psegrefbytes; WORD ne_ganglength; /* 3a Length of gangload area */ }; + + /* end of the ne_ver >= 5 structure for non-Windows modules */ + WORD ne_swaparea; /* 3c Minimum code swap area size */ WORD ne_expver; /* 3e Expected Windows version number */ } IMAGE_OS2_HEADER, *PIMAGE_OS2_HEADER; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7909
Nikolay Sivov (@nsivov) commented about include/winnt.h:
+ WORD ne_gangstart; /* 38 Offset to gangload area */ + }; + union { + /* ne_psegrefbytes is the incorrect name specified in Windows NT SDK. + Offset to segment ref. bytes was used only for the in-memory structure + of loaded NE modules on Windows 1.x and 2.x. On-disk structure always + contains length of gangload area (if gangload area is present). */ + WORD ne_psegrefbytes; + WORD ne_ganglength; /* 3a Length of gangload area */ + }; + + /* end of the ne_ver >= 5 structure for non-Windows modules */ + + WORD ne_swaparea; /* 3c Minimum code swap area size */ WORD ne_expver; /* 3e Expected Windows version number */ } IMAGE_OS2_HEADER, *PIMAGE_OS2_HEADER; This is a public type, so whatever is in current SDK is basically "correct". Do you mean that it differs from some older definition?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_101941
On Sat Apr 26 15:27:58 2025 +0000, Nikolay Sivov wrote:
This is a public type, so whatever is in current SDK is basically "correct". Do you mean that it differs from some older definition? Yes, I wrote it in the commit message.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_101943
There doesn't seem to be any point to these changes. They are not compatible with the PSDK, and they are not fixing anything. If the goal is to document the subtleties of the NE header, there are better places to do this than in the Wine code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_101958
On Sun Apr 27 15:44:38 2025 +0000, Alexandre Julliard wrote:
There doesn't seem to be any point to these changes. They are not compatible with the PSDK, and they are not fixing anything. If the goal is to document the subtleties of the NE header, there are better places to do this than in the Wine code. Well, for the fixing issues in NE loader which refuse to load some binaries, I first needed to understand the code and fix the wrong comments/documents. With wrong documentation it is hard to fix issues.
So I split changes into more parts (as requested in other merge request) and this one is updating the fields and documentation for them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_101980
On Sun Apr 27 15:44:38 2025 +0000, Pali Rohár wrote:
Well, for the fixing issues in NE loader which refuse to load some binaries, I first needed to understand the code and fix the wrong comments/documents. With wrong documentation it is hard to fix issues. So I split changes into more parts (as requested in other merge request) and this one is updating the fields and documentation for them. I understand that you were frustrated by inaccuracy in Wine's winnt.h. We do have a few inaccuracies here and there in Wine headers.
However I must note that Wine headers are not intended to be a reliable source of Win32 documentation. If you accompany it with a patch that actially fix issues in NE loader *and* relies on all the header changes then that would definitely be welcome. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_101995
However I must note that Wine headers are not intended to be a reliable source of Win32 documentation. If you accompany it with a patch that actially fix issues in NE loader and relies on all the header changes then that would definitely be welcome.
Actually, code fixes would be welcome (provided they fix an actual app), but the header should not be changed. There's no reason to deviate from the PSDK here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_102012
On Mon Apr 28 08:44:36 2025 +0000, Alexandre Julliard wrote:
However I must note that Wine headers are not intended to be a reliable source of Win32 documentation. If you accompany it with a patch that actially fix issues in NE loader and relies on all the header changes then that would definitely be welcome. Actually, code fixes would be welcome (provided they fix an actual app), but the header should not be changed. There's no reason to deviate from the PSDK here. I stand corrected, so the existing definitions are consistent with PSDK.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_102013
On Mon Apr 28 10:22:59 2025 +0000, Jinoh Kang wrote:
I stand corrected, so the existing definitions are consistent with PSDK. Now I looked into PSDK and it also have "fixed" the comment for ne_swaparea field, like in this my change. So I do not understand what is wrong with fixing documentation.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_102463
On Sat May 3 09:52:38 2025 +0000, Pali Rohár wrote:
Now I looked into PSDK and it also have "fixed" the comment for ne_swaparea field, like in this my change. So I do not understand what is wrong with fixing documentation. "PSDK" refers to the official Windows SDK (formerly Platform SDK) installed with Visual Studio or EWDK, not one of those unofficial or leaked ones on the Internet.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909#note_102465
This merge request was closed by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7909
participants (4)
-
Alexandre Julliard (＠julliard) -
Jinoh Kang (＠iamahuman)
-
Nikolay Sivov (＠nsivov)
-
Pali Rohár