Skip to content

fix(buffer): guard capacity doubling against Int overflow - #3822

Draft
bobzhang wants to merge 1 commit into
mainfrom
hongbo/fix-buffer-grow-overflow
Draft

fix(buffer): guard capacity doubling against Int overflow#3822
bobzhang wants to merge 1 commit into
mainfrom
hongbo/fix-buffer-grow-overflow

Conversation

@bobzhang

Copy link
Copy Markdown
Contributor

Buffer::grow_if_necessary doubled capacity until it covered the requirement. Once capacity passes 2³⁰, space * 2 overflows Int:

  • a power-of-two capacity wraps to INT_MIN, then sticks at 0 → 0 → ... — writes needing more than ~1 GiB of buffer hung forever;
  • a non-power-of-two capacity bounces through wrapped values and accidentally terminates on whatever positive value the wraparound lands on (e.g. growing from 11 toward 1.5 G allocated 1,610,612,736 via a negative intermediate).

Fix

The capacity computation moves into a pure grow_capacity(current, required) helper that falls back to exactly required on the first overflowing doubling. Below the overflow threshold the doubling sequence is byte-for-byte unchanged; past it, the buffer allocates the exact requirement instead of hanging or trusting wraparound.

Whitebox tests (buffer/grow_wbtest.mbt) pin the normal doubling path and both overflow boundary cases — without allocating memory, so they're CI-safe.

Review

Reviewed by Codex CLI (codex-cli 0.144.1) in two rounds. Round 1 rejected the initial version: it disproved my "identical capacity for all previously-terminating inputs" claim with the 11 → 1.5 G wraparound counterexample, confirmed the power-of-two hang (2³⁰ → INT_MIN → 0 → 0), audited that no other grow path in buffer/ shares the bug, and demanded CI-safe tests via a pure helper. All applied. Round 2: "Approved. grow_capacity correctly detects overflowing doubles... the commit message accurately scopes the behavior difference."

Signed-off-by: Codex CLI codex@openai.com

Validation

  • moon check clean, moon fmt applied, no .mbti changes
  • moon test: 6717 passed, 0 failed (buffer 98/98, incl. 2 new boundary tests)

🤖 Generated with Claude Code

Buffer::grow_if_necessary doubled capacity until it covered the
requirement. Once capacity passes 2^30, `space * 2` overflows: a
power-of-two capacity wraps to INT_MIN and then sticks at 0, looping
forever (writes needing >1 GiB hung); non-power-of-two capacities
bounce through wrapped values before accidentally terminating on
whatever positive value the wraparound lands on.

Extract the capacity computation into a pure grow_capacity helper that
falls back to exactly `required` on the first overflowing doubling.
Below the overflow threshold the doubling sequence is unchanged; past
it, the buffer now allocates the exact requirement instead of hanging
(power-of-two case) or a wraparound-determined size (other cases).

Whitebox tests cover the normal doubling path and both overflow
boundary cases without allocating memory.
Reviewed by Codex CLI (codex-cli 0.144.1) in two rounds: the first
rejected an overbroad equivalence claim and asked for CI-safe boundary
tests via a pure helper; re-review: "Approved. grow_capacity correctly
detects overflowing doubles... the commit message accurately scopes
the behavior difference."

Signed-off-by: Codex CLI <codex@openai.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coveralls

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 5184

Coverage increased (+0.002%) to 91.27%

Details

  • Coverage increased (+0.002%) from the base build.
  • Patch coverage: 4 of 4 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 17216
Covered Lines: 15713
Line Coverage: 91.27%
Coverage Strength: 198815.36 hits per line

💛 - Coveralls

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.

2 participants