reject negative indices in numpy array bounds checks

[sayed0200]

Description

array::check_dimensions_impl (reached through byte_offset, offset_at, index_at, data, mutable_data, and array_t::at/mutable_at) only tested i >= *shape, and array::shape(dim)/array::strides(dim) only tested dim >= ndim(). A negative index or axis forwarded from Python passed the out-of-bounds guard and yielded a negative byte offset, i.e. an access before the start of the buffer. For at/data that is an out-of-bounds read; for mutable_at/mutable_data it is an out-of-bounds write at a caller-chosen negative offset.

Built against py::array_t<uint16_t>, a.at(-1) reads two bytes before the allocation (ASan reports heap-buffer-overflow) and a.mutable_at(-4096) = v writes far below it. These accessors already document that they throw when an index is out of bounds; a negative value is out of bounds, so the lower-bound check belongs in the accessor rather than at every binding call site. With the fix all three raise index_error the same way an over-large index does.

Added negative-index and negative-axis cases to the existing test_bounds_check and test_array_attributes; they fail on the current code and pass with the fix.

Suggested changelog entry:

• Negative indices/axes passed to array/array_t element and shape/stride accessors now raise IndexError instead of addressing memory before the start of the buffer.

[henryiii]

Since negative indices currently don't work, and we do check the upper edge, seems to make sense to do this. I pushed a slightly faster version.

[rwgk]

This PR is fine, but I'm exploring supporting negative indices, because that's more in line with the usual expectations for NumPy arrays.

[rwgk]

codex GPT-6.1-Sol xhigh

(some links below are broken; I didn't want to spend the time weeding them out)

User — 2026-10-10T11:08:56-07:00 (PDT) · +6m 19s after Codex · prior Codex turn: 56s

Hm, so as it stands, the PR goes from UB to "negative indices not supported", i.e. from UB to deliberately defined behavior.

If we want to support negative indices later, it would be a change of defined behavior. That doesn't seem desirable.

I guess the cost for supporting negative indices is:

• a little more code
• maybe slight runtime overhead

Does that sound right?

WDYT about this reasoning? Do you think it's worth working out the negative index wrapping code, to see how it compares in detail?

Codex — 2026-10-10T11:25:25-07:00 (PDT) · +3m 5s after Codex · +16m 29s since User

I agree with your reasoning. Once rejection is documented, callers can rely on IndexError; accepting negative indices later could change their behavior. I’d prefer to decide the intended semantics now.

I worked out a prototype patch. It uses NumPy’s rule: add the dimension size once when an index is negative, then check bounds. Values below -size still raise. NumPy’s documented semantics

The implementation cost is small:

• Combining validation with offset calculation produces zero net additional header lines relative to the PR.
• It supports negative axes in shape() and strides() too.
• There is no allocation on valid accesses or ABI change.
• Unchecked proxies retain their existing behavior and overhead.

The runtime cost is measurable. For positive indices, an isolated C++ benchmark gave these median times per offset calculation:

Compiler	Dimensions	PR rejection	Normalization
GCC 13	1	1.06 ns	1.46 ns
GCC 13	2	1.78 ns	2.60 ns
Clang 18	1	5.56 ns	5.53 ns
Clang 18	2	5.57 ns	6.06 ns

These used -O3, ten million accesses and seven interleaved trials on one CPU. They measure a tight indexing loop; they don’t establish an equivalent slowdown for a whole binding call. Benchmark results

I also tried moving normalization behind the existing bounds check into a separate helper. That helped Clang but hurt GCC, so I kept the simpler implementation.

I’d lean toward supporting negative indices in this PR. Your compatibility argument is persuasive, the implementation stays small, and the remaining tradeoff is performance in tight checked-access loops.

The uncommitted prototype is on rwgk/pr6187-negative-index-prototype, based on PR head d5d782d3. Both full incremental scons suites passed: default 1,381 passed, 2 skipped; free-threaded 1,362 passed, 21 skipped. Each passed 28 Catch2 cases / 1,633 assertions. Lint passed, and the eight new wrapping cases fail against the rejection implementation.

Logs: default build / tests, free-threaded build / tests.