test(webp): close the crate's first mutation survey by justin13888 · Pull Request #496 · visualcommons/gamut

justin13888 · 2026-09-02T21:21:26Z

Refs #110.

gamut-webp's first mutation survey: 3290 caught, 30 survivors across 18 sites, 7 timeouts —
the largest crate in the workspace at 3422 mutants. All 30 are closed here.

The crate's own blind spot: a stream that is valid but not the one intended

The libwebp differential asks whether a stream is conformant and self-consistent, not whether the
encoder chose what it meant to — and several survivors live precisely in that gap.

The clearest is the per-macroblock segment lookup. segment_map[mb_y * mb_cols + mb_x] can be
indexed with mb_y / mb_cols and everything still passes, because the segment a macroblock was
assigned is recorded in MbRecord::segment and transmitted: both decoders read the same declared
map, agree with each other, and agree with the encoder's own reconstruction. What changes is only
which quantizer each macroblock got — a worse encode that is still a valid one.

That took three attempts to pin, and the failures were instructive. recon == decode cannot see
it (the stream is coherent). Nor can the obvious fixture: pattern's wrapping ramp sweeps 0..255
inside every macroblock, so each averages near 128, every one lands in the same bucket, and the
segment map comes out uniform — where any index is as good as any other. It needed banded content
and a byte-exact assertion, the same answer the tiff codecs needed.

The same fixture later grew from two bands to four, because SEGMENT_QUANT_DELTAS is
[-12, -4, 4, 12] and a dark/bright split only reaches segments 0 and 3 — so half the deltas were
never applied to anything.

Untested paths, not weak assertions

A skipped B_PRED macroblock. reconstruct_bpred_zero could be replaced with () outright:
B_PRED is covered, skipping is covered, never the two together. The fixture had to be measured
rather than guessed — detailed is too contrasty to skip even at q=127, flat content is too
plain for B_PRED to win — and a coarse 4-pixel step sits between them. Making that path reachable
closed three of the 30 at once.
A solid-colour image, which nothing encoded losslessly. An instrumented run showed the
code-length trim never went below 7 entries anywhere in the suite; solid content drives it to its
floor of 4.
A partition ≥ 64 KiB, so the third byte of the 24-bit size was always zero — on both the read
and the write side.
A sparse alphabet in canonical_codes, and the two degenerate histograms
build_length_limited_lengths documents but nothing asserted.

Equivalent mutants, removed rather than excluded where possible

Two disjoint-lane | sites (the 24-bit partition size, read and written) join the workspace tally —
now 13 instances across five crates. And value_to_prefix computed its extra value as
v - offset, where both offsets are multiples of 2^extra_bits and the result is always smaller:
the subtraction only ever cleared bits the write clears anyway, so one of its two shifts was
untestable while the other was not. Masking says what the format means and leaves no slack.

tokenize's if cache_bits > 0 guard restated a rejection ColorCache::new already makes;
deleting it removes the mutant outright.

Making exclusions precise enough to be honest

Where a mutant genuinely cannot be killed, the exclusion has to name it without silencing its
neighbours — and in this crate the neighbours kept being real gaps:

function	equivalent	but its sibling	fix
`write_description`	the trim floor	`symbol0 > 1` and the `max_symbol` trim are both real gaps	extracted `code_length_count`
`get_bool`	`range` renormalisation hangs	reversing the `value` shift is caught	wrote `range *= 2`
`build_length_limited_lengths`	the used-count `>=`	the `find`'s `> 0` fails 16 tests	wrote `h != 0`

Each is the same move: give the statement a mutation description of its own, so the exclusion can
name an operator instead of a function (which would hide the gap) or a line number (which stops
matching the moment anything above it moves).

The five hang exclusions follow the same discipline. i *= 1 pins a cursor and the loop never
returns; i -= 1 underflows and panics, so it terminates, is caught, and stays in the survey.
Each hang was confirmed by applying it under a hard timeout rather than trusting the TIMEOUT label.

Validation

cargo test -p gamut-webp --all-features, mise run fmt-check, mise run check-tests, clippy and
mise run check-commits all clean. Every mutant was applied by hand and confirmed; where the
verification harness itself failed (an ambiguous anchor, a mis-called helper) it now refuses to
report rather than printing a verdict it did not earn.

`canonical_codes` was only ever handed fully dense length arrays, so the assignment was pinned nowhere for the case that actually occurs -- a VP8L alphabet almost always has unused symbols. The test asserts the RFC 1951 section 3.2.2 assignment for a small alphabet and, separately, that prepending an unused symbol leaves every other symbol's code untouched. The mutants that pointed here are a different matter: both of that function's guards are equivalent under two of their mutations. I expected a coverage gap and wrote this test to close it; applying the mutant showed it still passed, which is what sent me looking for a reason rather than assuming one. COUNTING GUARD. Under `||` -- or `>=`, which admits a zero length the same way -- a zero is counted into `bl_count[0]`, read back at `bits == 1`, so `next_code[L]` becomes `correct + 2^L * z` for `z` unused symbols. Every code is emitted through `reverse_bits(c, len)`, whose loop consumes only the low `len` bits, and `2^L * z` is congruent to 0 modulo `2^L`. The offset is exactly a multiple of the mask at every length. ASSIGNMENT GUARD. A zero-length symbol writes `reverse_bits(c, 0)`, whose loop runs zero times and returns 0 -- already the slot's value -- and the `next_code[0] += 1` beside it is dead, since nothing reads that entry. Four mutants in all, each confirmed by applying it. The exclusions name `>=` and `||` specifically rather than every comparison in the function, because the neighbouring `== 0` is *not* equivalent -- it inverts the guard, and it is caught. That one stays in the survey, which is checked against `--list`. The test stays regardless. It pins behaviour that was genuinely unpinned, and its doc says plainly that it does not kill those mutants. Refs #110

`segment_map[mb_y * mb_cols + mb_x]` can be indexed with `mb_y / mb_cols` instead and every check in the suite still passes. Three things had to be true for that, and each one took a failed attempt to find. The segment a macroblock was assigned is kept in `MbRecord::segment` and transmitted from there, so a stream built with the wrong lookup is internally coherent: both decoders read the same declared map, agree with each other, and agree with the encoder's reconstruction. So `recon == decode` cannot see it, and neither can the libwebp differential -- it asks whether the stream is valid and consistent, not whether the encoder chose what it meant to. The loop filter *does* read the true map, but this configuration sets `filter_strength: [0; 4]`, so every segment filters identically. And the obvious fixture does not help. `pattern`'s wrapping ramp sweeps the full 0..255 range inside every macroblock, so each averages near 128, every one lands in the same bucket, and the segment map comes out uniform -- where any index is as good as any other. The new `banded` fixture splits dark over bright across macroblock *rows*, which is the axis a wrong row stride gets wrong. What the mutant actually changes is which quantizer each macroblock got: a worse encode that is still a valid one. That is the tiff codecs' blind spot again, and it takes the same answer -- assert the output, not its self-consistency. So the segmented stream is pinned byte for byte, on the contract `tests/default_bytes.rs` already sets: re-pinning is expected, and the commit that moves the number says why. The recon-equals-decoder invariant is also extended to a segmented encode. It does not catch this mutant, and its doc says so; it is worth having on its own, since that invariant had only ever run with segmentation off. Refs #110

Three mutants in `write_description`, all of the same kind: they change how many bits a description spends without changing what it means. Every width and count they touch is transmitted, so a decoder reads the result correctly either way -- only the size moves, which nothing asserted. `symbol0 > 1` picks a 1-bit or 8-bit field for a simple code's first symbol. `>= 1` misjudges only a first symbol of exactly 1, and no fixture had one, so the description silently grew seven bits. Now asserted at 0, 1 and 2 -- the boundary and both sides of it. `rposition(|&l| l > 0)` finds the last used symbol so the description can stop there. Relaxed to `>= 0` it is true for every `u8`, so the search returns the last index whatever the lengths are and the trimmed variant stops trimming. Asserted as the property that matters: trimming must be shorter than not trimming. The third is the trim's floor, `count > 4`, and that one is equivalent. Reaching 3 would underflow the caller's `- 4`, but the loop cannot get there: `CODE_LENGTH_CODE_ORDER` begins `[17, 18, 0, 1, 2, ...]`, so every length value 1..=15 sits at index 3 or later, and any real prefix code has some symbol of nonzero length -- which puts a nonzero entry at index >= 3 and stops the loop on the zero check. The floor is never the binding condition. That loop is lifted into `code_length_count` so the exclusion can name it. All three mutants read as `replace > with >= in write_description`, differing only by line and column, so an exclusion written for that function would have hidden the two real gaps above -- and line-anchoring it would break the moment anything above it moved. `the_code_length_trim_stops_at_four_entries` pins that the trim does reach 4, on the one- and two-symbol alphabets a solid image produces. Nothing had encoded one: an instrumented run of the whole suite never drove the count below 7, because every fixture is a busy image. The two-symbol case gets there because `simple_symbols` declines any symbol above 0xff and the second symbol is 273 -- a backward-reference length, which is how a solid image codes its repetition. Refs #110

`tokenize` wrote `if cache_bits > 0 { ColorCache::new(cache_bits).ok() } else { None }`, and `ColorCache::new` already rejects anything outside `MIN_CACHE_BITS..=MAX_CACHE_BITS`. Since `MIN_CACHE_BITS` is 1, zero comes back as an error and `.ok()` turns it into the same `None` the `else` produced. So the guard restated a rejection the constructor makes, and that left `> 0` with an unkillable `>= 0` twin: on a `u32` the mutated condition is always true, and the branch it then takes returns `None` for zero anyway. Removing the guard removes the mutant, which is what #110 prefers over an exclusion, and it is behaviour-preserving at every input: 1..=11 construct, everything else is `None` either way. The other 22 mutants in `tokenize` stay in the survey. Refs #110

…byte A token-partition size is 24-bit little-endian, spelled `s[0] | s[1] << 8 | s[2] << 16` -- one byte per lane, and disjoint lanes are where `|` and `^` agree, so those operators carried unkillable twins. It reads as `u32::from_le_bytes([s[0], s[1], s[2], 0])` now, which is the same fix gamut-png's bigram index and gamut-deflate's hash took. Thirteenth instance of the shape in this workspace. Dropping the high byte entirely also went unnoticed, because every partition in the suite is under 64 KiB and that byte was always zero. The new test asserts the boundary from both sides: a buffer exactly long enough for a 65536-byte partition is accepted, one byte shorter is refused. A reader that ignores the high byte computes a size of zero and accepts both. Byte-order and dropped-byte mutations of the new form are each confirmed caught. Two hang mutants are excluded with their reasons. `decode_image_data` advances `i` with `i += 1` in three branches of one `while i < n` loop; `*= 1` pins it in each and the loop never returns. `code_length_count` counts down with `count -= 1`; `/= 1` leaves it unchanged and spins. Their siblings are deliberately left in the survey, and the distinction is the point: `i -= 1` underflows at `i == 0` and panics, and `count += 1` grows past the array and panics. Those terminate, so they are caught, and an exclusion broad enough to cover them would have hidden real coverage. Refs #110

`BoolDecoder::get_bool` renormalises with two shifts on adjacent lines -- `self.value <<= 1` and `self.range <<= 1` -- and only one of them may be excluded. Reversing the range shift leaves it shrinking toward zero under `while self.range < 128`, so the loop never returns and cargo-mutants can only report a TIMEOUT. Reversing the value shift corrupts the decode and is caught. Written with the same operator, the two mutants differ only by line and column, so an exclusion could name the function -- and hide the caught one -- or anchor on a line number, which stops matching the moment anything above it moves. That is the brittleness the gamut-jpeg entries already live with. `self.range *= 2` is identical on a `u32` and gives the statement its own mutation description, so the exclusion can name the operator instead. Verified against `--list`: the hang goes, while `<<= with >>=` on the value shift and `*= with +=` on the range both stay in the survey. Refs #110

`reconstruct_bpred_zero` could be replaced with `()` outright and nothing failed. B_PRED is covered and skipping is covered, but never the two together -- and that pair is the only thing the function exists for: a skipped B_PRED macroblock is reconstructed from prediction alone, with no residual. The fixture took some finding, because the two requirements pull against each other. `detailed` is too contrasty: its residual survives even at q=127, so those macroblocks never skip. Flat content is too plain: a whole-block mode predicts it perfectly and is cheaper, so B_PRED is never chosen. Measured across four candidates, a coarse gradient stepping every four pixels sits between them and yields three macroblocks that are both, at q=110. `mode_stats` now counts that combination rather than only the two separately, so the test asserts the pair directly. Without it this would quietly stop covering anything the moment the encoder's mode decisions shifted, and look no different while doing so -- which is how the gap arose in the first place. Refs #110

The writer's counterpart to the read fixed earlier in this branch. A token partition's size goes out as `[len as u8, (len >> 8) as u8, (len >> 16) as u8]` -- one byte per lane, and with every partition in the suite under 64 KiB the top byte is zero whichever way the shift goes, so `>> 16` and `<< 16` are indistinguishable. `len.to_le_bytes()[..3]` says the same thing without hand-cutting the bytes, and reads the same way round as `split_token_partitions` does. Both shift mutants disappear with it; the `n - 1` arithmetic beside it stays in the survey. Refs #110

…fset `value_to_prefix` computed the extra value as `v - offset`, choosing between `2^(extra_bits+1)` and `3 * 2^extra_bits`. Both offsets are multiples of `2^extra_bits`, and the result is always smaller than that, so the subtraction only ever cleared bits that the `extra_bits`-wide write clears anyway. Which is exactly why one of those shifts could not be tested. Any offset congruent to 0 modulo `2^extra_bits` produces identical output, so `1u32 << (extra_bits + 1)` mutated to `>>` -- an offset of 0 -- round-tripped perfectly through the exhaustive inverse test. The other shift, `3 << extra_bits`, is *not* equivalent: at `extra_bits == 1` its mutant differs by 5, which is not a multiple of 2, and it is caught. Two identical-looking shifts in one function, one testable and one not. Masking says what the format means -- the extra bits *are* the low bits of `v` -- and leaves no such slack. Both remaining shifts and the mask's `- 1` are each confirmed caught, so this trades an untestable expression for testable ones rather than moving the problem. No exclusion needed. The exhaustive `value_to_prefix_inverts_read_lz77_value` covers the change: every length in `1..=MAX_COPY_LENGTH` plus seven large distances, unchanged. Refs #110

…h != `build_length_limited_lengths` documents two special cases -- "an empty histogram yields all-zero lengths; a single nonzero symbol gets length 1" -- and every fixture fed it a dense histogram, so neither was asserted. Both are now. The mutant that led here is equivalent, and the test does not kill it: counting used symbols with `h >= 0` makes `used` the whole alphabet, which skips both early returns, and the general path then produces exactly what they would have. They are an optimisation, not behaviour. Rather than exclude it, the count is written `h != 0` -- identical on a `u32`, but with no always-true variant. Its `==` mutant counts the unused symbols instead and breaks every dense histogram, so it is caught. That mattered more than tidiness here. The same function has a second `> 0`, in the `find` that locates the lone symbol, and *that* one is not equivalent -- it fails 16 tests. Both render as `replace > with >= in build_length_limited_lengths`, so an exclusion naming the function would have hidden a real gap while silencing the harmless one. Refs #110

`write_main_image` emits the entropy image's block-size exponent as `prefix_bits - 2`, and mutating that to `/ 2` changed nothing anywhere in the suite. Two things had to line up for it to hide that well. The arithmetic agrees at the value that matters most: `DEFAULT_PREFIX_BITS` is 4, and `4 - 2` and `4 / 2` are both 2, so every default encode writes the same field either way. `3 - 2` and `3 / 2` are both 1 as well. Only 2 and 5 separate them. And those appear only on higher-effort candidate plans, which reach this line just when they win the race against every other plan for the whole image. So the test hands the plan in directly rather than hoping -- the same reasoning the lazy-matching test above already records for the same encoder. Asserted on the bits themselves: skip the colour-cache header, check the meta prefix flag is set, then read the three-bit exponent. `prefix_bits = 2` gives 4-pixel blocks, so a 64x64 fixture has 256 of them and their signatures genuinely differ, which is what makes the meta branch run at all. Refs #110

`tokenize` advances `i` with `i += 1` in three branches of one `while i < n` loop -- the encoder's mirror of `decode_image_data`. Mutated to `*= 1` each pins the cursor and the loop never returns, so cargo-mutants can only report TIMEOUT. The `-=` siblings stay in the survey, and that was checked rather than assumed: applying one fails two tests, because it underflows and panics instead of hanging. An exclusion broad enough to cover them would have hidden real coverage. Refs #110

`build_length_limited_lengths` raises the histogram's floor and rebuilds the tree until it fits `max_len`, and `if max_depth <= max_len { return }` is the loop's only exit. Reversed to `>`, it returns exactly when it should keep going and spins when it should stop: once the tree fits, `count_min` saturates and every further pass rebuilds the same already-flat tree. Non-terminating, so cargo-mutants can only report TIMEOUT. It is the sole `<=` in the function, so the entry names it unambiguously; the other six mutants there stay in the survey. Refs #110

`BoolDecoder::get_tree_start` walks a coding tree until it reaches a leaf, which `i <= 0` marks. That test is the descent's only exit, so reversing it keeps the loop going at every leaf it finds. Confirmed by applying the mutant under a 120s cap: the suite exceeds it rather than failing, so this is a hang and not a slow failure. It is the sole `<=` in the function, and the other six mutants there stay in the survey. Refs #110

Adding the partition-size test anchored on the function line rather than its attribute, which left the original `#[test]` orphaned onto the new test's doc comment -- a duplicate rather than a lost test this time, since the anchor was re-emitted with its own. Clippy names both, which is the argument for the lint gate being blocking: a duplicated attribute is invisible to the suite, and the same slip in gamut-tiff silently disabled a test. The entropy-image test also looped over a single `prefix_bits`, which now reads as the binding it is.

`SEGMENT_QUANT_DELTAS` is `[-12, -4, 4, 12]`, and deleting the minus on `-4` was invisible: the fixture split dark over bright, which lands in segments 0 and 3, so the deltas for 1 and 2 were never applied to anything. Four horizontal bands averaging about 23, 87, 151 and 215 put one macroblock row in each segment. Every delta is now observable -- all four confirmed by mutating them individually, including the two the old fixture could not reach. The bands stay horizontal because that is the axis a wrong row stride gets wrong, which is what this fixture was built for in the first place; it now covers both concerns rather than one. The frame grows to 64x64 so four macroblock rows exist to hold four bands. The byte-stability golden moves with it, from 686 bytes to 875. That is the re-pinning its own contract anticipates: the fixture legitimately changed, and this is the commit that says why. Refs #110

justin13888 added 16 commits September 2, 2026 13:32

justin13888 merged commit ff3fe71 into master Sep 2, 2026
8 checks passed

Navigation Menu

Uh oh!

test(webp): close the crate's first mutation survey - #496

test(webp): close the crate's first mutation survey#496
justin13888 merged 16 commits into
masterfrom
fix/110-webp-survivors

justin13888 commented Sep 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

justin13888 commented Sep 2, 2026

The crate's own blind spot: a stream that is valid but not the one intended

Untested paths, not weak assertions

Equivalent mutants, removed rather than excluded where possible

Making exclusions precise enough to be honest

Validation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant