Stop StateHistory panicking when the trace buffer has zero capacity - #183
Open
deepspace28 wants to merge 1 commit into
Open
Stop StateHistory panicking when the trace buffer has zero capacity#183deepspace28 wants to merge 1 commit into
deepspace28 wants to merge 1 commit into
Conversation
spacewasm-trace --limit 0 aborts on the first traced instruction:
thread 'main' panicked at crates/spacewasm_util/src/trace.rs:38:27:
index out of bounds: the len is 0 but the index is 0
--limit parses straight into StateHistory::new and accepts 0. record then
takes the else branch and indexes an empty Vec; past that line the modulo
would divide by zero. Return early instead, so a zero capacity buffer holds
nothing and the tool prints an empty trace.
Adds tests/state_history.rs. spacewasm_util had no tests, so these cover the
ring buffer rather than only the zero case: ordering below capacity, at
capacity, across wraps, capacity one, and dump output including metadata.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tk7jKQZvb41gfozf2wxw1T
|
Welcome, new contributor! Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
spacewasm-trace --limit 0crashes:--limitparses straight intoStateHistory::new, and0is accepted. On the first recorded instructionrecordtakes the else branch and indexes an emptyVec. Past that line,(self.index + 1) % self.capacitywould divide by zero.recordnow returns early when capacity is zero, so the buffer holds nothing and the tool prints an empty trace instead of aborting.Before, on a module with one exported function:
After:
Also adds
tests/state_history.rs.spacewasm_utilhad no tests at all, so these cover the ring buffer rather than only the zero case: ordering below capacity, at capacity, across several wraps, capacity one, empty iteration, anddumpoutput including metadata. With the fix reverted,zero_capacity_records_nothingfails and the other eight pass.cargo fmt --all -- --checkandcargo clippy -p spacewasm_util --all-targets --all-features -- -D warningsare both clean.Generative AI disclosure
Per
AI_POLICY.md. This change is confined tocrates/*; nosrc/*code was touched.crates/spacewasm_util/src/trace.rs,crates/spacewasm_util/tests/state_history.rs