ReviewOS

home-lang/home

build: cache-key hashing must not mutate dependency hashes above 256

#904
Closed glennmichael123 opened this 8 days ago ·
8 days ago

In packages/build/src/ir_cache.zig, generateCacheKey copies dependency hashes into a 256-element stack buffer only for len <= 256. For larger lists it aliases the caller-provided []const CacheHash, casts away const, and sorts that slice in place. This violates the API contract, can mutate the ParallelBuilder dependency list, and can fault on read-only input. It also makes cache key generation have surprising side effects only at the 257th dependency.

Acceptance: copy all dependencies before sorting; preserve the existing sorted-hash key format and order independence; propagate allocation failure without mutating input or hiding it; cover the 256/257 boundary, shuffled input, unchanged input, and allocation failure in real build-package tests on the pinned and Zig 0.18 toolchains. Update public example/API usage. Related to #884 build-package compatibility; not a claim of complete Zig migration or Bun parity.

8 days ago

Fixed on origin/main in 4ef6f1f (four files). Key generation now sorts a private copy above 256 dependencies and keeps the existing sorted-hash format. The caller allocator is explicit; allocation failure returns OutOfMemory without changing input. The red baseline was 32 pass / 1 fail at 257 dependencies. Final build-package library/check/tests pass 33/33 in Debug and ReleaseFast on both pinned Zig 0.17 and Zig 0.18. Tests cover the 256/257 boundary, reversed and immutable input, stable key, and failed allocation. The usage example and both callers are updated. This closes this focused cache mutation bug; it does not complete #884 or #66.

Sign in to comment on this issue.