diff --git a/crates/volamos-core/src/dispatch.rs b/crates/volamos-core/src/dispatch.rs index 96aea89..8792af8 100644 --- a/crates/volamos-core/src/dispatch.rs +++ b/crates/volamos-core/src/dispatch.rs @@ -2767,11 +2767,9 @@ impl Runtime { // startup may read this instead of (or in addition to) A0/D0. mem.write_u32(task + exectask::PR_ARGUMENTS_OFFSET, args_addr); - // ReadArgs's default source (rdargs == NULL): the same - // command-line buffer just built above, per crate::dosargs's - // module docs. Registers hold this too, but a guest program is - // free to clobber A0/D0 before ever calling ReadArgs. - dos.cmdline = Some((args_addr, line_len)); + // The same text in the Input() buffer, where ReadArgs reads it, + // as RunCommand puts it there on AmigaOS: see DosState::input_buf. + dos.input_buf = line.clone(); Self { cpu, @@ -4675,6 +4673,19 @@ mod tests { ); } + #[test] + fn the_command_line_is_also_in_the_input_buffer_for_readargs() { + // vbcc's startup code writes a NUL over the command line's '\n' + // as it splits the A0 buffer into argv; ReadArgs, reading the A0 + // buffer, then saw "LIST\0" where the user typed LIST. It reads + // the Input() buffer now, as on AmigaOS. + let mut rt = runtime_with_program_and_args(&[RTS], vec!["LIST".to_string()]); + let a0 = rt.cpu.address_register(AddressRegister(0)); + let d0 = rt.cpu.data_register(DataRegister(0)); + rt.mem.write_u8(a0 + d0 - 1, 0); + assert_eq!(rt.dos.input_buf, b"LIST \n"); + } + #[test] fn empty_args_still_produce_a_bare_newline_command_line() { let rt = runtime_with_program_and_args(&[RTS], Vec::new()); diff --git a/crates/volamos-core/src/dosargs.rs b/crates/volamos-core/src/dosargs.rs index bda29e5..d57e706 100644 --- a/crates/volamos-core/src/dosargs.rs +++ b/crates/volamos-core/src/dosargs.rs @@ -6,15 +6,16 @@ //! //! # Scope and deviations from real `ReadArgs` //! -//! - **Default source only.** `rdargs == NULL` reads from -//! [`DosState::cmdline`] -- the same `A0`/`D0` command-line buffer -//! [`crate::dispatch::Runtime::new`] builds, exactly mirroring how real -//! AmigaOS delivers the CLI command tail through the process's buffered -//! `Input()`. [`DosState::cmdline_pos`] persists across calls, so -//! repeated `ReadArgs(NULL)` calls in one process walk forward through -//! the same buffer rather than each re-parsing it from the start -- -//! real `ReadArgs(NULL)` shares this property because it reads off a -//! stateful buffered stream. A caller-supplied `rdargs` with its own +//! - **Default source only.** `rdargs == NULL` reads the command line +//! `RunCommand` stuffed into the buffer of `Input()`, +//! [`DosState::input_buf`], as real `ReadArgs` "normally gets the +//! arguments by reading buffered IO from Input()" (autodoc). Reading +//! advances [`DosState::input_pos`], which buffered reads share, so +//! repeated `ReadArgs(NULL)` calls walk forward through the stream +//! rather than each re-parsing it from the start. Once that buffer is +//! used up, `ReadArgs` sees an empty line rather than waiting on host +//! stdin, as AROS does for an interactive `Input()` with an empty +//! buffer. A caller-supplied `rdargs` with its own //! `RDA_Source` (a custom string to parse instead of the command line) //! is not implemented: `rdargs` is accepted only as an *identity* for //! `FreeArgs` bookkeeping (see below), and parsing still always reads @@ -567,15 +568,10 @@ fn read_args( ) -> Result { let template = parse_template(template_bytes)?; - let (cmdline_addr, cmdline_len) = dos.cmdline.unwrap_or((0, 0)); - let mut buf = Vec::with_capacity(cmdline_len as usize); - for offset in 0..cmdline_len { - buf.push(mem.read_u8(cmdline_addr.wrapping_add(offset))); - } - - let mut reader = Reader::new(&buf, dos.cmdline_pos as usize); + let buf = dos.input_buf.clone(); + let mut reader = Reader::new(&buf, dos.input_pos); let result = match_args(&mut reader, &template); - dos.cmdline_pos = reader.pos as u32; + dos.input_pos = reader.pos; let slots = result?; let allocations = materialize(heap, mem, &template, slots, array_addr)?; @@ -685,7 +681,7 @@ fn find_arg_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Dispa /// Registers `ReadArgs`/`FreeArgs` onto [`DOS_LIBRARY_BASE`], looked up /// by name through [`DOS_LVOS`]. Called from [`crate::dispatch:: /// Runtime::new`] alongside the other `dos.library` registrations; these -/// handlers need [`DosState::cmdline`] (always set by `Runtime::new`), +/// handlers need [`DosState::input_buf`] (always set by `Runtime::new`), /// not a `Vfs`, so they work regardless of whether one is installed. pub fn register_dosargs_handlers( table: &mut LibraryTable, @@ -728,20 +724,16 @@ mod tests { use super::*; use crate::memory::FlatMemory; - /// Builds a fresh `(heap, mem, dos)` triple with `dos.cmdline` - /// pointing at `cmdline` (space-joined-and-newline-terminated, same - /// framing `Runtime::new` gives the real `A0`/`D0` buffer). + /// Builds a fresh `(heap, mem, dos)` triple with `cmdline` in + /// `dos.input_buf` (space-joined and newline-terminated, the framing + /// `Runtime::new` gives it). fn setup(cmdline: &[&str]) -> (GuestHeap, FlatMemory, DosState) { - let mut mem = FlatMemory::new(0x4000); - let mut heap = GuestHeap::new(0x100, 0x3000); + let mem = FlatMemory::new(0x4000); + let heap = GuestHeap::new(0x100, 0x3000); let mut line = cmdline.join(" ").into_bytes(); line.push(b'\n'); - let addr = heap.alloc(&mut mem, line.len() as u32).unwrap(); - for (i, &b) in line.iter().enumerate() { - mem.write_u8(addr + i as u32, b); - } let mut dos = DosState::new(None); - dos.cmdline = Some((addr, line.len() as u32)); + dos.input_buf = line; (heap, mem, dos) } diff --git a/crates/volamos-core/src/dosbuf.rs b/crates/volamos-core/src/dosbuf.rs index abf456d..299d78a 100644 --- a/crates/volamos-core/src/dosbuf.rs +++ b/crates/volamos-core/src/dosbuf.rs @@ -25,13 +25,14 @@ //! `FGetC` consults it before ever touching the host file, so a //! `FGetC`/`UnGetC`/`FGetC` sequence round-trips correctly. //! +//! The other is the command line `RunCommand` stuffs into the buffer of +//! `Input()` (`crate::dosfile::DosState::input_buf`): buffered reads of +//! the current `Input()` return it before any host stdin, as they do on +//! AmigaOS, where `ReadArgs` reads its arguments that way. +//! //! `SelectInput`/`SelectOutput` (redirecting a *process's* default -//! `Input()`/`Output()` handle) and interactive-file-specific behavior -//! (`IsInteractive`, the BCPL command-line-parameters-in-the-input- -//! buffer quirk `files.md` documents for `Input()`) are out of scope -- -//! this runtime's `Input()` is backed by real host stdin already (see -//! `crate::dosfile`'s module docs), not a command-line buffer, so that -//! particular real-AmigaOS quirk doesn't apply here in the first place. +//! `Input()`/`Output()` handle) and other interactive-file-specific +//! behavior (`IsInteractive`) are out of scope. use crate::cpu::{Cpu, DataRegister}; use crate::dispatch::{DOS_LIBRARY_BASE, DispatchError, HandlerContext, LibraryTable}; @@ -54,9 +55,9 @@ fn fgetc(ctx: &mut HandlerContext<'_, impl Cpu>, addr: u32) -> i32 { let value = if let Some(pushed) = ctx.dos.ungetc_buf.remove(&addr) { pushed } else { - match ctx.dos.read(addr, 1) { - Ok(bytes) if !bytes.is_empty() => i32::from(bytes[0]), - Ok(_) => { + match ctx.dos.read_buffered_byte(addr) { + Ok(Some(byte)) => i32::from(byte), + Ok(None) => { ctx.dos.set_io_err(0); // clean EOF, not an error ENDSTREAMCH } @@ -155,13 +156,12 @@ fn fread_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Dispatch let total_wanted = (block_len as u64) * (block_count as u64); let mut got = 0u64; while got < total_wanted { - match ctx.dos.read(addr, 1) { - Ok(bytes) if !bytes.is_empty() => { - ctx.mem - .write_u8(buf_addr.wrapping_add(got as u32), bytes[0]); + match ctx.dos.read_buffered_byte(addr) { + Ok(Some(byte)) => { + ctx.mem.write_u8(buf_addr.wrapping_add(got as u32), byte); got += 1; } - Ok(_) => break, // EOF + Ok(None) => break, // EOF Err(code) => { ctx.dos.set_io_err(code); break; diff --git a/crates/volamos-core/src/dosfile.rs b/crates/volamos-core/src/dosfile.rs index 0fdf3dd..2df716d 100644 --- a/crates/volamos-core/src/dosfile.rs +++ b/crates/volamos-core/src/dosfile.rs @@ -442,20 +442,25 @@ pub struct DosState { // the sibling module" convention as the T11/Phase-3-stage-7 fields // above), but the fields themselves are declared here per that same // convention. - /// Guest address of the process's command-line buffer (the `A0`/`D0` - /// buffer [`crate::dispatch::Runtime::new`] builds from - /// [`crate::dispatch::StartConfig::args`]) and its length in bytes - /// (including the trailing `'\n'`). This is `ReadArgs`'s default - /// input source (`rdargs == NULL`), mirroring how real AmigaOS - /// delivers the CLI command tail through the process's buffered - /// `Input()` -- see `crate::dosargs`'s module docs. - pub(crate) cmdline: Option<(u32, u32)>, - /// Byte offset into `cmdline` that the next default-source `ReadArgs` - /// call resumes from -- real `ReadArgs(NULL)` reads from a shared, - /// stateful input stream, so repeated calls in one process walk - /// forward through the same buffer rather than each re-parsing it - /// from the start. - pub(crate) cmdline_pos: u32, + /// The command line, as `RunCommand` stuffs it into the buffer of + /// the process's `Input()`, including the trailing `'\n'`: the text + /// of the `A0`/`D0` buffer [`crate::dispatch::Runtime::new`] builds + /// from [`crate::dispatch::StartConfig::args`], or of the arguments + /// `RunCommand` gets. The `RunCommand` autodoc: it "takes care of + /// setting up the current input filehandle in such a way that + /// ReadArgs() can be used in the program, and restores the state of + /// the buffering before returning"; `ReadArgs` "normally gets the + /// arguments by reading buffered IO from Input()". AROS does the same + /// (`rom/dos/runcommand.c`, `vbuf_inject()`), after the Guru Book. + /// + /// So `ReadArgs(NULL)` and buffered reads of the current `Input()` + /// (`FGetC`, `FGets`, `FRead`) take these bytes before the host + /// stream, and the guest's `A0` buffer is not involved: C startup + /// code splits that into argv in place (vbcc's writes a NUL over the + /// `'\n'`), which real `ReadArgs` never sees. + pub(crate) input_buf: Vec, + /// How much of `input_buf` has been read. + pub(crate) input_pos: usize, /// Live `ReadArgs` results, keyed by the `struct RDArgs*` anchor /// address returned in `D0` (see `crate::dosargs::RDARGS_STRUCT_SIZE`). /// `FreeArgs` looks up and frees every heap block an entry lists, plus @@ -545,8 +550,8 @@ impl DosState { seglists: HashMap::new(), overlay_roots: HashMap::new(), system_runner: None, - cmdline: None, - cmdline_pos: 0, + input_buf: Vec::new(), + input_pos: 0, rdargs: HashMap::new(), local_vars: HashMap::new(), anchor_states: HashMap::new(), @@ -962,6 +967,18 @@ impl DosState { } } + /// One byte of buffered input from the handle at `addr`, `None` at + /// EOF: the rest of [`Self::input_buf`] first when `addr` is the + /// current `Input()`, then the handle itself. Plain `Read` bypasses + /// the buffer, as it does on AmigaOS, so it uses [`Self::read`]. + pub fn read_buffered_byte(&mut self, addr: u32) -> Result, i32> { + if self.current_input == Some(addr) && self.input_pos < self.input_buf.len() { + self.input_pos += 1; + return Ok(Some(self.input_buf[self.input_pos - 1])); + } + Ok(self.read(addr, 1)?.first().copied()) + } + /// `Read`: reads up to `len` bytes from the handle at `addr`, /// returning the bytes actually read (may be shorter than `len`, /// including empty at EOF -- matching real `Read`'s "actual count" @@ -2280,6 +2297,24 @@ mod tests { assert_eq!(err, ERROR_INVALID_LOCK); } + #[test] + fn buffered_reads_of_input_return_the_command_line_first() { + // RunCommand stuffs the arguments into the buffer of Input(), so + // FGetC(Input()) sees them before anything else; another handle + // does not. + let mut heap = GuestHeap::new(0x1000, 0x4000); + let mut mem = FlatMemory::new(0x4000); + let mut dos = DosState::new(None); + dos.input_buf = b"A\n".to_vec(); + let input = dos.input_addr(&mut heap, &mut mem).unwrap(); + let other = dos.alloc_file_handle(&mut heap, &mut mem, false).unwrap(); + + assert_eq!(dos.read_buffered_byte(other), Err(ERROR_INVALID_LOCK)); + assert_eq!(dos.read_buffered_byte(input), Ok(Some(b'A'))); + assert_eq!(dos.read_buffered_byte(input), Ok(Some(b'\n'))); + assert_eq!(dos.input_pos, 2); + } + #[test] fn open_into_handle_resolves_relative_to_a_parent_lock_and_creates_the_file() { let tmp = TempDir::new("open-into-handle"); diff --git a/crates/volamos-core/src/dosseg.rs b/crates/volamos-core/src/dosseg.rs index d6e46cf..15a0fdd 100644 --- a/crates/volamos-core/src/dosseg.rs +++ b/crates/volamos-core/src/dosseg.rs @@ -247,7 +247,7 @@ //! [`crate::guestmem::MIN_STACK_SIZE`], matching //! [`crate::dispatch::Runtime::new`]'s own clamp) from the *caller's* //! [`GuestHeap`], saving the current task's `A7`/`tc_SPLower`/ -//! `tc_SPUpper`/`pr_Arguments`/[`DosState::cmdline`]/`cmdline_pos`/ +//! `tc_SPUpper`/`pr_Arguments`/[`DosState::input_buf`]/`input_pos`/ //! `A6` first -- //! same bookkeeping [`crate::exectask`]'s `StackSwap` handler already //! does for the same reason (a stack swap entirely within one @@ -285,6 +285,7 @@ use crate::dosfile::{ DosState, ERROR_FILE_NOT_OBJECT, ERROR_NO_FREE_STORE, MODE_OLDFILE, OFFSET_CURRENT, map_io_error, map_vfs_error, }; +use crate::dospattern::ERROR_LINE_TOO_LONG; use crate::exectask::{PR_ARGUMENTS_OFFSET, TC_SPLOWER, TC_SPUPPER}; use crate::guestmem::{ GuestHeap, GuestHeapError, MIN_STACK_SIZE, addr_from_bptr, bptr_from_addr, read_c_string, @@ -1022,6 +1023,15 @@ fn run_command_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Di // segment's payload address, immediately past its header. let entry = alloc_addrs[0].wrapping_add(SEG_HEADER_SIZE); + // The arguments are copied into the Input() buffer below, so they + // must lie inside guest memory: a length past it cannot be a real + // argument string, and must not make the host allocate gigabytes. + if u64::from(param_ptr) + u64::from(param_len) > ctx.mem.len() as u64 { + ctx.dos.set_io_err(ERROR_LINE_TOO_LONG); + ctx.cpu.set_data_register(DataRegister(0), (-1i32) as u32); + return Ok(()); + } + let stack_size = requested_stack.max(MIN_STACK_SIZE); let stack_base = match ctx.heap.alloc(ctx.mem, stack_size) { Ok(addr) => addr, @@ -1047,8 +1057,8 @@ fn run_command_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Di let old_sp_lower = ctx.mem.read_u32(task + TC_SPLOWER); let old_sp_upper = ctx.mem.read_u32(task + TC_SPUPPER); let old_pr_arguments = ctx.mem.read_u32(task + PR_ARGUMENTS_OFFSET); - let old_cmdline = ctx.dos.cmdline; - let old_cmdline_pos = ctx.dos.cmdline_pos; + let old_input_buf = std::mem::take(&mut ctx.dos.input_buf); + let old_input_pos = ctx.dos.input_pos; let entry_sp = stack_top & !3; ctx.cpu.set_address_register(AddressRegister(7), entry_sp); @@ -1058,15 +1068,15 @@ fn run_command_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Di // a guest startup may read this instead of (or in addition to) // A0/D0, so it has to describe the same buffer. ctx.mem.write_u32(task + PR_ARGUMENTS_OFFSET, param_ptr); - ctx.dos.cmdline = Some((param_ptr, param_len)); - // ReadArgs's cursor into dos.cmdline -- must restart at the child's - // own buffer's beginning, not wherever the caller's own prior - // ReadArgs call(s) left it (this field persists across calls, see - // crate::dosargs's module doc): without resetting it here, a - // RunCommand'd program's own first ReadArgs call would start - // partway (often at the very end) of *its* command line instead of - // the start, parsing as if no arguments were given at all. - ctx.dos.cmdline_pos = 0; + // The child's arguments go into the Input() buffer, read from the + // start, and the caller's buffer comes back when the child returns: + // RunCommand "takes care of setting up the current input filehandle + // in such a way that ReadArgs() can be used in the program, and + // restores the state of the buffering before returning" (autodoc). + ctx.dos.input_buf = (0..param_len) + .map(|i| ctx.mem.read_u8(param_ptr.wrapping_add(i))) + .collect(); + ctx.dos.input_pos = 0; ctx.cpu .set_address_register(AddressRegister(6), DOS_LIBRARY_BASE); // A0/D0 = argptr/argsize verbatim, exactly as RunCommand's caller @@ -1095,8 +1105,8 @@ fn run_command_handler(ctx: &mut HandlerContext<'_, C>) -> Result<(), Di ctx.mem.write_u32(task + TC_SPUPPER, old_sp_upper); ctx.mem .write_u32(task + PR_ARGUMENTS_OFFSET, old_pr_arguments); - ctx.dos.cmdline = old_cmdline; - ctx.dos.cmdline_pos = old_cmdline_pos; + ctx.dos.input_buf = old_input_buf; + ctx.dos.input_pos = old_input_pos; if let Some(shadow) = ctx.mem.shadow_mut() { shadow.reset_stack_tracking(old_sp_lower, old_sp_upper, old_a7); } @@ -1829,6 +1839,49 @@ mod tests { ); } + #[test] + #[allow(clippy::vec_init_then_push)] // one `.push()` per word, each commented + fn run_command_rejects_arguments_past_guest_memory() { + // The arguments are copied into the Input() buffer, so a length + // reaching past guest memory has to fail before the host tries to + // allocate and copy it. The child would exit with 42. + let child_words = [move_imm_to_d(0), 0, 42, RTS]; + let tmp = TempDir::new("args-past-memory"); + fs::write(tmp.path().join("child"), hunk_file_from_words(&child_words)).unwrap(); + let name = b"SYS:child\0"; + + let mut words = Vec::new(); + let name_idx = words.len(); + words.push(move_imm_to_d(1)); // D1 = name ptr (patched below) + words.push(0); + words.push(0); + push_jsr(&mut words, 6, -150); // LoadSeg + words.push(move_d0_to_d(1)); // D1 = seglist BPTR + words.push(move_imm_to_d(2)); // D2 = stack size + words.push(0); + words.push(8192); + words.push(move_imm_to_d(3)); // D3 = paramptr + words.push(0); + words.push(0); + words.push(move_imm_to_d(4)); // D4 = paramlen: 4 GB + words.push(0xFFFF); + words.push(0xFF00); + push_jsr(&mut words, 6, -504); // RunCommand + words.push(RTS); + + let name_addr = TRAP_TABLE_END + (words.len() as u32) * 2; + patch_imm32(&mut words, name_idx, name_addr); + + let mut rt = runtime_with_program_and_extra(&words, name_addr, name, Some(tmp.path())); + let mut out = Vec::new(); + let code = rt.run(&mut out, None).expect("run should succeed"); + + assert_eq!( + code, -1, + "RunCommand should refuse the length, not run the child" + ); + } + #[test] fn run_command_child_inherits_the_callers_fpcr() { // Child: FMOVE.L FPCR,D0 ; RTS -- reports back, as its own exit @@ -1940,9 +1993,9 @@ mod tests { #[allow(clippy::vec_init_then_push)] // one `.push()` per word, each commented fn run_command_resets_readargs_cursor_for_the_child_instead_of_inheriting_the_callers() { // The parent calls ReadArgs on *its own* command line first, - // advancing DosState::cmdline_pos partway (here, all the way) + // advancing DosState::input_pos partway (here, all the way) // into the parent's buffer. Without resetting that cursor for - // the child (DosState::cmdline_pos is a single, shared field -- + // the child (DosState::input_pos is a single, shared field -- // see crate::dosargs's module doc), the child's own ReadArgs // call would start wherever the parent's left off, typically // past the end of the *child's* entirely different buffer, @@ -1988,7 +2041,7 @@ mod tests { let mut words = Vec::new(); // Parent's own ReadArgs call first, against its own command - // line ("PARENT_TOKEN", below) -- advances cmdline_pos. + // line ("PARENT_TOKEN", below) -- advances input_pos. let parent_template_idx = words.len(); words.push(move_imm_to_d(1)); words.push(0); @@ -2067,7 +2120,7 @@ mod tests { read_c_string(rt.memory(), matched_ptr), b"CHILD_TOKEN", "the child's ReadArgs should parse the child's own command line from its start, \ - not wherever the parent's prior ReadArgs call left DosState::cmdline_pos" + not wherever the parent's prior ReadArgs call left DosState::input_pos" ); }