General guard for the over-read class, and the audit showing it was unique
Searched for the pattern that caused the over-read -- a record length derived from
the first difference between two offsets:
grep -rn "\[1\] - \[0\]" crates/gc-formats/src/*.rs
crates/gc-formats/src/chunks.rs:357 (offsets[1] - offsets[0])
One hit, in the one parser that walks an explicit offset table.
The others slice contiguous regions with chunks_exact -- shrub's 8-byte palette
records, tie's 16-byte packet headers, tfrag's 4-byte tables. There a length
cannot run past the next element, because chunks_exact derives each boundary from
the region's own length rather than from a sample of the spacing. Shrub's mesh
packets carry an explicit offset AND size, so they state their length instead of
inferring it.
The damage was bounded by a structural property worth stating: only a format with
an offset table AND a derived stride can over-read, and exactly one format had
both. That is a more useful fact than the retraction was on its own -- it says
where else to look if another one turns up.
The guard is positive rather than a comment.
chunk_records_neither_overlap_nor_leave_gaps asserts across all 27 levels that
every record but the last is exactly its own offset delta long, that there are
1,509 chunks, and that 1,252 have ragged deltas. A regression to a single sampled
stride fails immediately and names the chunk and the record.
245 tests, clippy clean.
This commit is contained in:
@@ -4624,3 +4624,73 @@ fn chunk_records_share_a_layout_measured_by_constant_byte_positions() {
|
||||
"constant byte positions left the 10-30% band: {lo:.3}..{hi:.3}"
|
||||
);
|
||||
}
|
||||
|
||||
/// No two records in a chunk may overlap, and `resolve` must reproduce the
|
||||
/// offset table exactly.
|
||||
///
|
||||
/// This is the general guard for the class of bug that was in `resolve` when it
|
||||
/// sliced every record at the chunk's first offset delta. That produced records
|
||||
/// that ran 4 to 8 bytes into their successors on 1,252 of 1,509 chunks, which
|
||||
/// is how a committed claim about moby class IDs evaporated.
|
||||
///
|
||||
/// `chunks` is the only format in `gc-formats` that walks an explicit offset
|
||||
/// table and derives a stride from it -- searched, and the other parsers slice
|
||||
/// contiguous regions with `chunks_exact`, where a length cannot run past the
|
||||
/// next element. So the invariant belongs here, stated positively: consecutive
|
||||
/// records abut, and each record's slice is exactly the distance to the next.
|
||||
#[test]
|
||||
fn chunk_records_neither_overlap_nor_leave_gaps() {
|
||||
require_disc!();
|
||||
|
||||
let data = std::env::var_os("GC_EXTRACTED")
|
||||
.map(PathBuf::from)
|
||||
.unwrap_or_else(|| {
|
||||
let home = std::env::var("HOME").unwrap_or_else(|_| ".".to_string());
|
||||
PathBuf::from(home).join("Documents/gc-extracted")
|
||||
});
|
||||
if !data.exists() {
|
||||
eprintln!("skipped: unpack the disc first");
|
||||
return;
|
||||
}
|
||||
|
||||
use gc_formats::chunks::parse_all_chunks;
|
||||
|
||||
let mut chunks = 0usize;
|
||||
let mut ragged = 0usize;
|
||||
for lv in 0..27usize {
|
||||
let Ok(core) = std::fs::read(data.join(format!("level{lv}/core_data.bin"))) else {
|
||||
continue;
|
||||
};
|
||||
for c in parse_all_chunks(&core).expect("chunks") {
|
||||
chunks += 1;
|
||||
let offs = &c.chunk.offsets;
|
||||
if offs.len() < 2 {
|
||||
continue;
|
||||
}
|
||||
let deltas: Vec<u32> = offs.windows(2).map(|w| w[1] - w[0]).collect();
|
||||
if !deltas.windows(2).all(|w| w[0] == w[1]) {
|
||||
ragged += 1;
|
||||
}
|
||||
// every record except the last is exactly its own delta long
|
||||
for (i, (record, want)) in c
|
||||
.records
|
||||
.iter()
|
||||
.zip(&deltas)
|
||||
.take(c.records.len().saturating_sub(1))
|
||||
.enumerate()
|
||||
{
|
||||
assert_eq!(
|
||||
record.len(),
|
||||
*want as usize,
|
||||
"level{lv} chunk at {:#x}: record {i} is {} bytes, \
|
||||
but its offset delta is {want}",
|
||||
c.chunk.offset,
|
||||
record.len()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
eprintln!("chunk record boundaries: {chunks} chunks, {ragged} with ragged deltas");
|
||||
assert_eq!(chunks, 1509, "chunk total moved");
|
||||
assert_eq!(ragged, 1252, "ragged chunk count moved");
|
||||
}
|
||||
|
||||
@@ -2486,3 +2486,30 @@ comparing only the span every record in the chunk has:
|
||||
|
||||
Still inside the documented band, now asserted rather than only described, in
|
||||
`chunk_records_share_a_layout_measured_by_constant_byte_positions`.
|
||||
|
||||
## The over-read bug was unique to one format, and why
|
||||
|
||||
Searched for the pattern that caused it -- a record length derived from the
|
||||
*first* difference between two offsets:
|
||||
|
||||
grep -rn "\[1\] - \[0\]" crates/gc-formats/src/*.rs
|
||||
crates/gc-formats/src/chunks.rs:357 (offsets[1] - offsets[0])
|
||||
|
||||
One hit, in the one parser that walks an explicit offset table.
|
||||
|
||||
The other parsers slice contiguous regions with `chunks_exact` -- shrub's 8-byte
|
||||
palette records, tie's 16-byte packet headers, tfrag's 4-byte tables. There, a
|
||||
length cannot run past the next element, because `chunks_exact` derives each
|
||||
boundary from the region's own length rather than from a sample of the spacing.
|
||||
Shrub's mesh packets are named by an explicit offset *and size* table, so they
|
||||
carry their length rather than inferring it.
|
||||
|
||||
So the damage was bounded by a structural property: **only a format with an
|
||||
offset table and a derived stride can over-read**, and exactly one format had
|
||||
both.
|
||||
|
||||
The general guard is now positive rather than a comment.
|
||||
`chunk_records_neither_overlap_nor_leave_gaps` asserts, across all 27 levels,
|
||||
that every record except the last is exactly its own offset delta long, and that
|
||||
1,509 chunks contain 1,252 with ragged deltas. A regression to a single sampled
|
||||
stride fails immediately and says which chunk and which record.
|
||||
|
||||
Reference in New Issue
Block a user