-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Closed
Labels
area-vmUse area-vm for VM related issues, including code coverage, and the AOT and JIT backends.Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.library-ffi
Milestone
Description
Amazingly, it turns out that some Win32 structs are packed. (In my case, OPENFILENAMEW
). I don't know how many Win32 structs are packed, but the lack of struct packing makes it difficult to interface with Win32.
Example:
class OpenFileNameW extends Struct<OpenFileNameW> {
@Int32()
int lStructSize; // DWORD lStructSize;
@IntPtr()
int hwndOwner; // HWND hwndOwner;
@IntPtr()
int hInstance; // HINSTANCE hInstance;
Pointer<Uint16 /* CWSTR */ > lpstrFilter; // LPCWSTR lpstrFilter;
Pointer<Uint16 /* WSTR */ > lpstrCustomFilter; // LPWSTR lpstrCustomFilter;
@Int32()
int nMaxCustFilter; // DWORD nMaxCustFilter;
@Int32()
int nFilterIndex; // DWORD nFilterIndex;
Pointer<Uint16 /* WSTR */ > lpstrFile; // LPWSTR lpstrFile;
@Int32()
int nMaxFile; // DWORD nMaxFile;
Pointer<Uint16 /* WSTR */ > lpstrFileTitle; // LPWSTR lpstrFileTitle;
@Int32()
int nMaxFileTitle; // DWORD nMaxFileTitle;
Pointer<Uint16 /* CWSTR */ > lpstrInitialDir; // LPCWSTR lpstrInitialDir;
Pointer<Uint16 /* CWSTR */ > lpstrTitle; // LPCWSTR lpstrTitle;
@Int32()
int Flags; // DWORD Flags;
@Int16()
int nFileOffset; // WORD nFileOffset;
@Int16()
int nFileExtension; // WORD nFileExtension;
Pointer<Uint16 /* CWSTR */ > lpstrDefExt; // LPCWSTR lpstrDefExt;
@IntPtr()
int lCustData; // LPARAM lCustData;
Pointer<Void /* OFNHOOKPROC */ > lpfnHook; // LPOFNHOOKPROC lpfnHook;
Pointer<Uint16 /* CWSTR */ > lpTemplateName; // LPCWSTR lpTemplateName;
Pointer<Void /* EditMenu */ > lpEditInfo; // LPEDITMENU lpEditInfo;
Pointer<CString> lpstrPrompt; // LPCSTR lpstrPrompt;
Pointer<Void> pvReserved; // void *pvReserved;
@Int32()
int dwReserved; // DWORD dwReserved;
@Int32()
int FlagsEx; // DWORD FlagsEx;
}
sizeOf returns 168, but in Godbolt with the actual comdlg headers, the result is 152. https://godbolt.org/z/6Tyt3h
Metadata
Metadata
Assignees
Labels
area-vmUse area-vm for VM related issues, including code coverage, and the AOT and JIT backends.Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.library-ffi
Type
Projects
Relationships
Development
Select code repository
Activity
sjindel-google commentedon Sep 1, 2019
I don't see any indication in the commdlg.h that the struct is not using the normal alignment rules.
However, I did notice that some of the fields are conditionally included:
Are you sure you were correct to include these?
ds84182 commentedon Sep 1, 2019
Ah sorry, my struct definition is incorrect, since the one in Win32 API docs don't match up. Thanks Microsoft. https://docs.microsoft.com/en-us/windows/win32/api/commdlg/ns-commdlg-openfilenamew
commdlg does include
pshpack1.h
which turns on byte packing on 32-bit targets, so 32-bit platforms are affected by it. https://godbolt.org/z/JAHfQ7dcharkes commentedon Oct 25, 2019
@ds84182 did using the correct struct definition solve your problem?
dcharkes commentedon Oct 25, 2019
For reference: most compilers support a form of struct packing with pragmas or attributes.
If we want to support those pragma's or attributes, then we should also think about supporting alignment attributes/pragmas on individual members of structs.
cc @mkustermann
ds84182 commentedon Oct 27, 2019
@dcharkes Yep, it solved my problem on 64-bit Windows, but I know on 32-bit Windows it'll be an issue still. I can't find any open-source C libraries that use packed structs via github search right now,
dcharkes commentedon Oct 28, 2019
@ds84182 Do you need packed structs in Win32, or is it just that the struct is different on Win32? You could work around the struct being different on 64 and 32 bit Windows by introducing a second struct just for win32, and duplicating the function signatures that use that struct. If that is really really un-ergonomic, that gives us a reason to address #35768.
dcharkes commentedon Oct 28, 2019
Adding support for packed structs should be relatively easy. (1) We need to specify a way that a struct is packed, and (2) our offset and size calculations need to take this into account.
Possible syntax
However, we might want to add
.valueUnaligned
and.valueUnaligned =
to support unaligned reads and writes. Otherwise these packed structs might be unusable on 32-bit arm.timsneath commentedon Aug 4, 2020
An example of a packed struct that I've just hit on Win32 is TASKDIALOGCONFIG, and I'm blocked because of this issue.
Per the declaration in CommCtrl.h:
The
sizeOf()
for this struct is ultimately 16 bytes too long because of alignment issues.[vm/ffi] Add class to `vm:ffi:struct-fields` pragma
12 remaining items