From d552fdb6009c65588f520d5d142e2238135979fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Sat, 12 Sep 2026 14:03:27 +0200 Subject: [PATCH 1/2] add Memory::read_cstr for the bytes of a C string lstrlenA, lstrcpyA, lstrcatA, wsprintfA and the NLS length computations work on bytes, so hand them the bytes instead of going through a &str and back. This keeps them exact once read_str starts decoding text that isn't UTF-8, which would otherwise change lengths and re-encode copies. Co-Authored-By: Claude Fable 5.1 --- runtime/src/memory.rs | 10 +++++++--- win32/winapi/src/kernel32/nls.rs | 4 ++-- win32/winapi/src/kernel32/strings.rs | 16 +++++++--------- win32/winapi/src/user32/misc.rs | 6 +++--- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/runtime/src/memory.rs b/runtime/src/memory.rs index e1edcc6f..4ef0e1bf 100644 --- a/runtime/src/memory.rs +++ b/runtime/src/memory.rs @@ -62,12 +62,16 @@ 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] + } + + pub fn read_str(&self, addr: u32) -> &str { + std::str::from_utf8(self.read_cstr(addr)).unwrap() } /// This returns an allocated string rather than a reference due to alignment. diff --git a/win32/winapi/src/kernel32/nls.rs b/win32/winapi/src/kernel32/nls.rs index 9e28d7ed..c96b4f79 100644 --- a/win32/winapi/src/kernel32/nls.rs +++ b/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 }; diff --git a/win32/winapi/src/kernel32/strings.rs b/win32/winapi/src/kernel32/strings.rs index bbc5582d..ad001479 100644 --- a/win32/winapi/src/kernel32/strings.rs +++ b/win32/winapi/src/kernel32/strings.rs @@ -18,14 +18,13 @@ const NORM_IGNORECASE: u32 = 1; pub fn lstrlenA(ctx: &mut Context, lpString: Ptr) -> 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, lpString2: Ptr) -> 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::(lpString1.addr + bytes.len() as u32, 0); lpString1.addr @@ -33,10 +32,9 @@ pub fn lstrcpyA(ctx: &mut Context, lpString1: Ptr, lpString2: Ptr) -> u3 #[win32_derive::dllexport] pub fn lstrcatA(ctx: &mut Context, lpString1: Ptr, lpString2: Ptr) -> 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::(lpString1.addr + dst_len + bytes.len() as u32, 0); lpString1.addr @@ -44,7 +42,7 @@ pub fn lstrcatA(ctx: &mut Context, lpString1: Ptr, lpString2: Ptr) -> u3 fn read_counted_a(ctx: &Context, addr: u32, count: i32) -> Vec { 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() } diff --git a/win32/winapi/src/user32/misc.rs b/win32/winapi/src/user32/misc.rs index dae9d082..b2453dfa 100644 --- a/win32/winapi/src/user32/misc.rs +++ b/win32/winapi/src/user32/misc.rs @@ -164,10 +164,10 @@ pub fn wsprintfA(ctx: &mut Context) -> i32 { let esp = ctx.cpu.regs.esp; let dst = ctx.memory.read::(esp + 4); let fmt_addr = ctx.memory.read::(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 = 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 From 8c48acd8bef4cf01eaafb660e676bedba6bf7b78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Sat, 12 Sep 2026 14:04:00 +0200 Subject: [PATCH 2/2] decode non-UTF-8 strings as Latin-1 instead of panicking Programs of this era use the Windows-1252 code page; a window title with a copyright sign in it took read_str's from_utf8 unwrap down. Decode such bytes as Latin-1 instead. read_str returns a String: it is the convenient reading, not the correct one, and a caller that cares about the encoding has read_cstr. Co-Authored-By: Claude Fable 5.1 --- dos/src/dosapi.rs | 4 ++-- runtime/src/memory.rs | 13 +++++++++++-- win32/winapi/src/kernel32/dll.rs | 7 +++---- win32/winapi/src/kernel32/file.rs | 8 ++++---- win32/winapi/src/user32/misc.rs | 2 +- win32/winapi/src/winmm/mmio.rs | 2 +- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/dos/src/dosapi.rs b/dos/src/dosapi.rs index d9a14b7a..f3dea46b 100644 --- a/dos/src/dosapi.rs +++ b/dos/src/dosapi.rs @@ -83,7 +83,7 @@ pub fn int21(ctx: &mut Context) -> Option { 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 { let seg = ctx.memory.read::(params_addr); let relo = ctx.memory.read::(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(); diff --git a/runtime/src/memory.rs b/runtime/src/memory.rs index 4ef0e1bf..7dd88688 100644 --- a/runtime/src/memory.rs +++ b/runtime/src/memory.rs @@ -70,8 +70,17 @@ impl<'a> Memory<'a> { &buf[..nul] } - pub fn read_str(&self, addr: u32) -> &str { - std::str::from_utf8(self.read_cstr(addr)).unwrap() + /// 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. diff --git a/win32/winapi/src/kernel32/dll.rs b/win32/winapi/src/kernel32/dll.rs index 70ee512b..4811ebca 100644 --- a/win32/winapi/src/kernel32/dll.rs +++ b/win32/winapi/src/kernel32/dll.rs @@ -110,8 +110,7 @@ pub fn GetModuleFileNameA( #[win32_derive::dllexport] pub fn GetModuleHandleA(ctx: &mut Context, lpModuleName: Ptr) -> 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) -> HMODULE { #[win32_derive::dllexport] pub fn LoadLibraryA(ctx: &mut Context, lpLibFileName: Ptr) -> 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) 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 { diff --git a/win32/winapi/src/kernel32/file.rs b/win32/winapi/src/kernel32/file.rs index e8fc9ef0..e40e43b2 100644 --- a/win32/winapi/src/kernel32/file.rs +++ b/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) -> 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) -> 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, lpFindFileData: Ptr, ) -> 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..]), diff --git a/win32/winapi/src/user32/misc.rs b/win32/winapi/src/user32/misc.rs index b2453dfa..4a606a11 100644 --- a/win32/winapi/src/user32/misc.rs +++ b/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)); diff --git a/win32/winapi/src/winmm/mmio.rs b/win32/winapi/src/winmm/mmio.rs index 14ba8215..144533b9 100644 --- a/win32/winapi/src/winmm/mmio.rs +++ b/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,