sourcemap: bit-packed InternalSourceMap (~2.4 B/mapping, no decode) - #29358
Code review found 3 important issues
Found 3 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 3 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/StandaloneModuleGraph.zig:267-285 |
ParsedSourceMap.deinit() would free borrowed slices and use mismatched allocation size for standalone module graph PSMs |
| 🔴 Important | src/sourcemap/InternalSourceMap.zig:248-268 |
readVarint() leaves pos at unread continuation byte when shift > 28 guard fires |
| 🔴 Important | src/sourcemap/InternalSourceMap.zig:375-387 |
nextRare() accesses self.bytes[p] out-of-bounds after last gen_line_exc pair |
Annotations
Check failure on line 285 in src/StandaloneModuleGraph.zig
claude / Claude Code Review
ParsedSourceMap.deinit() would free borrowed slices and use mismatched allocation size for standalone module graph PSMs
In `ParsedSourceMap.deinit()`, the `external_source_names` cleanup block (lines 113–117 of `src/sourcemap/ParsedSourceMap.zig`) lacks the `is_standalone_module_graph` guard that the adjacent `ism.deinit()` call correctly has. For standalone module graph PSMs, `external_source_names` elements are borrowed sub-slices of `serialized.bytes` (not heap-allocated), and the backing allocation is `alloc(?[]u8, source_files.len * 2)` while `external_source_names` only covers the first half — so if `deinit
Check failure on line 268 in src/sourcemap/InternalSourceMap.zig
claude / Claude Code Review
readVarint() leaves pos at unread continuation byte when shift > 28 guard fires
When the `shift > 28` guard fires at line 255 of `readVarint()`, the loop breaks BEFORE executing `const byte = bytes[i]; i += 1;`, so `pos.* = i` points to the unread continuation byte (which always has MSB set, 0x80). The next `readVarint()` call starts from that byte and treats it as the start of a new multi-byte varint, silently misaligning all subsequent varint reads in that window stream. For valid data this path is unreachable (i32 fits in ≤5 bytes), but for a corrupted `RuntimeTranspiler
Check failure on line 387 in src/sourcemap/InternalSourceMap.zig
claude / Claude Code Review
nextRare() accesses self.bytes[p] out-of-bounds after last gen_line_exc pair
In `WindowReader.nextRare()`, after `readVarint(self.bytes, &p)` advances `p`, the code unconditionally accesses `self.bytes[p]` (line 378) without a bounds check. For corrupted blobs — a missing 0xFF terminator in the gen_line_exc section — `readVarint` can legally advance `p` to `bytes.len` (by consuming the 1-byte `stream_tail_pad`), making `self.bytes[bytes.len]` a one-past-end OOB read: a clean index-out-of-bounds panic in Debug/ReleaseSafe builds and silent undefined behavior in ReleaseFas