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"