Reach the 32-bit RAM banks everywhere an address is bounded#283
Merged
Conversation
plausible_ptr() learned the motherboard, CPU-slot, and Zorro III windows in PR #279, but the AmigaDOS walkers around it still carried their own hard 16 MiB ceilings, so nothing downstream benefited. process_seglist(), walk_seglist(), read_bstr(), and process_command_name() each bounded a BPTR at $00400000 -- a $01000000 address once shifted -- and cpu.rs's task-catch peeks rejected an ExecBase or task node at or above $01000000. Those bounds hold on a small-box machine, where every RAM bank fits below $01000000. On an A3000/A4000 or an accelerated machine they do not: exec allocates out of the Ramsey bank at $04000000-$07FFFFFF, the CPU-slot bank at $08000000-$0FFFFFFF, or a Zorro III board, and OS 3.2+ SetPatch deliberately moves ExecBase into fast RAM. A program loaded there had its seglist rejected at the first BPTR, so the debugger's module tracker and gdb's qXfer:libraries:read reported nothing, and the task catch never armed. Add plausible_bptr(), which shifts and then applies the same window test, and route all four walkers plus the task-catch peeks through the shared predicates. ln_Name keeps no address bound at all: the ROM devices' node names are Kickstart strings, and node_name() already documents that case.
The Memory tab's Find button and the console FIND command both walked a hard-coded $01000000-byte space in 4 KiB chunks and masked the hit back to 24 bits. On a machine whose RAM sits above that -- the Ramsey motherboard bank at $04000000, the CPU-slot bank at $08000000, a Zorro III board -- the search could never reach it, and a hit reported from anywhere above 16 MiB aliased down to the wrong address. Add Bus::searchable_regions(): the writable RAM banks plus the Kickstart and extended-ROM windows, in ascending address order. Chip RAM is offered as the whole $000000-$1FFFFF select window rather than the fitted bank, because Agnus decodes fewer address bits than the window on the smaller parts and the bank repeats inside it -- a search of CPU-visible memory sees every image, which is the existing behaviour the regression test pins. writable_ram_regions() keeps reporting the fitted bank so the memory hunt gets no duplicate candidates. search_cpu_memory() now takes that region list and sweeps it twice (from the start address to the top of the map, then the bottom back up), so the wrap is exactly once over what the machine actually decodes. Skipping the undecoded gaps also drops the reads a search costs on a big-box configuration. The Save-region spec had the same 24-bit mask on its start address, so a dump requested above 16 MiB silently wrote a different region; the address is now taken as written and only the length stays capped.
debug_run_to_pc(), the reverse-debug replay scan tt_scan_stop(), and the CCP run-to-PC target all masked the PC with a literal $00FF_FFFF. The machine already carries the right width -- ui_addr_mask() is A0-A23 on the 68000, 68010, and 68EC020 and the full 32 bits on the 020/030/040/060 -- so the literal both aliased distinct 32-bit PCs onto each other and stopped a breakpoint set in RAM above 16 MiB from ever being recognised as the target. Route all three through ui_addr_mask(). On the 24-bit models the behaviour is unchanged, since the mask is the same constant there.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes remaining hard-coded 24-bit / 16 MiB ceilings in debugger- and OS-structure code paths so address plausibility tests, memory searching, PC comparisons, and region dumps correctly cover 32-bit RAM windows (motherboard, CPU-slot/accelerator, and Zorro III) on big-box configurations.
Changes:
- Extend pointer/BPTR plausibility and OS-structure walkers to accept the A3000/A4000 motherboard + CPU-slot RAM windows (and keep Zorro III coverage).
- Rework debugger “Find” (GUI + console) to sweep the decoded memory map via
Bus::searchable_regions()instead of scanning a fixed 16 MiB span. - Replace literal 24-bit PC masking in run-to-PC / reverse-debug / headless control paths with
ui_addr_mask()to prevent aliasing of distinct 32-bit PCs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/video/window/tests.rs | Adds a 030+ fixture with optional accel RAM and a regression test proving Find can hit >24-bit RAM. |
| src/video/window/console.rs | Refactors pattern search into region-based sweeps and updates stack-walk return-address gating to ui_addr_mask(). |
| src/video/window.rs | Switches Memory tab Find implementation to the shared region-based search helper. |
| src/video/ui.rs | Changes Save-region parsing to stop forcibly masking dump start addresses to 24-bit. |
| src/emulator.rs | Uses ui_addr_mask() for breakpoint/PC comparisons during replay + debug_run_to_pc. |
| src/cpu.rs | Makes task-catch pointer plausibility rely on the shared amigaos::plausible_ptr. |
| src/control/headless.rs | Fixes headless run-to-PC comparisons to use ui_addr_mask() instead of a 24-bit literal. |
| src/bus/tests.rs | Adds regression coverage ensuring searchable_regions() includes all 32-bit RAM banks + ROM windows. |
| src/bus.rs | Introduces Bus::searchable_regions() built from writable RAM + ROM windows, sorted by base, with special chip-window handling. |
| src/amigaos.rs | Exposes plausible_ptr, adds plausible_bptr, and updates seglist/BSTR walkers + tests for 32-bit windows. |
| docs/debugger/window.md | Documents the updated Find behavior (decoded map sweep incl. 32-bit banks). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Review follow-ups on the address-window sweep. The console FIND command passed its optional START token through unmasked while the Memory tab's Find button masked its start with ui_addr_mask(). A START past a 24-bit bus then named no region at all, so the first sweep matched nothing and the search silently restarted from the bottom of the map, reporting an earlier copy of the pattern than the one asked for. Take START through the address bus, as the GUI path does. region_spec() no longer masks, which is what lets a dump start above the 24-bit space on an 020+, but the save path reads through debug_read_memory(), which masks to the bus width. On a 24-bit model the file name and the OSD therefore named an address the bytes had not come from. Mask in debugger_mem_save_region() instead, alongside the identical ui_addr_mask() the neighbouring Writer? path already applies -- display and content agree again on 24-bit models, and a 32-bit dump address still passes through. The per-span reads deliberately run one pattern short of a chunk past the span end. Document why: that tail is what carries a match across a chunk boundary, and at a span end it is what carries a match across two banks that abut in the map -- a full motherboard bank ends at $08000000, exactly where the CPU-slot bank begins. The window count is the chunk length, so a reported hit always starts inside the span it was searched from. Pinned with a regression test for a pattern straddling that seam.
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
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.
Follow-up to #279, which taught
plausible_ptr()about the A3000/A4000motherboard and CPU-slot RAM windows. That is the third time this class of bug
has landed (#264 for bus-master DMA, e54c601 for exec structures, #279), so
this is a sweep of every remaining place that bounds an address with a literal
16 MiB / 24-bit ceiling.
The four windows any "is this a real address" test has to cover:
$000000-$9FFFFF,$C00000-$D7FFFF$04000000-$07FFFFFFMB_RAM_TOP)$08000000-$0FFFFFFFACCEL_RAM_BASE, grows up)$10000000-$7FFFFFFFWhat was still broken
OS-structure walkers (
src/amigaos.rs,src/cpu.rs) -- the callers aroundthe function #279 fixed kept their own ceilings, so nothing downstream
benefited.
process_seglist(),walk_seglist(),read_bstr(), andprocess_command_name()each bounded an AmigaDOS BPTR at$00400000; thetask-catch peeks rejected ExecBase and task nodes at
>= $01000000. A programloaded into a 32-bit bank had its seglist rejected at the first BPTR, so the
module tracker and gdb's
qXfer:libraries:readsaw nothing, and with OS 3.2+SetPatch moving ExecBase to fast RAM the task catch simply never armed. New
shared
plausible_bptr()shifts and then applies the same window test.Console
STACK/BT-- the return-address heuristic rejected>= $01000000, so no frame executing from 32-bit RAM was ever walked. Nowgated on
ui_addr_mask(); unmapped words peek as 0, which matches no JSR/BSRencoding, so the opcode test still does the filtering.
Memory-tab Find / console
FIND-- scanned a hard-coded 16 MiB and maskedhits back to 24 bits. Replaced with a sweep of the new
Bus::searchable_regions(). Chip RAM is offered as the whole$000000-$1FFFFFselect window so Agnus's image repeats are still found;
writable_ram_regions()keeps reporting the fitted bank so
HUNTgets no duplicate candidates. Skippingthe undecoded gaps also cuts the reads a search costs on a big-box config.
PC comparisons --
debug_run_to_pc(), the reverse-debug replay scantt_scan_stop(), and the CCP run-to-PC target all masked with a literal$00FF_FFFF, aliasing distinct 32-bit PCs onto each other. All three now useui_addr_mask()(identical constant on the 24-bit models).Save-region spec -- masked its start address to 24 bits, so a dump
requested above 16 MiB silently wrote a different region.
Left alone deliberately
The A2091 and CDTV DMACs, Akiko, and the copper/blitter/disk chip-bus pointers
are genuinely 24-bit (or narrower) hardware;
romtags::in_romis a ROM-windowtest.
zorro_device's DMA decode,filesys'sGuestBus,peek_word_any,writable_ram_regions, and the savestate banks were already complete.segment_plausible'ssize < $01000000is a sanity bound on the loader's sizelongword, not an address limit -- value kept, stale comment corrected.
Testing
cargo build --release,cargo clippy --all-targets,cargo fmt --checkallclean. Unit suite passes apart from
video::bitplane::tests::sprite_ctl_write_stops_dma_data_reuse_by_later_position_write,which fails identically on unmodified
main(subtract overflow, arrivedwith c7898d7 / #281) and is unrelated to this branch.
New regression tests:
plausible_pointers_cover_the_32_bit_ram_windowswalks_a_seglist_loaded_above_the_24_bit_spacereads_a_bstr_above_the_24_bit_spacesearchable_regions_cover_the_32_bit_ram_banksmemory_tab_find_reaches_ram_above_the_24_bit_space(new 030-with-accel-RAMwindow fixture)
Also smoke-tested a headless A4000/68040 boot.
docs/debugger/window.mdupdated for the new Find behaviour.