pq/libfreemkv
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

tests: harden UDF allocation-descriptor + bad-sector recovery paths

Browse Source 
Spec-grounded unit tests for the silent-corruption surfaces, each verified to
fail under a targeted source mutation (no vacuous tests).

udf (10): Extended-AD 20-byte stride + extent LBA at off+12, type-1 sparse
extents skipped not emitted, zero-length type-0 terminator, continuation-loop
bound (anti-hang), UTF-16BE and 8-bit name decoding, FID L_IU offset, parent
(..) FID skip, d-string length-byte cap. Locks the spec branches a future
allocation-descriptor refactor must not silently break.

recovery (9): Pass-N damage-skip range bounds (forward/reverse cursor stays in
range), one-quarter-of-remaining skip cap, below-threshold no-op, work-done
accounting, and bridge-degradation retry-to-budget fall-through.
This commit is contained in:
Matthew Jackson
2026-06-07 20:46:00 -07:00
parent 06c30aa466
commit c1b4f3cbb3
3 changed files with 636 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/disc/patch.rs
+234
View File
@@ -1856,4 +1856,238 @@ mod tests {
assert_eq!(r, Some((1000, 300)));
assert_eq!(skip_limit_remainder(true, 1000, 2000, 1000), None);
}
// ----------------------------------------------------------------
// compute_damage_skip - range-boundary + size-aware-cap coverage.
//
// These exercise the Pass-N damage-cluster skip documented in
// CLAUDE.md "Patch (Pass N)": skip is capped at 1/4 of the
// remaining bad range "so a single jump can't blow past a good
// middle", and the per-iteration cursor (`block_end`) must never
// cross the range boundary in either walk direction. A bug here
// silently abandons recoverable sectors (over-skip) or downgrades
// already-recovered sectors (cursor crossing the boundary).
//
// All byte offsets are multiples of 2048 (the sector size the code
// divides by at `range_remaining_bytes / 2048`).
// ----------------------------------------------------------------
/// Build a `PatchOptions` with only `reverse` meaningful for the
/// pure helpers under test (no I/O is performed).
fn opts_with_reverse(reverse: bool) -> crate::disc::PatchOptions<'static> {
crate::disc::PatchOptions {
decrypt: false,
block_sectors: Some(1),
full_recovery: false,
reverse,
wedged_threshold: 50,
progress: None,
halt: None,
}
}
/// A `PatchLoopState` whose damage window is full (16 entries) with
/// exactly `bad` failures - enough to evaluate the
/// `PASSN_DAMAGE_THRESHOLD_PCT` gate. `escalation` seeds
/// `consecutive_skips_without_recovery` so we can drive the
/// `PASSN_SKIP_SECTORS_BASE << escalation` size.
fn state_with_window(bad: usize, escalation: u32) -> PatchLoopState {
let mut s = PatchLoopState::new(0, 1 << 40, 1, false, 1 << 40);
s.damage_window.clear();
for i in 0..PASSN_DAMAGE_WINDOW {
s.damage_window.push(i >= bad); // first `bad` entries = false
}
s.consecutive_skips_without_recovery = escalation;
s
}
#[test]
fn damage_skip_forward_advances_cursor_toward_end_and_stays_in_range() {
// Forward walk: the per-iteration cursor moves UP, toward `end`,
// so the attempted region grows as [range_pos, block_end). A
// damage skip must push block_end FORWARD (higher) and never
// past `end` (the call site breaks on `block_end >= end`). Range
// [0, 80 KiB) = 40 sectors, cursor mid-range at 20 KiB. Spec:
// CLAUDE.md Pass-N reverse=false walks start->end.
// Mutation that makes this RED: swap the forward branch to
// subtract (reverse direction) e.g. `block_end - skip_bytes` ->
// the cursor moves the WRONG way and re-attempts recovered
// sectors / never converges. (Confirmed: the assertion
// block_end > before fails.)
let mut state = state_with_window(4, 0);
let opts = opts_with_reverse(false);
let mut frame = RangeFrame {
range_idx: 0,
range_pos: 0,
range_size: 80 * 1024,
end: 80 * 1024,
block_end: 20 * 1024, // 10 sectors in
range_budget_secs: 1,
range_sectors: 40,
};
let before = frame.block_end;
let did = compute_damage_skip(&mut state, &mut frame, &opts, 0, 2048);
assert!(did, "threshold crossed: a skip must apply");
assert!(
frame.block_end > before,
"forward skip must advance the cursor UP (toward end): {} !> {}",
frame.block_end,
before
);
assert!(
frame.block_end <= frame.end,
"forward cursor {} overshot range end {}",
frame.block_end,
frame.end
);
}
#[test]
fn damage_skip_reverse_moves_cursor_toward_range_start_and_stays_in_range() {
// Reverse walk (the recovery walker default): the cursor moves
// DOWN, toward `range_pos`, so the attempted region grows as
// [block_end, end). A damage skip must push block_end BACKWARD
// (lower) and never below `range_pos` (the call site breaks on
// `block_end <= range_pos`). Spec: CLAUDE.md "Patch (Pass N) -
// Default: reverse mode ... within each range from end to start."
// Mutation that makes this RED: the reverse branch adds instead
// of subtracts (copy-paste of the forward formula) -> cursor
// moves UP, away from range_pos, and the walk never converges on
// the low end of the range, silently abandoning those sectors.
let mut state = state_with_window(4, 0);
let opts = opts_with_reverse(true);
let range_pos = 40 * 1024;
let mut frame = RangeFrame {
range_idx: 0,
range_pos,
range_size: 80 * 1024,
end: range_pos + 80 * 1024,
block_end: range_pos + 60 * 1024, // 30 sectors above range_pos
range_budget_secs: 1,
range_sectors: 40,
};
let before = frame.block_end;
let did = compute_damage_skip(&mut state, &mut frame, &opts, 0, 2048);
assert!(did, "threshold crossed: a skip must apply");
assert!(
frame.block_end < before,
"reverse skip must move the cursor DOWN (toward range_pos): {} !< {}",
frame.block_end,
before
);
assert!(
frame.block_end >= frame.range_pos,
"reverse cursor {} descended below range_pos {}",
frame.block_end,
frame.range_pos
);
}
#[test]
fn damage_skip_caps_at_one_quarter_of_remaining_range() {
// CLAUDE.md: skip "capped at 1/4 of the remaining bad range so
// a single jump can't blow past a good middle." Forward walk,
// range [0, 80 KiB) = 40 sectors, cursor at start (block_end=0)
// so remaining = 40 sectors and quarter = 10 sectors. Drive a
// large escalation so the raw escalated skip far exceeds 10.
// The applied gap must be <= quarter (10 sectors = 20480 bytes).
// Mutation that makes this RED: remove `.min(range_quarter)`
// from `skip_sectors` -> the jump leaps the entire good middle.
let mut state = state_with_window(4, 10);
let opts = opts_with_reverse(false);
let mut frame = RangeFrame {
range_idx: 0,
range_pos: 0,
range_size: 80 * 1024,
end: 80 * 1024,
block_end: 0,
range_budget_secs: 1,
range_sectors: 40,
};
let did = compute_damage_skip(&mut state, &mut frame, &opts, 0, 2048);
assert!(did, "threshold crossed: a skip must apply");
let quarter_bytes = (40u64 / 4) * 2048; // 10 sectors
assert!(
frame.block_end <= quarter_bytes,
"skip advanced cursor to {} bytes, exceeding the 1/4 cap of {} bytes",
frame.block_end,
quarter_bytes
);
assert!(frame.block_end > 0, "a real skip must advance the cursor");
}
#[test]
fn damage_skip_below_threshold_does_not_skip_or_mutate_state() {
// With an all-good window (bad=0) the damage threshold is NOT
// crossed, so compute_damage_skip must be a no-op: it must NOT
// advance the cursor, increment skip_count, or charge work_done.
// A spurious skip here silently abandons readable sectors that
// the patch loop would otherwise retry.
// Mutation that makes this RED: invert/weaken the threshold
// guard (e.g. `>=` -> `<`) so a clean window still skips.
let mut state = state_with_window(/*bad=*/ 0, /*escalation=*/ 0);
let opts = opts_with_reverse(false);
let work_before = state.work_done;
let skips_before = state.skip_count;
let mut frame = RangeFrame {
range_idx: 0,
range_pos: 0,
range_size: 80 * 1024,
end: 80 * 1024,
block_end: 4096,
range_budget_secs: 1,
range_sectors: 40,
};
let did = compute_damage_skip(&mut state, &mut frame, &opts, 0, 2048);
assert!(!did, "clean window must not trigger a damage skip");
assert_eq!(frame.block_end, 4096, "cursor must not move on a no-op");
assert_eq!(
state.work_done, work_before,
"no-op must not charge work_done"
);
assert_eq!(
state.skip_count, skips_before,
"no-op must not bump skip_count"
);
}
#[test]
fn damage_skip_work_done_equals_actual_gap_skipped() {
// Progress accounting: when a skip fires, `work_done` must grow
// by EXACTLY the number of bytes the cursor moved (the gap),
// and skip_count must increment by exactly 1. Reverse range
// [0, 64 KiB) = 32 sectors, cursor at the top so remaining =
// 32, quarter = 8 sectors, escalation 0 -> escalated = base
// (32) capped to quarter (8). Expected gap = 8 sectors.
// Mutation that makes this RED: compute `gap_bytes` from the
// wrong endpoints or double-add it.
let mut state = state_with_window(/*bad=*/ 4, /*escalation=*/ 0);
let opts = opts_with_reverse(true);
let end = 64 * 1024;
let mut frame = RangeFrame {
range_idx: 0,
range_pos: 0,
range_size: end,
end,
block_end: end,
range_budget_secs: 1,
range_sectors: 32,
};
let before = frame.block_end;
let work_before = state.work_done;
let did = compute_damage_skip(&mut state, &mut frame, &opts, 0, 2048);
assert!(did, "threshold crossed: a skip must apply");
let expected_gap = 8 * 2048u64; // min(base=32, quarter=8) sectors
let actual_gap = before - frame.block_end; // reverse: cursor moved down
assert_eq!(
actual_gap, expected_gap,
"reverse skip should move the cursor down by the quarter-cap gap"
);
assert_eq!(
state.work_done - work_before,
actual_gap,
"work_done must grow by exactly the gap skipped"
);
assert_eq!(state.skip_count, 1, "exactly one skip must be recorded");
}
}
src/disc/read_error.rs
+166
View File
@@ -1205,4 +1205,170 @@ mod tests {
assert_eq!(ctx.consecutive_failures, 0);
assert!(*ctx.damage_window.last().unwrap());
}
// ----------------------------------------------------------------
// Additional hardening: retry-budget boundaries, transport-abort
// precedence, and the bounded-jump invariant. These guard against
// off-by-one in the retry caps (which would either hammer a wedging
// drive or give up a recovery one attempt early) and against an
// unbounded jump multiplier skipping the rest of the disc.
// ----------------------------------------------------------------
/// NOT_READY check-condition (status 0x02 so it is NOT classified as
/// bridge degradation, which keys off non-standard status bytes).
/// sense_key=2 with a generic ASC routes to the NOT_READY retry path.
fn not_ready_err() -> Error {
Error::DiscRead {
sector: 100,
status: Some(crate::scsi::SCSI_STATUS_CHECK_CONDITION),
sense: Some(ScsiSense {
sense_key: scsi::SENSE_KEY_NOT_READY,
asc: 0x04,
ascq: 0x00, // not 0x3E, so not the bridge-degradation signature
}),
}
}
/// Transport failure: SCSI status 0xFF (bridge crash). CLAUDE.md
/// "Bad-sector handling": this aborts the copy.
fn transport_failure_err() -> Error {
Error::DiscRead {
sector: 100,
status: Some(crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE),
sense: None,
}
}
/// Bridge degradation: a non-standard status byte (0x04 - neither
/// GOOD/CHECK/TRANSPORT) with empty sense, per `Error::is_bridge_degradation`.
fn bridge_degradation_err() -> Error {
Error::DiscRead {
sector: 100,
status: Some(0x04),
sense: None,
}
}
#[test]
fn not_ready_retries_capped_at_three_then_falls_through() {
// CLAUDE.md "Bad-sector handling" mode 1: NOT READY -> "Pause 3s,
// retry up to 3x, then mark NonTrimmed." NOT_READY_MAX_RETRIES=3.
// The 1st-3rd NOT_READY must Retry; the 4th must NOT Retry (it
// falls through to skip). Pass N (batch=1) so the marginal-bisect
// branch is irrelevant.
// Mutation that makes this RED: change `ctx.not_ready_retries <
// NOT_READY_MAX_RETRIES` to `<=` (retries 4 times) or to `>`
// (never retries).
let mut ctx = ReadCtx::for_patch(1);
for i in 0..NOT_READY_MAX_RETRIES {
let a = handle_read_error(&not_ready_err(), &mut ctx);
assert!(
matches!(a, ReadAction::Retry { .. }),
"NOT_READY attempt {i} should Retry, got {a:?}"
);
}
// Budget exhausted: the next NOT_READY must not Retry.
let a = handle_read_error(&not_ready_err(), &mut ctx);
assert!(
!matches!(a, ReadAction::Retry { .. }),
"NOT_READY past the retry cap must fall through, got {a:?}"
);
}
#[test]
fn transport_failure_aborts_even_mid_bisect() {
// CLAUDE.md "Bad-sector handling" mode 2: a transport failure
// (bridge crash, status 0xFF) aborts the pass so the outer loop
// can re-enumerate the bridge. This must hold even while
// bisecting and even on Pass N - the wedge-skip/jump paths must
// NOT swallow a real transport crash into a JumpAhead.
// Mutation that makes this RED: move the transport-failure check
// below the HARDWARE/ILLEGAL wedge arm, so a transport failure
// that also carried a wedge-family sense would JumpAhead instead.
let mut ctx = ReadCtx::for_patch(32);
ctx.bisecting = true;
assert_eq!(
handle_read_error(&transport_failure_err(), &mut ctx),
ReadAction::AbortPass
);
// And on a fresh Pass 1 context, still AbortPass.
let mut ctx1 = ReadCtx::for_sweep(32);
assert_eq!(
handle_read_error(&transport_failure_err(), &mut ctx1),
ReadAction::AbortPass
);
}
#[test]
fn bridge_degradation_retries_to_budget_then_falls_through() {
// The bridge-degradation cooldown retry is bounded by
// BRIDGE_DEGRADATION_MAX_RETRIES (=5). The first 5 degradation
// errors must Retry with the long bridge cooldown; the 6th must
// fall through to skip/jump rather than retrying forever and
// stalling the pass.
// Mutation that makes this RED: change the budget comparison
// `ctx.bridge_degradation_count < BRIDGE_DEGRADATION_MAX_RETRIES`
// to `<=` (retries 6 times).
let mut ctx = ReadCtx::for_patch(1);
for i in 0..BRIDGE_DEGRADATION_MAX_RETRIES {
let a = handle_read_error(&bridge_degradation_err(), &mut ctx);
match a {
ReadAction::Retry { pause_secs } => {
assert_eq!(
pause_secs, BRIDGE_DEGRADATION_PAUSE_SECS,
"bridge retry {i} should use the bridge cooldown"
);
}
other => panic!("bridge degradation attempt {i} should Retry, got {other:?}"),
}
}
let a = handle_read_error(&bridge_degradation_err(), &mut ctx);
assert!(
!matches!(a, ReadAction::Retry { .. }),
"bridge degradation past the retry budget must fall through, got {a:?}"
);
}
#[test]
fn jump_multiplier_caps_and_jump_distance_stays_bounded() {
// CLAUDE.md damage-jump: multiplier doubles per jump but is
// capped at MAX_JUMP_MULTIPLIER=64 (the "4 GiB cap"); a single
// jump must never be allowed to grow without bound and skip the
// rest of the disc. Drive a long single-sector failure streak on
// a sweep ctx with a tiny window so window-trigger jumps fire
// repeatedly, and verify the multiplier saturates at 64 and the
// emitted jump distance equals JUMP_BASE_SECTORS * batch * 64.
// Mutation that makes this RED: remove the
// `.min(MAX_JUMP_MULTIPLIER)` on the multiplier doubling, or use
// wrapping/non-saturating mul -> distance overshoots or panics.
const MAX_JUMP_MULTIPLIER: u64 = 64;
let batch: u16 = 32;
let mut ctx = ReadCtx::for_sweep(batch);
// Small window + 0% threshold so every failure can window-trigger
// a jump and keep doubling the multiplier toward the cap.
ctx.damage_window_max = 2;
ctx.damage_threshold_pct = 0;
let mut last_jump_sectors = 0u64;
for _ in 0..40 {
// Reset bisecting flag defensively; these are outer failures.
ctx.bisecting = false;
if let ReadAction::JumpAhead { sectors, .. } =
handle_read_error(&medium_err(), &mut ctx)
{
last_jump_sectors = sectors;
}
assert!(
ctx.jump_multiplier <= MAX_JUMP_MULTIPLIER,
"jump_multiplier {} exceeded the cap {}",
ctx.jump_multiplier,
MAX_JUMP_MULTIPLIER
);
}
// After saturation, the jump distance is exactly base*batch*cap.
let expected = JUMP_BASE_SECTORS * batch as u64 * MAX_JUMP_MULTIPLIER;
assert_eq!(
last_jump_sectors, expected,
"saturated jump distance must equal base*batch*64"
);
}
}