gh-97588: Fix ctypes structs

[matthiasgoergens]

The existing code is quite a mess and doesn't correspond to what gcc nor clang nor msvc are doing.

This PR fixes all the issues below.

• Issue: ctypes: bit field data does not survive round trip #97588

• Issue: ctypes: bitfield lost data with union on linux platform #95496

• Issue: bitfield layout wrong in ctypes #84039

• Issue: [Linux] ctypes packs bitfields Incorrectly #73939

• Issue: ctypes mixed-types bitfield layout nonsensical; doesn't match compiler. #59324

• Issue: Incorrect struct definition with bitfields #86098

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @encukou for commit ba61051 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[encukou]

This is about as good as it'll be for 3.13. So there is a decision to make:

• Put this in, and iterate with bugfixes. This PR is an improvement, but if more of those come in 3.14, things might break twice for users that rely on workarounds.
• Delay to 3.14, and iterate with more freedom. I can fix this properly in 2024, if I'm allowed to spend dev-in-rez time on it.

I'm torn here. Does anyone want to play a more impartial judge? Or just chip in with an opinion? @zooba, @gpshead

---

For an idea about what could go into 3.14:

Things that are under-tested or missing, where fixes might break working code:

• unions
• Matching unsupported (non-PEP11) architectures & compilers
• handling types whose alignment != size
• GCC-style packed structs/bitfields (requires straddling bitfields, which needs a bigger refactoring)

Missing features that would IMO be needed before we can claim that ctypes can match a C struct in the common ABIs:

• support for zero-length bit fields, which to most compilers means “avoid packing a bitfield together with the previous one” (but ctypes uses zero as “not a bitfield”)

Things needed for better testability or implementability of the above:

• fields/types that are stored as PyLong_AsNativeBytes arguments, rather than struct codes
• better reflection of ctypes
• Hypothesis tests & fuzzing

---

For this PR: all buildbots fail on only:

• s390x (here for example) -- might be due to stricter tests; today I can probably either fix this or at least build confidence to skip the test.
• Stuff that's not ctypes-related: test_pyrepl, test_import and a few more pre-existing failures, test_cext, No space left on device.

[matthiasgoergens]

This stuff has been broken since forever, and even this PR has been slumbering for a long time.

So I would vote in favour of taking your time and fixing this properly, and merging it into whatever version of Python is the one currently being worked on when you are ready.

But I have no strong feelings either way.

[encukou]

!buildbot s390x SLES PR

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @encukou for commit bc1225b 🤖

The command will test the builders whose names match following regular expression: s390x SLES PR

The builders matched are:

• s390x SLES PR

[gvanrossum]

This stuff has been broken since forever, and even this PR has been slumbering for a long time.

So I would vote in favour of taking your time and fixing this properly, and merging it into whatever version of Python is the one currently being worked on when you are ready.

But I have no strong feelings either way.

This is a tough one. I'm leaning in the same direction -- this is substantial work, and if you merge now you may spend the entire beta period having to tweak it, and possibly the 3.14 release cycle regretting it.

If you had come with this two weeks ago I would have said "put it in", but now it's very close to beta 1.

You could talk to the RM for a 3rd opinion.

[encukou]

@gpshead, you asked me to look into this. Was that to try to get it in 3.13, or just to move it forward?

[gpshead]

I like the conclusions above that Matthias and Guido make: Save this for 3.14.

IIRC I asked you to look into it because it seemed hairy enough to understand the details of that I didn't imagine it would ever go forward without a committer dedicating time to understand it as you have. You've done a lot. Even if this went in for 3.13 I'd call it an improvement, but given more potential work to be done has been identified in the process, waiting for 3.14 isn't going to hurt anyone and will avoid repeated behavior-fix-transition pain.

[matthiasgoergens]

Btw, if you are taking your time to fix this properly, I would like to suggest migrating the intricate logic that calculates the offsets etc from the specification from C to Python.

That way it's much easier to understand and maintain, and this calculation only happens once per struct definition, so it shouldn't be in any inner loop and doesn't need to be C-level fast.

When I wrote my prototype that you kindly took over, I actually started sketching out my logic in Python before translating it into C.

[encukou]

Thank you for confirming! 3.14 it is.

Yes, rewriting this in Python is the way to go.

it seemed hairy enough to understand the details of that I didn't imagine it would ever go forward without a committer dedicating time to understand it

Yup. There are a few modules like that; I was diving into argparse when you invited me here :)

[encukou]

I'll merge this when I'll be available to fix any issues quickly: after PyCon US, or at the sprints if someone wants to pair up there.

[encukou]

Argh, I hit Enter while I was editing the commit message. Sorry for that :(
Should have been something like:

Structure layout, and especially bitfields, sometimes resulted in clearly
wrong behaviour like overlapping fields. This fixes the bug, and adds the
attribute `_layout_` to select either MSVC-like or GCC-like behavior.
This API may still change for 3.14 as we add more flexible struct packing
options.

Co-authored-by: Gregory P. Smith <[email protected]>
Co-authored-by: Petr Viktorin <[email protected]>

[zware]

FYI: this appears to have broken tests on x86 (32-bit) Linux, see gh-121938.