From eac097bb4455f73701d7ac263e9b1a2a5923d403 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 1 Jul 2026 10:39:49 -0600 Subject: [PATCH 1/3] Fix three codegen soundness bugs found by round-trip fuzzing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packTab/__init__.py | 67 ++++++++++++++++++++++++++------- packTab/test.py | 91 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 137 insertions(+), 21 deletions(-) diff --git a/packTab/__init__.py b/packTab/__init__.py index 837ee89..8d6214e 100644 --- a/packTab/__init__.py +++ b/packTab/__init__.py @@ -875,7 +875,12 @@ def genCode(self, code, name=None, var="u", language="c", data_multiplier=1): # inline it as a constant instead of emitting an array. # Non-integer data (string identifiers) and negative values cannot # be inlined (negative values would produce invalid unsigned literals). - can_inline = len(data) * 8 <= 64 and all( + # ``data`` holds packed bytes when unitBits < 8 (see _combine), but holds + # unitBits-wide elements when unitBits >= 8. Inlining packs each element at + # ``elem_bits`` stride and must match the extraction stride below, so use the + # element width — not a hardcoded 8 — for both the fit check and the packing. + elem_bits = unitBits if unitBits >= 8 else 8 + can_inline = len(data) * elem_bits <= 64 and all( isinstance(v, int) and v >= 0 for v in data ) @@ -908,8 +913,8 @@ def genCode(self, code, name=None, var="u", language="c", data_multiplier=1): # (CONSTANT >> (index << log2(unitBits))) & ((1 << unitBits) - 1) packed = 0 for i, b in enumerate(data): - packed |= b << (i * 8) - const_typ = language.type_for(0, (1 << (len(data) * 8)) - 1) + packed |= b << (i * elem_bits) + const_typ = language.type_for(0, (1 << (len(data) * elem_bits)) - 1) lit = language.uint_literal(packed, const_typ) elementMask = (1 << unitBits) - 1 shift2 = int(round(log2(unitBits))) if unitBits > 1 else 0 @@ -1041,14 +1046,32 @@ def _first_non_default_index(data, default): def _prune_pareto_solutions(solutions): - """Remove solutions dominated on (nLookups, fullCost).""" - solutions = sorted(solutions, key=lambda s: (s.nLookups, s.fullCost)) + """Keep the (nLookups, fullCost) frontier, PLUS the (nLookups, cost) frontier. + + The size/speed heuristic (compression 1..9) navigates the fullCost frontier and + is unaffected: the extra cost-frontier solutions are always fullCost-dominated, so + the monotone score ``nLookups + c*log2(fullCost)`` can never prefer them, and on an + exact score tie the fullCost-frontier incumbent is ordered first. Retaining the + cost frontier lets the byte-minimizing (compression>=10) and flat (compression<=0) + selectors reach the true optimum, which pruning on fullCost alone would discard. + """ kept = [] - best_cost = float("inf") - for s in solutions: - if s.fullCost < best_cost: - kept.append(s) - best_cost = s.fullCost + seen = set() + + def add_frontier(key): + best = float("inf") + for s in sorted(solutions, key=lambda s: (s.nLookups, key(s))): + k = key(s) + if k < best: + best = k + if id(s) not in seen: + seen.add(id(s)) + kept.append(s) + + add_frontier(lambda s: s.fullCost) # incumbents first: 1..9 frontier, unchanged + add_frontier(lambda s: s.cost) # byte-optimal solutions for compression>=10 + # Stable sort keeps fullCost incumbents ahead of any cost-only tie. + kept.sort(key=lambda s: (s.nLookups, s.fullCost)) return kept @@ -1221,7 +1244,15 @@ def genCode(self, code, name=None, var="u", language="c", private=True): if isinstance(language, str): language = languages[language] - typ = language.type_for(self.layer.minV, self.layer.maxV) + # The return type must also hold ``default`` — it is emitted in the + # out-of-range branch and returned for culled default-prefix indices, so a + # negative or oversized default would otherwise wrap (C) or fail to compile + # (Rust) even though the stored data fits a narrower type. + lo, hi = self.layer.minV, self.layer.maxV + d = self.layer.default + if isinstance(lo, int) and isinstance(d, int): + lo, hi = min(lo, d), max(hi, d) + typ = language.type_for(lo, hi) retType = fastType(typ) lookup_var = var if self.layer.base: @@ -1297,8 +1328,13 @@ def genCode(self, code, name=None, var="u", language="c", private=True): if isinstance(language, str): language = languages[language] - # Determine return type based on original value range - typ = language.type_for(self.layer.minV, self.layer.maxV) + # Determine return type based on original value range, widened to hold + # ``default`` (returned out-of-range / for culled default-prefix indices). + lo, hi = self.layer.minV, self.layer.maxV + d = self.layer.default + if isinstance(lo, int) and isinstance(d, int): + lo, hi = min(lo, d), max(hi, d) + typ = language.type_for(lo, hi) retType = fastType(typ) # Generate palette array containing unique values @@ -1523,6 +1559,11 @@ def __init__(self, data, default, base=None): palette_solutions = self._try_palette_encoding(data, extraCost) self.solutions.extend(palette_solutions) + # Prune the combined (inner + palette) set to the union frontier, dropping + # solutions that are redundant — dominated on both fullCost and cost — such + # as an inner split beaten outright by a palette solution. + self.solutions = _prune_pareto_solutions(self.solutions) + def _try_palette_encoding(self, data, extraCost): """Try palette encoding: store indices + unique values table. diff --git a/packTab/test.py b/packTab/test.py index acc7481..580d250 100644 --- a/packTab/test.py +++ b/packTab/test.py @@ -1337,17 +1337,22 @@ def test_palette_cost_calculation(self): assert palette_sol.cost < 64 # Better than direct (64 bytes) def test_palette_in_pareto_frontier(self): - """Palette solution should be on Pareto frontier.""" + """Every returned solution is minimal on the fullCost or the cost axis. + + The frontier is the union of the (nLookups, fullCost) frontier (navigated by + compression 1..9) and the (nLookups, cost) frontier (needed for compression + >=10 to reach the true minimum bytes). So a solution may be fullCost-dominated + yet still belong, provided it is cost-minimal for its lookup count; what must + never happen is a fully redundant solution — dominated on both axes. + """ data = [1, 2, 3, 2, 3, 2, 1, 0, 2, 1, 2, 2, 3, 3, 1, 11110124] solutions = pack_table(data, default=0, compression=None) - # All returned solutions should be non-dominated - for a in solutions: - for b in solutions: - if a is b: - continue - # a should not dominate b (otherwise b wouldn't be in frontier) - assert not (a.nLookups <= b.nLookups and a.fullCost <= b.fullCost) + for s in solutions: + peers = [t for t in solutions if t.nLookups <= s.nLookups] + on_fullcost = s.fullCost <= min(t.fullCost for t in peers) + on_cost = s.cost <= min(t.cost for t in peers) + assert on_fullcost or on_cost, (s.nLookups, s.fullCost, s.cost) def test_palette_selected_large_dataset(self): """Palette should be selected for large dataset with outliers.""" @@ -1518,3 +1523,73 @@ def test_none_default_considers_both_boundary_values(self): assert inferred_costs <= candidate_costs assert inferred_costs + + +class TestCodegenSoundnessRegression: + """Round-trip regressions for codegen soundness bugs found by fuzzing.""" + + # ── F2: inline-constant path with >= 16-bit values ── + # A wide value packed into the inline path used to be corrupted because the + # packing assumed 8-bit elements. This picks the inline solution at the + # default compression and must round-trip. + def test_inline_16bit_value_roundtrips(self, language): + data = [0] * 36 + [255] + [0] * 55 + [65000, 0, 0, 255] + [0] * 8 + code = _generate(data, language=language) + _compile_and_run(code, data, 0, language) + + def test_inline_various_wide_values_roundtrip(self, language): + for data in ( + [0, 65000, 0, 300], + [0, 0, 70000, 0], # 32-bit + [5, 5, 5, 4000, 5, 5, 5, 5], + ): + code = _generate(data, language=language) + _compile_and_run(code, data, 0, language) + + # ── F3: return type must hold `default` ── + def test_negative_default_roundtrips(self, language): + data = [0, 1, 2, 3] + code = _generate(data, default=-1, language=language) + _compile_and_run(code, data, -1, language) + + def test_large_default_roundtrips(self, language): + data = [0, 1, 2, 3] + code = _generate(data, default=1000, language=language) + _compile_and_run(code, data, 1000, language) + + def test_default_outside_range_constant_data(self, language): + data = [0] * 8 + code = _generate(data, default=-5, language=language) + _compile_and_run(code, data, -5, language) + + def test_negative_default_culls_prefix(self, language): + # Inferred default picks a boundary value that base-rebasing then culls; + # the culled index must still return the (correctly typed) default. + data = [-2, 1] + code = _generate(data, default=-2, language=language) + _compile_and_run(code, data, -2, language) + + # ── F1: compression>=10 must reach the true minimum-byte solution ── + def test_compression_10_is_global_min_bytes(self): + cases = [ + [0] * 8 + [9999] + [0] * 30 + [5] + [0] * 60 + [1] + [0] * 40, + [0] * 200 + [7] + [0] * 55, + [i % 4 for i in range(400)], + ] + for data in cases: + frontier = pack_table(data, default=0, compression=None) + true_min = min(s.cost for s in frontier) + picked = pack_table(data, default=0, compression=10) + assert picked.cost == true_min, (data[:8], picked.cost, true_min) + + def test_compression_1to9_picks_unchanged_by_frontier_enrichment(self): + # The cost-frontier solutions re-admitted for compression>=10 are always + # fullCost-dominated, so they must never change a 1..9 pick. + data = [0] * 64 + [1, 2, 3, 0, 0, 5] + [0] * 40 + [255] + [0] * 20 + frontier = pack_table(data, default=0, compression=None) + for c in range(1, 10): + picked = pick_solution(list(frontier), c) + best = min( + frontier, key=lambda s: s.nLookups + c * __import__("math").log2(s.fullCost) + ) + assert (picked.cost, picked.nLookups) == (best.cost, best.nLookups) From 97bfb6080b0a3d846c2f7f9ec5b99113578e7fa6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 1 Jul 2026 10:43:43 -0600 Subject: [PATCH 2/3] Fix palette lookup ignoring its shared-array start offset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packTab/__init__.py | 13 ++++++++--- packTab/test.py | 53 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/packTab/__init__.py b/packTab/__init__.py index 8d6214e..2ff12fc 100644 --- a/packTab/__init__.py +++ b/packTab/__init__.py @@ -1339,7 +1339,7 @@ def genCode(self, code, name=None, var="u", language="c", private=True): # Generate palette array containing unique values palette_typ = language.type_for(min(self.palette), max(self.palette)) - palette_name, _ = code.addArray(palette_typ, "palette", self.palette) + palette_name, palette_start = code.addArray(palette_typ, "palette", self.palette) lookup_var = var if self.layer.base: lookup_var = language.wrapping_sub( @@ -1350,9 +1350,16 @@ def genCode(self, code, name=None, var="u", language="c", private=True): # This returns an expression that evaluates to a palette index (_, index_expr) = self.next.genCode(code, None, lookup_var, language=language) - # Look up value in palette: palette[index] - # Cast index to usize for Rust array indexing + # Look up value in palette: palette[start + index] + # Cast index to usize for Rust array indexing. The palette array may be + # shared across solutions in one Code, so honor the start offset returned by + # addArray (as the data-array path does), rather than indexing from zero. index_expr_usize = language.as_usize(index_expr) + if palette_start: + index_expr_usize = "%s+%s" % ( + language.usize_literal(palette_start), + index_expr_usize, + ) expr = language.array_index(palette_name, index_expr_usize) expr = language.cast(retType, expr) diff --git a/packTab/test.py b/packTab/test.py index 580d250..ffbd96a 100644 --- a/packTab/test.py +++ b/packTab/test.py @@ -1582,6 +1582,59 @@ def test_compression_10_is_global_min_bytes(self): picked = pack_table(data, default=0, compression=10) assert picked.cost == true_min, (data[:8], picked.cost, true_min) + # ── F4: palette lookup must honor its shared-array start offset ── + def test_two_palettes_in_one_code_roundtrip(self, language): + def palette_solution(d): + return [ + s for s in pack_table(d, 0, compression=None) + if isinstance(getattr(s, "palette", None), list) + ][0] + + data1 = ([10, 20, 30, 20] * 12) + [999999] + data2 = ([40, 50, 60, 70, 50] * 10) + [888888] + lang = languageClasses[language]() + code = Code("t") + palette_solution(data1).genCode(code, "g1", language=lang, private=False) + # Second palette accumulates into the SAME Code / shared "palette" array. + palette_solution(data2).genCode(code, "g2", language=lang, private=False) + buf = io.StringIO() + code.print_code(file=buf, language=lang) + gen = buf.getvalue() + + if language == "c": + checks = "\n".join( + " assert((long long)t_g2(%d)==(%dLL));" % (i, v) + for i, v in enumerate(data2) + ) + src = ( + "#include \n#include \n" + gen + + "\nint main(){\n" + checks + "\n return 0;\n}\n" + ) + suffix = ".c" + else: + checks = "\n".join( + " assert_eq!(t_g2(%d) as i64, %di64);" % (i, v) + for i, v in enumerate(data2) + ) + src = gen + "\nfn main(){\n" + checks + '\n println!("ok");\n}\n' + suffix = ".rs" + + with tempfile.NamedTemporaryFile(suffix=suffix, mode="w", delete=False) as f: + f.write(src) + path = f.name + out = path[: -len(suffix)] + if language == "c": + compile_cmd = ["cc", "-o", out, path, "-std=c99", "-w"] + else: + compile_cmd = ["rustc", "-A", "warnings", "-o", out, path] + try: + subprocess.check_call(compile_cmd, stderr=subprocess.PIPE) + subprocess.check_call([out]) + finally: + os.unlink(path) + if os.path.exists(out): + os.unlink(out) + def test_compression_1to9_picks_unchanged_by_frontier_enrichment(self): # The cost-frontier solutions re-admitted for compression>=10 are always # fullCost-dominated, so they must never change a 1..9 pick. From d6114d7ebbd0ea080f0c0f874c31d733c32ae8f9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 1 Jul 2026 11:14:46 -0600 Subject: [PATCH 3/3] Fix odd-length non-integer data leaking split() padding as out-of-range value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packTab/__init__.py | 6 +++++- packTab/test.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/packTab/__init__.py b/packTab/__init__.py index 2ff12fc..9d09cdd 100644 --- a/packTab/__init__.py +++ b/packTab/__init__.py @@ -1093,7 +1093,11 @@ class InnerLayer(Layer): """ def __init__(self, data): - Layer.__init__(self, data) + # Own a private copy: split() appends a padding element to self.data, and + # the caller's list may be aliased (OuterLayer passes its own data through + # unchanged for non-integer values). Mutating it would extend the bounds + # check so out-of-range indices return the padding instead of the default. + Layer.__init__(self, list(data)) self.maxV = max(data) self.minV = min(data) diff --git a/packTab/test.py b/packTab/test.py index ffbd96a..f237e55 100644 --- a/packTab/test.py +++ b/packTab/test.py @@ -1582,6 +1582,47 @@ def test_compression_10_is_global_min_bytes(self): picked = pack_table(data, default=0, compression=10) assert picked.cost == true_min, (data[:8], picked.cost, true_min) + # ── F5: odd-length string data must not leak the split() padding ── + def test_odd_length_string_data_out_of_range_default(self): + data = ["A", "B", "C"] # odd length -> split() pads to 4 + sol = pack_table(data, default="DEF", compression=1) + lang = languageClasses["c"]() + code = Code("data") + sol.genCode(code, "get", language=lang, private=False) + buf = io.StringIO() + code.print_code(file=buf, language=lang) + gen = buf.getvalue() + # Define the string identifiers so the generated C compiles, and assert the + # out-of-range index returns DEF (not the 'A' padding). + checks = "\n".join( + " assert(data_get(%d)==%s);" % (i, v) for i, v in enumerate(data) + ) + checks += "\n assert(data_get(3)==DEF);\n assert(data_get(9)==DEF);" + src = ( + "#include \n#include \n" + "#define A 1\n#define B 2\n#define C 3\n#define DEF 9\n" + + gen + + "\nint main(){\n" + checks + "\n return 0;\n}\n" + ) + with tempfile.NamedTemporaryFile(suffix=".c", mode="w", delete=False) as f: + f.write(src) + path = f.name + out = path[:-2] + try: + subprocess.check_call( + ["cc", "-o", out, path, "-std=c99", "-w"], stderr=subprocess.PIPE + ) + subprocess.check_call([out]) + finally: + os.unlink(path) + if os.path.exists(out): + os.unlink(out) + + def test_inner_layer_does_not_mutate_caller_list(self): + original = ["A", "B", "C"] + layer = OuterLayer(list(original), "DEF") + assert len(layer.data) == 3 + # ── F4: palette lookup must honor its shared-array start offset ── def test_two_palettes_in_one_code_roundtrip(self, language): def palette_solution(d):