Skip to content

add support for abi3t#556

Draft
Icxolu wants to merge 2 commits into
PyO3:mainfrom
Icxolu:abi3t
Draft

add support for abi3t#556
Icxolu wants to merge 2 commits into
PyO3:mainfrom
Icxolu:abi3t

Conversation

@Icxolu

@Icxolu Icxolu commented Jun 15, 2026

Copy link
Copy Markdown
Member

This adds abi3t support by

  • switching to opaque structs
  • exposing data access helpers from slots 369-378
  • redirect all field accesses through the access helpers

Todo:

  • changelog entry
  • CI testing for 3.15/3.15t

See numpy/numpy#31091
Closes #555

@ngoldbaum

Copy link
Copy Markdown
Contributor

Ping @kumaraditya303 - we were just talking about this today.

@neutrinoceros

neutrinoceros commented Jun 17, 2026

Copy link
Copy Markdown

I confirmed that this resolved the runtime error I reported in #555
example logs (remaining failures are unrelated, they're just due to numpy not having cp315 nightlies for windows yet)

Comment thread src/npyffi/array.rs Outdated
Comment thread src/npyffi/array.rs Outdated
Comment thread src/npyffi/array.rs Outdated
Comment thread src/npyffi/objects.rs
@kumaraditya303

Copy link
Copy Markdown

I think we should also add a check that when abi3t is used, the minimum numpy version should be 2.5.

@Icxolu Icxolu force-pushed the abi3t branch 4 times, most recently from cfef0a3 to 23f2955 Compare June 20, 2026 09:28
@Icxolu

Icxolu commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Looks like 32bit windows still has some issues... not sure why though
There was also https://github.com/PyO3/rust-numpy/actions/runs/27866655836/job/82471861978 earlier

@neutrinoceros

Copy link
Copy Markdown

My guess is that'd be because there are no wheels for cp315*-*win32
xref numpy/numpy-release#53

@Icxolu

Icxolu commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

To me it looks like there are cp315-win32 wheels here and they installed fine. The version specific run had no issues as well. It's just the abi3t run that failed, so I think this is a genuine failure, but I'm not sure which fault it could be: ours, pyo3, numpy or python

@kumaraditya303

Copy link
Copy Markdown

I think it could alignment issue on 32 bit MSVC, I am not very familiar with rust but it seems flags is defined as npy_uint64 which rust gives 8 bytes alignment whereas in C I think it is 4 bytes which could be an issue.

@kumaraditya303

Copy link
Copy Markdown

@Icxolu Can you try forcing a 4 byte alignment layout using #[cfg_attr(all(Py_LIMITED_API, Py_GIL_DISABLED), repr(C, packed(4)))]?

@ngoldbaum

Copy link
Copy Markdown
Contributor

If you all don't sort this out in the meantime I'll try to look on Monday. While win32 isn't so important per se, I do find it useful to detect and sort out CPU architecture-sensitive issues. It's possible this issue is in any one of CPython, NumPy, or rust-numpy.

Glad to see everything mostly works otherwise!

@Icxolu

Icxolu commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

I did a bit of digging with compiler explorer: https://godbolt.org/z/qYe3Ghz7a
Both rustc and msvc agree on both architectures on the structure size and that flags is at offset 16 (and thus 8 byte aligned as it should), so I think there is no issue with our definitions here. I did however also find this: rust-lang/rust#112480, which does not really paint a great picture about win32... perhaps we ran into that...

@ngoldbaum

Copy link
Copy Markdown
Contributor

I think I know what the issue is.

PyArray_Descr_fields has a field that causes alignment issues on 32bit builds:

https://github.com/numpy/numpy/blob/1d656e454dea882b2f2a20ccdd93b02f3c7880c1/numpy/_core/include/numpy/ndarraytypes.h#L650

It's explicitly npy_uint64 - 8 bytes. In this case, it happens to lead to a field access with incorrect alignment and you get a segfault.

I think the fix needs to live in NumPy's headers. There probably needs to be a mirrored change here to ensure correct alignment for the flags field in PyArray_Descr_fields and _PyArray_LegacyDescr_fields. I'll have a PR open on NumPy shortly.

@ngoldbaum

Copy link
Copy Markdown
Contributor

I opened numpy/numpy#31759

@ngoldbaum

ngoldbaum commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

It turns out my first attempt at a fix had a flaw and I think the only real fix requires NumPy to add new a C API function to fix this issue: numpy/numpy#31762

@kumaraditya303

This comment was marked as outdated.

@kumaraditya303

This comment was marked as outdated.

@Icxolu

Icxolu commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Thank you both for continuing to look into this, much appreciated ❤️. That patch seems to make it run indeed. What I don't really understand is, why this should work. If I got it correctly, then from numpy's definition there are padding bytes inserted to align the flags field on 32bit. If we now tell Rust to use a packed layout, wouldn't we read the padding bytes instead of the field? I'd also assume that this than effects all 32bit platforms, not just windows and not just x86, we just don't have any CI configured for other cases.

@ngoldbaum

Copy link
Copy Markdown
Contributor

I think we need a similar fix on the NumPy side too.

This does affect all 32 bit platforms, I think.

@kumaraditya303

Copy link
Copy Markdown

This is fixed in numpy main branch, I think the latest nightly would contain the fix.

@Icxolu Icxolu added the CI-no-fail-fast If one job fails, allow the rest to keep testing label Jul 10, 2026
@Icxolu

Icxolu commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

Thanks for the heads up. I added the corresponding changes in 6437f4b.

As in Rust alignment constraints can only be applied to types and not individual fields, I removed the conditional ob_base from the _field variants and added it unconditionally to the corresponding "full" structs if they are not opaque. Additional alignment is applied to the fields for 3.15+.

I think the 3.15t-x86 failure is caused by the nightly builds not being up-to date. I build some numpy wheels locally and it worked for me.

I'm not really sure why 3.14t-x86 is failing now. I can reproduce the failure locally with both the latest numpy release as well as with a locally built wheel. As this worked in the initial version before 6437f4b I feel like something is not quite right in there, but I currently don't see it. Maybe one of you is able to spot it.

Comment thread src/npyffi/objects.rs
if is_numpy_2(py) {
unsafe {
(*(dtype as *mut _PyArray_DescrNumPy2)).elsize = size;
(*_PyDataType_GET_ITEM_DATA(dtype).cast::<_PyArray_DescrNumPy2_fields>()).elsize = size;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the cast to _PyArray_DescrNumPy2_fields correct here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. _PyDataType_GET_ITEM_DATA returns *mut PyArray_Descr_fields which for us only contains the common fields between v1 and v2. Here we did a runtime check that we are running on v2, so we can cast to the v2 specific variant to get access to the v2 fields. That's the idea anyway.

Comment thread src/npyffi/objects.rs
#[repr(C)]
pub struct _PyArray_DescrNumPy2 {
pub ob_base: PyObject,
#[cfg_attr(Py_3_15, repr(align(8)))]

@ngoldbaum ngoldbaum Jul 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[cfg_attr(Py_3_15, repr(align(8)))]
#[cfg_attr(Py_3_15, repr(align(8)))]
#[cfg_attr(
all(not(Py_3_15), Py_GIL_DISABLED, target_pointer_width = "32"),
repr(packed(4))
)]

Since the fix only applies on 3.15 and newer, otherwise we'd break NumPy's ABI on 32-bit free-threaded builds. Not that anyone is actually using that in practice, I think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a bug in my original suggestion so I edited it. Doing VCS over github comments is fun!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look! I wonder if in this case we should just declare this configuration unsupported and reject the build. I'm quite hesitant to add packed structs here, as they bring a bunch of pitfalls with them that we then have to deal with. References are nowadays rejected by the compiler already, but loads and stores also need to be properly aligned unless they go though read_unaligned/write_unaligned. So we need to carefully check which fields may now be underaligned and make sure to handle them carefully and properly, and we have to keep maintaining that into the future. This which feels like a lot of effort for a rather niche case, IMO. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure if that makes sense to you. I believe that this is actually the current ABI for 3.14t as exported by NumPy for almost a year now. So if no one has noticed it’s probably OK to just tell people not to use 32 bit free-threaded builds until 3.15.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-no-fail-fast If one job fails, allow the rest to keep testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: function built with -F pyo3/abi3t raises TypeError at runtime (CPython 3.15.0b2-free-threaded))

4 participants