Repository navigation
EPIC-10: latent layout and timing bugs - #10
Merged
diegoparrilla merged 9 commits intoSep 15, 2026
Merged
Conversation
…10 STORY-02 STORY-03) The image reaches RAM through the XIP stream, whose address register has no bits 1:0. target_firmware is a uint16_t array, only 2-byte aligned, so at an address that is 2 mod 4 every word would be shifted and the ST would boot without the cartridge. It has been aligned by luck. firmware.py now emits aligned(4), and COPY_FIRMWARE_TO_RAM falls back to a word copy for any source that is not 4-byte aligned. The stream also moves 32-bit words, so an odd word count lost the last 16-bit word. It is now copied separately. The 64 KB window is cleared before the copy, so nothing from a previous run (a jump from Booster, a crash reboot) survives past the end of the image. COPY_FIRMWARE_TO_RAM_MEMCPY took bytes while every other copy macro takes words; it takes words now. Debug builds check the window against target_firmware after the copy and count non-zero bytes past the image. target_firmware.h is regenerated by the m68k build; only the declaration and the header's build date change.
…TORY-01) __rom_in_ram_start__ was declared as `extern unsigned int`, so to the compiler it is a 4-byte object and every read or write past those bytes is undefined behaviour it may optimise on at -O3. Clearing the window with memset made GCC warn that 65,536 bytes overflow a 4-byte region. Declare it as an array of unknown size in constants.h and in the two files that redeclare it. Every use takes its address and casts it, so no call site changes and the code size is identical.
[G] and [U] make the m68k install GEMDRIVE at the relocation address and memtop the RP publishes when the ST sends HELLO at cold boot. After the RP reboots while the ST keeps running (a crash, a SELECT reset, a flash) no HELLO arrives. Recovery only worked because the previous run's shared variables survived in RAM; with the window now cleared, the address reads 0 and the ST halts with "Reloc/stack overlap". The launch commands now refuse until HELLO has landed since the RP booted, keep the full menu on screen and say "Reset the Atari ST first" on the bottom strip. When HELLO lands the message clears, and if the refused launch came from the countdown, the countdown starts again. Keystrokes the ST sends while it boots are dropped at that point, or the main loop would halt the restarted countdown before its first tick. Verified on hardware (release): a launch after an RP reboot is refused from the countdown and from [U] with the menu intact; after an ST reset the countdown restarts, runs to 0 and Runner launches without the banner.
DPRINTF blocks on the UART. At the SDK's default 115,200 baud a debug build printed its flash layout and both settings dumps before copying the cartridge image, and a power-cycled ST booted into GEM because the cartridge was not live yet; release, which prints nothing, showed the menu. At 921,600 the same output is eight times faster and the power-cycled ST shows the DevOps menu on the debug build too. The UART clock stays at 48 MHz after the 225 MHz overclock, so the divider gives 921,413 baud, a 0.02 % error. Only debug builds get the define; release has no UART console. Set the serial terminal to 921600.
…RY-04) DPRINTF waited until every byte had left the UART, so each trace stalled the main loop, the ROM3 command ring and the ST's commands for about 1 ms at 921,600 baud, and bursts such as GEMDRIVE's per-call traces for much longer. DEBUG_BUFFERED_CONSOLE in debug.h, default 1, toggles a buffered console. DPRINTF formats with fprintf into a funopen stream that queues the text in a 4 KB ring buffer, and the UART transmit interrupt sends it. When the ring is full the caller waits for room, or sends bytes itself with interrupts off, so nothing is lost. Inside an exception the queue and the new text are sent at once, so crash reports stay complete and in order. stderr and stdout are redirected to the same queue, so nothing else writes to the UART behind the interrupt. Set it to 0 for the previous blocking console, which keeps the last lines before a hang. Everything lives in debug.h: the shared state and functions are weak definitions merged by the linker into one copy. debug.h replaces DPRINTF definitions made earlier, and settings.c includes it, so the settings library's own fallback DPRINTF no longer bypasses the queue. Debug builds only: 4 KB less heap and about 500 bytes of flash. Release is unchanged. Verified on hardware: a power-cycled boot log is complete and in order, and a forced panic prints its report whole, in order, before the reboot.
usbcdc_init() called stdio_init_all() a second time in debug builds, assuming it was idempotent. It is not: stdio_uart_init() runs uart_init(), which resets the UART and throws away up to 32 bytes still in the console FIFO, and stdio_usb_init() claims another IRQ and background task on every call. With the buffered console this showed as exactly 32 bytes missing from a boot trace. Start USB stdio only when TinyUSB is not running yet. Release builds, where main.c does not initialise stdio, behave as before.
__flash_binary_start and the ROM temp, Booster, config, lookup and global config region symbols were declared as `extern unsigned int`, 4-byte objects to the compiler. aconfig.c reads the whole 4 KB lookup table through a pointer taken from _global_lookup_flash_start, which is undefined behaviour the optimiser may use. Declare them as arrays of unknown size, as __rom_in_ram_start__ already is. Every use takes the address and casts it, so no call site changes.
…-05) The streaming listing checked for 32 bytes of room, enough for the closing envelope, before reading the next entry. An entry needs about 80 bytes, up to 350 with a long name, so once a chunk had between 32 and 80 bytes left the entry just read did not fit: it was dropped and the listing ended with "truncated": true. Any folder with more entries than fit in one 1 KB chunk, about 15, came back cut short. Check for room for the largest possible entry before f_readdir, since an entry that has been read cannot be put back; the listing then continues in the next chunk. Found by the EPIC-10 smoke script; verified on hardware with 100 and 500 files: all entries listed, "truncated": false.
…-10 STORY-05) FOLDER defaulted to the microfirmware template's "/test", which md-devops never uses, so every boot created an empty /test folder on the card. FOLDER now defaults to "/devops" and is created at boot if it does not exist. Settings saved with the old "/test" value are changed to "/devops" and saved once at boot, then the folder is created if needed. Verified on hardware: the first boot logged the change and rewrote the app settings with every other value kept; the next power-on boot read FOLDER=/devops and mounted the card with no /test folder involved.
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.
Third epic of v1.1.0: find and fix the bugs that only showed up in some builds, and in
Releaserather thanMinSizeRel, so both build types can ship.MinSizeRelwas the symptom. The causes were an alignment-dependent copy of the cartridge image, undefined behaviour in linker symbol declarations, and debug traces that delayed the cartridge enough to miss the ST's boot.What changes
uint16_tarray streamed out of XIP flash with a word-sized DMA copy that assumed 4-byte alignment. The array is nowaligned(4), and a misaligned image falls back to a word copy. An odd word count copies its trailing word, and the 64 KB window is cleared before the copy. Debug builds compare the copy with the image at boot and count non-zero bytes after it.__rom_in_ram_start__) and the six flash-region symbols were declared as 4-byteunsigned ints, then read and written as 64 KB and 4 KB regions: undefined behaviour that GCC warned about once the window was cleared. Code size is identical.[G]and[U]launch only after the ST's HELLO. Launching before the ST has booted the cartridge code left it in GEM without the resident blob. The menu now saysReset the Atari ST first: no HELLO since RP boot., and the countdown restarts when HELLO arrives.DEBUG_BUFFERED_CONSOLEindebug.h(default 1) queuesDPRINTFtext in a 4 KB RAM ring sent by the UART TX interrupt. When the ring is full it waits instead of dropping text, and the exception path writes synchronously. Every library usingDPRINTFgoes through it, includingsettings/. Set it to 0 for the blocking console when chasing a hang.usbcdc_initre-ranstdio_init_all(), which lost the console bytes in flight.truncated: true./devops.FOLDERkept the template's/test, which md-devops never uses, so every boot created an empty/testfolder. Stored settings with/testare changed to/devopsonce at boot; the folder is created if missing.Verified on hardware
A new smoke script checks health before and after each step and fails on any reboot, crash or ROM3 overrun. It covers: cold boot,
[G]desktop and file copy, 256 KB upload, download, rename and delete, a listing of 500 files, Runner load, exec and unload with a debug tail, and SELECT.releasedebug, buffered console at 921,600releaseanddebugwith flash code and constants shifted about 6 KBreleaseanddebugwith RAM code and data shifted about 1 KBdebugFOLDER=/test/devopsand saved once; other settings keptA
-Wall -Wextrareview of application code found no uninitialised-use, array-bounds or aliasing warnings beyond the two declaration fixes.Found and filed, not fixed here
f_mkdirasks for 32 KB underPICO_MALLOC_PANIC, more than the whole heap (EPIC-11). Before this PR the same happened with/test.commRingis 8 KB-aligned, so.datagrowing past an 8 KB boundary costs 8 KB of heap (EPIC-12).reset_device()flood on SELECT) is halted; the buffered console removed the stalls.