Support pragma pack
nanonyme opened this issue · 7 comments
Hi,
I have this problem that I am using CFFI in ABI mode and have a header using MSVC pragma pack convention.
Following is an example struct that results in different size with 64bit Python on Windows depending on whether pack is enabled or not. (36 bytes with packing, 40 bytes without packing)
import cffi
ffi = cffi.FFI()
ffi.cdef("""
typedef enum { FOO, BAR } Test;
typedef struct {
size_t mysize;
Test someenum;
uint64_t x;
uint64_t y;
uint64_t z;
} Foo;""", packed=True)
assert ffi.sizeof("Foo") == 36
what CFFI does is it wipes the pragma pack declaration out completely. I may be able to workaround the issue as per above by using packed=True, however, CFFI functionality of silently generating invalid structs in ABI mode without any warning or error seems scary to me. Ideally it would be great if CFFI handled scoped packing correctly but it seems minimal scope would be at least erroring out if you have packing requested in cdef but packed==False.
pragma pack spec contained is described in https://learn.microsoft.com/en-us/cpp/preprocessor/pack?view=msvc-170
what CFFI does is it wipes the pragma pack declaration out completely.
I'm not sure I understand what you mean. CFFI did not "wipe out" any pragma pack declaration in your example, as there is none to start with. There is only the "packed" keyword argument you can give to cdef() or not, and doing so changes the packing of structures declared inside that cdef.
CFFI functionality of silently generating invalid structs in ABI mode without any warning or error seems scary to me.
That's what the API mode is for. There is nothing we can do in ABI mode to prevent cdefs from silently generating invalid structs if the code or the "packed" argument given to cdef() are bogus...
As a wild guess, maybe you used some third-party tool to generate the source code you pass to the cdef(), and that tool ignored the "#pragma"? In that case, the problem is in the third-party tool and not in CFFI.
I am referring to this commit as for the "wipe out" part 6da6407. This code block was clearly being triggered by the real code which did have pragma pack, above is a synthetic example to generate a struct where packing actually matters since I don't at the moment have access to the original C API header.
Simply removing that "ignore pragmas" is not enough (nor did I really expect it to do so) to fix the generation of the struct but it would have shown that the C API header had code that CFFI cannot handle in ABI mode. (rather than silently generating incorrect data structure)
I can try to obtain the pragma pack declarations from original header for Friday. I thought I remembered the header well enough by heart to create a full-fledged example but had forgotten the exact pragma pack lines involved so I left them out.
You're right, CFFI should issue a warning instead of silently ignoring all #pragma... Even better, of course, if CFFI would recognize and parse the #pragma pack like MSVC does. At one point in time, the issue was that if you didn't install the latest version of pycparser then the pragma lines were ignored by pycparser itself. But that is already many years ago, so it should be fine now.
The warning sounds great. We're going to discuss in team whether or not we actually want to continue using pragma pack for this case in the future and we can probably use the parameter as a workaround for now if we do. (the only downside seems to be that pragma pack allows having packed and not packed structs in same header whereas parameter has full cdef scope; we can probably live with that) But it's important when things fail that there is proper context in logs.