Skip to content

Commit 1169724

Browse files
metsw24-maxmeta-codesync[bot]
authored andcommitted
fix index overflow in enumerate with large start
Summary: **Index overflow in `enumerate` with a large start** The per-element index is built as `k as i32 + start`, so when the caller passes a `start` near `i32::MAX` the addition overflows on the first elements rather than at some unreachable list length. Cause is doing the arithmetic in `i32` even though Starlark integers are arbitrary precision, so the correct result does not fit. Fix widens the running index to `i64` before adding the offset. `enumerate(['a', 'b'], 2147483647)` panics with "attempt to add with overflow" in debug and wraps to a negative index in release; it should yield `[(2147483647, 'a'), (2147483648, 'b')]`. Added a regression test alongside the existing builtin tests. Should be safe as the index can no longer exceed `i64` for any in-memory iterable, though a reviewer may prefer matching whatever width the range work settled on. X-link: facebook/starlark-rust#203 Reviewed By: JakobDegen Differential Revision: D108556073 fbshipit-source-id: 2d2b80304aba92dc806f4338e8227c143820fc69
1 parent 237a56c commit 1169724

1 file changed

Lines changed: 9 additions & 1 deletion

File tree

  • starlark-rust/starlark/src/stdlib/funcs

starlark-rust/starlark/src/stdlib/funcs/other.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ pub(crate) fn register_other(builder: &mut GlobalsBuilder) {
166166
.get()
167167
.iterate(heap)?
168168
.enumerate()
169-
.map(move |(k, v)| (k as i32 + start, v));
169+
.map(move |(k, v)| (k as i64 + start as i64, v));
170170
Ok(AllocList(v))
171171
}
172172

@@ -413,6 +413,14 @@ mod tests {
413413
assert::is_true("isinstance(abs(1), int)");
414414
}
415415

416+
#[test]
417+
fn test_enumerate_start_overflow() {
418+
assert::eq(
419+
"enumerate(['a', 'b'], 2147483647)",
420+
"[(2147483647, 'a'), (2147483648, 'b')]",
421+
);
422+
}
423+
416424
#[test]
417425
fn test_constants() {
418426
assert::is_true("not None");

0 commit comments

Comments
 (0)