Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shim for realpath on unix #2294

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const UNIX_IO_ERROR_TABLE: &[(std::io::ErrorKind, &str)] = {
(NotFound, "ENOENT"),
(Interrupted, "EINTR"),
(InvalidInput, "EINVAL"),
(InvalidFilename, "ENAMETOOLONG"),
(TimedOut, "ETIMEDOUT"),
(AlreadyExists, "EEXIST"),
(WouldBlock, "EWOULDBLOCK"),
Expand Down
5 changes: 5 additions & 0 deletions src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// fadvise is only informational, we can ignore it.
this.write_null(dest)?;
}
"realpath" => {
let [path, resolved_path] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let result = this.realpath(path, resolved_path)?;
this.write_pointer(result, dest)?;
}

// Time related shims
"gettimeofday" => {
Expand Down
62 changes: 62 additions & 0 deletions src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::convert::TryInto;
use std::fs::{
read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir,
};
Expand Down Expand Up @@ -1659,6 +1660,67 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
}
}

fn realpath(
LegNeato marked this conversation as resolved.
Show resolved Hide resolved
&mut self,
path_op: &OpTy<'tcx, Tag>,
processed_path_op: &OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, Pointer<Option<Tag>>> {
let this = self.eval_context_mut();
this.assert_target_os_is_unix("realpath");

let pathname = this.read_path_from_c_str(this.read_pointer(path_op)?)?;
LegNeato marked this conversation as resolved.
Show resolved Hide resolved
let processed_ptr = this.read_pointer(processed_path_op)?;

// Reject if isolation is enabled.
if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op {
this.reject_in_isolation("`realpath`", reject_with)?;
let eacc = this.eval_libc("EACCES")?;
this.set_last_error(eacc)?;
return Ok(Pointer::null());
}

let result = std::fs::canonicalize(pathname);
match result {
Ok(resolved) => {
let path_max = this
.eval_libc_i32("PATH_MAX")?
.try_into()
.expect("PATH_MAX does not fit in u64");
let dest = if this.ptr_is_null(processed_ptr)? {
// POSIX says behavior when passing a null pointer is implementation-defined,
// but GNU/linux, freebsd, netbsd, bionic/android, and macos all treat a null pointer
// similarly to:
//
// "If resolved_path is specified as NULL, then realpath() uses
LegNeato marked this conversation as resolved.
Show resolved Hide resolved
// malloc(3) to allocate a buffer of up to PATH_MAX bytes to hold
// the resolved pathname, and returns a pointer to this buffer. The
// caller should deallocate this buffer using free(3)."
// <https://man7.org/linux/man-pages/man3/realpath.3.html>
this.alloc_os_str_as_c_str(resolved.as_os_str(), MiriMemoryKind::C.into())?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I just realized this is a general os_str function, but it should be a path function to do the directory separator conversion. We need alloc_path_as_c_str but such a function does not exist yet. Can you add it?

} else {
let (wrote_path, _) =
this.write_path_to_c_str(&resolved, processed_ptr, path_max)?;

if !wrote_path {
// Note that we do not explicitly handle `FILENAME_MAX`
// (different from `PATH_MAX` above) as it is Linux-specific and
// seems like a bit of a mess anyway: <https://eklitzke.org/path-max-is-tricky>.
let enametoolong = this.eval_libc("ENAMETOOLONG")?;
this.set_last_error(enametoolong)?;
return Ok(Pointer::null());
}
processed_ptr
};

Ok(dest)
}
Err(e) => {
this.set_last_error_from_io_error(e.kind())?;
Ok(Pointer::null())
}
}
}
}

/// Extracts the number of seconds and nanoseconds elapsed between `time` and the unix epoch when
Expand Down
6 changes: 6 additions & 0 deletions src/shims/unix/macos/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let result = this.ftruncate64(fd, length)?;
this.write_scalar(Scalar::from_i32(result), dest)?;
}
"realpath$DARWIN_EXTSN" => {
let [path, resolved_path] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let result = this.realpath(path, resolved_path)?;
this.write_pointer(result, dest)?;
}

// Environment related shims
"_NSGetEnviron" => {
Expand Down
19 changes: 19 additions & 0 deletions tests/pass/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ fn main() {
test_errors();
test_rename();
test_directory();
test_canonicalize();
test_dup_stdout_stderr();

// These all require unix, if the test is changed to no longer `ignore-windows`, move these to a unix test
Expand Down Expand Up @@ -364,6 +365,24 @@ fn test_rename() {
remove_file(&path2).unwrap();
}

fn test_canonicalize() {
use std::fs::canonicalize;
let dir_path = prepare_dir("miri_test_fs_dir");
create_dir(&dir_path).unwrap();
let path = dir_path.join("test_file");
drop(File::create(&path).unwrap());

let p = canonicalize(format!("{}/./test_file", dir_path.to_string_lossy())).unwrap();
assert_eq!(p.to_string_lossy().find('.'), None);

remove_dir_all(&dir_path).unwrap();

// Make sure we get an error for long paths.
use std::convert::TryInto;
let too_long = "x/".repeat(libc::PATH_MAX.try_into().unwrap());
assert!(canonicalize(too_long).is_err());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we can also test the error kind, can't we?

}

fn test_directory() {
let dir_path = prepare_dir("miri_test_fs_dir");
// Creating a directory should succeed.
Expand Down
140 changes: 137 additions & 3 deletions tests/pass/libc.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,147 @@
// ignore-windows: No libc on Windows
// compile-flags: -Zmiri-disable-isolation

#![feature(io_error_more)]
#![feature(rustc_private)]

use std::path::PathBuf;

extern crate libc;

#[cfg(any(target_os = "linux", target_os = "freebsd"))]
fn tmp() -> std::path::PathBuf {
fn tmp() -> PathBuf {
std::env::var("MIRI_TEMP")
.map(std::path::PathBuf::from)
.map(|tmp| {
// MIRI_TEMP is set outside of our emulated
// program, so it may have path separators that don't
// correspond to our target platform. We normalize them here
// before constructing a `PathBuf`

#[cfg(windows)]
return PathBuf::from(tmp.replace("/", "\\"));

#[cfg(not(windows))]
return PathBuf::from(tmp.replace("\\", "/"));
Comment on lines +17 to +23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// before constructing a `PathBuf`
#[cfg(windows)]
return PathBuf::from(tmp.replace("/", "\\"));
#[cfg(not(windows))]
return PathBuf::from(tmp.replace("\\", "/"));
// before constructing a `PathBuf`.
return PathBuf::from(tmp.replace("\\", "/"));

This is a non-windows test.

})
.unwrap_or_else(|_| std::env::temp_dir())
LegNeato marked this conversation as resolved.
Show resolved Hide resolved
}

/// Test allocating variant of `realpath`.
fn test_posix_realpath_alloc() {
use std::ffi::OsString;
use std::ffi::{CStr, CString};
use std::fs::{remove_file, File};
use std::os::unix::ffi::OsStrExt;
use std::os::unix::ffi::OsStringExt;

let buf;
let path = tmp().join("miri_test_libc_posix_realpath_alloc");
let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed");

// Cleanup before test.
remove_file(&path).ok();
// Create file.
drop(File::create(&path).unwrap());
unsafe {
let r = libc::realpath(c_path.as_ptr(), std::ptr::null_mut());
assert!(!r.is_null());
buf = CStr::from_ptr(r).to_bytes().to_vec();
libc::free(r as *mut _);
}
let canonical = PathBuf::from(OsString::from_vec(buf));
assert_eq!(path.file_name(), canonical.file_name());

// Cleanup after test.
LegNeato marked this conversation as resolved.
Show resolved Hide resolved
remove_file(&path).unwrap();
}

/// Test non-allocating variant of `realpath`.
fn test_posix_realpath_noalloc() {
use std::ffi::{CStr, CString};
use std::fs::{remove_file, File};
use std::os::unix::ffi::OsStrExt;

let path = tmp().join("miri_test_libc_posix_realpath_noalloc");
let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed");

let mut v = vec![0; libc::PATH_MAX as usize];

// Cleanup before test.
remove_file(&path).ok();
// Create file.
drop(File::create(&path).unwrap());
unsafe {
let r = libc::realpath(c_path.as_ptr(), v.as_mut_ptr());
assert!(!r.is_null());
}
let c = unsafe { CStr::from_ptr(v.as_ptr()) };
let canonical = PathBuf::from(c.to_str().expect("CStr to str"));

assert_eq!(path.file_name(), canonical.file_name());

// Cleanup after test.
remove_file(&path).unwrap();
}

/// Test failure cases for `realpath`.
fn test_posix_realpath_errors() {
use std::convert::TryInto;
use std::ffi::CString;
use std::fs::{create_dir_all, remove_dir_all};
use std::io::ErrorKind;
use std::os::unix::ffi::OsStrExt;
use std::os::unix::fs::symlink;

// Test non-existent path returns an error.
let c_path = CString::new("./nothing_to_see_here").expect("CString::new failed");
let r = unsafe { libc::realpath(c_path.as_ptr(), std::ptr::null_mut()) };
assert!(r.is_null());
let e = std::io::Error::last_os_error();
assert_eq!(e.raw_os_error(), Some(libc::ENOENT));
assert_eq!(e.kind(), ErrorKind::NotFound);

// Test that a long path returns an error.
//
// Linux first checks if the path exists and macos does not.
// Using an existing path ensures all platforms return `ENAMETOOLONG` given a long path.
//
// Rather than creating a bunch of directories, we create two directories containing symlinks.
// Sadly we can't avoid creating directories and instead use a path like "./././././" or "./../../" as linux
// appears to collapse "." and ".." before checking path length.
Copy link
Member

@RalfJung RalfJung Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this test makes any sense... the actual real path after resolving all those symlinks to /tmp/.../x, i.e., it is very short. The test creates a very long input path but the interesting case is a very long output.

On my system this test produces a FilesystemLoop error.

let path = tmp().join("posix_realpath_errors");
// Cleanup before test.
remove_dir_all(&path).ok();

// The directories we will put symlinks in.
let x = path.join("x/");
let y = path.join("y/");

// The symlinks in each directory pointing to each other.
let yx_sym = y.join("x");
let xy_sym = x.join("y");

// Create directories.
create_dir_all(&x).expect("dir x");
create_dir_all(&y).expect("dir y");

// Create symlinks between directories.
symlink(&x, &yx_sym).expect("symlink x");
symlink(&y, &xy_sym).expect("symlink y ");

// This path exists due to the symlinks created above.
let too_long = path.join("x/y/".repeat(libc::PATH_MAX.try_into().unwrap()));

let c_path = CString::new(too_long.into_os_string().as_bytes()).expect("CString::new failed");
let r = unsafe { libc::realpath(c_path.as_ptr(), std::ptr::null_mut()) };
let e = std::io::Error::last_os_error();

assert!(r.is_null());
assert_eq!(e.raw_os_error(), Some(libc::ENAMETOOLONG));
assert_eq!(e.kind(), ErrorKind::InvalidFilename);

// Cleanup after test.
remove_dir_all(&path).ok();
}

#[cfg(any(target_os = "linux", target_os = "freebsd"))]
fn test_posix_fadvise() {
use std::convert::TryInto;
Expand Down Expand Up @@ -317,6 +447,10 @@ fn main() {

test_posix_gettimeofday();

test_posix_realpath_alloc();
test_posix_realpath_noalloc();
test_posix_realpath_errors();

#[cfg(any(target_os = "linux"))]
test_sync_file_range();

Expand Down