From 4b1febb5dd7dcdb7099230b45877b8b02a0201e3 Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:07:15 -0500 Subject: [PATCH] =?UTF-8?q?fix(build):=20.incbin=20escaped=20the=20checkou?= =?UTF-8?q?t=20from=20a=20worktree=20=E2=80=94=20use=20--bin-include-dir?= =?UTF-8?q?=20(#116)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ca65 resolves a relative `.incbin` operand against the CURRENT DIRECTORY first, falling back to the including source file's directory only on a miss. Every `.incbin` here was spelled `../../..//` on the belief that source-relative was the rule. From the repo root the two interpretations coincide, so it worked — but a git worktree lives at `.claude/worktrees//`, exactly three levels down, so `../../../` climbed out of the worktree and into the PRIMARY checkout. Every worktree build therefore embedded the primary checkout's blob while make's dependency graph tracked the worktree's. Nothing diagnosed it, and nothing could: the two copies are normally byte-identical, so the PRG hash is unchanged and the entire existing evidence bar passes. It goes wrong the moment they can differ — bump the ip65 submodule on a branch and the PRG silently ships the old network stack; or run concurrent agents in sibling worktrees, where whoever last wrote the primary's copy decides what everyone else compiles. Fix, per the measurements on #116: CA65FLAGS += --bin-include-dir $(abspath $(IP65_BUILD)) CA65FLAGS += --bin-include-dir $(abspath build) with the `../` dropped from every operand. Note `-I` does NOT feed `.incbin` — that is the source-include path, and aiming it at a blob directory fails with "Cannot open include file" even though the file is sitting in it. Binary includes have their own search path. ABSOLUTE roots, because a relative one is cwd-relative and would re-acquire this exact bug the moment a recipe assembles from a subdirectory. FOUR sites, not one. #116 was filed against the ip65 blob; the same escape was in both overlay TUs: src/net/ip65/ip65_blob.s ../../../ip65-build/ip65-c64.bin src/crypto/shared/p256_overlay_blobs.s ../../../build/lib/nistcurves-p256-verify.bin src/crypto/shared/p384_overlay_blobs.s ../../../build/lib/overlay-p384-{sha384,curve}.bin The overlay ones are the worse of the three: `ip65-build/` is deterministic, but `build/` is branch- and flag-dependent, so a borrowed overlay image could have come from a different pin entirely. All three files also *documented* the wrong rule ("resolved by ca65 relative to this source file"), which is what made the `../../../` look deliberate and kept it alive. Replaced rather than left beside a corrected path. ACCEPTANCE GATE — a marker test, not a hash ------------------------------------------- Hashes cannot see this bug, so "the build is green and the hash matches" is not evidence. Planting distinguishable bytes in the two copies and checking which reaches the image is. Checked in the LINKED PRG, not the object: ca65 stores .incbin data in fragments (measured — only 8 bytes of the blob appear contiguously in the .o), so an object-level string search cannot see the marker even when the right file was read. worktree blob marked, both copies present: pre-fix -> PRG lacks the marker (read the PRIMARY's) FAIL post-fix -> PRG contains the WORKTREE marker PASS worktree blob absent: pre-fix -> assembles anyway (escaped) FAIL post-fix -> Cannot open include file 'ip65-c64.bin' PASS Gate exits 1 pre-fix and 0 post-fix, so it discriminates rather than merely passing. The primary checkout's blob is never written by the test. Verification ------------ all 5 backend x profile builds PASS, hashes IDENTICAL to master (d522e684 / b181ec08 / 66e37037 / 118241e9 / 211ad1bd) — output-neutral marker gate PASS post-fix, FAIL pre-fix overlay operands resolve with a stub .bin present, fail loudly when absent (both TUs, real flags, real defines) The two overlay-embed BUILDS remain blocked by pre-existing defects unrelated to this change, confirmed identical on master: P-256 dies at `LIB_NISTCURVES_P256_CODE overflows OVERLAY_REGION by 644 bytes` (and from clean, earlier, at `No rule to make target 'build/labels.txt'`), P-384 at its SHA-384 table overflow. Their `.incbin` lines are therefore never reached by a real build, which is why the operands are verified in isolation above rather than end-to-end. Stated plainly rather than implied by a green matrix. Refs #116. Co-Authored-By: Claude Opus 5 (1M context) --- Makefile | 25 ++++++++++++ src/crypto/shared/p256_overlay_blobs.s | 17 +++++++-- src/crypto/shared/p384_overlay_blobs.s | 13 ++++--- src/net/ip65/ip65_blob.s | 53 ++++++++++++++++++-------- 4 files changed, 83 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index 9608de4..4e03aa6 100644 --- a/Makefile +++ b/Makefile @@ -36,6 +36,31 @@ IP65_BUILD := ip65-build IP65_BIN := $(IP65_BUILD)/ip65-c64.bin CA65FLAGS := -I src -I src/inc -I src/crypto/shared -I src/net/$(BACKEND) -I build --debug-info +# Binary-include search roots for `.incbin` (issue #116). +# +# `-I` above does NOT feed `.incbin` — that is a separate search path in ca65, +# and pointing `-I` at a blob directory fails with "Cannot open include file" +# even though the file is right there (measured, ca65 V2.18). Binary includes +# need `--bin-include-dir`. +# +# Why this exists at all: ca65 resolves a relative `.incbin` operand against +# the CURRENT DIRECTORY FIRST, falling back to the including source file's +# directory only if that misses. Every `.incbin` here used to be spelled +# `../../..//` on the belief that source-relative was the rule. From +# the repo root those two interpretations coincide, so it worked — but an agent +# worktree lives at `.claude/worktrees//`, exactly three levels down, so +# `../../../` climbed straight out into the PRIMARY checkout and every worktree +# build silently embedded the primary's blob. The worktree's own freshly-built +# copy was never read, by anything, while make's dependency graph tracked it. +# +# ABSOLUTE paths, not relative: a relative root would be cwd-relative and would +# re-acquire the same class of bug the moment any recipe assembles from a +# subdirectory. `$(abspath ...)` pins these to make's own working directory. +# The operands in the sources are then written WITHOUT any `../`, so there is +# no escape hatch left to resolve. A missing blob now fails the assemble +# loudly instead of quietly resolving somewhere else. +CA65FLAGS += --bin-include-dir $(abspath $(IP65_BUILD)) +CA65FLAGS += --bin-include-dir $(abspath build) # Which backend is selected, as a ca65 define. Sources that must name a # cfg-level symbol need this, because the two cfgs give the same region # different names: the crypto code+rodata region is CRYPTO_HOT under UCI and diff --git a/src/crypto/shared/p256_overlay_blobs.s b/src/crypto/shared/p256_overlay_blobs.s index cc618a5..0c10acc 100644 --- a/src/crypto/shared/p256_overlay_blobs.s +++ b/src/crypto/shared/p256_overlay_blobs.s @@ -36,13 +36,22 @@ ; P-256 verify overlay image (REU_OVERLAY_P256_VERIFY source) ; ; Loads into the live CRYPTO_OVERLAY slot at $4200-$5FFF at PRG load -; time, then boot DMAs it to REU bank 2 slot $22100. The .incbin path -; is resolved by ca65 relative to this source file: from -; src/crypto/shared/ the build/ tree is two levels up. +; time, then boot DMAs it to REU bank 2 slot $22100. +; +; Path is relative to ca65's BINARY include path, set by the Makefile to +; an absolute $(abspath build). The `../../../` this used to carry was +; an escape hatch, not a path: ca65 resolves .incbin against the CURRENT +; DIRECTORY first and only falls back to this file's directory, so from a +; git worktree (three levels down, at .claude/worktrees//) it +; climbed into the PRIMARY checkout and embedded that tree's overlay +; image. Worse here than for the ip65 blob, which is deterministic: +; build/ is branch- and flag-dependent, so the borrowed image could have +; come from a different pin entirely, with no diagnostic. See issue #116 +; and the longer note in src/net/ip65/ip65_blob.s. ; ----------------------------------------------------------------------------- .segment "OVERLAY_BLOB_P256" p256_overlay_verify_blob: - .incbin "../../../build/lib/nistcurves-p256-verify.bin" + .incbin "lib/nistcurves-p256-verify.bin" p256_overlay_verify_blob_end: .endif ; .ifdef USE_OVERLAY_P256_EMBED diff --git a/src/crypto/shared/p384_overlay_blobs.s b/src/crypto/shared/p384_overlay_blobs.s index 5eb892a..df5c9e1 100644 --- a/src/crypto/shared/p384_overlay_blobs.s +++ b/src/crypto/shared/p384_overlay_blobs.s @@ -87,13 +87,16 @@ ; SHA-384 overlay image (REU bank 6 source) ; ; Loads into the live CRYPTO_OVERLAY slot at $4200-$5FFF at PRG load -; time, then boot DMAs it to REU bank 6. The .incbin path is resolved -; by ca65 relative to this source file: from src/crypto/shared/ the -; build/ tree is two levels up. +; time, then boot DMAs it to REU bank 6. +; +; Path is relative to ca65's BINARY include path (absolute $(abspath +; build), set by the Makefile). The former `../../../` escaped the +; checkout entirely when assembled from a git worktree — see issue #116 +; and the note in src/net/ip65/ip65_blob.s. ; ----------------------------------------------------------------------------- .segment "OVERLAY_BLOB_SHA384" p384_overlay_sha384_blob: - .incbin "../../../build/lib/overlay-p384-sha384.bin" + .incbin "lib/overlay-p384-sha384.bin" p384_overlay_sha384_blob_end: ; ----------------------------------------------------------------------------- @@ -104,7 +107,7 @@ p384_overlay_sha384_blob_end: ; ----------------------------------------------------------------------------- .segment "OVERLAY_BLOB_CURVE" p384_overlay_curve_blob: - .incbin "../../../build/lib/overlay-p384-curve.bin" + .incbin "lib/overlay-p384-curve.bin" p384_overlay_curve_blob_end: .endif ; .ifdef USE_OVERLAY_P384_EMBED diff --git a/src/net/ip65/ip65_blob.s b/src/net/ip65/ip65_blob.s index dc7dedf..47f0e7e 100644 --- a/src/net/ip65/ip65_blob.s +++ b/src/net/ip65/ip65_blob.s @@ -17,20 +17,41 @@ .segment "NET_CODE" -; Three levels up from this file's directory (src/net/ip65/) is the repo -; root, where ip65-build/ lives. -; -; CAUTION: ca65 does NOT resolve .incbin relative to the including source -; file only — it also tries the path relative to the current directory, -; which is the repo root when make runs. From there `../../../` escapes -; three levels ABOVE the checkout. A git worktree under -; /.claude/worktrees// sits at exactly that depth, so if its -; own ip65-build/ip65-c64.bin is missing this silently picks up the parent -; checkout's blob and the build looks fine. Measure blob behaviour in a -; real clone, never in a nested worktree. -; -; The build order is guaranteed by an explicit dependency edge in the -; Makefile (build/net/ip65/ip65_blob.o: $(IP65_BIN)) — make cannot see -; through .incbin, so without it this object could be assembled before +; The blob is found through ca65's BINARY include path, set by the +; Makefile to an absolute $(abspath $(IP65_BUILD)). Hence the bare +; filename: there is deliberately no `../` here to resolve. +; +; It used to read `../../../ip65-build/ip65-c64.bin`, on the belief that +; ca65 resolves .incbin relative to the including source file. It does +; not — or rather, not first. **ca65 tries the CURRENT DIRECTORY first** +; and falls back to the source file's directory only on a miss. From the +; repo root the two coincide, which is why the old spelling worked and +; why the wrong rule survived in this comment for so long. +; +; They stop coinciding inside a git worktree. One lives at +; /.claude/worktrees// — exactly three levels down — so +; `../../../` climbed out of the worktree and into the PRIMARY checkout, +; and every worktree build embedded the primary's blob while make's +; dependency graph tracked the worktree's. Nothing diagnosed it: the two +; copies are normally byte-identical, so it stayed invisible until +; someone bumped the ip65 submodule on a branch. Measured, ca65 V2.18, +; with distinguishable bytes planted in each copy (issue #116): +; +; .incbin "../../../ip65-build/…" -> PRIMARY's bytes +; --bin-include-dir + .incbin "ip65-c64.bin" -> worktree's +; +; Note `-I` does NOT feed .incbin — that is the source-include path, and +; pointing it at a blob directory fails with "Cannot open include file" +; even though the file is sitting in it. Binary includes have their own +; search path, hence --bin-include-dir. +; +; Two properties worth preserving if this is ever respelled: an absent +; blob now FAILS the assemble instead of quietly resolving elsewhere, and +; adding the flag while leaving a `../` operand in place would silently +; keep the old behaviour — the operand is the load-bearing half. +; +; The build ORDER is guaranteed separately, by an explicit dependency edge +; in the Makefile (build/net/ip65/ip65_blob.o: $(IP65_BIN)) — make cannot +; see through .incbin, so without it this object could be assembled before ; the blob exists (issue #89). -.incbin "../../../ip65-build/ip65-c64.bin" +.incbin "ip65-c64.bin"