Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement struct bitfield layout rules according to GCC #1926

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

BCSharp
Copy link
Member

@BCSharp BCSharp commented Mar 6, 2025

This new layout rules only apply on POSIX. The rules for Clang are the same as for GCC (but different than the MSVC rules used by IronPython so far). On a few occasions, CPython produces different results, which frankly don't make sense to me and look like a bug in CPython: extra unnecessary padding bits, bitfield container unit not properly aligned, or even storage bits of two bitfields overlapping — the last case signals that there is a serious problem with the CPython's layout rules. More importantly, the layout that CPython produces is not the same as GCC/Clang's. In case of such a discrepancy, IronPython follows GCC, not CPython. After all, the purpose of ctypes is interoperability with C not CPython. To see the problematic cases, scan added tests for comment # bug in CPython.

What is still not supported:

  • _pack_ with bifields — requires more investigation of what the actual rules are
  • zero-width anonymous bitfields — these are not supported by CPython, but supported by C

On the bright side, since CPython doesn't support (or supports incorrectly) some of the cases, it just tells me that those problematic scenarios are not commonly being used in the wild (or they would have been reported to the CPython community already).

@slozier
Copy link
Contributor

slozier commented Mar 6, 2025

Just a quick note, looks like 3.14 has some fixes for the bit fields:

The layout of bit fields in Structure and Union now matches platform defaults (GCC/Clang or MSVC) more closely. In particular, fields no longer overlap. (Contributed by Matthias Görgens in gh-97702.)

After some quick tests it seems to match up with your is_cli branches.

@BCSharp
Copy link
Member Author

BCSharp commented Mar 8, 2025

Thanks for the pointer. I am updating the tests to verify the 3.14 behaviour. For what I can see:

  • 3.14 fixed all standard discrepancies I have identified between CPython and GCC. This means that IronPython and CPython 3.14 are in agreement in all cases that I have implemented.
  • The changes in 3.14 change how packed structs are being laid out, but they are still incorrect (wrt GCC/Clang), just in a different way than before. Not a big deal, because IPY does not support packed structs with bitfields at all. Time permitting though, this was something I wanted to implement as well, since I have this whole thing fresh in my memory. This is not simple however, because the rules are quite obscure and compilcated, something I see being acknowledged in gh-9702 as well. Particularly, there seems to be a difference between #pragma pack and __attribute__ ((packed)). I have included/corrected a couple of tests for packed structs, just for the record, but they are currently being skipped by IPY, and since CPython 3.14 is in alpha, this may change there too. Anyway, in terms of interoperability, I think it is a bad idea to try to use packed structs for communication between Python and C anyway (due to the fact that the actual layout depends on so many C attributes, pragmas, and compiler options), so somebody doing it is basically asking for trouble.
  • Zero-length anonymous bitfields are still not supported by both CPython 3.14a4 and IronPython.

@BCSharp
Copy link
Member Author

BCSharp commented Mar 8, 2025

  • The changes in 3.14 change how packed structs are being laid out, but they are still incorrect (wrt GCC/Clang), just in a different way than before.

I've been reading CPython's ctypes/_layout.py and it explains why the above is so: when _pack_ is used, CPython silently switches to the MSVC layout, even on non-Windows systems (simply because the GCC layout is not implemented?). The fact that it is not implemented/supported is by itself OK, but that the change is silent is worrisome. I think it is better to simply report an error (like IronPython currently does), and let the user to select the MSVC layout explicitly, something that was apparently added to CPython recently (attribute _layout_). Perhaps this will still change before the final release.

Anyway, _pack_ is for another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants