Fix C core correctness bugs in extended format and init paths - #328
Merged
Conversation
Five bugs in the C core, with the compressor fixes mirrored in the
pure-Python reference implementation:
* compressor.c: silent data loss in the extended-format RLE path. A lone
run byte consumed into rle_count by one poll was discarded when a later
poll saw the run had ended ("AAB" round-tripped as "AB" with
byte-at-a-time sink/poll). The byte is now re-emitted as a literal,
mirroring the existing rle_count==1 drain in tamp_compressor_flush.
* compressor.c: silent output corruption with lazy_matching + extended
(the configuration shipped in the Python wheels). poll_extended_handling
consumed input and mutated the window without invalidating the cached
lazy match, so a stale match was later emitted for data that no longer
matched. Found by round-trip fuzzing (~5% of run-heavy seeds corrupted).
The cache is now cleared whenever extended handling consumes input.
* compressor.c: tamp_compress_stream spun forever when
TAMP_STREAM_WORK_BUFFER_SIZE/2 < 6: an extended match token could never
fit in the output half-buffer, poll returned TAMP_OUTPUT_FULL with zero
progress, and the loop never detected the stall. Now a compile-time
#error (buffer must be >= 12 when extended compression is compiled in).
* decompressor.c: with TAMP_EXTENDED_DECOMPRESS=0, extended streams were
rejected only after committing conf and setting configured=true, so a
retrying caller silently decoded the extended stream as classic format.
Validation now happens before any state is committed.
* compressor_find_match_desktop.c: input_word_ext was unconditionally
assembled from input_bytes[2..9] even when only input_bytes[0..max-1]
were initialized (uninitialized stack read, UB, MSan finding). The
array is now zero-initialized; the extra bytes never affect the result.
Tests:
* ctests: new test_compressor_extended_rle_lone_byte_sink_poll (fails
pre-fix: "Expected 3 Was 2") and test_compressor_extended_lazy_rle_fuzz
(1000 deterministic seeds; pre-fix corrupts ~5% of them).
* ctests now build with -DTAMP_LAZY_MATCHING=1. The lazy-matching
configuration shipped in every Python wheel was previously untested by
make c-test, which is how the lazy-cache corruption survived.
* Makefile: fixed the c-test-embedded link rule (missing LittleFS/FatFS
objects; the target did not link from a clean build).
* tests/test_compressor_decompressor.py: round-trip regression tests for
the stale lazy-match cache (150 LCG seeds; 13 corrupt pre-fix) and the
dropped lone RLE byte, covering the pure-Python and Cython
implementations.
No bitstream format changes; compressed output can differ from prior
releases for run-heavy inputs (the previously-corrupt paths). Building
with TAMP_STREAM_WORK_BUFFER_SIZE in [4, 11] and extended compression
enabled is now a compile error instead of a runtime hang.
Verified: pytest 131 passed (+235 subtests); micropython unittest OK;
make c-test and c-test-embedded 31/31; ruff clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013UMgpFKUcWjqCDKcZwiPog
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes several correctness issues in Tamp’s C core around extended-format compression/decompression (RLE, extended match continuation, lazy matching) and strengthens regression coverage across both the C test suite and the Python implementation to prevent silent corruption/data loss.
Changes:
- Fix extended+lazy matching stale cached-match emission by invalidating the lazy cache when extended handling consumes input/mutates the window (C and Python).
- Fix extended RLE “lone byte” being dropped in low-level sink/poll style usage by re-emitting it as a literal (C and Python).
- Improve robustness in edge configurations: reject extended streams before committing decompressor state when extended-decompress is disabled; add a compile-time guard for too-small stream work buffers; eliminate a masked-but-UB uninitialized read in desktop match-finding.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_compressor_decompressor.py | Adds Python-level regressions for extended+lazy cache staleness and lone-byte RLE handling. |
| tamp/compressor.py | Mirrors C fixes in the pure-Python compressor (lazy-cache invalidation + lone-byte RLE re-emit). |
| tamp/_c_src/tamp/decompressor.c | Rejects extended streams before committing decompressor state when extended support is disabled. |
| tamp/_c_src/tamp/compressor.c | Fixes stale lazy-cache interaction with extended handling, lone-byte RLE re-emit, and adds a stream buffer-size compile-time guard. |
| tamp/_c_src/tamp/compressor_find_match_desktop.c | Zero-initializes preloaded input bytes to avoid UB/uninitialized reads under sanitizers. |
| Makefile | Enables TAMP_LAZY_MATCHING in ctests and fixes embedded test runner linking for FS-backed stream tests. |
| ctests/test_runner.c | Registers new compressor regression tests. |
| ctests/test_compressor.c | Adds C regressions for lone-byte extended RLE sink/poll and extended+lazy+RLE fuzz roundtrips. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* ctests: assert exact success codes instead of res >= 0 — TAMP_OK for poll, TAMP_INPUT_EXHAUSTED for decompress (the documented success return in lieu of TAMP_OK when all input is consumed). * ctests: replace libc rand()/srand() in the lazy-RLE fuzz test with the same fixed LCG the Python regression test uses, so the corpus is identical across toolchains and failing seeds are cross-platform reproducible. Verified the LCG corpus still fails pre-fix. * tests: test_extended_lazy_rle_stale_cache now explicitly skips on MicroPython (only the native module is available there and it lacks the lazy_matching kwarg) instead of silently passing as a no-op. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013UMgpFKUcWjqCDKcZwiPog
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes (ordered by severity)
compressor.c) — affects shipped builds.poll_extended_handlingconsumed input and mutated the window without invalidating the cached lazy match, so a stale match was later emitted for data that no longer matched: silent output corruption.lazy_matching + extendedis the configuration compiled into every Python wheel and the WASM build, withextended=Truethe default. Data-dependent: found by round-trip fuzzing (~5% of run-heavy seeds corrupt); text corpora never trigger it — enwik8/Silesia output is bit-identical pre/post fix. People purely using Tamp as an on-device embedded compressor are unaffected since extended=False is the recommended configuration on microcontrollers.compressor.c) — low-level API users only. A lone run byte consumed intorle_countby one poll was silently discarded when a later poll saw the run had ended ("AAB"round-tripped as"AB"). Unreachable via the high-level API —tamp_compressor_compressonly polls a full input buffer, andflushalready drained the tail correctly — so it affects only direct sink/poll callers that poll a partially-filled buffer (which goes against recommended use). The byte is now re-emitted as a literal, mirroring the existingrle_count==1drain intamp_compressor_flush.compressor.c) — nonstandard builds only.tamp_compress_streamspun forever whenTAMP_STREAM_WORK_BUFFER_SIZE/2 < 6because an extended match token could never fit in the output half-buffer. Only custom builds with the buffer set to 4–11 bytes were affected (default 32, docs recommend 256+). Now a#error(buffer must be ≥ 12 when extended compression is compiled in).decompressor.c) — nonstandard builds only. WithTAMP_EXTENDED_DECOMPRESS=0, an extended stream was rejected only after committing conf and settingconfigured=true; the firstdecompresscall correctly errored, but a caller retrying on the same decompressor then silently decoded the extended bitstream as classic format (garbage output, no error). Validation now happens before any state is committed. Requires that build + an extended stream + a retrying caller, so real-world impact is unlikely — but it turned a clean rejection into silent garbage.compressor_find_match_desktop.c) — no behavioral impact.input_word_extwas assembled frominput_bytes[2..9]even when onlyinput_bytes[0..max-1]were initialized. The garbage bytes are masked off before use, so output was always correct; still formally UB and an MSan finding. The array is now zero-initialized.The two corruption/data-loss compressor fixes are mirrored in
tamp/compressor.py.