GeezerVison

All commit authors signed the Contributor License Agreement.

This failure is a 10 minute timeout in test_subprocess's test_check on the macOS Intel runner, unrelated to this change (which only touches _ctypes/callbacks.c).

I couldn't write a test that fails before this fix and passes after it on any CI architecture. The existing test_byte/test_short/test_int callback tests in test_callbacks.py already round-trip narrow return values through a real closure call, and they pass with or without this patch on x86-64 and arm64 — those backends already read back only the bytes they need, so the missing widening never surfaces there. The corruption only shows up on an ABI where the caller trusts the full register width, which in this report is s390x, not one of the PR-gating runners, and I don't have that hardware to verify against. I'm glad to add a value-check test for the narrow int types as basic coverage if you'd still like one, though it won't reproduce the bug itself on x86-64/arm64.

I'm confused; how did you ensure that your fix works if you can't test on s390x? This feels a lot like you fed an issue into Codex/Claude/whatever, and you just sent the PR to us. Please don't try to fix issues that you can't even reproduce.

It was AI-assisted, yes. I reviewed and drove the actual fix, and I get why the s390x question is fair.

Here's what I actually checked: the widening logic follows libffi's own contract for ffi_prep_closure_loc — narrow integer types have to fill a full ffi_arg-sized register, sign- or zero-extended. The old code just shifted a pointer for big-endian and never actually widened anything. I ran the full test_ctypes suite (628 tests) before and after on arm64, no regressions, and I also tried to reproduce the bug directly through register-priming plus a reinterpreted function pointer — it doesn't reproduce on arm64 or x86-64, because those libffi backends only read back the bytes they actually need. That's also why no portable regression test is possible on any architecture CI runs; s390x is the only one that trusts the full register.

If someone with s390x access can confirm the fix before/after, that's better evidence than anything I can produce here — happy to wait on that if you'd rather not merge on the reasoning alone.

@miladfarca can you help testing this fix or find someone with the access to a s390x machine?

I really don't appreciate you copy-pasting AI output as a response. Please use your own words.

I'm going to refrain from reviewing this until someone (preferably from the LLVM team) can confirm that this is the right solution for s390x. I don't completely understand this fix, and given that you can't even reproduce the bug locally, I'm not so sure that you do either.

I have created this small test case to test this PR, it fails without the patch and passes with it on s390x, I cannot run the entire test suite as I don't have the proper environment:

diff --git a/Lib/test/test_ctypes/test_callbacks.py b/Lib/test/test_ctypes/test_callbacks.py
index 6c7c2e52707..204eb6da301 100644
--- a/Lib/test/test_ctypes/test_callbacks.py
+++ b/Lib/test/test_ctypes/test_callbacks.py
@@ -328,6 +328,14 @@ def func():
                              f"of ctypes callback function {func!r}")
             self.assertIsNone(cm.unraisable.object)
 
+    def test_narrow_int_return_widened(self):
+        @CFUNCTYPE(c_int)
+        def cb():
+            return -1
+
+        addr = ctypes.cast(cb, ctypes.c_void_p).value
+        wide = CFUNCTYPE(c_longlong)(addr)
+        self.assertEqual(wide(), -1)
 
 if __name__ == '__main__':
     unittest.main()

Thanks for testing this on s390x! I added your test in ebf7568. I put a 64-bit skip on it, because on 32-bit targets a long long return uses two registers and the callback only sets one. Here on arm64 macOS it passes with and without the fix, so s390x is still the only place where it catches the bug.

I limited the test to s390x in 9338911. It failed on x86-64 CI even with the fix: libffi zero-extends a 32-bit result there, and the x86-64 ABI leaves the upper bits undefined, so reading an int result as long long is not reliable on that platform. On s390x the ABI requires the callee to extend the result, so the test is valid there and the s390x buildbots will run it.

Thanks @miladfarca for confirming. I think we do have a few s390x buildbots lying around too. I'll try to run them once the review comments are addressed.

@lazerg In the future, please only work on issues that you can actually verify, and please don't use LLM output to reply to comments. I get that AI is good at solving issues like this, but it ultimately just shifts the cognitive effort to the maintainer/reviewer.

Yes, understood.

!buildbot s390x

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit 5693c85 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F157045%2Fmerge

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

The builders matched are:

• s390x CentOS10 Clang Installed PR
• s390x Fedora Stable LTO PR
• s390x Fedora Stable Clang Installed PR
• s390x Fedora Rawhide NoGIL refleaks PR
• s390x Fedora Stable LTO + PGO PR
• s390x Fedora Rawhide LTO + PGO PR
• s390x CentOS10 LTO PR
• s390x Fedora Rawhide Clang PR
• s390x RHEL9 LTO PR
• s390x Fedora Stable PR
• s390x CentOS10 Clang PR
• s390x Fedora Rawhide Clang Installed PR
• s390x RHEL9 PR
• s390x CentOS10 Refleaks PR
• s390x RHEL9 LTO + PGO PR
• s390x CentOS10 PR
• s390x Fedora Stable Refleaks PR
• s390x Fedora Rawhide NoGIL PR
• s390x Fedora Rawhide PR
• s390x CentOS10 LTO + PGO PR
• s390x Fedora Rawhide LTO PR
• s390x Fedora Rawhide Refleaks PR
• s390x RHEL9 Refleaks PR
• s390x Fedora Stable Clang PR

Thanks @lazerg for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

GH-158295 is a backport of this pull request to the 3.15 branch.

Sorry, @lazerg and @ZeroIntensity, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker b6f9a50e654dd76394bece71e6a8240ef28107ab 3.13

GH-158296 is a backport of this pull request to the 3.14 branch.

GH-158297 is a backport of this pull request to the 3.13 branch.