diff --git a/crates/volamos-core/src/backend.rs b/crates/volamos-core/src/backend.rs index 5de8c2d..990adac 100644 --- a/crates/volamos-core/src/backend.rs +++ b/crates/volamos-core/src/backend.rs @@ -218,6 +218,37 @@ impl M68kCpu { } } +/// How many bytes of guest address space `cpu_type` can actually reach +/// (issue #98): `0x100_0000` (16 MiB) for the models with a 24-bit +/// address bus -- the 68000 and 68010 -- and the full `0x1_0000_0000` +/// for the 32-bit ones. +/// +/// This exists because an address space *larger* than this is not merely +/// wasteful, it is broken: the guest stack sits at the top of the +/// address space, so a 32 MiB space on a 68000 puts `A7` somewhere the +/// CPU cannot express. The address wraps to 24 bits, a `JSR` into a +/// library pushes its return address into low memory instead, and the +/// `RTS` pops whatever happened to be there -- which is faithful to the +/// hardware, and useless. The CLI rejects that combination up front +/// rather than letting a program die several calls later with a +/// baffling "continuation stub trapped" message; see the `--ram` doc in +/// `main.rs`. +/// +/// **Asked of the `m68k` crate rather than hardcoded here.** The crate +/// owns `CpuCore::address_mask` and sets it per model, so the one +/// authority on which models wrap is the emulator that does the +/// wrapping. A table duplicated here would be a second source of truth +/// that silently goes stale the next time the crate refines a model -- +/// the 68EC020, for instance, has a 24-bit *external* bus on real +/// silicon, which the crate does not currently model, and volamos +/// should follow the crate's behavior rather than reject a +/// configuration that demonstrably works today. +pub fn addressable_bytes(cpu_type: CpuType) -> u64 { + let mut core = CpuCore::new(); + core.set_cpu_type(cpu_type); + u64::from(core.address_mask) + 1 +} + impl Default for M68kCpu { fn default() -> Self { Self::new() diff --git a/crates/volamos/src/main.rs b/crates/volamos/src/main.rs index 2175b98..e390fff 100644 --- a/crates/volamos/src/main.rs +++ b/crates/volamos/src/main.rs @@ -52,7 +52,11 @@ //! [`run_nested_program`] check this upfront and fail with a clear //! error (rather than letting [`volamos_core::dispatch::Runtime::new`] //! panic deep inside guest-heap setup) if `--stack` is too close to or -//! exceeds `--ram`. +//! exceeds `--ram`. `--ram` must also be an address space the `--cpu` +//! model can actually reach: a 68000/68010 has a 24-bit address bus, so +//! anything above 16 MiB puts the guest stack at an address the CPU +//! cannot express, and [`check_ram_addressable`] refuses that +//! combination up front too. //! //! `--cpu MODEL` picks the emulated [`CpuType`] (default `68000`, the //! lowest common denominator every Kickstart 3.1 machine shares -- see @@ -78,7 +82,7 @@ use std::io; use std::path::PathBuf; use std::process::ExitCode; -use volamos_core::backend::{CpuType, M68kCpu, TRAP_TABLE_END}; +use volamos_core::backend::{CpuType, M68kCpu, TRAP_TABLE_END, addressable_bytes}; use volamos_core::dispatch::{Runtime, StartConfig, TraceEvent}; use volamos_core::exectask::install_host_break_handler; use volamos_core::loader::Location; @@ -283,7 +287,8 @@ fn print_usage(program_name: &str) { ); eprintln!(" same K/M suffix syntax as --stack. --stack must leave"); eprintln!(" real room within this for the loaded program and the"); - eprintln!(" runtime's own guest heap"); + eprintln!(" runtime's own guest heap. Above 16M needs --cpu 68020"); + eprintln!(" or later: a 68000/68010 cannot address more than that"); eprintln!(" --cpu MODEL emulated CPU (default 68000): 68000, 68010, 68020,"); eprintln!(" 68ec020, 68030, 68ec030, 68040, 68ec040, 68lc040,"); eprintln!(" 68060, or scc68070"); @@ -431,6 +436,31 @@ fn parse_cpu_type(s: &str) -> Result { } } +/// The `--cpu MODEL` spelling for `cpu_type` -- the inverse of +/// [`parse_cpu_type`], so a diagnostic can name the model the way the +/// user wrote it rather than as a Rust enum variant. +fn cpu_model_name(cpu_type: CpuType) -> &'static str { + match cpu_type { + CpuType::M68000 => "68000", + CpuType::M68010 => "68010", + CpuType::M68020 => "68020", + CpuType::M68EC020 => "68ec020", + CpuType::M68030 => "68030", + CpuType::M68EC030 => "68ec030", + CpuType::M68040 => "68040", + CpuType::M68EC040 => "68ec040", + CpuType::M68LC040 => "68lc040", + CpuType::M68060 => "68060", + CpuType::SCC68070 => "scc68070", + // `CpuType` is the `m68k` crate's, so it can grow a variant (or + // hand back its `Invalid` sentinel) without this match knowing. + other => { + debug_assert!(false, "unnamed CpuType {other:?}"); + "CPU" + } + } +} + /// Computes `ExecBase.AttnFlags` (`exec/execbase.h`'s `AFF_68010`/ /// `AFF_68020`/`AFF_68030`/`AFF_68040`/`AFF_68060`, plus `AFF_68881`/ /// `AFF_68882` for a coprocessor FPU or `AFF_FPU40` for the 68040's @@ -841,6 +871,36 @@ fn report_sanitizer_violations( } } +/// Checks that `ram_size` is an address space the configured CPU can +/// actually reach (issue #98). +/// +/// The guest stack lives at the top of the address space, so a `--ram` +/// larger than the CPU's address bus can express puts `A7` at an +/// address that wraps: the `JSR` into a library pushes its return +/// address into low memory instead, and the `RTS` pops whatever was +/// there. That is what real hardware does, and it manifests as the +/// baffling "continuation stub trapped at 0x000000c4 with no pending +/// continuation" several calls into a run rather than as a complaint +/// about the flags -- so, like an oversized `--stack` (see +/// [`check_ram_fits`]), it is refused up front instead. +/// +/// Only bites someone who raises `--ram` past 16 MiB without also +/// asking for a 32-bit CPU, since the default is exactly 16 MiB. +fn check_ram_addressable(ram_size: u32, cpu_type: CpuType) -> Result<(), String> { + let addressable = addressable_bytes(cpu_type); + if u64::from(ram_size) <= addressable { + return Ok(()); + } + let model = cpu_model_name(cpu_type); + Err(format!( + "--ram {ram_size} is more address space than a {model} can reach: that CPU has a \ + 24-bit address bus, so it can only address {addressable} bytes, and the guest \ + stack at the top of a larger space would be at an address it cannot express -- \ + ask for a 32-bit address bus with --cpu 68020 (or any later model), or lower the \ + size to {addressable} or less (--ram on the command line, RAM= in a config file)" + )) +} + /// Checks that `stack_size` plus [`MIN_HEAP_HEADROOM`] actually fits /// between `load_end` (the loaded program's own end address) and /// `ram_size` (the top of the guest address space) -- see @@ -992,6 +1052,11 @@ fn build_runtime_with_vfs( } fn run(opts: &Options) -> Result { + // Before anything is read or loaded: this one depends only on the + // flags, and a run that violates it fails much later in a way that + // does not point at them. + check_ram_addressable(opts.ram_size, opts.cpu_type)?; + let bytes = std::fs::read(&opts.program) .map_err(|e| format!("couldn't read '{}': {e}", opts.program))?; @@ -1566,6 +1631,73 @@ mod tests { assert!(check_ram_fits(u32::MAX, u32::MAX, u32::MAX).is_err()); } + #[test] + fn the_default_ram_size_is_exactly_what_a_68000_can_address() { + // Issue #98: the two constants have to agree, or the default + // configuration is either broken or needlessly small. If a + // future `DEFAULT_RAM_SIZE` bump ever wants more, it has to come + // with a `--cpu` default change (or a clamp) as well. + assert_eq!( + u64::from(DEFAULT_RAM_SIZE), + addressable_bytes(CpuType::M68000) + ); + assert!(check_ram_addressable(DEFAULT_RAM_SIZE, CpuType::M68000).is_ok()); + } + + #[test] + fn check_ram_addressable_rejects_over_16m_on_a_24_bit_cpu() { + // One byte over is enough: the guest stack sits at the top of + // the address space, so this is not a "close enough" situation. + for cpu in [CpuType::M68000, CpuType::M68010] { + let err = check_ram_addressable(DEFAULT_RAM_SIZE + 1, cpu).unwrap_err(); + assert!(err.contains("24-bit address bus"), "{err}"); + assert!(err.contains(cpu_model_name(cpu)), "{err}"); + assert!(err.contains("--cpu 68020"), "{err}"); + } + } + + #[test] + fn check_ram_addressable_accepts_any_size_on_a_32_bit_cpu() { + for cpu in [ + CpuType::M68020, + CpuType::M68EC020, + CpuType::M68030, + CpuType::M68EC030, + CpuType::M68040, + CpuType::M68EC040, + CpuType::M68LC040, + CpuType::M68060, + CpuType::SCC68070, + ] { + assert!( + check_ram_addressable(u32::MAX, cpu).is_ok(), + "{} should take any u32 address space", + cpu_model_name(cpu) + ); + } + } + + #[test] + fn cpu_model_name_round_trips_through_parse_cpu_type() { + // Keeps the diagnostic spelling honest: whatever name an error + // message shows has to be a name the user could have typed. + for cpu in [ + CpuType::M68000, + CpuType::M68010, + CpuType::M68020, + CpuType::M68EC020, + CpuType::M68030, + CpuType::M68EC030, + CpuType::M68040, + CpuType::M68EC040, + CpuType::M68LC040, + CpuType::M68060, + CpuType::SCC68070, + ] { + assert_eq!(parse_cpu_type(cpu_model_name(cpu)), Ok(cpu)); + } + } + #[test] fn default_cpu_is_68000_with_no_fpu() { let opts = parse_args(args(&["prog"])).unwrap(); diff --git a/userdocs/CLI-Reference.md b/userdocs/CLI-Reference.md index bfaad61..0ff4b4c 100644 --- a/userdocs/CLI-Reference.md +++ b/userdocs/CLI-Reference.md @@ -170,7 +170,7 @@ suffixed `K`/`k` (KiB) or `M`/`m` (MiB): ```sh volamos --ram 4M fixtures/hello -volamos --ram 64M --stack 1M fixtures/hello # room for a much larger stack +volamos --ram 8M --stack 1M fixtures/hello # room for a much larger stack ``` The default comfortably covers the tiny CLI binaries volamos currently @@ -178,6 +178,26 @@ targets, with plenty of headroom for a larger-than-default `--stack`. Raise it if a guest program needs more address space than that (e.g. a larger `--stack`, or a program that allocates a lot via `AllocMem`). +**Above 16 MiB needs `--cpu 68020` or later.** A 68000 or 68010 has a +24-bit address bus, so 16 MiB is all it can address at all. Since the +guest stack sits at the top of the address space, a larger `--ram` would +put the stack pointer at an address the CPU cannot express — so volamos +refuses the combination rather than letting the program fail confusingly +later on: + +```sh +$ volamos --ram 64M fixtures/hello +volamos: --ram 67108864 is more address space than a 68000 can reach: that +CPU has a 24-bit address bus, so it can only address 16777216 bytes, and the +guest stack at the top of a larger space would be at an address it cannot +express -- ask for a 32-bit address bus with --cpu 68020 (or any later +model), or lower the size to 16777216 or less (--ram on the command line, +RAM= in a config file) + +$ volamos --ram 64M --cpu 68020 fixtures/hello +Hello from volamos +``` + ## `--cpu MODEL` Picks the emulated CPU model. Default `68000` — the lowest common diff --git a/userdocs/Changelog.md b/userdocs/Changelog.md index cd4d2b4..0c7b939 100644 --- a/userdocs/Changelog.md +++ b/userdocs/Changelog.md @@ -5,6 +5,43 @@ version scheme in `Cargo.toml`. ## 0.7 +- **`--ram` above 16 MiB is now refused on a 68000/68010** (issue #98) + instead of producing a program that dies confusingly later. Those CPUs + have a 24-bit address bus, and the guest stack sits at the top of the + address space, so `--ram 32M` on the default `--cpu 68000` put the + stack pointer at an address the CPU cannot express: it wrapped, a + `JSR` into a library pushed its return address into low memory, and + the `RTS` popped whatever was there — surfacing several calls later as + "continuation stub trapped at 0x000000c4 with no pending continuation", + which points at nothing useful. It now fails up front naming both + flags, the way an oversized `--stack` already did. Pass `--cpu 68020` + (or later) for a 32-bit address bus. The default `--ram` is exactly + 16 MiB, so only a raised `--ram` was ever affected. + +- **`--sanitize` no longer reports a recycled heap block as + use-after-free** (issue #95, reported by Bernie Innocenti). Once a run + freed a block and the heap handed those same addresses back out, every + write into the *new* allocation was reported against the *old* one's + poison — one violation per byte, which slowed a BenchWork run from 10 + seconds to 6 minutes and looked like a hang. + + The clearing now happens inside the guest heap, at the single point + where a block leaves its control, and the allocator takes the address + space as a parameter so a new allocation site cannot compile without + it. That matters because `AllocMem`/`AllocVec`/`AllocPooled` are only + 3 of some four dozen places that carve guest structures out of that + heap — the `FileHandle` a `dos` `Open` returns, the `FileLock` a `Lock` + returns, a `ReadArgs` argument buffer and many more — and any of them + can be handed a poisoned block. Fixing only the exec allocators left a + guest that freed a large block and then opened a file still reporting + 46 phantom use-after-free writes. + + Separately, `--sanitize`'s slowdown on a violation-heavy run is gone: + the per-byte access check was deciding "did that byte report?" by + summing the hit counts of every violation already logged, so the cost + grew with the log. A 400,000-violation run goes from 1.33s to 0.03s, + with identical output. + - **The heap detectors now cover four more allocators** (issue #83, tier 0): `utility.library`'s `AllocateTagItems`, `dos.library`'s `AllocDosObject`, and `exec.library`'s `CreateIORequest` and diff --git a/userdocs/Examples.md b/userdocs/Examples.md index d7726e7..84d3ecf 100644 --- a/userdocs/Examples.md +++ b/userdocs/Examples.md @@ -147,7 +147,11 @@ fails cleanly rather than crashing if `--stack` doesn't actually fit inside `--ram`: ```sh -volamos --stack 1M --ram 64M fixtures/hello +volamos --stack 1M --ram 8M fixtures/hello + +# Past 16 MiB of RAM, ask for a 32-bit CPU too -- a 68000 cannot address +# more than that, so volamos refuses the pairing rather than misbehaving. +volamos --stack 1M --ram 64M --cpu 68020 fixtures/hello ``` See [CLI Reference](CLI-Reference.md#-stack-size) and