Skip to content

Instantly share code, notes, and snippets.

@esc
Last active August 7, 2026 12:51
Show Gist options
  • Select an option

  • Save esc/f9f1d2ae9bf0cc2cf107477d70b9a9e0 to your computer and use it in GitHub Desktop.

Select an option

Save esc/f9f1d2ae9bf0cc2cf107477d70b9a9e0 to your computer and use it in GitHub Desktop.
Numba PR 10761 Review by Kimi K3 and esc

Code Review — PR #10761: Fix np.clip writing past the end of a caller-provided out= array

  • Reviewer: Kilo (senior-engineer pass) and Kimi K3
  • Date: 2026-08-05
  • Branch: fix-10682-clip-out-shape @ 162521a9a0, merge base 0d0938a717
  • Local env: macOS arm64, Python 3.14, NumPy 2.3.5, numba 0.68.0dev0+14.g162521a9a0 (in-place build)

Verdict

Approve. This fixes a genuine memory-corruption bug: a too-small out= was written past its end with no error, and a too-large out= silently returned a wrong array. The fix is minimal, covers all eight implementation branches and all four call spellings, and every claim in the PR description that I audited checks out (see "Claims audit" below). No blockers. Two non-blocking discussion items and a few nits.

Verification

Convention: every step below consists of a self-contained python snippet (all imports and variables defined inside it — save it to a file, e.g. `v1.py`, or paste into a Python session) plus a bash block with the exact commands to run it from the repo root of the PR checkout. Fragments that need the pre-PR behavior carry the source swap inline — the only changed file is pure Python, so checking out its merge-base revision switches the implementation without a rebuild (compiled extensions are untouched):

git checkout 0d0938a717 -- numba/np/arrayobj.py   # merge-base (pre-PR) implementation
# ... run the python snippet ...
git checkout HEAD -- numba/np/arrayobj.py         # restore PR state

All restores during this review were verified with git status (tracked tree clean). Outputs below are the exact observed outputs from executing these snippets.

V1. NumPy oracle — what the correct behavior is

import numpy as np

a = np.arange(5.0)

for label, target, out in [
        ("too-small (3,) for a (5,)", a, np.full(3, -1.0)),
        ("too-large (8,) for a (5,)", a, np.full(8, -1.0)),
        ("per-dim mismatch (3,2) for a (2,3)", np.arange(6.0).reshape(2, 3), np.empty((3, 2))),
]:
    try:
        np.clip(target, 0.0, 3.0, out=out)
        print(f"{label}: NO ERROR")
    except ValueError as e:
        print(f"{label}: ValueError: {e}")

print("numpy", np.__version__)
python v1.py   # pure NumPy; build state irrelevant

Expected/observed output:

too-small (3,) for a (5,): ValueError: operands could not be broadcast together with shapes (5,) () () (3,)
too-large (8,) for a (5,): ValueError: operands could not be broadcast together with shapes (5,) () () (8,)
per-dim mismatch (3,2) for a (2,3): ValueError: operands could not be broadcast together with shapes (2,3) () () (3,2)
numpy 2.3.5

V2. The defect on the merge base (pre-PR)

import numpy as np
from numba import njit

@njit
def clip_out(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

a = np.arange(5.0)

buf = np.full(8, 999.0)
r = clip_out(a, 0.0, 3.0, buf[:3])           # out too small: view of larger buffer
print(f"too-small: ret={r}, buf={buf}")

r = clip_out(a, 0.0, 3.0, np.full(8, -1.0))  # out too large
print(f"too-large: ret={r}")
git checkout 0d0938a717 -- numba/np/arrayobj.py   # pre-PR implementation
python v2.py
git checkout HEAD -- numba/np/arrayobj.py         # restore PR state

Expected/observed output (pre-PR):

too-small: ret=[0. 1. 2.], buf=[  0.   1.   2.   3.   3. 999. 999. 999.]
too-large: ret=[ 0.  1.  2.  3.  3. -1. -1. -1.]

i.e. buf[3] and buf[4] are written past the end of the 3-element out, the returned array is wrong, and nothing is raised — the reported defect, reproduced.

V3. The fix on the PR build

import numpy as np
from numba import njit

@njit
def clip_out(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

a = np.arange(5.0)

# too small: 3-element view into an 8-element buffer
buf = np.full(8, 999.0)
try:
    clip_out(a, 0.0, 3.0, buf[:3])
    print("too-small: NO ERROR")
except ValueError as e:
    print(f"too-small: ValueError: {e}")
print(f"buffer intact: {np.array_equal(buf, np.full(8, 999.0))}")

# too large
try:
    clip_out(a, 0.0, 3.0, np.full(8, -1.0))
    print("too-large: NO ERROR")
except ValueError as e:
    print(f"too-large: ValueError: {e}")

# valid out: accepted, returned by identity, values correct
out = np.full(5, -1.0)
r = clip_out(a, 0.0, 3.0, out)
print(f"valid out: ret={r}, returned-is-out={r is out}, "
      f"values-match={np.array_equal(r, np.clip(a, 0.0, 3.0))}")
git checkout HEAD -- numba/np/arrayobj.py   # ensure PR state (no-op on default checkout)
python v3.py

Expected/observed output (PR build):

too-small: ValueError: clip: the shape of the 'out' array does not match the shape of the result
buffer intact: True
too-large: ValueError: clip: the shape of the 'out' array does not match the shape of the result
valid out: ret=[0. 1. 2. 3. 3.], returned-is-out=True, values-match=True

The raise happens before any element is written — no partial writes, adjacent memory untouched; a valid out is written and returned by identity, with NumPy-correct values.

V4. Non-Array bounds (tuple / list / nested tuple) — risk to the getattr(t, 'ndim', 0) typing computation

Concern: type_can_asarray admits Sequence/Tuple/Number at the gate, and those types have no .ndim, so the new result_ndim = max(getattr(t, 'ndim', 0) ...) would compute 0 for them. Run on both builds:

import numpy as np
from numba import njit

@njit
def clip_b(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

cases = [
    ("list bound",         (np.arange(5.0), [0, 1, 2, 3, 4], 3.0, np.full(5, -1.0))),
    ("tuple bound",        (np.arange(5.0), (0, 1, 2, 3, 4), 3.0, np.full(5, -1.0))),
    ("nested-tuple bound", (np.arange(5.0), ((0, 1, 2, 3, 4), (0, 1, 2, 3, 4)), 3.0, np.empty((2, 5)))),
    ("tuple a",            ((0., 1., 2., 3., 4.), 0.0, 3.0, np.full(5, -1.0))),
]
for label, args in cases:
    try:
        clip_b(*args)
        print(f"{label}: compiled and ran")
    except Exception as e:
        sig = [l.strip() for l in str(e).splitlines() if l.strip().startswith(">>> clip(")]
        print(f"{label}: {type(e).__name__}: No implementation ... {sig[0] if sig else ''}")
python v4.py                                    # PR build
git checkout 0d0938a717 -- numba/np/arrayobj.py
python v4.py                                    # merge-base build
git checkout HEAD -- numba/np/arrayobj.py       # restore PR state

Expected/observed output — identical on both builds, each run:

list bound: TypingError: No implementation ... >>> clip(array(float64, 1d, C), reflected list(int64)<iv=None>, float64, array(float64, 1d, C))
tuple bound: TypingError: No implementation ... >>> clip(array(float64, 1d, C), UniTuple(int64 x 5), float64, array(float64, 1d, C))
nested-tuple bound: TypingError: No implementation ... >>> clip(array(float64, 1d, C), UniTuple(UniTuple(int64 x 5) x 2), float64, array(float64, 2d, C))
tuple a: TypingError: No implementation ... >>> clip(UniTuple(float64 x 5), float64, float64, array(float64, 1d, C))

None of these signatures ever compiled, before or after the PR, so the getattr fallback is unreachable for any compilable call. Concern discarded (kept as nit 2).

V5. dtype-mismatched out — did the helper drop an implicit check?

The old np.empty_like(a) if out is None else out IfExp forced unification of empty_like(a) with out, implicitly rejecting dtype-mismatched out. The new helper could, in principle, have lost that.

import numpy as np
from numba import njit

@njit
def clip_out(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

for label, args in [
    ("float64 a, int64 out", (np.arange(5.0), 0.0, 3.5, np.full(5, -1))),
    ("int64 a, float64 out", (np.arange(5), 0, 3, np.full(5, -1.0))),
]:
    try:
        clip_out(*args)
        print(f"{label}: NO ERROR")
    except Exception as e:
        first = str(e).splitlines()[0]
        print(f"{label}: {type(e).__name__}: {first}")

# NumPy oracle for the first call:
try:
    np.clip(np.arange(5.0), 0.0, 3.5, out=np.full(5, -1))
except Exception as e:
    print(f"numpy oracle: {type(e).__name__}: {e}")
python v5.py   # PR build

Expected/observed output (PR build):

float64 a, int64 out: TypingError: Failed in nopython mode pipeline (step: nopython frontend)
int64 a, float64 out: TypingError: Failed in nopython mode pipeline (step: nopython frontend)
numpy oracle: UFuncTypeError: Cannot cast ufunc 'clip' output from dtype('float64') to dtype('int64') with casting rule 'same_kind'

Still rejected (as No implementation of function clip under the hood) — the helper's two return paths (np.empty_like(a) vs out) still force unification, so no silent-truncation path was opened. Concern discarded.

V6. Ordering corner: both bounds None + bad-ndim out

import numpy as np
from numba import njit

@njit
def clip_out(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

try:
    clip_out(np.arange(5.0), None, None, np.empty((2, 5)))   # bad-ndim out
except Exception as e:
    hit = 'must have the same number of dimensions' in str(e)
    print(f"bad-ndim out: {type(e).__name__}, ndim-message-present={hit}")

try:
    clip_out(np.arange(5.0), None, None, np.empty(5))        # good-ndim out
except Exception as e:
    print(f"good-ndim out: {type(e).__name__}: {e}")
python v6.py   # PR build

Expected/observed output (PR build):

bad-ndim out: TypingError, ndim-message-present=True
good-ndim out: ValueError: array_clip: must set either max or min

The new typing-time ndim check runs before branch selection, so the first call now fails at compile time instead of raising the runtime "must set either max or min". Harmless — the call is an error either way (see Discussion 2).

V7. Positive paths (PR build)

import numpy as np
from numba import njit

@njit
def clip_out(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

# aliasing: in-place clip (out is a)
b = np.arange(5.0)
r = clip_out(b, 0.0, 3.0, b)
print("aliasing:", r is b, r)

# F-order out
a2 = np.arange(6.0).reshape(2, 3)
out = np.asfortranarray(np.full((2, 3), -1.0))
r = clip_out(a2, 0.0, 3.0, out)
print("F-order:", r is out, np.array_equal(out, np.clip(a2, 0.0, 3.0)))

# 0-d a, 0-d out
out = np.zeros(())
r = clip_out(np.array(7.0), 0.0, 3.0, out)
print("0-d:", r)

# method spelling (ndarray.clip) with bad out
@njit
def clip_m(a, a_min, a_max, out):
    return a.clip(a_min, a_max, out)

try:
    clip_m(np.arange(5.0), 0.0, 3.0, np.full(3, -1.0))
except ValueError as e:
    print("method spelling:", type(e).__name__ + ":", e)
python v7.py   # PR build

Expected/observed output:

aliasing: True [0. 1. 2. 3. 3.]
F-order: True True
0-d: 3.0
method spelling: ValueError: clip: the shape of the 'out' array does not match the shape of the result

np.empty_like(a) allocation on the out=None path is unchanged by the diff (layout of returned arrays preserved).

V8. Broadcast-larger result with explicit out (PR build)

import numpy as np
from numba import njit

@njit
def clip_o(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

a = np.arange(5.0).reshape(1, 5)      # (1,5)
a_max = np.full((3, 5), 3.0)          # (3,5) -> broadcast result (3,5)

out = np.full((3, 5), -1.0)
r = clip_o(a, 0.0, a_max, out)        # valid out, larger than a
print("valid (3,5) out:", r is out, np.array_equal(r, np.clip(a, 0.0, a_max)))

try:
    clip_o(a, 0.0, a_max, np.full((2, 5), -1.0))   # same ndim, wrong shape
except ValueError as e:
    print("bad (2,5) out:", type(e).__name__ + ":", e)

a_min_b = np.zeros((3, 1))            # array/array branch, broadcast on both sides
out = np.full((3, 5), -1.0)
r = clip_o(a, a_min_b, a_max, out)
print("array/array branch:", np.array_equal(r, np.clip(a, a_min_b, a_max)))
python v8.py   # PR build

Expected/observed output:

valid (3,5) out: True True
bad (2,5) out: ValueError: clip: the shape of the 'out' array does not match the shape of the result
array/array branch: True

V9. The deferred #10760 case — identical pre/post PR (out of scope for this PR)

import numpy as np
from numba import njit

@njit
def clip_n(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

a = np.arange(5.0).reshape(1, 5)
a_max = np.full((3, 5), 3.0)
r = clip_n(a, 0.0, a_max, None)     # out=None, broadcast result (3,5) > a (1,5)
print(f"ret.shape={r.shape} (numpy: {np.clip(a, 0.0, a_max).shape})", flush=True)
print(f"ret={r}", flush=True)
python v9.py ; echo "exit=$?"                     # PR build
git checkout 0d0938a717 -- numba/np/arrayobj.py
python v9.py ; echo "exit=$?"                     # merge-base build
git checkout HEAD -- numba/np/arrayobj.py         # restore PR state

Observed output — identical on both builds, each run:

ret.shape=(1, 5) (numpy: (3, 5))
ret=[[0. 1. 2. 3. 3.]]
exit=0

with exit=139 (SIGSEGV) in several earlier runs on both builds. The wrong shape is deterministic and proves the out-of-bounds write: the loop writes 15 elements into the 5-element np.empty_like(a) allocation. Whether the resulting heap corruption crashes the process at teardown is nondeterministic (segfault on some runs, clean exit on others). Confirms the author's scoping: pre-existing memory corruption on the out=None path, untouched (neither fixed nor regressed) by this PR. Worth prioritizing #10760.

V10. NumPy's actual out rule vs. the PR's rule vs. numba's ufunc path

import numpy as np
from numba import njit

# NumPy: out only needs to be a valid broadcast *target*
out = np.full((2, 5), -1.0)
np.clip(np.arange(5.0), 0.0, 3.0, out=out)   # accepted: inputs broadcast ONTO out
print("numpy clip, out (2,5) for a (5,):")
print(out)

# numba ufunc path agrees with NumPy:
@njit
def add_out(a, out):
    return np.add(a, 1.0, out)

out = np.full((2, 5), -1.0)
r = add_out(np.arange(5.0), out)
print("numba np.add, out (2,5) for a (5,):", r is out,
      np.array_equal(out, np.broadcast_to(np.arange(5.0) + 1.0, (2, 5))))

# numba clip, PR build: same call shape is rejected
@njit
def clip_out(a, a_min, a_max, out):
    return np.clip(a, a_min, a_max, out)

try:
    clip_out(np.arange(5.0), 0.0, 3.0, np.empty((2, 5)))
except Exception as e:
    hit = 'must have the same number of dimensions' in str(e)
    print(f"numba clip (PR), out (2,5) for a (5,): {type(e).__name__}, "
          f"ndim-message-present={hit}")
python v10.py   # PR build

Observed output:

numpy clip, out (2,5) for a (5,):
[[0. 1. 2. 3. 3.]
 [0. 1. 2. 3. 3.]]
numba np.add, out (2,5) for a (5,): True True
numba clip (PR), out (2,5) for a (5,): TypingError, ndim-message-present=True

Post-PR clip rejects the call shape that NumPy and numba's own ufunc path accept. Not a regression — pre-PR clip also rejected it (V2/V4-style No implementation, from the IfExp unification) — but the PR's exact-shape check is stricter than both NumPy and numba's own ufunc path. See Discussion 1.

V11. Tests and hygiene

# new tests on the PR build
python -m numba.runtests \
  numba.tests.test_array_methods.TestArrayMethods.test_clip_out_shape_mismatch \
  numba.tests.test_array_methods.TestArrayMethods.test_clip_out_no_out_of_bounds_write \
  numba.tests.test_array_methods.TestArrayMethods.test_clip_out_bad_ndim
# observed: Ran 3 tests in 4.958s — OK

# same three tests against the merge-base implementation (author claim: all fail on main)
git checkout 0d0938a717 -- numba/np/arrayobj.py
python -m numba.runtests \
  numba.tests.test_array_methods.TestArrayMethods.test_clip_out_shape_mismatch \
  numba.tests.test_array_methods.TestArrayMethods.test_clip_out_no_out_of_bounds_write \
  numba.tests.test_array_methods.TestArrayMethods.test_clip_out_bad_ndim
# observed: AssertionError: ValueError not raised ... Ran 3 tests — FAILED (failures=3)
git checkout HEAD -- numba/np/arrayobj.py   # restore PR state

# clip subset
python -m numba.runtests numba.tests.test_array_methods -k clip
# observed: Ran 12 tests in 43.754s — OK

# full affected module
python -m numba.runtests numba.tests.test_array_methods
# observed: Ran 83 tests in 190.551s — OK (skipped=2)

python -m flake8 numba/np/arrayobj.py numba/tests/test_array_methods.py
# observed: clean (no output)

python maint/towncrier_rst_validator.py --pull_request_id 10761
# observed: Passed: Filename is valid / Passed: File contents are valid
# (rstcheck binary absent locally; title/underline lengths checked manually: 71/71)

Findings

Discussion 1 (non-blocking): the PR codifies a stricter out rule than both NumPy and numba's own ufunc path

NumPy does not require out.shape == broadcast(inputs); it requires out to be a valid broadcast target — extra leading dims and size-1 dims are accepted and written through (V10). Numba's ufunc machinery (np.add(a_1d, 1.0, out_2x5)) agrees with NumPy. Post-PR clip rejects such calls with the (clear) typing-time error. This is not a regression — ndim-mismatched out never compiled before either (V2/V4 evidence: opaque No implementation of function clip from the IfExp unification) — so the PR strictly improves the error for a call that was already unsupported.

Still, the exact-shape rule is now baked into a helper and a typing check, which makes the divergence from NumPy/ufunc semantics more permanent than the accidental unification failure was. Supporting broadcast-target out requires the same loop/allocation redesign as #10760 (the result shape can exceed a.shape). Suggestion: a one-line comment in _np_clip_prepare_out noting that accepting broadcast-target out is deliberately deferred and tied to #10760, so the next person doesn't read the check as a statement of NumPy semantics. Not gating.

Discussion 2 (minor): ordering change for the both-bounds-None corner

np.clip(a, None, None, out=wrong_ndim) previously raised the runtime ValueError: array_clip: must set either max or min; it now raises the compile-time TypingError from the ndim check, which runs before branch selection (V6). Harmless — the call is an error either way — noted for completeness.

Nits

  1. Error message could carry the shapes. ValueError("clip: the shape of the 'out' array does not match the shape of the result") tells the user what but not how it mismatched. Including out.shape and the result shape (both runtime values, formattable in nopython) would aid debugging, e.g. NumPy's non-broadcastable output operand with shape (3,) doesn't match the broadcast shape (5,).
  2. getattr(t, 'ndim', 0) is a silent assumption about future bound types. Verified unreachable for any compilable signature today (V4), but if sequence bounds are ever implemented this line would quietly compute a wrong result_ndim. A comment word to that effect would do; not worth restructuring.
  3. Test adjacency coverage is single-branch. test_clip_out_no_out_of_bounds_write exercises the scalar/scalar branch only. The check precedes the loop in every branch and the matrix test asserts the raise for all eight, so this is smoke-level by design — acceptable, but parametrizing it across branches would be cheap insurance against a future branch reordering the two statements.

Claims audit

Claim in PR description Result
Too-small out written past its end, nothing raised Verified (V2: buffer corruption demonstrated)
Too-large out accepted where NumPy raises Verified (V1, V2)
Same class as #9166 / not covered by the #10671 ufunc fix Confirmed — clip has its own impl, no _build_array involvement
All 8 impl branches + 4 spellings covered Verified by code read (helper replaces the line in every branch) and by the branch-matrix test
ndim mismatch reported at typing time; previously failed with No implementation Verified (V2/V4: pre-PR TypingError: No implementation of function clip from IfExp unification; post-PR targeted message)
Allocation on out=None path unchanged Verified — still np.empty_like(a), layout preserved (V7)
3 new tests fail on main, pass with the change Verified by swapping arrayobj.py between revisions (V11)
boundscheck=True doesn't reach overload-generated indices Plausible, consistent with the issue reporter's observation; not re-derived
#10760 (out=None, broadcast-larger result) deliberately excluded Verified identical pre/post PR (V9): returns (1,5) instead of (3,5) on both builds — an out-of-bounds write that segfaulted the process in several runs (exit 139) on both builds. The exclusion regresses nothing; #10760 itself deserves priority.
Textual conflict with #10594 Not re-verified; lines touched are indeed the same clip region
flake8 clean; test_array_methods 83 tests pass Reproduced (V11: flake8 clean; 83 tests OK, 2 skipped)

Notes on the error contract

The split — ndim mismatch → compile-time TypingError, same-ndim shape mismatch → runtime ValueError — is a defensible use of numba's typing model (ndim is part of the array type) and matches how the pre-existing code surfaced these failures. NumPy raises runtime ValueError for both; users coming from NumPy may be mildly surprised, but the messages are clear and the tests pin the behavior.

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