decode non-UTF-8 strings as Latin-1 instead of panicking

dos/src/dosapi.rs

@@ -83,7 +83,7 @@ pub fn int21(ctx: &mut Context) -> Option<runtime::Cont> {
                 log::warn!("TODO: file access {access:x}");
             }
             let mut state = state();
-            let Some(buf) = state.read_file(name) else {
+            let Some(buf) = state.read_file(&name) else {
                 log::warn!("open {name:?}: not found");
                 ctx.cpu.regs.set_ax(/* file not found */ 2);
                 ctx.cpu.flags.insert(runtime::Flags::CF);
@@ -205,7 +205,7 @@ pub fn int21(ctx: &mut Context) -> Option<runtime::Cont> {
                     let seg = ctx.memory.read::<u16>(params_addr);
                     let relo = ctx.memory.read::<u16>(params_addr + 2);
 
-                    let Some(buf) = state().read_file(cmd) else {
+                    let Some(buf) = state().read_file(&cmd) else {
                         panic!()
                     };
                     let header = exe::DOS::parse(&buf).unwrap();

runtime/src/memory.rs

@@ -62,12 +62,25 @@ impl<'a> Memory<'a> {
             .unwrap();
     }
 
-    pub fn read_str(&self, addr: u32) -> &str {
+    /// Read the bytes of a NUL-terminated string, without the terminator.
+    pub fn read_cstr(&self, addr: u32) -> &[u8] {
         self.check_access(addr);
         let buf = &self.bytes[addr as usize..];
         let nul = buf.iter().position(|&c| c == 0).unwrap();
-        let buf = &buf[..nul];
-        std::str::from_utf8(buf).unwrap()
+        &buf[..nul]
+    }
+
+    /// Read a NUL-terminated string as text, for paths, names and logging.
+    /// Programs of this era use the Windows-1252 code page, so bytes that
+    /// aren't valid UTF-8 are decoded as Latin-1 rather than rejected. This is
+    /// convenient rather than correct: a caller that cares about the encoding
+    /// should take the bytes from read_cstr.
+    pub fn read_str(&self, addr: u32) -> String {
+        let buf = self.read_cstr(addr);
+        match std::str::from_utf8(buf) {
+            Ok(s) => s.to_string(),
+            Err(_) => buf.iter().map(|&b| b as char).collect(),
+        }
     }
 
     /// This returns an allocated string rather than a reference due to alignment.

win32/winapi/src/kernel32/dll.rs

@@ -110,8 +110,7 @@ pub fn GetModuleFileNameA(
 #[win32_derive::dllexport]
 pub fn GetModuleHandleA(ctx: &mut Context, lpModuleName: Ptr<u8>) -> HMODULE {
     let kernel32 = lock();
-    let Some(name) =
-        (lpModuleName.addr != 0).then(|| ctx.memory.read_str(lpModuleName.addr).to_owned())
+    let Some(name) = (lpModuleName.addr != 0).then(|| ctx.memory.read_str(lpModuleName.addr))
     else {
         // A null name asks for the running executable itself.
         return kernel32.image_base;
@@ -127,7 +126,7 @@ pub fn GetModuleHandleA(ctx: &mut Context, lpModuleName: Ptr<u8>) -> HMODULE {
 
 #[win32_derive::dllexport]
 pub fn LoadLibraryA(ctx: &mut Context, lpLibFileName: Ptr<u8>) -> HMODULE {
-    let filename = ctx.memory.read_str(lpLibFileName.addr).to_owned();
+    let filename = ctx.memory.read_str(lpLibFileName.addr);
     let addr = lock().dlls.load_library(&filename);
     if addr == 0 {
         log::warn!("LoadLibrary({filename}): not supported, returning null");
@@ -147,7 +146,7 @@ pub fn GetProcAddress(ctx: &mut Context, hModule: HMODULE, lpProcName: Ptr<u8>)
     let name = if lpProcName.addr < 0x1000 {
         format!("ordinal{}", lpProcName.addr)
     } else {
-        ctx.memory.read_str(lpProcName.addr).to_owned()
+        ctx.memory.read_str(lpProcName.addr)
     };
     let addr = lock().dlls.get_proc_address(hModule, &name);
     if addr == 0 {

win32/winapi/src/kernel32/file.rs

@@ -115,7 +115,7 @@ pub fn CreateFileA(
     _dwFlagsAndAttributes: u32,
     _hTemplateFile: u32,
 ) -> crate::HANDLE {
-    let name = ctx.memory.read_str(lpFileName.addr).to_owned();
+    let name = ctx.memory.read_str(lpFileName.addr);
     let path = resolve_path(&name);
     let write = dwDesiredAccess & GENERIC_WRITE != 0;
     let read = dwDesiredAccess & GENERIC_READ != 0;
@@ -300,7 +300,7 @@ pub fn CloseHandle(_ctx: &mut Context, hObject: crate::HANDLE) -> bool {
 
 #[win32_derive::dllexport]
 pub fn DeleteFileA(ctx: &mut Context, lpFileName: Ptr<u8>) -> bool {
-    let name = ctx.memory.read_str(lpFileName.addr).to_owned();
+    let name = ctx.memory.read_str(lpFileName.addr);
     host::fs::remove_file(&resolve_path(&name)).is_ok()
 }
 
@@ -352,7 +352,7 @@ pub fn GetCurrentDirectoryA(ctx: &mut Context, nBufferLength: u32, lpBuffer: Ptr
 
 #[win32_derive::dllexport]
 pub fn SetCurrentDirectoryA(ctx: &mut Context, lpPathName: Ptr<u8>) -> bool {
-    let name = ctx.memory.read_str(lpPathName.addr).to_owned();
+    let name = ctx.memory.read_str(lpPathName.addr);
     let path = resolve_path(&name);
     match host::fs::set_current_dir(&path) {
         Ok(()) => true,
@@ -457,7 +457,7 @@ pub fn FindFirstFileA(
     lpFileName: Ptr<u8>,
     lpFindFileData: Ptr<WIN32_FIND_DATAA>,
 ) -> crate::HANDLE {
-    let pattern = ctx.memory.read_str(lpFileName.addr).to_owned();
+    let pattern = ctx.memory.read_str(lpFileName.addr);
     let pattern = pattern.replace('\\', "/");
     let (dir, file_pattern) = match pattern.rfind('/') {
         Some(pos) => (&pattern[..pos], &pattern[pos + 1..]),

win32/winapi/src/kernel32/nls.rs

@@ -80,7 +80,7 @@ pub fn GetStringTypeA(
         return false;
     }
     let len = if cchSrc < 0 {
-        ctx.memory.read_str(lpSrcStr.addr).len() + 1
+        ctx.memory.read_cstr(lpSrcStr.addr).len() + 1
     } else {
         cchSrc as usize
     };
@@ -147,7 +147,7 @@ pub fn LCMapStringA(
     cchDest: i32,
 ) -> i32 {
     let len = if cchSrc < 0 {
-        ctx.memory.read_str(lpSrcStr.addr).len() as u32 + 1
+        ctx.memory.read_cstr(lpSrcStr.addr).len() as u32 + 1
     } else {
         cchSrc as u32
     };

win32/winapi/src/kernel32/strings.rs

@@ -18,33 +18,31 @@ const NORM_IGNORECASE: u32 = 1;
 pub fn lstrlenA(ctx: &mut Context, lpString: Ptr<u8>) -> i32 {
     // A null string faults on Windows too, so let the null page guard catch it
     // rather than inventing a length.
-    ctx.memory.read_str(lpString.addr).len() as i32
+    ctx.memory.read_cstr(lpString.addr).len() as i32
 }
 
 #[win32_derive::dllexport]
 pub fn lstrcpyA(ctx: &mut Context, lpString1: Ptr<u8>, lpString2: Ptr<u8>) -> u32 {
-    let src = ctx.memory.read_str(lpString2.addr).to_owned();
-    let bytes = src.as_bytes();
-    ctx.memory[lpString1.addr..][..bytes.len()].copy_from_slice(bytes);
+    let bytes = ctx.memory.read_cstr(lpString2.addr).to_vec();
+    ctx.memory[lpString1.addr..][..bytes.len()].copy_from_slice(&bytes);
     ctx.memory
         .write::<u8>(lpString1.addr + bytes.len() as u32, 0);
     lpString1.addr
 }
 
 #[win32_derive::dllexport]
 pub fn lstrcatA(ctx: &mut Context, lpString1: Ptr<u8>, lpString2: Ptr<u8>) -> u32 {
-    let dst_len = ctx.memory.read_str(lpString1.addr).len() as u32;
-    let src = ctx.memory.read_str(lpString2.addr).to_owned();
-    let bytes = src.as_bytes();
-    ctx.memory[lpString1.addr + dst_len..][..bytes.len()].copy_from_slice(bytes);
+    let dst_len = ctx.memory.read_cstr(lpString1.addr).len() as u32;
+    let bytes = ctx.memory.read_cstr(lpString2.addr).to_vec();
+    ctx.memory[lpString1.addr + dst_len..][..bytes.len()].copy_from_slice(&bytes);
     ctx.memory
         .write::<u8>(lpString1.addr + dst_len + bytes.len() as u32, 0);
     lpString1.addr
 }
 
 fn read_counted_a(ctx: &Context, addr: u32, count: i32) -> Vec<u8> {
     if count < 0 {
-        ctx.memory.read_str(addr).as_bytes().to_vec()
+        ctx.memory.read_cstr(addr).to_vec()
     } else {
         ctx.memory[addr..][..count as usize].to_vec()
     }

win32/winapi/src/user32/misc.rs

@@ -93,7 +93,7 @@ pub fn MessageBoxA(
         if ptr.addr == 0 {
             String::new()
         } else {
-            ctx.memory.read_str(ptr.addr).to_owned()
+            ctx.memory.read_str(ptr.addr)
         }
     };
     log::warn!("MessageBox: {} / {}", read(lpCaption), read(lpText));
@@ -164,10 +164,10 @@ pub fn wsprintfA(ctx: &mut Context) -> i32 {
     let esp = ctx.cpu.regs.esp;
     let dst = ctx.memory.read::<u32>(esp + 4);
     let fmt_addr = ctx.memory.read::<u32>(esp + 8);
-    let fmt = ctx.memory.read_str(fmt_addr).to_owned();
+    let fmt = ctx.memory.read_cstr(fmt_addr).to_vec();
     let mut arg_addr = esp + 12;
 
-    let bytes = fmt.as_bytes();
+    let bytes = fmt.as_slice();
     let mut out: Vec<u8> = Vec::new();
     let mut i = 0;
     while i < bytes.len() {
@@ -229,7 +229,7 @@ pub fn wsprintfA(ctx: &mut Context) -> i32 {
             b'c' => vec![next_arg() as u8],
             b's' => {
                 let addr = next_arg();
-                ctx.memory.read_str(addr).as_bytes().to_vec()
+                ctx.memory.read_cstr(addr).to_vec()
             }
             _ => {
                 // Consume the arg anyway: skipping it would shift every

win32/winapi/src/winmm/mmio.rs

@@ -154,7 +154,7 @@ pub fn mmioOpenA(ctx: &mut Context, szFilename: u32, _lpmmioinfo: u32, _dwOpenFl
         log::warn!("mmioOpenA: no filename");
         return 0;
     }
-    let name = ctx.memory.read_str(szFilename).to_owned();
+    let name = ctx.memory.read_str(szFilename);
     let path = kernel32::resolve_path(&name);
     let data = match host::fs::read(&path) {
         Ok(data) => data,