Auto merge of #3466 - RalfJung:GetFullPathNameW, r=RalfJung

bors · bors · commit 0dee2b13ebd1 · 2024-04-15T15:13:43.000Z
add some basic support for GetFullPathNameW

This is the last missing piece to make std `path::` tests work on Windows.
diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs
@@ -1285,3 +1285,18 @@ pub(crate) fn simd_element_to_bool(elem: ImmTy<'_, Provenance>) -> InterpResult<
         _ => throw_ub_format!("each element of a SIMD mask must be all-0-bits or all-1-bits"),
     })
 }
+
+/// Check whether an operation that writes to a target buffer was successful.
+/// Accordingly select return value.
+/// Local helper function to be used in Windows shims.
+pub(crate) fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 {
+    if success {
+        // If the function succeeds, the return value is the number of characters stored in the target buffer,
+        // not including the terminating null character.
+        u32::try_from(len.checked_sub(1).unwrap()).unwrap()
+    } else {
+        // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters,
+        // required to hold the string and its terminating null character.
+        u32::try_from(len).unwrap()
+    }
+}
diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs
@@ -13,6 +13,7 @@
 #![feature(let_chains)]
 #![feature(lint_reasons)]
 #![feature(trait_upcasting)]
+#![feature(absolute_path)]
 // Configure clippy and other lints
 #![allow(
     clippy::collapsible_else_if,
diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs
@@ -9,21 +9,7 @@ use rustc_middle::ty::Ty;
 use rustc_target::abi::Size;
 
 use crate::*;
-
-/// Check whether an operation that writes to a target buffer was successful.
-/// Accordingly select return value.
-/// Local helper function to be used in Windows shims.
-fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 {
-    if success {
-        // If the function succeeds, the return value is the number of characters stored in the target buffer,
-        // not including the terminating null character.
-        u32::try_from(len.checked_sub(1).unwrap()).unwrap()
-    } else {
-        // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters,
-        // required to hold the string and its terminating null character.
-        u32::try_from(len).unwrap()
-    }
-}
+use helpers::windows_check_buffer_size;
 
 #[derive(Default)]
 pub struct EnvVars<'tcx> {
@@ -164,7 +150,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let name_ptr = this.read_pointer(name_op)?;
         let name = this.read_os_str_from_wide_str(name_ptr)?;
         Ok(match this.machine.env_vars.map.get(&name) {
-            Some(var_ptr) => {
+            Some(&var_ptr) => {
+                this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error
                 // The offset is used to strip the "{name}=" part of the string.
                 #[rustfmt::skip]
                 let name_offset_bytes = u64::try_from(name.len()).unwrap()
@@ -375,10 +362,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         // If we cannot get the current directory, we return 0
         match env::current_dir() {
-            Ok(cwd) =>
+            Ok(cwd) => {
+                this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error
                 return Ok(Scalar::from_u32(windows_check_buffer_size(
                     this.write_path_to_wide_str(&cwd, buf, size, /*truncate*/ false)?,
-                ))),
+                )));
+            }
             Err(e) => this.set_last_error_from_io_error(e.kind())?,
         }
         Ok(Scalar::from_u32(0))
diff --git a/src/tools/miri/src/shims/os_str.rs b/src/tools/miri/src/shims/os_str.rs
@@ -316,9 +316,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             // We also have to ensure that absolute paths remain absolute.
             match direction {
                 PathConversion::HostToTarget => {
-                    // If this start withs a `\`, we add `\\?` so it starts with `\\?\` which is
-                    // some magic path on Windows that *is* considered absolute.
-                    if converted.get(0).copied() == Some(b'\\') {
+                    // If the path is `/C:/`, the leading backslash was probably added by the below
+                    // driver letter handling and we should get rid of it again.
+                    if converted.get(0).copied() == Some(b'\\')
+                        && converted.get(2).copied() == Some(b':')
+                        && converted.get(3).copied() == Some(b'\\')
+                    {
+                        converted.remove(0);
+                    }
+                    // If this start withs a `\` but not a `\\`, then for Windows this is a relative
+                    // path. But the host path is absolute as it started with `/`. We add `\\?` so
+                    // it starts with `\\?\` which is some magic path on Windows that *is*
+                    // considered absolute.
+                    else if converted.get(0).copied() == Some(b'\\')
+                        && converted.get(1).copied() != Some(b'\\')
+                    {
                         converted.splice(0..0, b"\\\\?".iter().copied());
                     }
                 }
@@ -333,6 +345,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         // Remove first 3 characters
                         converted.splice(0..3, std::iter::empty());
                     }
+                    // If it starts with a drive letter, convert it to an absolute Unix path.
+                    else if converted.get(1).copied() == Some(b':')
+                        && converted.get(2).copied() == Some(b'/')
+                    {
+                        converted.insert(0, b'/');
+                    }
                 }
             }
             Cow::Owned(OsString::from_vec(converted))
diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs
@@ -1,5 +1,7 @@
 use std::ffi::OsStr;
+use std::io;
 use std::iter;
+use std::path::{self, Path, PathBuf};
 use std::str;
 
 use rustc_span::Symbol;
@@ -21,6 +23,61 @@ fn is_dyn_sym(name: &str) -> bool {
     )
 }
 
+#[cfg(windows)]
+fn win_absolute<'tcx>(path: &Path) -> InterpResult<'tcx, io::Result<PathBuf>> {
+    // We are on Windows so we can simply lte the host do this.
+    return Ok(path::absolute(path));
+}
+
+#[cfg(unix)]
+#[allow(clippy::get_first, clippy::arithmetic_side_effects)]
+fn win_absolute<'tcx>(path: &Path) -> InterpResult<'tcx, io::Result<PathBuf>> {
+    // We are on Unix, so we need to implement parts of the logic ourselves.
+    let bytes = path.as_os_str().as_encoded_bytes();
+    // If it starts with `//` (these were backslashes but are already converted)
+    // then this is a magic special path, we just leave it unchanged.
+    if bytes.get(0).copied() == Some(b'/') && bytes.get(1).copied() == Some(b'/') {
+        return Ok(Ok(path.into()));
+    };
+    // Special treatment for Windows' magic filenames: they are treated as being relative to `\\.\`.
+    let magic_filenames = &[
+        "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8",
+        "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9",
+    ];
+    if magic_filenames.iter().any(|m| m.as_bytes() == bytes) {
+        let mut result: Vec<u8> = br"//./".into();
+        result.extend(bytes);
+        return Ok(Ok(bytes_to_os_str(&result)?.into()));
+    }
+    // Otherwise we try to do something kind of close to what Windows does, but this is probably not
+    // right in all cases. We iterate over the components between `/`, and remove trailing `.`,
+    // except that trailing `..` remain unchanged.
+    let mut result = vec![];
+    let mut bytes = bytes; // the remaining bytes to process
+    loop {
+        let len = bytes.iter().position(|&b| b == b'/').unwrap_or(bytes.len());
+        let mut component = &bytes[..len];
+        if len >= 2 && component[len - 1] == b'.' && component[len - 2] != b'.' {
+            // Strip trailing `.`
+            component = &component[..len - 1];
+        }
+        // Add this component to output.
+        result.extend(component);
+        // Prepare next iteration.
+        if len < bytes.len() {
+            // There's a component after this; add `/` and process remaining bytes.
+            result.push(b'/');
+            bytes = &bytes[len + 1..];
+            continue;
+        } else {
+            // This was the last component and it did not have a trailing `/`.
+            break;
+        }
+    }
+    // Let the host `absolute` function do working-dir handling
+    Ok(path::absolute(bytes_to_os_str(&result)?))
+}
+
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     fn emulate_foreign_item_inner(
@@ -112,7 +169,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
                 let written = if handle == -11 || handle == -12 {
                     // stdout/stderr
-                    use std::io::{self, Write};
+                    use io::Write;
 
                     let buf_cont =
                         this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(u64::from(n)))?;
@@ -146,6 +203,40 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     dest,
                 )?;
             }
+            "GetFullPathNameW" => {
+                let [filename, size, buffer, filepart] =
+                    this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
+                this.check_no_isolation("`GetFullPathNameW`")?;
+
+                let filename = this.read_pointer(filename)?;
+                let size = this.read_scalar(size)?.to_u32()?;
+                let buffer = this.read_pointer(buffer)?;
+                let filepart = this.read_pointer(filepart)?;
+
+                if !this.ptr_is_null(filepart)? {
+                    throw_unsup_format!("GetFullPathNameW: non-null `lpFilePart` is not supported");
+                }
+
+                let filename = this.read_path_from_wide_str(filename)?;
+                let result = match win_absolute(&filename)? {
+                    Err(err) => {
+                        this.set_last_error_from_io_error(err.kind())?;
+                        Scalar::from_u32(0) // return zero upon failure
+                    }
+                    Ok(abs_filename) => {
+                        this.set_last_error(Scalar::from_u32(0))?; // make sure this is unambiguously not an error
+                        Scalar::from_u32(helpers::windows_check_buffer_size(
+                            this.write_path_to_wide_str(
+                                &abs_filename,
+                                buffer,
+                                size.into(),
+                                /*truncate*/ false,
+                            )?,
+                        ))
+                    }
+                };
+                this.write_scalar(result, dest)?;
+            }
 
             // Allocation
             "HeapAlloc" => {
diff --git a/src/tools/miri/tests/pass/shims/path.rs b/src/tools/miri/tests/pass/shims/path.rs
@@ -0,0 +1,38 @@
+//@compile-flags: -Zmiri-disable-isolation
+#![feature(absolute_path)]
+use std::path::{absolute, Path};
+
+#[track_caller]
+fn test_absolute(in_: &str, out: &str) {
+    assert_eq!(absolute(in_).unwrap().as_os_str(), Path::new(out).as_os_str());
+}
+
+fn main() {
+    if cfg!(unix) {
+        test_absolute("/a/b/c", "/a/b/c");
+        test_absolute("/a/b/c", "/a/b/c");
+        test_absolute("/a//b/c", "/a/b/c");
+        test_absolute("//a/b/c", "//a/b/c");
+        test_absolute("///a/b/c", "/a/b/c");
+        test_absolute("/a/b/c/", "/a/b/c/");
+        test_absolute("/a/./b/../c/.././..", "/a/b/../c/../..");
+    } else if cfg!(windows) {
+        // Test that all these are unchanged
+        test_absolute(r"C:\path\to\file", r"C:\path\to\file");
+        test_absolute(r"C:\path\to\file\", r"C:\path\to\file\");
+        test_absolute(r"\\server\share\to\file", r"\\server\share\to\file");
+        test_absolute(r"\\server.\share.\to\file", r"\\server.\share.\to\file");
+        test_absolute(r"\\.\PIPE\name", r"\\.\PIPE\name");
+        test_absolute(r"\\.\C:\path\to\COM1", r"\\.\C:\path\to\COM1");
+        test_absolute(r"\\?\C:\path\to\file", r"\\?\C:\path\to\file");
+        test_absolute(r"\\?\UNC\server\share\to\file", r"\\?\UNC\server\share\to\file");
+        test_absolute(r"\\?\PIPE\name", r"\\?\PIPE\name");
+        // Verbatim paths are always unchanged, no matter what.
+        test_absolute(r"\\?\path.\to/file..", r"\\?\path.\to/file..");
+
+        test_absolute(r"C:\path..\to.\file.", r"C:\path..\to\file");
+        test_absolute(r"COM1", r"\\.\COM1");
+    } else {
+        panic!("unsupported OS");
+    }
+}
