Skip to content

fix: integer-address pointer reconstruction (inttoptr + GEP) - #4251

Draft
ianna wants to merge 4 commits into
mainfrom
ianna/integer-address_pointer_reconstruction
Draft

fix: integer-address pointer reconstruction (inttoptr + GEP)#4251
ianna wants to merge 4 commits into
mainfrom
ianna/integer-address_pointer_reconstruction

Conversation

@ianna

@ianna ianna commented Jul 27, 2026

Copy link
Copy Markdown
Member

Summary

This PR updates Awkward's Numba lowering to explicitly reconstruct typed pointers from integer addresses before performing pointer arithmetic.

Awkward stores buffer addresses in its lookup table as integer values (intp). Several lowering paths reconstructed element addresses by performing integer address arithmetic and relying on cgutils.pointer_add to produce the final pointer. Following Numba PR numba/numba#10696, pointer_add now preserves pointer provenance by using LLVM's getelementptr (GEP), which requires its base operand to already be a pointer.

This PR updates these call sites to:

  • reconstruct typed pointers explicitly with builder.inttoptr(),
  • perform element addressing using builder.gep(),
  • avoid integer address arithmetic after pointer reconstruction.

The resulting code is compatible with the updated pointer_add semantics while making pointer provenance explicit in the generated LLVM IR.

Motivation

Numba PR #10696 changed cgutils.pointer_add to use GEP instead of byte-wise pointer arithmetic in order to avoid LLVM miscompilations related to pointer provenance.

Awkward intentionally stores external buffer addresses as integers in its lookup table. Those addresses should therefore be converted back into typed pointers explicitly before any pointer arithmetic is performed. This makes the intended semantics explicit and avoids relying on implicit integer-to-pointer conversions.

This change is independent of the recent work on native string lowering and instead updates the underlying pointer handling used by the Numba integration.

Changes

  • replace integer-address pointer arithmetic with explicit builder.inttoptr() calls where appropriate,
  • use builder.gep() for element addressing once a typed pointer has been reconstructed,
  • remove assumptions that cgutils.pointer_add accepts integer addresses.

No user-facing API changes are introduced.

@github-actions github-actions Bot added the type/fix PR title type: fix (set automatically) label Jul 27, 2026
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.07%. Comparing base (eae50c8) to head (b952bd3).
✅ All tests successful. No failed tests found.

Additional details and impacted files
Files with missing lines Coverage Δ
src/awkward/_connect/numba/arrayview.py 93.86% <100.00%> (+0.02%) ⬆️
src/awkward/_connect/numba/layout.py 83.58% <100.00%> (-0.15%) ⬇️

builder,
data_pointer_type = context.get_value_type(numba.types.CPointer(rettype.dtype))

typed_arrayptr = builder.inttoptr(

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 inttoptr necessary? In numba, we have seen LLVM loop-unrolling passes that ignores the pointer provenance from inttoptr. I highly recommend avoiding it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I saw

("arrayptrs", numba.types.CPointer(numba.intp)),

Can you model arrayptrs as i8** (so like a void**) instead of intp*?

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 @sklam ! One thing I'd like to clarify is the scope of this PR change.

This PR updates Awkward to explicitly reconstruct typed pointers with inttoptr before performing pointer arithmetic with getelementptr, making it compatible with Numba's recent changes to pointer_add.

However, I don't think we should describe this as fully solving pointer provenance. Awkward's lookup table still stores buffer addresses as intp values, so the original pointer provenance has already been discarded before the lowering reconstructs a pointer. This PR improves correctness relative to the new Numba implementation, but it doesn't eliminate the underlying design issue.

I think preserving pointer provenance end-to-end would require a larger redesign of the lookup representation (for example, separating integer layout metadata from a table of actual buffer pointers). I've opened a follow-up issue to discuss that independently: #4252

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm recommending extra caution here because:

a) The pointer provenance issue is complicated by the uncertain state in LLVM. It is a LLVM design bug that is being addressed. See https://www.npopov.com/2026/01/31/This-year-in-LLVM-2025.html#ptrtoaddr .
b) The effect from it is very difficult to predict, replicate or track down. Undefined behavior gives LLVM the license to do very unusual things (see LLVM blog). For example, Numba saw the issue to only appear in a very particular loop nest pattern and on a specific CPU ISA due to LLVM's target cost model.

There are more details from my debugging in the original Numba issue here: numba/numba#10695 (comment).

All these made me scared about having inttoptrand ptrtoint.

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 @sklam ! You are right it is a dangerous territory! I ought to look into a safer fix.

@github-actions

Copy link
Copy Markdown

The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR4251

@ianna
ianna marked this pull request as draft July 27, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/fix PR title type: fix (set automatically)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants