Dricmoy commented on PR #52451:
URL: https://github.com/apache/arrow/pull/52451#issuecomment-6808417120
Thanks for the reviews. The latest commit addresses the remaining overflows
(branch also rebased onto current main):
- **Size estimate:** split into `MaxSliceBytes` (clamps start/stop) and
`NumSelectedBytes` (`ceil(span / |step|)`, with a step at least as large
as the
span handled separately). `"abc"`, start=1, default stop, step=INT64_MAX no
longer overflows.
- **Clamp lower bound** is now `-len - 1` instead of `-len`. When slicing
backward,
`-len - 1` means "before the first byte". The previous formula
overestimated
enough to hide this, but an exact estimate needs it.
- **Same overflow class elsewhere in the kernel:** `SliceBackwardRange`
(`start + 1` / `stop + 1` at INT64_MAX), the copy loops (pointer advanced
by a
huge step), and `FixedOutputSize`.
- **Tests** are added to the existing PosPos / NegNeg / NegPos tests. Each
one fails
under ASan/UBSan if the matching change is reverted.
Verification: Debug build with ASan + UBSan; `arrow-compute-scalar-type-test`
passes. I also ran a local, uncommitted differential test against Python's
slice
semantics: 19,600 start/stop/step combinations (INT64_MIN/MAX, ±INT64_MAX/2,
-10..10) over binary, large_binary, single-string and fixed-size-binary
inputs.
All matched.
AI disclosure: I used an AI assistant (Claude) to help find the remaining
overflow
paths and to write the change and tests. I have reviewed every change and can
explain it.
`utf8_slice_codeunits` still has the `+ step - 1` overflow in its size
estimate
for step=INT64_MAX. I'll open a separate issue for that instead of widening
this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
