Fix bit manipulation overflow, VCD ID overflow, and corner tile detection#14
Open
hobostay wants to merge 1 commit intoMidstallSoftware:masterfrom
Open
Fix bit manipulation overflow, VCD ID overflow, and corner tile detection#14hobostay wants to merge 1 commit intoMidstallSoftware:masterfrom
hobostay wants to merge 1 commit intoMidstallSoftware:masterfrom
Conversation
…tion
1. Fix integer overflow in bitstream read/write (tile_bits.rs)
- `1 << i` defaults to i32, panics in debug when i >= 32
- `write_bits`: changed to `1u64 << i` to match the u64 value type
- `read_bits`: changed to `1u64 << i` to match the u64 accumulator
- `clear_bits`: changed to `1u8 <<` for the byte mask type
- Without this fix, configs wider than 32 bits would panic or corrupt
2. Fix VCD writer signal ID overflow (aegis-sim/src/lib.rs)
- `next_id` would wrap past '~' (126) into non-printable/control
characters, producing invalid VCD files
- Added guard that panics with a clear message when the limit is reached
3. Fix incorrect corner tile detection in nextpnr (aegis.cc)
- Used `x == y` / `x != y` to skip corner tiles, which only works
for square grids (W == H)
- For non-square grids (e.g. 3x5 fabric), this would skip wrong tiles
(e.g. tile at (5,0) is a corner but x != y) or fail to skip actual
corners (e.g. tile at (0,6) when W=5)
- Fixed in all three locations: init_wires, init_bels, init_pips
- Now correctly checks all four corners regardless of grid aspect ratio
Co-Authored-By: Claude Opus 4.6 <[email protected]>
RossComputerGuy
requested changes
Apr 9, 2026
| if id < '~' { | ||
| self.next_id = (self.next_id as u8 + 1) as char; | ||
| } else { | ||
| panic!("VcdWriter: too many signals for single-character VCD IDs"); |
Member
There was a problem hiding this comment.
Should return an error, please use thiserror to define a new error type.
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
2 participants
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.
Summary
Fix integer overflow in bitstream read/write (
crates/aegis-ip/src/tile_bits.rs) —1 << idefaults toi32in Rust, which panics in debug mode wheni >= 32. Changed to1u64 << iand1u8 <<for correct types.Fix VCD writer signal ID overflow (
crates/aegis-sim/src/lib.rs) —next_idwraps past'~'into invalid characters. Added guard to panic with clear message.Fix incorrect corner tile detection in nextpnr (
nextpnr-aegis/aegis.cc) —x == yonly works for square grids. Fixed to explicitly check all four corners.Test plan
🤖 Generated with Claude Code