From d54b057935179cd480a04c89a2b2e20ce23f5d62 Mon Sep 17 00:00:00 2001 From: JC_000 <3798556+JC-000@users.noreply.github.com> Date: Sun, 10 May 2026 14:09:07 -0500 Subject: [PATCH 1/3] chore(tools/uci): use Labels.from_file() for cross-format label support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5 UCI test scripts had bespoke `_load_labels()` parsers that called `parts[1].split(":", 1)`, assuming every line in build/labels.txt has the form `al C:XXXX .name`. The Makefile sed only rewrites 4-digit addresses with leading zeros; raw 6-digit ld65 emissions (e.g. `al 022100 .REU_OVERLAY_P256`) survive verbatim and crash the parser with `ValueError: not enough values to unpack`. The harness's `Labels.from_file()` already handles both forms via `r"al\s+(?:C:)?([0-9a-fA-F]+)\s+\.(\S+)"` and is used by the other two UCI scripts (`test_https_local`, `bench_ecdsa_u64e`) — converge on it as the single source of truth. Affected: phase3_tcp_echo (crashed), test_http_local (crashed), test_http_live, _analyze_ecdsa_trace, phase2_check (latent — would have crashed when any new ≥0x10000 symbol landed). phase2_check preserves its custom `KeyError` message via an explicit guard. Co-Authored-By: Claude Opus 4.7 (1M context) --- tools/uci/_analyze_ecdsa_trace.py | 13 +++++-------- tools/uci/phase2_check.py | 13 +++++-------- tools/uci/phase3_tcp_echo.py | 10 ++-------- tools/uci/test_http_live.py | 10 ++-------- tools/uci/test_http_local.py | 10 ++-------- 5 files changed, 16 insertions(+), 40 deletions(-) diff --git a/tools/uci/_analyze_ecdsa_trace.py b/tools/uci/_analyze_ecdsa_trace.py index ef2b132..666d31a 100644 --- a/tools/uci/_analyze_ecdsa_trace.py +++ b/tools/uci/_analyze_ecdsa_trace.py @@ -21,6 +21,8 @@ from collections import Counter from pathlib import Path +from c64_test_harness import Labels + REPO_ROOT = Path(__file__).resolve().parents[2] DEFAULT_BASE = Path("/tmp/ecdsa_debug") @@ -32,14 +34,9 @@ def _load_labels() -> dict[int, str]: if not labels_file.is_file(): return {} out: dict[int, str] = {} - for line in labels_file.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - a = int(hex_addr, 16) - # Prefer the first (public) label if multiple map to same addr - out.setdefault(a, name) + for name, a in Labels.from_file(labels_file).items(): + # Prefer the first (public) label if multiple map to same addr + out.setdefault(a, name) return out diff --git a/tools/uci/phase2_check.py b/tools/uci/phase2_check.py index 0caf388..fb66e0e 100644 --- a/tools/uci/phase2_check.py +++ b/tools/uci/phase2_check.py @@ -31,6 +31,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -74,14 +75,10 @@ def load_label(name: str) -> int: Entries look like: `al C:BC4B .net_local_ip` — the `.name` token is unambiguous across backend cfgs. """ - token = f".{name}" - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2] == token: - addr_tok = parts[1] # "C:BC4B" - _, hex_addr = addr_tok.split(":", 1) - return int(hex_addr, 16) - raise KeyError(f"label {name!r} not found in {LABELS_PATH}") + labels = Labels.from_file(LABELS_PATH) + if name not in labels: + raise KeyError(f"label {name!r} not found in {LABELS_PATH}") + return labels[name] def is_plausible_private(ip: tuple[int, int, int, int]) -> bool: diff --git a/tools/uci/phase3_tcp_echo.py b/tools/uci/phase3_tcp_echo.py index b861d0f..30395b0 100644 --- a/tools/uci/phase3_tcp_echo.py +++ b/tools/uci/phase3_tcp_echo.py @@ -35,6 +35,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -107,14 +108,7 @@ def _run_echo_server(bind_ip: str, port: int, result: dict) -> None: def _load_labels() -> dict[str, int]: - labels: dict[str, int] = {} - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - labels[name] = int(hex_addr, 16) - return labels + return dict(Labels.from_file(LABELS_PATH)) def _build_test_routine(labels: dict[str, int], host_ip: str, port: int) -> bytes: diff --git a/tools/uci/test_http_live.py b/tools/uci/test_http_live.py index 792a4c8..2c0c9ae 100644 --- a/tools/uci/test_http_live.py +++ b/tools/uci/test_http_live.py @@ -18,6 +18,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -44,14 +45,7 @@ def _load_labels() -> dict[str, int]: - labels: dict[str, int] = {} - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - labels[name] = int(hex_addr, 16) - return labels + return dict(Labels.from_file(LABELS_PATH)) def _build_http_routine(labels: dict[str, int], hostname_len: int, port: int) -> bytes: diff --git a/tools/uci/test_http_local.py b/tools/uci/test_http_local.py index 3da097d..7ba9981 100644 --- a/tools/uci/test_http_local.py +++ b/tools/uci/test_http_local.py @@ -26,6 +26,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -97,14 +98,7 @@ def _run_http_server(bind_ip: str, port: int, result: dict) -> None: def _load_labels() -> dict[str, int]: - labels: dict[str, int] = {} - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - labels[name] = int(hex_addr, 16) - return labels + return dict(Labels.from_file(LABELS_PATH)) def _build_http_routine(labels: dict[str, int], port: int) -> bytes: From 4d839d0ea244afe541f33623a4b920f09218daf8 Mon Sep 17 00:00:00 2001 From: JC_000 <3798556+JC-000@users.noreply.github.com> Date: Sun, 10 May 2026 14:09:33 -0500 Subject: [PATCH 2/3] fix(uci): bound uci_wait_idle via TOD; detect net_tcp_connect short-read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related defects on the UCI/U64E TCP path that together turned a non-deterministic firmware OPEN_TCP failure into a 600s sentinel hang in tools/uci/test_https_local. Triaged in c64-test-harness#90. #36 — net_tcp_connect masked OPEN_TCP short-read as success Read the firmware response into uci_socket_id but unconditionally set net_tcp_state = UCI_TCP_CONNECTED regardless of how many bytes came back. A short/empty response left uci_socket_id = 0, the C64 proceeded to the TLS layer, and the ClientHello pushed bytes via SOCKET_WRITE on a phantom socket the firmware never opened. Fix: pre-zero uci_socket_id before the read so a short-read leaves a known sentinel; validate uci_resp_count != 0 AND uci_socket_id != 0 before committing CONNECTED. On failure: net_last_error = UCI_ERR_NO_SOCKET, net_tcp_state = UCI_TCP_CONNECT_FAIL, return C=1. All three existing call sites (src/http.s:87, :680; src/boot.s:465) already check carry, no caller updates needed. #37 — uci_wait_idle was an unbounded spin Spun forever waiting for UCI_STATUS to clear, so a wedged FPGA or the phantom-socket retry loop above became a 600s harness sentinel timeout instead of a clean error. Fix: wall-clock-bounded via CIA1 TOD ($DC08-$DC0B) per the CLAUDE.md "bounded timeouts must use wall-clock time" design note. TOD ticks at 10 Hz independent of CPU turbo, which is why a cycle-counted budget breaks at 48 MHz (cf. abandoned feat/net-drain-abi). 5-second budget; on exhaustion: net_last_error = UCI_ERR_WAIT_TIMEOUT, C=1. Read order is HOUR (latch) → MIN → SEC → TENTHS (unlatch); the SMC state bytes inside the routine match the file's no-ZP convention. All 4 callers (net_dhcp_acquire, net_tcp_connect, net_tcp_send, net_tcp_close) gained `bcs` bails to surface the timeout. Verified on U64E @ 10.43.23.81: test_https_local PASS 79.9s (full TLS handshake, HTTP 200, body OK — happy path uci_socket_id=0x07); test_https_print_body PASS 84.9s; phase2_check PASS; phase3_tcp_echo PASS. Both new error paths surface as immediate C=1 with a populated net_last_error byte. Fixes #36 Fixes #37 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/net/uci/net.s | 45 +++++++++++++++++++++++++++++ src/net/uci/uci_cmd.s | 58 ++++++++++++++++++++++++++++++++++++-- src/net/uci/uci_errors.inc | 3 ++ 3 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/net/uci/net.s b/src/net/uci/net.s index e7cb1ec..c27964b 100644 --- a/src/net/uci/net.s +++ b/src/net/uci/net.s @@ -313,6 +313,7 @@ net_poll: ; ============================================================================= net_dhcp_acquire: jsr uci_wait_idle + bcs @dhcp_wait_to ; FPGA wedged — bail with C=1 lda #UCI_TARGET_NETWORK jsr uci_begin_cmd @@ -332,6 +333,7 @@ net_dhcp_acquire: lda #UCI_ERR_CMD_FAILED sta net_last_error +@dhcp_wait_to: sec rts @@ -399,6 +401,14 @@ net_tcp_connect: stx uci_connect_port_hi jsr uci_wait_idle + bcc :+ + ; FPGA wedged before we even queued anything — surface the timeout + ; (net_last_error already set) with the connect-fail tcp_state. + lda #UCI_TCP_CONNECT_FAIL + sta net_tcp_state + sec + rts +: lda #UCI_TARGET_NETWORK jsr uci_begin_cmd @@ -446,6 +456,10 @@ net_tcp_connect: @tc_no_err: ; Read 1-byte socket_id response. + ; Pre-zero uci_socket_id so a short-read leaves a known sentinel + ; (uci_read_resp_bytes only writes the bytes it actually receives). + lda #$00 + sta uci_socket_id lda #uci_socket_id @@ -458,11 +472,29 @@ net_tcp_connect: jsr uci_drain_status jsr uci_ack + ; Validate the response: firmware must have returned at least 1 + ; byte (uci_resp_count) AND a non-zero socket_id. Issue #36 — at + ; least one observed U64E firmware path returns no payload while + ; clearing the error bit, leaving uci_socket_id = 0 and us writing + ; into a phantom socket. Convert that into a clean failure. + lda uci_resp_count + beq @tc_no_socket + lda uci_socket_id + beq @tc_no_socket + lda #UCI_TCP_CONNECTED sta net_tcp_state clc rts +@tc_no_socket: + lda #UCI_ERR_NO_SOCKET + sta net_last_error + lda #UCI_TCP_CONNECT_FAIL + sta net_tcp_state + sec + rts + ; ============================================================================= ; net_tcp_send — push up to net_send_len bytes from (AX) through SOCKET_WRITE. ; @@ -519,6 +551,12 @@ net_tcp_send: @begin_chunk: jsr uci_wait_idle + bcc :+ + ; FPGA wedged mid-send — surface as send-fail (net_last_error already + ; set to UCI_ERR_WAIT_TIMEOUT inside uci_wait_idle). + sec + rts +: lda #UCI_TARGET_NETWORK jsr uci_begin_cmd @@ -646,6 +684,13 @@ net_tcp_send: ; ============================================================================= net_tcp_close: jsr uci_wait_idle + bcc :+ + ; FPGA wedged on close — force CLOSED state and bail. Best-effort + ; semantics already match the existing close path (no return code). + lda #UCI_TCP_CLOSED + sta net_tcp_state + rts +: lda #UCI_TARGET_NETWORK jsr uci_begin_cmd diff --git a/src/net/uci/uci_cmd.s b/src/net/uci/uci_cmd.s index 6ff81eb..b07cd70 100644 --- a/src/net/uci/uci_cmd.s +++ b/src/net/uci/uci_cmd.s @@ -26,6 +26,10 @@ ; one interface-index parameter). Later phases will extend as needed. .include "uci_regs.inc" +.include "uci_errors.inc" + +; net_last_error lives in net.s's BSS — we set it on wait timeout (#37). +.import net_last_error .export uci_abort .export uci_wait_idle @@ -61,19 +65,69 @@ uci_abort: rts ; ============================================================================= -; uci_wait_idle — spin until STATE==0 AND CMD_BUSY==0 +; uci_wait_idle — spin until STATE==0 AND CMD_BUSY==0, with wall-clock cap ; UCI_STAT_STATE ($30) covers the state field; CMD_BUSY ($01) is bit 0. ; ORing them (MASK $31) and looping while nonzero gives "fully idle". +; +; Issue #37 — the historical unbounded spin converts an FPGA wedge into a +; 600 s test sentinel timeout. The cap below uses CIA1 TOD (CIA_TOD_TENTHS, +; ticks at 10 Hz) — the only clock that runs at the same wall-clock rate +; regardless of CPU turbo speed. Cycle-counted budgets do NOT work here: +; the per-iteration cost scales with turbo (each fence is ~38 us of FPGA +; wall time but only a few CPU cycles at 48 MHz), so a budget tuned at +; 1 MHz collapses at 48 MHz (and vice versa). A prior attempt on +; feat/net-drain-abi shipped cycle-counted budgets and broke turbo DHCP +; for exactly this reason. +; +; CIA TOD read protocol: reading the HOUR register latches the four +; registers atomically; reading the TENTHS register unlatches them. +; We only need TENTHS for our 5-second budget, but we still latch+unlatch +; properly so we don't disturb other code that might be reading TOD. +; +; Budget: UCI_WAIT_IDLE_BUDGET_TENTHS (50 = 5 s). On expiry: set +; net_last_error = UCI_ERR_WAIT_TIMEOUT, return C=1. +; +; Output: C=0 on idle, C=1 on timeout. ; Clobbers: A ; ============================================================================= +CIA_TOD_TENTHS = $DC08 +CIA_TOD_HOUR = $DC0B +UCI_WAIT_IDLE_BUDGET_TENTHS = 50 ; 5 seconds at 10 Hz + uci_wait_idle: + ; Sample initial TENTHS for delta-tracking. Latch via HOUR, + ; release via TENTHS. We don't care about the HOUR value itself. + lda CIA_TOD_HOUR + lda CIA_TOD_TENTHS + sta @wi_last_tenths + lda #$00 + sta @wi_elapsed +@wi_loop: lda UCI_STATUS uci_fence ; settle read before testing bits and #(UCI_STAT_STATE | UCI_STAT_CMD_BUSY) ; $31 beq @idle_done - jmp uci_wait_idle ; long branch: fence too wide for BNE + + ; Check TOD for elapsed tenths. Latch (HOUR) then read TENTHS. + lda CIA_TOD_HOUR + lda CIA_TOD_TENTHS + cmp @wi_last_tenths + beq @wi_loop ; no change — keep spinning + sta @wi_last_tenths + inc @wi_elapsed + lda @wi_elapsed + cmp #UCI_WAIT_IDLE_BUDGET_TENTHS + bcc @wi_loop ; under budget — continue + ; Timeout + lda #UCI_ERR_WAIT_TIMEOUT + sta net_last_error + sec + rts @idle_done: + clc rts +@wi_last_tenths: .byte 0 +@wi_elapsed: .byte 0 ; ============================================================================= ; uci_wait_not_busy — spin until CMD_BUSY==0 (ignore STATE) diff --git a/src/net/uci/uci_errors.inc b/src/net/uci/uci_errors.inc index 4ac13f3..6525e6c 100644 --- a/src/net/uci/uci_errors.inc +++ b/src/net/uci/uci_errors.inc @@ -12,11 +12,14 @@ UCI_ERR_CONNECT_FAIL = $84 ; TCP_CONNECT returned an error bit UCI_ERR_SEND_FAIL = $85 ; SOCKET_WRITE returned an error bit UCI_ERR_READ_FAIL = $86 ; SOCKET_READ returned an error bit UCI_ERR_SHORT_WRITE = $87 ; SOCKET_WRITE wrote fewer bytes than requested +UCI_ERR_NO_SOCKET = $88 ; TCP_CONNECT response yielded no socket_id +UCI_ERR_WAIT_TIMEOUT = $89 ; uci_wait_idle exceeded its wall-clock budget ; TCP state values stored in net_tcp_state UCI_TCP_CLOSED = $00 ; no active socket UCI_TCP_CONNECTED = $01 ; connected, reads/writes valid UCI_TCP_ERROR = $02 ; saw an error on a read — stop polling +UCI_TCP_CONNECT_FAIL = $03 ; TCP_CONNECT did not yield a usable socket ; UCI firmware data queue max per SOCKET_WRITE push (see uci_network.py) UCI_DATA_QUEUE_MAX = 800 From c0571462497708f4ec015efda0732f07d58a8c9b Mon Sep 17 00:00:00 2001 From: JC_000 <3798556+JC-000@users.noreply.github.com> Date: Sun, 10 May 2026 14:13:07 -0500 Subject: [PATCH 3/3] docs(uci): document bounded uci_wait_idle and new error codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote the "bounded timeouts must use wall-clock time" design note from "future work" to "implemented for uci_wait_idle, copy this pattern" — TOD read order (HOUR latch → MIN → SEC → TENTHS unlatch), 5 s budget, C=1 + UCI_ERR_WAIT_TIMEOUT on bail. Note that uci_push_wait and uci_end_cmd are still unbounded and should follow the same template when a wedge is observed. Add a new "UCI error codes" subsection enumerating the values surfaced via net_last_error and net_tcp_state — UCI_ERR_NO_SOCKET (short-read OPEN_TCP / phantom socket), UCI_ERR_WAIT_TIMEOUT, UCI_TCP_CONNECT_FAIL — with cross-refs to issues #36 and #37. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 51 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8ffcc2a..7f10ec0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -139,6 +139,29 @@ is **$C9**. See `src/net/uci/uci_regs.inc` for the full equate list `uci_read_data`, etc. No zero-page usage — all absolute addressing and self-modifying code. +`uci_wait_idle` is wall-clock-bounded (5 s budget via CIA1 TOD) per +the design note below. On timeout it returns C=1 with `net_last_error += UCI_ERR_WAIT_TIMEOUT`. The four callers (`net_dhcp_acquire`, +`net_tcp_connect`, `net_tcp_send`, `net_tcp_close`) all `bcs` out to +surface the failure rather than letting the C64 hang indefinitely on +a wedged FPGA. `uci_push_wait` and `uci_end_cmd` are still unbounded +and should be converted to the same TOD pattern if a wedge there is +ever observed. + +### UCI error codes + +`src/net/uci/uci_errors.inc` enumerates the values surfaced via +`net_last_error` and `net_tcp_state`. The most load-bearing: + + - `UCI_ERR_NO_SOCKET` — `net_tcp_connect` got a short-read on the + OPEN_TCP response (no socket-id byte), so the firmware never + actually opened the TCP connection. `net_tcp_state` is set to + `UCI_TCP_CONNECT_FAIL` and C=1 is returned. Without this check + the C64 would commit to a phantom socket and push TLS bytes + into nowhere (see issue #36). + - `UCI_ERR_WAIT_TIMEOUT` — `uci_wait_idle` exhausted its 5 s budget + (see issue #37). + ### DNS UCI firmware resolves hostnames internally during `TCP_CONNECT`. There @@ -360,16 +383,24 @@ touching TLS call sites. ### Design note — bounded timeouts must use wall-clock time -Any future robustness work on the UCI adapter's spin-wait helpers -(`uci_wait_idle`, `uci_push_wait`, etc.) MUST use a wall-clock time -source — CIA timer on stock C64, TOD clock on U64E — rather than a -cycle-counted iteration budget. The fences around every UCI register -access make per-iteration cost scale with CPU speed: a budget that is -ample at 1 MHz collapses to far too short at 48 MHz because turbo -scales CPU cycles but not the FPGA's wire-level operation durations. -A prior attempt on branch `feat/net-drain-abi` split waits into -fast/long tiers with cycle-count budgets and broke DHCP at turbo for -exactly this reason; the branch was abandoned. +Robustness work on the UCI adapter's spin-wait helpers (`uci_wait_idle`, +`uci_push_wait`, etc.) MUST use a wall-clock time source — CIA timer +on stock C64, TOD clock on U64E — rather than a cycle-counted iteration +budget. The fences around every UCI register access make per-iteration +cost scale with CPU speed: a budget that is ample at 1 MHz collapses +to far too short at 48 MHz because turbo scales CPU cycles but not the +FPGA's wire-level operation durations. A prior attempt on branch +`feat/net-drain-abi` split waits into fast/long tiers with cycle-count +budgets and broke DHCP at turbo for exactly this reason; the branch +was abandoned. + +`uci_wait_idle` is the first helper to follow this pattern (issue #37). +At entry it samples CIA1 TOD ($DC08-$DC0B) — read order is HOUR +(latch) → MIN → SEC → TENTHS (unlatch) — and on each spin pass re-reads +TENTHS, bailing with C=1 + `net_last_error = UCI_ERR_WAIT_TIMEOUT` +after 50 transitions (~5 s wall-clock, independent of CPU turbo). State +lives in two SMC bytes inside the routine to match the file's no-ZP +convention. Use this as the template for any future bounded helper. ## Memory layout