BUG: fix crash on 32 bit systems using abi3t by kumaraditya303 · Pull Request #31771 · numpy/numpy

kumaraditya303 · 2026-06-26T17:24:02Z

PR summary

This PR fixes the crash on 32 bit systems when using abi3t ABI. In this PR, the first member of the PyArrayDescr is now aligned to 8 bytes so that regardless of the size of the actual PyObject, the field accesses remain correct. 8 bytes is used instead of alignof(max_align_t) to preserve backwards compatibility with exisiting abi3 extensions where sizeof(PyObject) is a multiple of 8 thereby the added alignment is no-op and does not changes the offset for such extensions.

AI Disclosure

AI was used to create the test.

charris · 2026-06-27T12:41:49Z

The lint problem is unrelated and fixed in main.

seberg · 2026-06-28T09:41:38Z

         * object.
         */
-        PyTypeObject *typeobj;
+        NPY_DECL_ALIGNED(8) PyTypeObject *typeobj;


This is a neat idea! Inspired I had a bit of a brainstorming search:

How about we embed the "public" fields here as an anonymous struct? I feel like that may be even cleaner (it doesn't encode the actual alignment guarantees yet, see below).
That is C11 but major compilers support it earlier, that might be somewhat annoying (I hope not, but...). However, NPY_DECL_ALIGNED is also not defined in a fully compatible way, currently (i.e. also would need fixing).

With an anonymous struct, but also in general, I think it might be good to do this for all our structs maybe. And it would be nice to add static asserts that would tell us in case we ever break our assumptions.
(I think a static_assert(alignof(object_struct) <= 8/16) would do the trick. It is a bit more strict than needed, but that could be addressed if it ever breaks...)

How about we embed the "public" fields here as an anonymous struct? I feel like that may be even cleaner (it doesn't encode the actual alignment guarantees yet, see below).
That is C11 but major compilers support it earlier, that might be somewhat annoying (I hope not, but...).

I think we have considered anonymous struct before but avoided that because it is incompatible with C++ and requires C11. See #31091 (comment) where it was discussed previously.

However, NPY_DECL_ALIGNED is also not defined in a fully compatible way, currently (i.e. also would need fixing).

Yeah, I'll fix it, thanks!

I've added the asserts

Thanks, I forgot about that and we do need C++ of course.

Three points, mostly because I felt they are worth noting/considering once:

_Alignas is also C11, so either we require C11 for downstream for compilers other than gcc/clang/msvc, or we'd have to omit it and stop compiling for free-thread and opaque builds.
I am not exactly sure how unreasonable C11 + fun compiler is?!

Reducing alignment is a one way road for public fields (i.e. the asserts I asked for). We aren't reducing it on 32bit systems but are doing so on 64bit systems.
I don't know that there is a realistic need for 16 byte aligned types public types but if we do this we can only add them by telling the compiler to not only align them to 8 bytes.

As this is an ABI choice, it also (potentially) affects structs that are public but not exposed on opaque builds.
I have not checked, I suspect this is only the scalars and I think that is fine. So this may mostly be about diffusing knowledge: Adding this alignment is vital for all new types we might want to expose on the opaque API (sure for many it does nothing but that isn't obvious).
EDIT: I think we can make such structs public actually, but it is limited to future Python versions (i.e. right now we can for 3.15+ which is all there is anyway for opaque builds).

_Alignas is also C11, so either we require C11 for downstream for compilers other than gcc/clang/msvc, or we'd have to omit it and stop compiling for free-thread and opaque builds.
I am not exactly sure how unreasonable C11 + fun compiler is?!

I think it is reasonable to use C11 _Alignas because CPython itself require it for defining PyObject layout. https://github.com/python/cpython/blob/2670cb062c9ec31cd6df7be645f929a8398601c7/Include/pymacro.h#L65-L86

Reducing alignment is a one way road for public fields (i.e. the asserts I asked for). We aren't reducing it on 32bit systems but are doing so on 64bit systems.

I don't think we are actually reducing it on 64 bits, as noted in the CPython implementation, alignment can only be increased and the macros becomes no-op when it will reduce the alignment.

I think it is reasonable to use C11 _Alignas because CPython itself require it for defining PyObject layout.

An nice, shouldn't we match CPython and add that #ifndef to allow compilers that don't do this right? (Not that I expect this is actually used.)

I don't think we are actually reducing it on 64 bits, as noted in the CPython implementation, alignment can only be increased and the macros becomes no-op when it will reduce the alignment.

This assumes that Python promises that on 64bit systems the size of PyObject_HEAD will always remain a multiple of 16. Unless that is a promise that Python is willing to make officially, I don't think we should rely on it.
(Sure, we could just add a static_assert in our code with a comment "if this fails we have to think about ABI" in practice.)

Sorry if I seem pedantic, but I feel we need to be very clear about what ABI decisions we are making here that we cannot change easily.

EDIT: I'll note that I think there are one or two more issues to think about. But I need a pause from quick iteration and probably Nathan will find them as well anyway.

Sure, we could just add a static_assert in our code with a comment "if this fails we have to think about ABI" in practice.

IMO this is the most practical thing to do. There is overlap between the CPython team and the NumPy team (me and Kumar) and IMO we would be able to handle that static assert failing if it ever does come up in the future.

…ng it

…ing structs

kumaraditya303 · 2026-07-02T06:40:39Z

I've changed the code to only align the first field for 3.15+ as we discussed in community meeting. I tested it with https://github.com/kumaraditya303/np-test against 3.12 stable ABI extensions and 3.14t extensions and it passes on all of them.
See https://github.com/kumaraditya303/np-test/actions/runs/28570376628/job/84706582608 for the results.

Now since the alignment only affects 3.15+, we can also do 16 bytes alignment specifically for 64 bit versions for future proofing as @seberg suggested earlier, I have tested that at https://github.com/kumaraditya303/np-test/actions/runs/28571408746/job/84709836846 as that works fine as well. Ignore this part, I messed up the testing.

seberg · 2026-07-02T08:48:38Z

Now since the alignment only affects 3.15+, we can also do 16 bytes alignment specifically for 64 bit versions for future proofing as @seberg suggested earlier

Sorry, but this is winding me up: Can you explain what changed now?

About it: I was OK with just adding an assert to defer the decision to the future poor sods if it ever happens (hopefully us still, but who knows). However, this is not the assert we have.
Of course I would prefer to hear opinions from the specialists (your, maybe @mattip), because for new objects I would be tempted to just use alignof(max_align_t) to align with PEP 697, but it is correct that in practice it will probably never make a difference.
And because it'll probably never happen, I thought we agreed on just adding a static assert with a comment (not the one we have).

FWIW, I am wondering if we can't add a bit more docs around the core problem we have to ensure:

offsetof(struct_with_object_head.first_field) % alignof(struct_without_object_head) == 0

so that others don't have to think backwards from the bug condition. I am thinking there are likely cargo-culters and we spend enough time on this to leave more breadcrumbs for them/us (e.g. point out that for new objects sizeof(max_align_t) might make sense).

From there we can explain all of our choices:

historic constraints.
64bit choice of 8 bytes over 16 bytes being "deferred" (or not). Because alignof(struct) is (a) none of our structs (except the longdouble scalar) need an alignment of more than 8 and (b) we added an assert to re-consider if sizeof(PyObject) changes and 16 would actually be different.
Ideally a brief note why possibly indicating an over-alignment of 8 seems unproblematic in the case that alignof(max_align_t) < 8. I expect it is because I don't think a malloc alignment smaller 8 is a thing and even if this happens we would have to change the padding strategy, the padding itself is OK.
(The only slight "too bad" I can think of is that the padding may differ from that of PEP 967 on those builds, but we already have no choice but to differ on many 32bit platforms anyway...)

kumaraditya303 · 2026-07-02T09:35:58Z

Sorry, but this is winding me up: Can you explain what changed now?

Sorry for the noise, that comment is not correct. I messed up the testing ignore that part.

About it: I was OK with just adding an assert to defer the decision to the future poor sods if it ever happens (hopefully us still, but who knows). However, this is not the assert we have.
Of course I would prefer to hear opinions from the specialists (your, maybe @mattip), because for new objects I would be tempted to just use alignof(max_align_t) to align with PEP 697, but it is correct that in practice it will probably never make a difference.

Honestly given how difficult it is to get this right, I would prefer if for new objects, we do not expose it the way it is done here. For new objects you are correct and we should use max_align_t as you suggest but also I would suggest to not expose the struct at all in the ABI instead add functions to access the fields. We should also define a separate fields struct and object struct which embed the fields structs to be standard compliant, we couldn't do it for existing types because we did not want break extensions which access fields directly.

kumaraditya303 · 2026-07-02T10:00:06Z

Of course I would prefer to hear opinions from the specialists (your, maybe @mattip), because for new objects I would be tempted to just use alignof(max_align_t) to align with PEP 697, but it is correct that in practice it will probably never make a difference.

I mean something like this for new types:

struct NewNumpyStructFields {
    ... // Define the fields of the new struct here
};

struct NewNumpyStruct {
    PyObject_HEAD
    alignas(max_align_t) struct NewNumpyStructFields fields;
};

seberg · 2026-07-06T10:52:49Z

Review wise, there are still two things mentioned earlier that I am not sure we aligned on (maybe, maybe not); I may also be forgetting something, but hopefully nothing big:

Do we want a guard so we can support non-C11 niche compilers like Python does?
Maybe not for here, but previously we didn't have to worry about wanting to expose more structs. Now we do. If we don't add the padding now, it means abi3t builds that include a "future" struct . This may be nothing and maybe Nathan and you had already thought through this part? But if not, I would appreciate looking at a list (IMO minus scalars, I am not worried about them) to avoid forgetting something now that we may regret.
My changes need to be checked of course!

But, I guess I'll just let you and Nathan mop that up unless you wonder e.g. if I think a struct should get the padding.

ngoldbaum · 2026-07-08T18:52:16Z

Thanks @kumaraditya303!

Co-authored-by: Sebastian Berg <sebastianb@nvidia.com> Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>

BUG: fix crash on 32 bit systems using abi3t (#31771)

BUG: fix crash on 32 bit systems using abi3t

060f67c

charris added 00 - Bug 09 - Backport-Candidate PRs tagged should be backported labels Jun 27, 2026

seberg added the 63 - C API Changes or additions to the C API. Mailing list should usually be notified. label Jun 28, 2026

seberg reviewed Jun 28, 2026

View reviewed changes

kumaraditya303 added 6 commits June 28, 2026 16:33

fix NPY_DECL_ALIGNED for C++

54111ed

Merge branch 'main' of https://github.com/numpy/numpy into 32bit-crash

9e570de

use offsetof for all types

18d03f1

fix c++ compilation

e222990

fix lint

cbed361

align all first member of all opaque structs and add comment explaini…

bb765e5

…ng it

kumaraditya303 marked this pull request as ready for review June 28, 2026 11:45

kumaraditya303 requested a review from seberg June 28, 2026 11:46

kumaraditya303 added 2 commits June 28, 2026 17:25

add static asserts

3070fc4

remove unnecessary comment

905c69e

kumaraditya303 requested a review from ngoldbaum June 28, 2026 16:12

kumaraditya303 mentioned this pull request Jun 30, 2026

Dealing with opaque API, 32bit builds and Fields Getting #31764

Open

kumaraditya303 added 3 commits July 1, 2026 18:42

Merge branch 'main' of https://github.com/numpy/numpy into 32bit-crash

1e3cec1

use offset of the first field to skip PyObject header part while copy…

5b6b371

…ing structs

add assert about typeobj offset being multiple of 8

b7ad3d6

ngoldbaum mentioned this pull request Jul 1, 2026

TST: add tests for stable ABI numpy extensions #31822

Merged

kumaraditya303 added 2 commits July 2, 2026 11:53

add _NPY_OPAQUE_FIRST_FIELD and change the alignment for 3.15+ only

267f81f

fix static assert for 3.14t

4e49357

seberg force-pushed the 32bit-crash branch from 81e2ac5 to d03255a Compare July 6, 2026 10:30

Suggest more in-depth code comments about all of this.

61dcc11

seberg force-pushed the 32bit-crash branch from d03255a to 61dcc11 Compare July 6, 2026 10:34

kumaraditya303 and others added 4 commits July 6, 2026 20:07

Merge branch 'main' of https://github.com/numpy/numpy into 32bit-crash

bc44ee6

remove 32 bit abi3t test skip

bab5c4a

only define NPY_DECL_ALIGNED is not already defined

9ec9718

BUG: expose PyArray_StringDTypeObject as an opaque pointer under abi3t

7857a15

ngoldbaum merged commit 009c609 into numpy:main Jul 8, 2026
87 checks passed

kumaraditya303 deleted the 32bit-crash branch July 8, 2026 19:05

charris mentioned this pull request Jul 10, 2026

BUG: fix crash on 32 bit systems using abi3t (#31771) #31949

Merged

charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jul 10, 2026

charris pushed a commit that referenced this pull request Jul 10, 2026

BUG: fix crash on 32 bit systems using abi3t (#31771)

965440e

Co-authored-by: Sebastian Berg <sebastianb@nvidia.com> Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>

charris added a commit that referenced this pull request Jul 10, 2026

Merge pull request #31949 from charris/backport-31771

6419eec

BUG: fix crash on 32 bit systems using abi3t (#31771)

Icxolu added a commit to Icxolu/rust-numpy that referenced this pull request Jul 10, 2026

following numpy/numpy#31771

6155b7f

Icxolu added a commit to Icxolu/rust-numpy that referenced this pull request Jul 11, 2026

following numpy/numpy#31771

6437f4b

Uh oh!

Uh oh!

Conversation

kumaraditya303 commented Jun 26, 2026 • edited by ngoldbaum Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

AI Disclosure

Uh oh!

charris commented Jun 27, 2026

Uh oh!

seberg Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

kumaraditya303 Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

kumaraditya303 Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

seberg Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kumaraditya303 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

seberg Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

kumaraditya303 commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seberg commented Jul 2, 2026

Uh oh!

kumaraditya303 commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kumaraditya303 commented Jul 2, 2026

Uh oh!

seberg commented Jul 6, 2026

Uh oh!

ngoldbaum commented Jul 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kumaraditya303 commented Jun 26, 2026 •

edited by ngoldbaum

Loading

seberg Jun 29, 2026 •

edited

Loading

seberg Jun 29, 2026 •

edited

Loading

kumaraditya303 commented Jul 2, 2026 •

edited

Loading

kumaraditya303 commented Jul 2, 2026 •

edited

Loading