Conversation
VMABI.h says it is the whole contract, and for a module it is. For a host there was no equivalent: the slot geometry, the "@VM1" authorization, the EEPROM entry and exit protocol, the launch record, the manifest CRC and the failure record were spread across VMBootImage.h, VMRegistry.h, VMFail.h and Common_Defs.h, none of which a third party can compile against. Writing a host meant reading TeensyROM's source and reimplementing what it found. VMHostABI.h collects them, and vm/abi/vm_host_abi.h publishes it beside vm_abi.h. The split follows one rule: a bare constant is duplicated and compared by verify-extensions, an algorithm is moved so there is one copy. Duplicating an algorithm cannot be drift-checked, and these fail silently when they diverge -- the manifest CRC runs over the struct's padding, so a reimplementation that looks right returns a different number and every launch dies as ManifestCrc with nothing to debug against. Moved on that basis: the slot predicate, VmHostId, the marker, the launch record, the manifest parser, the failure record, vm_load_payload, the module table check, and the ITCM unlock -- which is a load step rather than a guard, because the core leaves the module window read-only and the payload copy faults without it. Duplicated instead: the EEPROM addresses and boot-indicator values, because Common_Defs.h is 29 KB of firmware-wide definitions a vendor must never include and its EEPROM map is a running accumulator that should not have three entries pulled out of it. checkEepromProtocol compares the two. Behaviour on the target is unchanged and the sizes say so. Against the parent, tr-plus --with-extensions: minimal identical in both flash and ITCM; main identical in ITCM, 264 bytes more flash code; the extension image 192 bytes more ITCM and 392 more flash, leaving 10360 bytes under the 96 KiB ITCM ceiling. The larger helpers keep FLASHMEM through VM_HOST_TEXT, which is what holds the extension image's ITCM steady -- without it the same refactor cost 1520 bytes there, because `inline` let GCC keep a copy in each caller. Two changes are host-build only, both scaffolding for the installer to come: the buffer standing in for the slot grows to its full 384 KiB, and install() takes a host name.
"A module needs VMABI.h and nothing else" has been in that header since the ABI was written, and the host contract added beside it makes the same promise for two files. Both are claims about absence, which nothing was checking: an include added to either header still builds everywhere in this repository, because every build here has the repository on its include path. It would break only for the vendor who copied the files out, which is the one place nobody runs the tests. Two gates, because neither alone reaches the vendor's position. The headers' includes are read as text against a published allow-list: a native compile settles the #if defined(__arm__) and #ifdef FLASHMEM branches the other way from the target, so an include under either would pass a build here. Then VMABI.h is compiled alone, and both headers with one test that runs, each in a directory holding nothing else and with no -I back into the tree. Mutations, each run and named: #include "Common_Defs.h" inside VMHostABI.h's __arm__ block compiles clean standalone and is refused by the allow-list; #include "VMHostABI.h" in VMABI.h is refused by both; a typo in vm_host_abi.h's relative path is caught by the shim compile, which is the only thing that would see it, since vm_abi.h has four in-tree includers and vm_host_abi.h has none. The shims told a vendor to copy themselves out, which cannot work -- their relative path does not resolve from a vendor directory. They say what travels instead, and the claim in VMHostABI.h is restored to naming the gate, which it could not honestly do when it was written one commit ago.
A vendor host adds callbacks behind its own service bit. Today a module that requires one cannot be expressed: vm_valid_header refuses any bit outside VM_KNOWN_SERVICES, so the image is malformed before anything gets to ask which host is installed. Mean Hamster Software's copy of the header works around it by redefining VM_KNOWN_SERVICES, which is a fork of the validator. vm_valid_header now judges structure and self-consistency only. Bit 7 stays, because it changes how reserved[1] is read; the rest is a question for a particular host's services, which VMRegistry.h's preflight already asks and answers by name. Measured on the registry test: with the clause in place, launching a package requiring bit 16 against the stock host says "VM package/client failed validation", which sends the author to look at their build. Without it, "TeensyROM host lacks service $10000". The bit numbers become a registry with assignees rather than four reserved numbers in a comment -- the eight Mean Hamster uses, two on request, and bit 16 to this repository's own examples and conformance fixtures. Policy moves to the packager, which is the machine that can afford it: buildImage refuses an unassigned number unless --allow-unassigned-services says otherwise, build-extension.mjs gains --services, and parseImage stops judging services at all, so it still mirrors the validator exactly. checkServiceRegistry compares the two copies of the assignment list, the base profile and bit 7, so a packager that refuses what the loader would serve fails the gate. image_test's four reserved bits invert from refused to accepted, joined by an assigned bit and an unassigned one. The registry fixture gains a third package requiring bit 16, and registry_test asserts the message names the number; restoring the deleted clause turns that assertion red at registry_test.cpp:70.
The moves into VMHostABI.h left the firmware's own host calling the published definitions instead of its private ones. The native suite compiles those headers but runs its own host against them, so only the target exercises this. A TeensyROM+ built from this branch launched the reference extension and drew the same four lines as a build from 7cce56f, before the moves.
A .TRH package is a 64-byte header followed by the raw slot image, so the payload stays byte-identical to what objcopy produced. The header is a separate container rather than a field in the image because the image's own descriptor sits inside the region a CRC would have to cover; writer and reader would otherwise both have to stream around the same hole. The install order is what the design turns on. Sector 0 is erased first and its FlexSPI tag programmed last, so an interrupted install leaves a slot vm_host_slot_valid rejects rather than one it enters and then faults in. The test cuts power at every operation an install performs and requires the slot to read as absent, as the old host, or as the new one. Erasing descending, or programming the tag along with the rest of its sector, turns that red. Native only; no firmware writes flash yet.
The .TRH header carried an ordering essay in three places -- the VMHostInstall.h banner, the BOOT_WORD table in extension.mjs and the fake_flash.h banner -- each arguing that an interruption cannot leave a slot the loader would enter. The argument belongs in the commit message and the test; repeating it at the reader is what the standard calls self-justification. extension.mjs pointed at a checkHostSlotPredicate() in verify-extensions.mjs that does not exist. Nothing pins hostSlotValid against vm_host_slot_valid, so the comment now says that. VMHostInstall.h referred to install steps as P6, P7 and P8, a numbering the tree does not carry.
The install relied on erasing sector 0 to stop the slot reading as a host. A sector erase fixes no order in which its cells reach the erased state, so an interrupted one can leave the FlexSPI tag at offset 0 standing while the descriptor beside it is gone: vm_host_slot_valid then accepts a slot whose MVH2 descriptor reads 0xffffffff, which is the identity() false the install order was written to avoid. vm_host_invalidate programs the tag to zero first. A program only clears bits, so a torn one still leaves a tag the check rejects, and the erase that follows is free to land in any order. The un-commit on a verify failure goes through the same helper. The sweep ran a torn erase over the low half of its sector only, which is why it did not see this. It now runs both halves.
The README lists what `npm run verify:extensions` covers. host_install_test joined that run and was not in the list, so the list understated it.
host_install_test runs in verify:extensions, and 3d3a74a added it to the list in vm/abi/README.md. The line the run prints at the end says what it covered, and still omitted it.
VMHostInstall.h already decides what is written in what order, and the power-loss sweep in host_install_test holds it to leaving the slot absent, the old host, or the new one. This is the device behind it: a flash object over the core's FlexSPI primitives, a reader over the SD/USB File, and DoHostInstall in FlashUpdate.ino beside the .hex updater it resembles. Validation reads only, so a package this firmware will not take is refused as an ordinary message with the C64 still running. Past that point the machine has to go: the menu executes from cartridge ROM served by isrPHI2, and the core's sector erase spins with interrupts off for the whole erase, so the 6510 is held in reset before the cartridge stops answering. The message goes out before the reset assert, because on fab 0.4 that assert pulls the pin isrExtResetDetect watches and the resulting BtnPressed ends the wait for the C64 to read it. Reading the slot back has to reach the part: it is XIP and cacheable, and the FlexSPI writes go around the cache, so each read-back drops the sector first. The install then ends in a reboot whichever way it went, and its outcome travels in the VmFail record the main image collects and shows. Success reports too -- captureHeld filters on code != Ok, and an install the menu never mentions looks from the couch like nothing happened. Menu wiring is rtFileTRH with its two C64 mirrors, the extension itself registered only in an extension build, and "trh" added to protectedExtensions so a manifest cannot claim it before the switch sees it.
Every other project under Source/C64 carries a .gitignore holding build/*; VMHello arrived without one, so running the suite leaves its assembler output untracked in the tree.
Review findings on 4ecbd44, all in what the new rtFileTRH item type reaches rather than in the installer itself. nfcScan's random picker takes everything from rtFilePrg up except rtFileHex, so a tag naming a directory could land on a .TRH and erase the host slot from a tap. rtFileHex was carved out by value; both sit behind IsDeviceWriteType now, beside the table where a file type is registered, so the next type that writes the device is added in one place. The extensions the stock menu owns are written down in four places and 4ecbd44 reached two. The packager would emit a manifest claiming trh that the firmware then refuses whole, and vm/abi/README.md still published the list without it. verify-extensions compares all four against the menu's own table now; dropping trh from each in turn fails it. VmSlotFlash::program reported success whatever the part did, while erase() beside it checked. vm_host_invalidate clears the tag and then erases the sector holding it, and VM_HOST_ID_OFFSET is 0x800, inside that same sector, so a write that quietly did nothing puts back the torn erase c11f860 removed. It reads back through map() now, which makes VmFail::InstallProgram reachable. BadFormat fires below the supported format as well as above it, so its message no longer says "too new".
SerialStringBuf is MaxPathLength+6 = 262 bytes. SendMsgPrintfln and SendMsgPrintf formatted into it with vsprintf, and DriveDirLoad.ino hands them a FullFilePath[MaxNamePathLength] = 358, which sprintf fills to 357 from a 256-byte DriveDirPath and a 100-byte item name. A deep enough directory on the card overran a global. Every message in the firmware goes through those two, so bounding them is the fix; the minimal image's own SendMsgPrintfln gets the same treatment, and so does the default-SID line in StatusFunctions, which builds a path into the same buffer without passing through either. SendMsgPrintfln takes two fewer bytes than the buffer holds, because it formats first and then shifts the text right to prepend the newline.
Bounding SendMsgPrintf/ln left three ways for card-supplied text to get past them. A menu item's name reached SendMsgPrintfln as the format string, so a file named %n on the card writes through it; vsnprintf bounds the output and not the argument walk. SIDLoadError and SendStrPrintfln passed a variable the same way. All three now format with "%s". GetCurrentFilePathName built "%s:%s/%s" from DriveDirPath and an item name with sprintf into whatever the caller handed it, and every caller hands it a MaxPathLength buffer that a deep directory overruns -- the same trigger one frame ahead of the message buffer. It takes the buffer's size and truncates now, which also keeps the hot-key and auto-launch callers from running EEPwriteStr past their MaxPathLength EEPROM slots. LatestSIDLoaded packs a source byte, a path and a name into malloc'd MaxPathLength; a 255-char DriveDirPath needs 257 bytes by itself. It is packed through SetLatestSIDLoaded, which leaves both fields terminated inside the block, and the two places that read the record back call TerminateSIDRecord first, since EEPROM may still hold one written before this. DriveDirPath grows through strcat with no bound of its own, so it can still overrun before any of this runs. Refusing a descent that will not fit changes what the C64 menu does and wants its own commit.
Bounding the message formats left the read that reaches them unbounded. LoadFile parses the main header into lclBuf[CRT_MAIN_HDR_LEN], and the 32-byte "Name" field ends at that buffer's last byte, so a name filling all 32 carries no terminator and %s walks off the stack. Both parsers read it as %.32s now. AT_DT's 100-byte Buf gets snprintf for the same reason its hostname is user-typed. Known-Issues.md proposed exactly this alongside the vsnprintf and still said none of the memory-safety items was fixed. Its entry for the unbounded vsprintf is retired, the sprintf-into-fixed-buffer audit records which instances are closed, and the summary names what the last three commits fixed. Also cuts three comments that narrate their own lines.
Every drift gate in verify-extensions.mjs finds its constant by matching the file as raw text, and String.match takes the first hit anywhere. A comment naming the constant satisfies the gate while the declaration beside it drifts: with "// historical: VM_HOST_SLOT_BASE = 0x60280000u" above the enum and the enumerator itself moved to 0x60990000, checkBootSlot printed PASS. The include allow-list had the same shape from the other side -- its pattern required whitespace after #include, so #include"Common_Defs.h" inside the __arm__ block was invisible to it and compiled clean standalone, because the native build never takes that branch. tools/lib/source-text.mjs now splices line continuations and strips comments before any gate reads, in the preprocessor's own order, and the include pattern accepts the unspaced form. It is a lib module rather than a local helper so that npm test can reach it: verify-extensions.mjs is run only by npm run verify:extensions, and nothing that broke a gate there would have reddened a test. Both near misses and the continued form are now refused alongside the spaced form that already was, and disabling any one of the three transforms turns source-text.test.mjs red.
Three things the header got wrong about itself once it became the file a vendor compiles against. vm_manifest_parse was `static` in a header every TU that includes VMFail.h or VMBootImage.h now pulls in, so a TU that does not call it warns: g++ -Wall -Wextra on vm/tests/fail_test.cpp reported "unused function". `static inline` silences it and costs nothing on the target -- a tr-plus --with-extensions build is size-identical to the one before this commit in all three images, so the ITCM the parser's FLASHMEM placement protects is untouched. vm_load_payload lost its precondition when it moved out of VMImageLoad.h, which said the caller validates the header first. It bounds nothing itself: a host that reads the 64-byte header and calls it copies code_bytes to VM_CODE_BASE and memsets bss_bytes past the window before the payload CRC is ever checked. The docblock names vm_valid_header again. The header called its dependencies stdint, stddef and string.h, but vm_manifest_extensions calls strcasecmp, which is POSIX <strings.h>. It resolved only because the libcs in use pull that in from string.h. The include is explicit now, and on the allow-list the standalone gate reads.
Relaxing vm_valid_header moved the service question out of the format validator, and image_test's four reject cases became accept cases with it. What was left holding the descriptor-less path -- a host installed before it could say what it provides, which the main image cannot ask in advance -- was one clause in VMHost.h that no test compiles. Deleting it at 9b3179d leaves npm test and npm run verify:extensions fully green. The clause becomes vm_host_serves() in VMHostABI.h, where a host vendor can see it: the published headers previously stated the obligation only as prose in VMABI.h and pointed at VMRegistry.h, which is not published. Two gates now hold it. host_abi_standalone.cpp exercises the predicate, and a wrong-but- compiling rewrite of it aborts there; checkHostAdmission reads the call in VMHost.h, and deleting that call fails the gate by name. The docblock's reason for keeping bit 7 in the validator was wrong: vm_image_ro_bytes returns reserved[1] unconditionally and no consumer of reserved[1] branches on bit 7. What bit 7 is checked against at VMABI.h:164 and :166 is the memory profile in reserved[0]. Corrected. tr-plus --with-extensions is size-identical in all three images.
checkServiceRegistry compared three ORed totals, so drift that preserved a total was invisible. Swapping SERVICE.FILES and SERVICE.CLOCK in the packager passed every gate; so did adding a bit to VM_HOST_SERVICES, which is where a service this loader starts providing would land and is the one case the gate exists to stop -- the packager would go on refusing a module for a service the loader serves. The five base enumerators are compared individually now, and VM_HOST_SERVICES by its expression, since it is named terms rather than literals. All three drifts above now fail the gate by name. --services advertised 0x1001b as an example mask. The base profile is 0x1f, so that omits VM_SERVICE_PACKETS; a user copying it ships an image that does not declare it needs packets. It is 0x1001f, and extension.test.mjs now reads the example back and compares it against BASE_SERVICES | SERVICE_EXAMPLE, so putting 0x1001b back turns npm test red. The note beside it fired only for bits assigned to another host, so a build forced through with --allow-unassigned-services -- the one case where the user overrode a guard -- printed nothing, and the wording was wrong for bit 16, which is assigned to this repository. It now names whatever the loader will not provide. The README's registry listed VM_SERVICE_RAM2_RO as "base profile" while VMABI.h says the base profile is bits 0..4 and bit 7 is this loader's optional memory profile. A module reading the table would conclude it may assume RAM2_RO.
`lda #NumPages-1` sends '0' wherever the page table happens to end. Append a page after Installed Extensions and '0' follows it there, while Pg_Index.asm still prints "0 Installed Extensions" and the assembler reports nothing. Derive the index from a label on the table entry. The docs that enumerate the settings pages still described nine of them.
8acfdef gave GetCurrentFilePathName the size of the buffer it writes into and moved every site to snprintf. FullPathToSelected, added three commits earlier by 4ecbd44, builds the same kind of path from the same two strings and kept sprintf. Both callers hand it a MaxNamePathLength buffer, which is MaxPathLength + MaxItemNameLength + 2 and so is wide enough for a terminated DriveDirPath and a menu item name. That is arithmetic about the destination rather than a bound on the source, and Known-Issues.md records DriveDirPath as overflowable by ordinary deep menu browsing -- it is one of the two strings this reads. The new case in c64-message-buffer.test.js goes red with the change reverted.
…ed on vm_host_scan reads flashMagic from sector 0 of its 4 KiB staging buffer and the other four boot words from sector 1, but vm_trh_valid only required payloadBytes > 0x1000. A payload stopping inside sector 1 leaves the rest of the buffer holding sector 0, so the mirror checks compare the header against bytes the package author also wrote, at offsets the file never had. Built against the worktree with the real fake_flash.h, a package of 0x1004 payload bytes carrying entry, bootBase and imageBytes at payload offsets 0x4, 0x20 and 0x24: vm_trh_valid : ACCEPTED vm_host_scan : ACCEPTED bootWords 42464346 432000d1 60282001 60280000 00001004 before : vm_host_installed = yes vm_host_install: status=0 detail=0x1004 -> VmFail::Installed after : vm_host_installed = no flash@0x1004 = ffffffff The incumbent host and the whole slot are erased, the C64 is told "extension host installed", and the slot then reads as no host at all. The floor is now two sectors, which is what tools/lib/extension.mjs already required of an image it writes; parseHostPackage took the same 0x1000 bound and now shares the constant. At 0x2000 a package still scans and installs and the slot reads back installed; 0x1fff and 0x1004 are refused as BadLength. Reverting the bound turns the reject assertion at host_install_test.cpp:52 red: "Assertion failed: (!r && r.status == want)", exit 134.
90811a1 added two reject cases and an accept case at the payload floor. The line the run prints still said "eight malformed-header cases", and there are now ten -- the same drift f8a76a5 and 3d3a74a fixed in this suite's summary and in the README that enumerates it. The count is not what the suite turns on, so it is gone rather than corrected, and the floor is named instead. The two derived numbers beside it stay: both are printf arguments the run computes.
hostSlotValid() in tools/lib/extension.mjs and vm_host_slot_valid() in VMHostABI.h answer the same question at package time and on the device, and the packager's refusal text promises the device will enter what it accepts. Nothing compared them, so either could narrow alone: a narrower device predicate installs a host that then reads as no host, and a narrower packager one refuses a legitimate host and blames the image. host_abi_standalone.cpp now prints its verdict for thirteen five-word vectors around the entry window, the image-size bounds and the two magics, and checkHostSlotPredicate() re-answers each with the JavaScript mirror. Narrowing the C++ entry window to base+0x2000 used to leave the whole suite at exit 0; it now stops at Error: vm_host_slot_valid says 0 and hostSlotValid in tools/lib/extension.mjs says 1 for flashMagic=0x42464346 vectorMagic=0x432000d1 entry=0x60283001 bootBase=0x60280000 imageBytes=0x4000 and narrowing the JavaScript side instead fails the same vector the other way.
C64-Software.md lists the page files "index -> content", and had Ethernet at index 5 with Time/RTC and Info: General after it. tblSettingsPages in SettingsMenu.asm runs Time/RTC, Info: General, Info: Ethernet in that order, which is also what Pg_Index.asm prints against keys 6, 7 and 8 and what docs/General_Usage.md numbers. The error predates the branch; 7c1c1c0 edited this sentence to add the tenth page and left the order alone. The same file carries a standing warning to check page indices against the source, so a wrong one there is worth more than its size.
90811a1 put the two-sector floor in vm_trh_valid, but the function that depends on it is vm_host_scan, which copies the boot words from staging+0x20 and +0x24 without knowing whether the read that filled that sector reached them. Today's one caller happens to validate first; the header is published to host vendors, whose caller might not. Removing the guard again turns the new case red: "Assertion failed: (!got && got.status == VmInstallStatus::BadLength), file host_install_test.cpp, line 107".
VmSlotFlash::program returns memcmp(map(offset), data, n) == 0, so the device reports ProgramFailed for a value the cells cannot hold. FakeFlash::program ANDed the bytes in and returned true, so the power-loss sweep's conclusion rested on looser primitives than the hardware's. Inverting vm_host_invalidate to erase sector 0 before clearing the tag leaves 0x00000000 where the final tag program needs 0x42464346. The install now ends ProgramFailed (14) with cells[0..3] = 00 00 00 00, where before it ran on and ended VerifyFailed (15) at the whole-payload CRC.
Three holes, each shown by reverting the thing the gate exists to protect and watching the suite stay green. The format scan ran line by line and required the first argument on the open paren's line, so reflowing a call hid it. `SendMsgPrintfln(\n MenuSelCpy.Name);` is the DriveDirLoad.ino defect reintroduced, and it compiles the same. The scan now reads the whole file, with comments blanked rather than cut so the offsets still name lines -- blankComments in tools/lib/source-text.mjs, beside the withoutComments the drift gates use. Nothing asserted the bounding itself. Reverting all four of vsnprintf and snprintf back to vsprintf and sprintf left seven of seven passing; it now turns "the formatters that write the C64 message buffer are bounded" red. Keeping the format a literal and keeping the output inside the buffer are separate properties and the suite now holds both. The two caller checks walked hard-coded file lists. An unsized GetCurrentFilePathName in nfcScan.ino and an eepAdDefaultSID read without TerminateSIDRecord in DriveDirLoad.ino both passed; each now fails by name.
The install message promised a blank screen, but the DMA write of D011 that blanks it is under Fab04_FullDMACapable, and --with-extensions is independent of the target. On a fab 0.2/0.3 extensions build the VIC keeps painting the frozen menu for the whole 45 seconds. checkEepromProtocol justified itself with "nothing links both". Teensy.ino includes Common_Defs.h and, since this branch, VMHostInstall.h; MinimalBoot.ino includes Common_Defs.h and VMFail.h. Both reach one translation unit, so the reason given was not the reason. "every single-byte corruption of a host package is caught" flipped a bit at eight of 32832 positions. The property does hold -- a sweep of all of them misses none -- but it costs thirty seconds, so the test now sweeps the whole header the way host_install_test.cpp already does, samples the payload, and is named for what it does. VMRegistry.h uses ten names from VMHostABI.h and included it only through VMBootImage.h; it now says so itself. Not a latent break -- VMBootImage.h needs those names too -- just include-what-you-use.
Seven fixes from the branch review. Each is a defect the review named with a failing input rather than a preference; the reviewer wrote them, and they land here as their own commit so the commits they were found in keep their SHAs. The unbounded sprintf path-build that HandleExecution replaced with FullPathToSelected was still at three other sites: LoadFile in both DriveDirLoad.ino and Min_DriveDirLoad.ino, and LoadBank in IOH_EasyFlash.c. SetDriveDirMenuNameType mallocs Name at strlen+1 with no MaxItemNameLength cap, so a 240-character SD filename under a 130-character path writes 372 bytes into a 358-byte stack buffer. All three now snprintf against sizeof. buildHostPackage sized its 0xFF pad from the image's own declared length before hostSlotValid bounded that word, so an image declaring 0xfffffff0 reached Buffer.alloc with 4 GB instead of the refusal the next line would have given it. vm_host_install held a second 4 KiB sector0 buffer on the stack for the whole install, on a path reached from the menu -- the class Known-Issues.md already blames for a reproducible crash in nfcReadTagLaunch(). Sector 0 is re-read through the caller's staging buffer at the point it is programmed instead. The power-cut sweep still holds the slot to absent, untouched or complete. A failed un-commit after a failed verify was discarded, so a slot still carrying a bootable tag over an image that did not verify reported as a clean refusal. The minimal image would have entered it on the next boot. DoHostInstall read the 64-byte header before consulting the file length, so ShortFile was unreachable on the device and a truncated .TRH blamed the card. vm_trh_valid reported reserved[0] whichever reserved word was set, so the only diagnostic the install path has named a field that is zero. The corruption sweep's descriptor position indexed the package rather than the payload, so the MVH2 refusal it is named for was never exercised. hostImage() moves to fixtures.mjs so the unit tests and the native fixture cannot drift.
…short one Review of bab62d2. Two of the four new assertions were `strlen(narrow)<4`, which is satisfied by `strlen == 0` -- so a displayName that rendered the host as nothing at all passed the case added to prove it truncates. Measured rather than argued: with if (!id) { snprintf(out, bytes > 15 ? bytes : 1, "%s", noDescriptor); return; } verify:extensions was green. The placeholder branches now assert the bytes they should have produced, "(un" and "(no", and the same mutation aborts at registry_test.cpp:159. The pair also tells the two placeholders apart at this width, which two identical length checks could not. strcmp rather than strlen for a second reason: strlen on a buffer displayName failed to terminate -- one of the two failures this block exists to catch -- walked off the end of a ten-byte `fenced` looking for a NUL nobody wrote, and only reached its verdict by reading whatever followed it on the stack. strcmp stops at the first byte that differs, inside the array, every time. The zero-length canary compared against a hand-counted "##########". Nothing tied those ten characters to `sizeof fenced`, so widening the fence would have read past the literal and compared against whatever sits after it in .rodata. The widths are now two constants the array, the canary checks and the calls all read from; set fenceBytes to 5 and the block still passes. Re-checked by mutation, each one restored afterwards: the loop guard to `n < bytes` aborts at :148, dropping `if (!bytes) return;` aborts at :165, emptying the (unnamed) branch at narrow widths aborts at :154, and emptying (no descriptor) aborts at :159. npm test 107/107, verify:extensions exit 0.
…the jitter slack Four defects in 9008a01, found reviewing it. **DrainCmdChannel had no exit the peer did not control.** Quiet is reset on every byte, so the loop ends only after 500 mS of silence -- and 9008a01's own rationale for adding it says the deadline branch "fires while the peer is still sending at full rate, where it is the rule". That is exactly the state in which the drain never returns. A TCP peer (which never authenticated) posting len=100000 and dribbling one byte per 400 mS trips the deadline at ~50 s, enters the drain, keeps dribbling, and the board never comes back. The commit set out to shorten a bounded 24.8-day hold and handed the abort path an unbounded one. Worse on WriteC64MemCommand, which is in the always-available tier: that drain runs while the C64 is being emulated, and a consumer that streams DMA continuously (c64cast is named in docs/ControlComms.md) never lets it out. Now capped at two quiet windows. Past the cap the remainder goes back to being scanned for tokens, which is where it was before the drain existed; unbounded is not anywhere it was before. **The send-side deadline was tested where time is not spent.** SendFileData checked it once per 64-byte chunk, but the thing 9008a01 set out to bound -- lastProgressTime, reset on every partial write -- still governs inside a chunk. The same peer it describes, one byte per 1.9 s, runs sizeof(chunk) * 1.9 s = 122 s past the deadline before the outer loop looks again. The check is now in the stall branch too, which is the only branch that can spend time: while availableForWrite keeps returning nonzero the loop makes progress and ends. **One SerialTimoutMillis of slack is not slack.** The deadline is tested *before* SerialAvailabeTimeout, so one legal per-byte wait can overrun it by a full window on its own, consuming the entire constant term. A 256-byte file over TCP got 628 mS against 22 mS of wire time; a single retransmit failed a transfer with nothing wrong with it. The per-channel floor 9008a01 added fixes the rate term and left this one, so small transfers never saw the fix. Ten windows is 5 s, which does not move the hostile bound -- that case is measured in hours. Also: ReceiveFloorBytesPer_mS is now TransferCeilingmS(len) and returns the whole deadline. One knob set the receive floor, the send floor and the DMA floor under a receive-only name, so a change made for an SD-write reason landed on the DMA path unannounced. Both new functions are FLASHMEM now, as every other function in both files is; without it they are copied to ITCM, which is a different budget from the flash headroom the build reports. And two cited line numbers were off by one: push.py:8 -> :7, ControlComms.md:53 -> :52. Not fixed: the drain still cannot tell an aborted file's remainder from the peer's next command, so a client retrying inside the quiet window has that command swallowed. The real fix is framing in ProcessCommand, which scans without alignment; that is a separate change. npm test 107/107, npm run verify:extensions pass, built --target tr-plus, exit 0, 1301.0 K of headroom.
Upstream moved twice while this branch was in review, both times in files the branch also rewrites: d937355 firmware: route the extension launch reboot through RebootTR() e8f35c7 vm/tests: shim RebootTR() for the native host conformance suite e8f35c7 auto-merged. d937355 conflicted with this branch only by adjacency: upstream rewrote tryLaunch()'s last line and this branch rewrote the line above it. Resolved by taking both -- VM_HOST_MARKER for the EEPROM write and RebootTR() for the reboot. VMHostABI.h:84 defines VM_HOST_MARKER as "@VM1", the literal upstream still had there, so the resolution changes no stored bytes. One behaviour does change at this site, and it is upstream's change, carried rather than adjusted: the hand-inlined form had a delay(20) between SetResetAssert and REBOOT, and RebootTR() (Common_Defs.h:525) has no delay. That now matches every other reboot call site in the firmware, which is what d937355 exists to do. Rebasing would have been the other option and was declined: it rewrites all 124 SHAs on the branch, and every commit here has a recorded review against its SHA. Gates on the merge result: npm test 107/107, npm run verify:extensions all PASS (ten conformance suites), python3 -m unittest discover -s tools/bench 64/64, node tools/build-firmware.mjs --target tr-plus --force exit 0 with 1301.0 K of self-update headroom.
…ines 8f73f65 fixed the row that ate a line on the Installed Extensions page and reported a sweep behind it: "Swept every !tx line ending in ChrReturn across Source/C64/*/source/*.asm for the same shape. This was the only one." That sweep cannot see the shape it is looking for. A row is everything drawn between two returns and can be spread over several !tx directives, so a fill whose last character is on one line and whose ChrReturn is on the next matches no "!tx line ending in ChrReturn" and reads as clean. The glob also stops at *.asm, and Source/C64 has 90 !tx directives in .s files -- 88 of them in MainMenuCRT/source/StringsMsgs.s, the main menu's message table. Measuring rows instead finds two the sweep missed, both pre-existing and both deliberate: the MIDI2SID banner (the return at M2Ssupport.asm:158) and the cart banner (TeensyROMC64.asm:155), each a full-width reverse-video bar drawn under a ruler comment counting to 40. They cost the row the wrap lands on -- the returns after them each advance a further line -- which is cosmetic in a standalone utility and invisible on a banner the main menu immediately clears. Left as they are and listed in the test, so a new one has to be added there on purpose. tools/lib/c64-screen.mjs models the part of CHROUT that moves the cursor: printables advance and wrap at 40, control codes do not (colour and reverse-video codes are control codes, which is why an !tx line's visible width is not its character count), ChrClear and ChrHome park at column 0, and a quote flips the editor into quote mode where control codes draw as reverse glyphs and do advance. Backslash escapes are consumed the way ACME consumes them -- TRExtPortCheck.asm:97 measures 41 as typed and 39 drawn for that reason, confirmed against the bytes in the committed TRExtPortCheck.prg.h. An expression it cannot evaluate is reported rather than assumed, so a scan that understood less than it looked like it did does not come back quietly clean. Verified by mutation, in both directions of the class: - restoring 8f73f65's 40-column row fails the sweep at Pg_InstalledExt.asm:126 - splitting that same row across two under-40 !tx directives fails it at Pg_InstalledExt.asm:127 -- the case the old sweep reads as clean - the tree as it stands reports exactly the two declared banners The comment 8f73f65 left in Pg_InstalledExt.asm said "keep every line below 40 visible columns", which is the per-line rule rather than the per-row one. It now says row, says a row can be several directives, and names the gate. Comment only: SettingsMenu.prg.h rebuilds byte-identical. npm test 117/117 (107 plus 10 new), npm run verify:extensions all PASS.
…the C64 bd2b836 upgraded the descriptor row to an unqualified yes and, with the hedge, removed the only sentence in vm/abi/README.md that said the quoted strings are the firmware's bytes rather than what the screen draws. It also changed the verb to "shows", so the row now asserts the C64 displays `TeensyROM ABI 2 services $409f`. It does not. The firmware formats ASCII into SerialStringBuf. The C64 reads it back a byte at a time and calls CHROUT unconverted -- StringFunctions.asm's PrintSerialStringLoaded, `SendChar` = `$ffd2` per MainMenuCRT/source/c64defs.i -- and CHROUT reads PETSCII. The settings menu never writes $d018, so it inherits the $17 the main menu sets (MainMenu.asm:1214, commented ";Lower case"), where PETSCII $41-$5A draw lowercase and $61-$7A draw uppercase. Every letter comes out case-inverted. `!convtab pet` in SettingsMenu.asm:2 converts the assembler's own literals; it cannot touch bytes that arrive at runtime. Verified by execution rather than from a PETSCII table: pushed each quoted string through the KERNAL's PETSCII-to-screen-code mapping and decoded the result with the repo's own tools/bench/c64.py petscii_row. 'TeensyROM ABI 2 services $409f' -> 'tEENSYrom abi 2 SERVICES $409F' 'Example ABI 2 services $0000' -> 'eXAMPLE abi 2 SERVICES $0000' 'Example host lacks service $1f' -> 'eXAMPLE HOST LACKS SERVICE $1F' 'None installed.' -> 'nONE INSTALLED.' Controls: the transform is not the identity, it is a clean involution (feeding the screen form back produces the source form), and digits and punctuation are unchanged -- so "letters invert" is the whole of the difference. Who this costs. Section 10 exists to be re-run; the rows marked no are there to be turned into yes. Someone re-running the descriptor row compares an HDMI capture or a screen dump against the documented string, sees a mismatch, and reads a passing row as a regression -- or worse, changes the firmware's literal to make the capture match. A dump reads the same way as the screen, because petscii_row decodes screen codes and has no swap to undo. Fixed once, above the table, rather than in each row. Three rows quote firmware strings -- the two bd2b836 wrote and the service-gate row from 83014ce -- and three copies of the caveat is how they drift apart. The note also points at hostops.Outcome.said, which folds case, so the bench scripts were never exposed to this; only a human comparing by eye was. Not fixed, and outside this commit: tools/bench/README.md:113 and tools/bench/c64.py:39 both name `hostops.Result.saw`. There is no Result class and no saw method -- hostops.py:68,75 define Outcome.said. Reported upstream rather than fixed here, to keep this commit scoped to the one under review. npm run verify:extensions all PASS (it parses this file at tools/verify-extensions.mjs:59), npm test 117/117. No hardware driven; none claimed.
…just from being one 4bda5b2 filtered the twelve descriptor bytes against the two PETSCII control ranges, which is right about every byte that *is* a control code and silent about the one byte that stops the next one working. $22 draws, so nameByteDraws passed it, and CHROUT toggles the screen editor's quote mode ($d4) on every quote it prints. In quote mode the next control code is drawn rather than executed. RETURN is what clears the flag, and MakeExtHostStr ends its row without one deliberately -- StatusFunctions.c says so in its first comment -- so an odd number of quotes in a name outlives the row it was printed on. The next control code after it is PrintBanner's ChrClear: MsgBanner1 in StringFunctions.asm is EscC,EscNameColor,ChrClear,... and Pg_InstalledExt.asm calls PrintBanner as the first thing `u` does. That $93 then draws as a glyph, the screen is never cleared, and the uninstall confirmation lands on top of the page it should have replaced -- while it is asking whether to erase the slot. The same outcome 4bda5b2 exists to prevent, reached one byte later, so it is the same class rather than a second one. The repo already models the rule it missed: tools/lib/c64-screen.mjs flips `quoted` on a quote and executes control codes only `&& !quoted`, over the same two ranges nameByteDraws used. Renamed to nameByteSafe, because "draws" is the question that produced the gap: $22 draws and is still not safe, and a reader adding the next byte needs to be asked whether CHROUT changes state on it, not whether it appears. The mirror in tools/lib/extension.mjs and the 256-byte comparison in verify-extensions.mjs follow, since that gate compares the two byte for byte and would otherwise go red on the first one changed. Second fix, same function. `drawn` was decided inside the loop bounded by the caller's buffer, so a narrow buffer could answer "(unnamed)" for a host named " TR" -- a different name, not a shortened one, and "(unnamed)" mid-erase is exactly the answer displayName exists to keep honest. Whether a descriptor names anything is a property of the twelve bytes; how much fits is a property of the buffer. They are decided separately now. No caller passes anything but nameBytes today, so this was a trap for the next one rather than a live bug -- the narrow case is the one the existing test block already says nothing else reaches. Both verified by mutation, not by assertion: - quote byte neutralised to $01, predicate shape kept: the gate fails naming $22 ("is true in VMBootImage.h but hostNameSafe ... says false"); with the mirror mutated to match so the native tests run, registry_test.cpp aborts at line 114 on the "A?B?C?" case. - the quote clause removed outright: the gate fails "cannot read the nameByteSafe() ranges", which is the shape coupling in checkHostNamePolicy failing loudly and in the safe direction. Noted in a comment there, since it means a future refinement of the predicate is a change to that regex too. - `named` put back inside the bounded loop: registry_test.cpp aborts at line 168 on the blank-prefix case, and the new pin in c64-message-buffer.test.js fails the JS suite as well. Gates: npm test 117/117, npm run verify:extensions 21/21 with 23 PASS lines, python3 -m unittest discover -s tools/bench 64/64. Not run: node tools/build-firmware.mjs, because a board was being flashed from build/firmware at the time and a concurrent build would have overwritten the hex mid-read. The changed header is compiled by the native conformance build that verify:extensions runs, so it is compiler-checked, but not by the ARM toolchain and not linked into an image. No hardware was driven and no measurement here comes from a board.
tools/bench/README.md:113 and tools/bench/c64.py:39 both told a caller to fold case "as hostops.Result.saw does". There is no Result class and no saw method in hostops.py; the case-folding comparison is Outcome.said, at hostops.py:68,75. Whoever writes the next bench script reads one of those two lines, calls Result.saw, and gets an AttributeError -- or, worse, writes their own case-sensitive comparison because the one they were pointed at did not resolve. Both sites now name hostops.Outcome.said. vm/abi/README.md:561 already cited it correctly and is left alone. Found by the bd2b836 reviewer, which reported it as outside its own commit rather than fixing it there. Docstring and prose only: no behaviour changes. python3 -m unittest discover -s tools/bench 64/64.
89d4794 placed the confirm prompt absolutely, which fixed it, and then placed the firmware's reply at row 9 column 0 to reproduce where that reply used to land. Row 9 is not free: the prompt is 47 drawn characters from column 0, so it fills row 8 and its last seven -- "to keep" -- are row 9 columns 0 to 6. What prints there first is not the reply. WaitForTRDots (SupportFunctions.asm :36) prints one '.' per elapsed TOD second at the cursor, and the uninstall is handed to it at line 108. Measured with the repo's own CHROUT cursor rules, one dot at row 9 column 0 gives: 8 | Remove it? y to remove, any other key | 9 |.o keep | while the next three rows are telling the user not to power off mid-erase. Before 89d4794 the cursor sat at row 9 column 7, just past "to keep", so dots appended; this is a regression the placement change brought with it. It needs the wait to cross a second boundary, so it is a minority of runs rather than all of them. Row 10 instead. That is free whatever the prompt is later changed to say, because the screen editor links at most two physical rows into one logical line, so a prompt placed on row 8 can never reach past row 9 -- where row 9 was a constant derived from the prompt's current length, which is the coupling 89d4794 exists to remove. The reply moves down one row with it: from row 10 to row 11, and "Press any key to return" from row 11 to row 12. Cosmetic, and now by construction rather than by where the text happened to stop. One byte of the assembled image changes, LDX #$09 -> LDX #$0A at the SetCursor before the control write; SettingsMenu.prg.h is regenerated. Also corrected, same screens, both measured rather than argued: - StatusFunctions.c said a line wider than the clamp "runs onto the row below, which on the settings page carries the uninstall option". It does not. The host line is row 5 there and the uninstall option is row 7; row 6 is blank, because MsgInstalledExtMenu spends two returns getting from row 4 to row 7. The confirmation screen's row below is row 7 and is blank too. An unclamped line reaches 48 characters and spills its last 11 onto a blank row, so what MaxLength = 37 buys is a line that stays on its own row -- not a collision avoided. Anyone deciding whether the bound can be relaxed was reading a reason that does not hold. - Pg_InstalledExt.asm quoted the no-loader arm as "No extension loader in this firmware" and called it 37 characters. The string ends in a stop; as quoted it is 36. The model used for the row numbers is the one in tools/lib/c64-screen.mjs, extended with absolute rows and the logical-line linkage that decides where a ChrReturn lands. It reproduces the three readings in 89d4794's own message (rows 8, 8 and 9 before the fix, all row 8 after) and both screen dumps taken off a TR+ at current firmware, row for row, before being used for anything this commit claims. npm test 117/117, npm run verify:extensions all PASS, node tools/build-c64.mjs --project SettingsMenu reproduces the committed header, node tools/build-firmware.mjs --target tr-plus --force exit 0 with 1301.0 K of headroom. No hardware driven -- the board is held by a resident module.
…-inverted The note above the §10 table said every letter of a firmware-formatted string arrives on the C64 case-inverted, and gave `TeensyROM ABI 2 services $409f` as reaching the screen as `tEENSYrom abi 2 SERVICES $409F`. It does not. The note modelled CHROUT and the `$d018` = `$17` charset correctly and missed the stage before them: the C64 reads these bytes out of `rwRegSerialString`, and `IOH_TeensyROM.c:675` passes every one through `ToPETSCII` (table at `:107`), which swaps letter case -- `'A'` to 97, `'a'` to 65. That swap and the charset's cancel, so the screen shows exactly the bytes the firmware formatted. Verified by execution: driving the real 128-entry table out of IOH_TeensyROM.c through CHROUT's PETSCII-to-screen-code ranges and back through tools/bench/c64.py's petscii_row is the identity for all three strings §10 quotes. Dropping the ToPETSCII stage from that model reproduces the note's `tEENSYrom abi 2 SERVICES $409F` exactly, which is what identifies the missing stage as the whole of the error. It also matches the board: screen dumps taken for the row-placement runs show `TeensyROM ABI 2 services $409f` and `WIDEHOSTNAME ABI 2 services $fffff>` reading naturally. Who it cost: §10 exists to be re-run. Someone capturing the descriptor row would have compared an exact match against a paragraph promising an inverted one, and the note told them explicitly that "a capture that differs only in case is the expected result" -- so a correct capture reads as suspicious and an actually case-broken one reads as fine. Anyone chasing a real case fault would have gone to `$d018` rather than to the conversion table. The corrected note names both stages and their line numbers, keeps the case-folded matching advice under a reason that is true (a launched program may switch charset, which is the Limits note in tools/bench/README.md), and says outright that nothing tests this round trip. tools/bench/README.md and tools/bench/c64.py make the charset-selection point only and never claimed inversion; they are left alone. The §10 service-gate row this was found under is itself correct: `Example host lacks service $1f` is what the screen shows, `ExampleHost.ino:63` publishes `services 0` under the name `Example`, and the built module's header carries `requiredServices` 31 = $1f, so the masked value is $1f. npm test 117/117, npm run verify:extensions all PASS (it parses this file), python3 -m unittest discover -s tools/bench 64/64.
…ing 0 85bf736 fixed two of exttest's three arms and left the third. A launch that enters the extension image and comes back reported its VmFail record and exited 0 whatever the record said -- so $03 "extension faulted", $15 "client CRT header bad", $20 "module refused" and the rest all passed. The commit's own reason for changing the third arm from 0 to 1 was that exit 0 there "passes a refusal"; the same reasoning had not been applied to this one. hostops.py already states the rule, for install and removal: it has more ways to fail than to succeed ... So the check is for the one success phrase rather than a list of failures -- a record that is absent, truncated, or carries a code this copy has never heard of then fails noisily instead of passing as "it rebooted, didn't it". exttest was the one place that did not follow it. A launch has three normal finishes rather than one -- $00 handed off, $04 module exited, $50 host returned -- so the positive match is a set, hostops.FINISHED_NORMALLY, and hostops.finished_normally() is the predicate. The boot output is Tee'd the way run_step Tee's it, because the record has to be read as well as shown, and the C64 screen is matched alongside it since report() puts the record there too. Exit status is now documented in the docstring and in the README row: 0 for a resident module or a normal finish, 1 for a launch nothing entered or a record that is a failure, absent, truncated or unrecognised. Unchanged: the resident arm (0) and the never-dropped arm (1). hostenter.py's no-phrase exit 0 is left alone -- it prints that it is asserting nothing and says how to assert, which is a documented decision rather than this defect. Coverage, because nothing exercised any of this and that is why the original bug shipped: - test_protocol.FailPhrases pins every VmFail phrase the bench matches on against VmFail::describe() in VMFail.h, so a renamed firmware string fails a test rather than turning every assertion into a silent pass. It carries its own control: a regex that stops matching fails on the arm count rather than comparing against an empty set. - test_protocol.LaunchOutcome covers the predicate, including that every arm of describe() outside FINISHED_NORMALLY fails, that an absent record fails, and that the screen half matches in either case. - test_trlink.ExtensionRun runs exttest end to end against the fake board for the arm a reboot alone cannot classify: finished -> 0, faulted -> 1, no record -> 1. Mutation-verified four ways: the pre-fix "every reboot a pass" makes the faulted and no-record tests fail while the finished one stays green; a phrase changed in FINISHED_NORMALLY fails both phrase tests; a broken describe() regex fails the control. python3 -m unittest discover -s tools/bench: 75/75 in 120 s (was 64 in ~110). npm test: 117/117. Neither the native suite nor the firmware build reads any of these files.
…ostops Three claims in tools/bench/README.md that a reader sizes work off, checked against the code as it stands rather than as it was when each was written. 1. The Limits note said "exttest.py's own reconnect path still needs a real reset". ab1c149 added test_trlink.ExtensionRun, which runs exttest.py end to end across a real port drop against the fake board, so that path is covered without hardware. Proven by mutation: making reconnect() always return None fails all three ExtensionRun tests. Understating coverage costs the reader work -- they go looking for a board to exercise a path the suite already gates. The note now says which arm is covered and which two are not: a module that stays resident, and a launch refused before the port ever drops. Checked that those two really are uncovered; only ExtensionRun runs exttest.py, and all three of its tests reboot. 2. The hostops.py row enumerated its callers as "the host*.py scripts, and test_bounds.py". Since 85bf736 and ab1c149, exttest.py imports FINISHED_NORMALLY, REBOOT_TIMEOUT, Tee and finished_normally from it, and its exit status is finished_normally()'s answer -- so the most consequential caller was the one missing. Someone editing a phrase there to track a firmware rename would check four scripts and a test and never learn they had moved a gate's verdict. test_protocol.py imports it too, and pins those phrases against VmFail::describe(). 3. The hostenter.py row -- the one 0542e4a wrote -- said a resident host "reads here as a board that did not return". It does, but only when minimal's orphan USB node is gone, and exttest.py's own docstring records that the node usually stays. With a node present, reconnect opens it, nothing answers the firmware check, and run_step goes on to read the screen through it; that read is what fails. Measured against a fake board publishing an unserviced pty -- the 2026-09-24 hardware shape: no reply -- DMA read is not compiled into this image, or this is not the main image exit 1, after the boot window rather than after REBOOT_TIMEOUT. That message names a bad flash, which is the wrong place to look when the host is running exactly as intended -- and this row exists to catch that mistake, so being wrong about it is the whole cost. Both node cases are now described. hostenter.py's own docstring made the same claim in more detail, so it is fixed at both sites rather than only the cited one. Not fixed, and reported upward instead: run_step() does not catch what exttest.py catches at exttest.py:95 -- SystemExit and OSError from a node that enumerated but is serviced by nobody. Making it catch them would change what hostinstall, hostuninstall and hostcycle do when a board half-returns, where a loud failure is the right answer, so it is a wider decision than a prose commit should take. Prose only; no behaviour changes. python3 -m unittest discover -s tools/bench 75/75, npm test 117/117, npm run verify:extensions all PASS.
… that does tools/bench/README.md cited "PR SensoriumEmbedded#31" twice -- for the extension image and for build/extensions/ -- as things that arrive later. Both claims are false in two ways at once. The number first: no PR has been opened from this branch, and the convention elsewhere in the repo (docs/Architecture/DMA-Timing-Known-Issues.md, PR SensoriumEmbedded#21) is to cite a PR retrospectively. Guessing a number sends a reader to whatever unrelated PR happens to land on it. The tense second, and it is the worse half. Neither thing is waiting on anything: upstream/main already carries Source/Teensy/VMBoot/VMBoot.ino, the USB_DISABLED extension build in tools/build-firmware.mjs, and the build:hello script in package.json. Someone reading either sentence concludes the image they are holding cannot do what the paragraph above just told them it does, and goes looking for a PR to wait for. The first citation is deleted -- the sentence before it already names all three images -- and the second now names the script that writes the directory, which is the thing the reader actually needs. Found by the 0542e4a reviewer and routed here rather than fixed there, since the number depends on when the PR opens. python3 -m unittest discover -s tools/bench 75/75.
…ever runs
Section 10 told a module author that when an entry point never returns, "the
button is the only way back". The sentence contradicts its own previous clause
and the code contradicts both.
isrButton (ISRs.c:25) sets BtnPressed and nothing else. The reboot it stands
for happens in loop() (VMBoot.ino:234-238), and vm_entry is called from
setup(), by way of VMHostBoot() (VMHost.h:146) and loadModule() (VMHost.h:96,
the synchronous call through h.entry). So a module that hangs in vm_entry hangs
setup(): loop() is never reached, the flag is set and never read, and both the
menu button and the C64 reset line are wired to that same ISR -- neither does
anything. Section 7 already contemplates this state a few paragraphs earlier
("An entry point that hangs writes no report, so it stays $00 and stays
silent"), which is what makes the pair a contradiction rather than a gap.
The cost lands on the reader this section is written for. A third-party module
author whose first entry point loops presses the button, gets nothing, and has
no documented next step -- the honest one being a C64 power cycle, which is
also the board power cycle, since the Teensy 5V/USB connection is cut during
assembly.
The paragraph now separates the resident case (vm_entry returned, loop running,
button works) from the hung case, and names the power cycle. It also says why
the alternate button is out: isrSpecial is left unattached in this image, which
was asserted without a reason before.
Checked for other sites making the same claim: docs/Architecture/
Extension-Hosts.md section 4 is about a host writing the boot indicator, a
different subject, and vm/hello/hello.cpp:79 says the reset button is the way
out from input(), which runs from loop() with a resident module and is correct.
npm run verify:extensions 23 PASS (it parses this file at
verify-extensions.mjs:59).
…ing one The sprintf/vsprintf audit listed DriveDirLoad.ino:406-417 (full firmware) and Min_DriveDirLoad.ino:406-417 (MinimalBoot) as "each has its own independent copy" of SendMsgPrintfln/SendMsgPrintf with an unbounded vsprintf. The full firmware has no such copy. Source/Teensy/DriveDirLoad.ino calls the formatter eighteen times and defines none; it holds no vsprintf and no vsnprintf, and it held none at this branch base (e8f35c7) either, so this is a long-standing mislocation rather than drift this branch introduced. This branch is where it costs something, because this branch appended "**Fixed**: vsnprintf in the minimal image's copy" to that same bullet. Read together, the two halves say the full-firmware copy exists and was not among what got fixed. The next person to run the dedicated sweep the section asks for goes looking in DriveDirLoad.ino for an overflow that was never there -- or records the full firmware as carrying a known, unfixed memory-safety bug when its formatter is the FileParsers.ino one in the bullet above, which is fixed and gated. The entry now names the one real copy, says plainly that DriveDirLoad.ino only calls it, and keeps the part that is true: LoadFile()/ParseCRTHeader() really are duplicated per build, which is why both Name: call sites needed %.32s. Verified rather than asserted: both %.32s sites and the minimal image's vsnprintf are pinned by Source/Teensy/tests/c64-message-buffer.test.js, whose "CRT name field" test loops over FileParsers.ino and Min_DriveDirLoad.ino, and whose "formatters are bounded" test fails either file back to vsprintf. Added those line references so the next reader can check the same way. npm test 117/117.
The ABI README opened by telling a third-party module author that a module
needs VMABI.h "and nothing else from this repository to build". Two more files
are required, and neither is optional:
vm/abi/module.ld -- tools/build-extension.mjs:127 passes it as -T. It sets
ENTRY(vm_entry), KEEPs .entry first, and fixes CODE to
0x00018000 and DATA to 0x20014000. Those are the exact
addresses vm_valid_header() checks (VMABI.h:188,
h.code_base != VM_CODE_BASE || h.ram_base !=
VM_DATA_BASE), so an image linked without it is
refused by the loader rather than merely laid out
differently.
vm/abi/vm_runtime.c -- compiled and linked unconditionally at
build-extension.mjs:118 and :127. Weak memset, memcpy,
memmove, memcmp, strlen, strcmp. A freestanding module
has no libc and GCC emits calls to these for code that
never names them, so a module that zeroes a struct
fails to link without it.
The reference module itself shows the header claim is narrow too:
vm/hello/hello.cpp includes ../abi/vm_abi.h, not VMABI.h directly. vm_abi.h is
a shim that includes VMABI.h, so the header a module actually names is the
former while the one that cannot drift from the firmware is the latter, and the
intro named only the latter.
The document already contradicted itself on this: section 5 says the entry
point is "linked first by module.ld" and links to it. Section 1 is the one a
reader acts on first, so a third party setting up their own build from the
intro copies one header, then fails at link on unresolved memset and on an
image the loader refuses -- with the contract document as their only
explanation of why.
Replaced with the three files, each with what it supplies and why it cannot be
dropped, and kept the closing promise that nothing else in the repository is
needed -- which is true, and is what the sentence was reaching for.
npm run verify:extensions 23 PASS (it parses this file) - npm test 117/117.
kfox
marked this pull request as draft
September 24, 2026 20:28
…ilds them CI's tools job fails at verify:extensions: host_abi_standalone.cpp builds VMHostABI.h with -Wall -Werror, and GCC's -Wmisleading-indentation rejects vm_manifest_extensions, where an `if(...)return false;` shares its line with the statements after it. Clang does not warn, so it passed on macOS. VMRegistry.h's extensionMatches has the same shape. Nothing builds it with -Werror yet, so it has not failed, but it is split the same way. Reproduced on node:24 (g++ 12.2), then verified there after the change: verify:extensions exits 0 with 23 PASS lines, and the bench suite passes 75/75. npm test 117/117 on macOS, and both --target tr-plus and --target tr build.
Seven SHA citations sat in four files. They mean nothing to a reader of the merged tree, and they go stale whenever the branch is rewritten. - IOH_TR_BASIC.c now names the rRegStreamData guard in IOH_TeensyROM.c rather than the two commits that added it. - c64-screen.mjs and its test name the row they describe, not the commit that fixed it. - Known-Issues.md gives the RAM1 figure against the extension-host PR and drops the second measurement, which differed only in `padding`.
kfox
force-pushed
the
extension-host
branch
from
September 24, 2026 20:55
1459a85 to
47ea044
Compare
kfox
marked this pull request as ready for review
September 24, 2026 20:59
kfox
marked this pull request as draft
September 25, 2026 00:49
…nside it No firmware hex carries an extension host any more. The TR+ build combines MinimalBoot and the main image only, and packages the stock host (VMBoot) as TeensyROM+_<ver>_VMBoot.TRH beside the hex, read back through the device's own checks before it is written. A board runs exactly as it would without the slot until someone installs a host into it. --host-sketch now builds that host and nothing else, writing TeensyROM+_<ver>_<dir>.TRH and no firmware hex: a third-party host installs onto a board running the stock firmware rather than shipping inside a firmware of its own. CI checks that each target's hex carries no host, that the TR+ build left exactly one .TRH beside its hex and the TR none, and builds the example host through the same path. Releases still publish the firmware hex only; the stock host's .TRH stays in the build artifact until there is a module a user would install it for. The slot is still at 0x60280000 here, inside the range the firmware updater stages into and erases, so an update erases the installed host. The next commit moves the slot out of that range.
… reach The slot moves from 0x60280000 to 0x60760000..0x607c0000, directly below the Teensy core's EEPROM emulation. With extensions on, FlashUpdate.ino's FLASH_RESERVE grows by the slot's size (256K -> 640K), so the updater never stages into or erases it and an installed host survives a firmware update. A static_assert holds the slot's top to the stock reserve's bottom, and hex.mjs's FLASH_LIMIT -- the top of what a firmware hex may occupy -- is now the slot's base. A plain TR keeps the 256K reserve. The main image is no longer capped at 2176K by a slot sitting above it; it is linked into 7168K and bounded by the self-update ceiling instead. The flash headroom check now counts the slot in the TR+ reserve. Measured on 0.8.0.11: 1301.0K of headroom with VMBoot baked in at 0x60280000, 1689.0K now (TR unchanged at 1914.0K).
With the slot at the top of flash, firmware that does not reserve it -- anything built without the extension loader, and every release before it -- sizes its update buffer by scanning down from 0x607bfffc for the first programmed word, and finds the installed host almost at once. A host left behind a cleared tag therefore left that firmware unable to update itself. vm_host_remove() clears the tag exactly as before, then erases every other sector. A tag that will not clear stops with the host intact; a sector that will not erase past that point is reported and the rest are still erased. The removal record keeps asking the slot, and now carries the operation's status as its detail, so a partial erase says so, and the bench scripts pass a removal only when that detail is $0. Uninstall runs for any slot that is not blank, not only one holding a host: an install that failed part way, or was cut off, leaves bytes that are no host and are still in that search's way. The Installed Extensions page says "None installed; slot not blank." for such a slot. The native suite covers the new paths, sweeps a power cut across every removal operation, and clears what a cut at every install operation leaves behind. The Installed Extensions page, the user guide and the bench docs say what removal does now, and that a host should be uninstalled before loading firmware without extension support.
kfox
marked this pull request as ready for review
September 25, 2026 19:09
This was referenced Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a 384 KiB extension host slot at
0x60760000— the top of flash, directly belowthe EEPROM emulation — that a third party can build for, package as a
.TRH, and installover USB serial or from the TR menu, with removal on both paths. No firmware hex
carries a host: extensions are opt-in, and a board runs exactly as it would without the
slot until someone installs one. The TR+ firmware builds with the extension loader and
the plain TR firmware builds without, both in CI.
What is here
One installable slot, outside the updater's reach. With extensions on,
FLASH_RESERVEgrows by the slot (256K → 640K), so the firmware updater never stagesinto or erases it and an installed host survives a firmware update. A
static_assertties the slot's top to the stock reserve's bottom. The main image is no longer capped
by a slot above it; it is bounded by the self-update ceiling, with 1689.0K of
headroom on 0.8.0.11 (the plain TR is unchanged at 1914.0K). A second slot, if one is
ever needed, goes directly below this one.
The stock host ships beside the firmware, never inside it.
npm run build:tr-pluswrites
TeensyROM+_<ver>_VMBoot.TRHnext to the hex. CI decodes each target's hex andfails if any byte reaches the slot, and checks that exactly one
.TRHsits beside theTR+ hex and none beside the TR. Releases publish the hex only; the
.TRHstays in thebuild artifact until there is a module a user would install it for.
--host-sketch <dir>builds that host and nothing else, writingTeensyROM+_<ver>_<dir>.TRHand no firmware hex: a third-party host installs onto aboard running the stock firmware rather than shipping inside a firmware of its own.
.TRH(TRH1) host package — 64-byte header plus the raw slot image, CRC overheader and payload, an ABI mirror the installer checks before it erases anything.
Install and uninstall over USB serial, and from Settings → Installed Extensions.
Neither needs a hand on the board, which is what lets the bench run a whole round trip
unattended. Removal over the network is refused: the remove token is accepted on the USB
device port only.
An example host and module (
Source/Teensy/ExampleHost,vm/hello) plus the ABIdocs a third party needs. CI builds the example host and asserts on its descriptor, so
the third-party story cannot rot quietly.
VM ABI 2 —
VmHostfrozen at 76 bytes,VmHostExitat 80,static_asserted.Bench tools for the round trip:
hostinstall.py,hostuninstall.py,hostcycle.py,hostenter.py, sharinghostops.py.Layout
Flash (TeensyROM+, 8 MiB at
0x60000000)Measured on the 0.8.0.11 TR+ build. With VMBoot built into the hex at
0x60280000,as earlier revisions of this PR had it, the same build had 1301K of headroom. A plain TR,
with no slot and the 256K reserve, has 1914K.
.TRH0x607BFFFCfor the first programmed word, so it can't update itself while anything is in the slot. Uninstall firstRAM: modules come from the SD card, every launch
Launch: one reset, two records
sequenceDiagram participant Main as Main firmware participant SD as SD card participant EE as EEPROM participant Min as MinimalBoot participant Host as Installed host participant Mod as Module Main->>Main: required_services vs the slot's MVH2 descriptor<br/>(refused on screen, no reboot, if missing or no host) Main->>SD: write /VMS/launch.vml, read it back Main->>EE: "@VM1" marker + ExecuteMin boot indicator Main->>Min: reset Min->>EE: read marker and indicator (SD not mounted yet) Min->>Host: validate the slot, jump Host->>SD: CRC-check launch.vml, load client.crt and engine.mvm Host->>Mod: copy into the RAM windows, call vm_entryExisting defects this also fixes
All present on
maintoday.Firmware
PerformDMAandCloseDMAspun onDMA_Stateforever, soa bus that never reached the target state hung the board. Every wait now has a deadline
and an absolute ceiling (a clocking bus cannot hold it open either), a transfer that
lands just as the deadline expires is reported as landed rather than failed, and
/DMAis released by the ISR on a PHI2 falling edge — the C64 and C128 both publish the rule
that DMA moves only while PHI2 is low — with an unsynchronised drop only as the last
resort on a bus that is not clocking.
TestDMAPageandTestDMAPatternread back intothe buffer already holding the expected pattern, so an aborted read compared equal on
every byte and the page was reported good having read nothing from the C64.
ReceiveFileDatahad a per-byte timeout only,and
WriteC64MemCommand's receive loop could run 65535 × 500 ms = 9.1 hours. Both nowcarry a deadline sized per channel (
USBHostSerialat 115200 8N1 is 11.52 bytes/mS atthe wire, so a single floor would fail real uploads), and both drain the channel on the
abort paths so the next command does not read the tail of the abandoned one.
isrPHI2. The menu index is nowclamped where it is formed (
MenuIdxFromRegs; page 0 made the product negative) andre-checked at the dereference (
MenuItemSel()answers NULL), because a menu change movesNumItemsFullandMenuSourceunder an already-formed index — four POKEs read past thebuilt-in menus with no SD card.
SetMenu()publishes base and count as one step. Alsobounded: the stream and serial-string reads (both streamed firmware memory back past
their terminator), the default IO1 read (
DE68..DEFFreturned heap past the 104-byteIO1), theIOHandler[]name index, and the string selector (gains adefault:).FreeDriveDirMenufreed theblock but left
MenuSource/NumItemsFullas they were, so ISR reads were use-after-freeuntil the next menu change. It now redirects to the static menu itself, covering all five
callers.
RAM_Image[StreamOffsetAddr++] = DatainIOH_TR_BASIC.chad no bound; atuint16_tit wrapped and was contained by accident.StreamOffsetAddrwasuint16_tagainst auint32_tsize. The C64 text viewer re-displayed the first 64 KiBforever.
?Stat: a5after a message the C64 never read.SendMsgSerialStringBufleft itsrsC64Messagesentinel inrwRegStatuson timeout, and the next poll dispatched it as astatus code. The sentinel is now taken back when it is still ours.
StatusFunction[]also gains a count
static_assertand a runtime null check.SendMsgPrintf/SendMsgPrintflnvsprintf'd paths of up to 357 bytes into the 262-byteSerialStringBuf. Both are nowbounded, as are the minimal image's copy and the default-SID line.
Tools
build-extension.mjs, sonpm run <script> -- --opt <v>lost to the script's own baked-in value. All buildscripts now share
tools/lib/cli-args.mjs, which refuses unknown, repeated andvalueless options.
exttest.pymisread a running extension. It keyed on whether a serial node exists,but the minimal image's node usually stays after the jump, so a running module was
reported as a failure, and a port that never dropped was reported as success. It now
asks which image answered, and exits non-zero unless the launch record is one of the
three normal finishes (
$00handed off,$04module exited,$50host returned).petscii_rowdecoded the wrong charset, so every screen scrape turned lower case intoupper and upper case into
..What is measured, and what is not
Gates on the current head:
npm test121/121 ·npm run verify:extensions21/21 plusall ten conformance suites ·
python3 -m unittest discover -s tools/bench78/78 · both--target tr-plusand--target trbuild clean.On the board, for the slot move and the new removal: firmware stamp
Sep 25 2026, 15:20:57, flashed through TeensyROM's own SD updater. It was built fromthis branch before the last review fixes, which change only what removal does when the tag
will not clear, a path no bench step can reach:
hostcycle.py— install/remove round trip at0x60760000, six asserted steps$0hostuninstall.py— whole-slot erase over USB$40, detail$0; 17 s from command to board answering againTeensyROM ABI 2 services $409fafterwards--no-extensions, 256K reserve), then an update from it back to this onevm/hellolaunched from the new slotOn the board, earlier on this branch (stamp
Sep 24 2026, 18:33:43, slot at0x60280000; none of these paths changed apart from the slot address):hostenter.py /NOSVC.crt "extension host returned"$50, detail$4F8,0,u,ywith a host installedWIDE.TRHwhose clamped line is the full-widthWIDEHOSTNAME ABI 2 services $fffff>fwupdate.py— hands-free reflash through the SD updaterexttest.py /HELLO.crtPrintFileNameExample ABI 2 services $0000on a firmware whose own constants areTeensyROM/$409fExample host lacks service $1fon the C64 —VMRegistry.h:133and nowhere else in the firmwarevm/helloend to endHostRemoveToken7f 9b 42 75 73 79 21 0a—FailTokenthenBusy!\n— fromncagainst port 2112. Board did not reboot; slot still readsTeensyROM ABI 2 services $409f. Control:hostcycle.pyremoves with the same token over the USB device port on the same firmwareNot measured — needs something this bench cannot supply.
AbortDMAceiling has never fired. Reproducing it wants a bus that stalls ondemand.
miscompares — a
zsweep withnS_DMADataHolddetuned into the marginal range.LaunchFileTokenis handled atSerUSBIO.ino:549, ahead of the gate at:633, with nochannel check, and reaches both
DoHostInstallandDoFlashUpdateon all three channels.Deliberately unfixed here — the right place is
LaunchFile(), and closing it is a changeto the general remote-launch path rather than to anything this PR adds.
Known gaps, not closed here
The six guards added to
IO1Hndlr_TeensyROM— the index clamp, the stream bound, theptrSerialStringNULL/end guard, theIO1Sizeread bound, theIOH_Num_Handlersbound andthe string-selector
default:— have zero execution coverage, taken or not-taken.Nothing in this repo compiles and calls any
.cor.inofirmware file. The native suitedoes build and execute real firmware headers —
VMRegistry.h,VMBootImage.handVMHostInstall.hare the code under test inverify:extensions, not models of it — but theIO1 dispatch lives in
IOH_TeensyROM.candStatusFunctions.c, which no test includes, andfail_test.cpp:41reproduces what the polling handler does rather than calling it. Closingthis means extracting the dispatch into something a host compiler can build and call with
mocked
IO1[],XferImageandptrSerialString; that is the obvious follow-up.DisplayTime(StringFunctions.asm:105) writes ten characters from column 29, leaving thecursor at row 1 column 39. One more character in the clock would wrap it, link rows 1
and 2 into a single logical line, and move every
ChrReturn-placed row on every settingspage. Nothing is wrong today; it is a one-column margin with no test on it, and
tools/lib/c64-screen.mjsis where such a test would go.