Skip to content

fix(stdlib): return error instead of panicking on out-of-range combinations size 🧮#173

Merged
timfennis merged 2 commits into
masterfrom
bugfix/combinations-capacity-overflow
Jun 2, 2026
Merged

fix(stdlib): return error instead of panicking on out-of-range combinations size 🧮#173
timfennis merged 2 commits into
masterfrom
bugfix/combinations-capacity-overflow

Conversation

@timfennis
Copy link
Copy Markdown
Owner

Context

While fuzzing the pipeline for panics, I found that combinations(seq, k) aborts the process with capacity overflow for out-of-range k:

  • combinations([1, 2, 3], -1)k as usize turns -1 into usize::MAX
  • combinations([1, 2, 3], 9223372036854775807) — a genuinely huge positive k

Both reach CombinationsIter::new, which eagerly built its index vector with (0..k).collect() and asked the allocator for a usize::MAX-element Vec.

Changes

  • ndc_vm: CombinationsIter now stores k and defers the indices allocation until ensure_index(k - 1) confirms the buffer holds at least k elements. The allocation is then bounded by data actually materialised, so an over-large k over a short source yields no combinations rather than overflowing. This also covers a large positive k, which the signature change alone does not.
  • ndc_stdlib: combinations, permutations, and subsequences took k/length as i64 and cast with as usize. They now take usize, so argument conversion rejects a negative value with the same error as the sibling windows/chunks/take functions. Previously permutations and subsequences silently returned [] for negative input.

Tests

Four regression tests under tests/functional/programs/900_bugs/: the huge-positive and negative cases for combinations, plus the negative cases for permutations and subsequences.

🤖

…ations size 🧮

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f957eb7ec9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ndc_vm/src/iterator.rs
Leaving `first = false` after a failed `ensure_index` relied on a subtle
non-local invariant (ensure_index nulling the source) to keep the else
branch from indexing the still-empty `indices`. Clear `first` only after
`indices` is built, so a re-poll re-enters the first-call branch and
returns None idempotently. Adds a re-poll regression case.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@timfennis timfennis enabled auto-merge (squash) June 2, 2026 06:43
@timfennis timfennis merged commit 8863df1 into master Jun 2, 2026
1 check passed
@timfennis timfennis deleted the bugfix/combinations-capacity-overflow branch June 2, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant