Fix/codegen soundness#9
Merged
Merged
Conversation
All three were surfaced by exhaustively round-tripping every Pareto solution
(not just the picked one) through compiled C and Rust, and reproduced minimally.
1. Inline-constant path corrupted values >= 16 bits. The eligibility check
`len(data)*8 <= 64` and packing `b << (i*8)` assumed 8-bit elements; after
_combine, unitBits>=8 data holds unitBits-wide elements, so wide values were
packed at the wrong stride into an undersized constant. This miscompiled on
the DEFAULT compression=1 path (e.g. a sparse table with a 65000 outlier).
Fix: use the element width for both the fit check and the packing.
2. Generated return type ignored `default`. The function return type was sized
from the stored (rebased/culled) data range only, so a negative or oversized
`default` wrapped in C and failed to compile in Rust — and corrupted in-bounds
culled default-prefix indices too. Fix: widen the return type to cover the
data range unioned with `default`.
3. compression>=10 ("minimum table bytes") and compression<=0 did not reach the
true optimum. Pareto pruning kept only the (nLookups, fullCost) frontier;
fullCost adds a per-lookup penalty, so deeper splits that shave raw bytes were
pruned before pick_solution saw them. Fix: also retain the (nLookups, cost)
frontier. These extra solutions are always fullCost-dominated, so the 1..9
heuristic picks are provably unchanged (verified byte-identical across 4000
inputs); compression>=10 now returns the true minimum. OuterLayer prunes its
combined inner+palette set so no fully-dominated (redundant) solution leaks out.
Adds round-trip regression tests for all three (the suite compiled and ran only
the picked solution at compression=1, so these paths were untested).
Full suite: 222 passing. A 1540-input C+Rust round-trip fuzz over all Pareto
solutions now reports zero failures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PaletteOuterSolution.genCode discarded the `start` returned by
addArray("palette", ...) and always indexed the palette from 0, unlike the
data-array path (which folds `start` into the index). When two palette
solutions are emitted into one shared Code object, the second palette's values
are appended at a nonzero offset but read from 0, silently returning the first
palette's entries — a miscompile that violates the documented Code array-
accumulation contract.
No shipped path shares one Code across top-level solutions today (the CLI uses
separate Code objects), so this is latent, but it is a real correctness gap in
the public Code API. Fix: honor the palette start offset. Adds a C+Rust
regression test that emits two palette solutions into one Code and round-trips.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ge value
InnerLayer.split() appends a padding element to self.data to make the length
even. For non-integer (string) data, OuterLayer hands its own data list to
InnerLayer unchanged, so the padding mutated OuterLayer.data too, extending the
generated bounds check (`u < len`) by one. Out-of-range indices then returned
the padding value instead of the default — e.g. ['A','B','C'] with default 'DEF'
built a length-4 array {A,B,C,A} guarded by `u<4`, so data_get(3) returned A.
(Integer data was unaffected because OuterLayer passes InnerLayer a freshly
reduced/divided copy, not its own list.)
Fix: InnerLayer takes a private copy of its data so split() can never mutate a
caller-owned list. Adds a compiled string round-trip regression test and a
non-mutation assertion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
behdad
added a commit
to behdad/packtab.rs
that referenced
this pull request
Jul 3, 2026
Port codegen-soundness fixes from harfbuzz/packtab#9
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
1 participant
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.
Claude work.