Measure every chunk's record offsets; the cause is ambiguous, so no fix
The previous commit fixed on one signature. Measuring all of them shows the
pattern is the norm. Taking delta = offset[i+1] - offset[i] against the chunk's
own first delta, levels 0-2, 2,953 deltas:
0 1,723 -4 549 +4 426
-8 146 +8 32
-12 71 +12 21
-16 34 +16 4
-20 27 +20 1
-24 13
Every deviation is a multiple of 4 and most are exactly +/-4; worst observed is 24
bytes in a 7-record chunk. 1,252 of 1,509 chunks have at least one, across 66 of
74 signatures.
WHY THIS DOES NOT BECOME A FIX
Two models produce this offset table and they are observationally identical:
1. records are genuinely variable-length, padded to 4 bytes, stride = largest
2. records are fixed at stride and the offset table carries inter-record
padding, so the gap to the next record is record + padding
Nothing in the offsets distinguishes them. Both say where records start. Both
produce 92, 88, 84, 92, 92, 92, 84, 88 for the case already discussed.
I wrote the change that slices each record against its successor -- model 1 -- and
then reverted it. Two things came out of writing it:
First, it broke three tests, which is the informative part. Those tests assert
"1,509 of 1,509 chunks have uniform record length" and they passed BECAUSE the
parser forced uniformity by slicing every record at a single stride. They were
asserting the parser's behaviour back at itself, and would have kept passing
forever regardless of what the disc contains.
Second, shipping it would have replaced one unverified model with another and
silently changed the record boundaries of 1,252 chunks on the strength of a
guess. The repo's own rule applies: a heuristic count is not an identification,
and here it is not even a count -- it is two equally good explanations of one
table.
WHAT WOULD SETTLE IT: a field inside the record whose end is independently
observable -- a terminator, a count implying a length, or a known value pinning
its position. Under model 2 the disputed bytes are padding and will be uniform or
zero; under model 1 they are data and will not be. For [75 42 ef] those 8 bytes
read a0 00 27 20 at both the tail of one slice and the head of the next, which is
arithmetic rather than evidence, since it is the same eight bytes seen twice.
The uniform model stays, the ambiguity is recorded in resolve()'s documentation
and here, and the 1,509-of-1,509 assertion stands as a statement about what the
parser does rather than about what the disc contains. 243 tests, clippy clean.
This commit is contained in:
@@ -102,11 +102,13 @@ pub const CHUNK_REGION_MARKER: [u8; 2] = [0xf6, 0x7c];
|
||||
|
||||
/// A chunk header plus the records its offsets address.
|
||||
///
|
||||
/// `stride` is the distance between the first two resolved offsets. Every
|
||||
/// offset after the first is `stride` from its predecessor within a few bytes
|
||||
/// on the measured levels, but the parser takes each offset literally rather
|
||||
/// than assuming a uniform stride, because the deltas are not exactly constant
|
||||
/// (e.g. 340, 340, 336, 340 on level 0).
|
||||
/// `stride` is the distance between the first two resolved offsets, and nothing
|
||||
/// more. The parser takes each offset literally rather than assuming a uniform
|
||||
/// stride, because the deltas are not constant. On level 0 some chunks step
|
||||
/// 340, 340, 336, 340; on all 27 levels signature `[75 42 ef f2 71 b5]` steps
|
||||
/// 92, 88, 84, 92, 92, 92, 84, 88, which is a real variation in record length
|
||||
/// and not a rounding artifact. Record *lengths* come from the deltas, in
|
||||
/// [`resolve`].
|
||||
pub struct Chunk {
|
||||
/// The 16 opaque header bytes.
|
||||
pub header: [u8; CHUNK_HEADER_BYTES],
|
||||
@@ -121,6 +123,7 @@ pub struct Chunk {
|
||||
/// The low 24 bits of each tagged offset, in file order.
|
||||
pub offsets: Vec<u32>,
|
||||
/// Record stride, from the first two offsets. Zero if fewer than two.
|
||||
///
|
||||
pub stride: usize,
|
||||
/// The `u16` immediately before the first record's payload -- the record's
|
||||
/// own leading word, which the loaded table points at.
|
||||
@@ -388,11 +391,23 @@ pub fn parse_chunk(region: &[u8], offset: usize) -> Result<Option<Chunk>> {
|
||||
|
||||
/// Resolve every record a chunk addresses, bounds-checked against `region`.
|
||||
///
|
||||
/// Each record is `region[chunk.offset + low .. + stride]`. A record that
|
||||
/// would run past the end of the region is dropped rather than clamped, so the
|
||||
/// returned length may be shorter than `chunk.offsets` -- on the measured
|
||||
/// levels it is not, but a parser that silently clamps would hide a malformed
|
||||
/// chunk rather than report it.
|
||||
/// Each record is `region[chunk.offset + low .. + stride]`, and `stride` is the
|
||||
/// chunk's first offset delta.
|
||||
///
|
||||
/// **This may slice past a record's real end.** 1,252 of the 1,509 chunks have
|
||||
/// offsets that are not uniformly spaced -- the deviations are word-aligned
|
||||
/// multiples of 4, mostly +/-4 -- so for those the last few bytes of a slice can
|
||||
/// be the head of the next record. Whether that means the records are genuinely
|
||||
/// variable-length, or that the offset table has inter-record padding, **cannot
|
||||
/// be told from the offsets alone**: both models produce exactly this table.
|
||||
/// Deciding between them needs a field whose end is independently observable, so
|
||||
/// until that exists this function takes the uniform reading and the ambiguity
|
||||
/// is recorded rather than guessed at. See `docs/formats/core-data-regions.md`.
|
||||
///
|
||||
/// A record that would run past the end of the region is dropped rather than
|
||||
/// clamped, so the returned length may be shorter than `chunk.offsets` -- on the
|
||||
/// measured levels it is not, but a parser that silently clamps would hide a
|
||||
/// malformed chunk rather than report it.
|
||||
pub fn resolve<'r>(chunk: &Chunk, region: &'r [u8]) -> Result<ResolvedChunk<'r>> {
|
||||
let mut records = Vec::with_capacity(chunk.offsets.len());
|
||||
for &low in &chunk.offsets {
|
||||
|
||||
Reference in New Issue
Block a user