diff --git a/src/lib.rs b/src/lib.rs index 479353bb98..97271c33a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,6 +9,7 @@ #![feature(is_some_and)] #![feature(nonzero_ops)] #![feature(local_key_cell_methods)] +#![feature(is_terminal)] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, diff --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index c21e0441ca..44a433df1e 100644 --- a/src/shims/unix/foreign_items.rs +++ b/src/shims/unix/foreign_items.rs @@ -452,7 +452,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { "isatty" => { let [fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.isatty(fd)?; - this.write_scalar(Scalar::from_i32(result), dest)?; + this.write_scalar(result, dest)?; } "pthread_atfork" => { let [prepare, parent, child] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index ed68976773..0610f65db1 100644 --- a/src/shims/unix/fs.rs +++ b/src/shims/unix/fs.rs @@ -4,7 +4,7 @@ use std::convert::TryInto; use std::fs::{ read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir, }; -use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; +use std::io::{self, ErrorKind, IsTerminal, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; use std::time::SystemTime; @@ -65,6 +65,8 @@ trait FileDescriptor: std::fmt::Debug { fn dup(&mut self) -> io::Result>; + fn is_tty(&self) -> bool; + #[cfg(unix)] fn as_unix_host_fd(&self) -> Option { None @@ -143,6 +145,10 @@ impl FileDescriptor for FileHandle { use std::os::unix::io::AsRawFd; Some(self.file.as_raw_fd()) } + + fn is_tty(&self) -> bool { + self.file.is_terminal() + } } impl FileDescriptor for io::Stdin { @@ -170,6 +176,10 @@ impl FileDescriptor for io::Stdin { fn as_unix_host_fd(&self) -> Option { Some(libc::STDIN_FILENO) } + + fn is_tty(&self) -> bool { + self.is_terminal() + } } impl FileDescriptor for io::Stdout { @@ -202,6 +212,10 @@ impl FileDescriptor for io::Stdout { fn as_unix_host_fd(&self) -> Option { Some(libc::STDOUT_FILENO) } + + fn is_tty(&self) -> bool { + self.is_terminal() + } } impl FileDescriptor for io::Stderr { @@ -227,12 +241,16 @@ impl FileDescriptor for io::Stderr { fn as_unix_host_fd(&self) -> Option { Some(libc::STDERR_FILENO) } + + fn is_tty(&self) -> bool { + self.is_terminal() + } } #[derive(Debug)] -struct DummyOutput; +struct NullOutput; -impl FileDescriptor for DummyOutput { +impl FileDescriptor for NullOutput { fn name(&self) -> &'static str { "stderr and stdout" } @@ -247,7 +265,11 @@ impl FileDescriptor for DummyOutput { } fn dup(&mut self) -> io::Result> { - Ok(Box::new(DummyOutput)) + Ok(Box::new(NullOutput)) + } + + fn is_tty(&self) -> bool { + false } } @@ -267,8 +289,8 @@ impl FileHandler { let mut handles: BTreeMap<_, Box> = BTreeMap::new(); handles.insert(0i32, Box::new(io::stdin())); if mute_stdout_stderr { - handles.insert(1i32, Box::new(DummyOutput)); - handles.insert(2i32, Box::new(DummyOutput)); + handles.insert(1i32, Box::new(NullOutput)); + handles.insert(2i32, Box::new(NullOutput)); } else { handles.insert(1i32, Box::new(io::stdout())); handles.insert(2i32, Box::new(io::stderr())); @@ -1662,35 +1684,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } #[cfg_attr(not(unix), allow(unused))] - fn isatty(&mut self, miri_fd: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { + fn isatty( + &mut self, + miri_fd: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - #[cfg(unix)] + // "returns 1 if fd is an open file descriptor referring to a terminal; + // otherwise 0 is returned, and errno is set to indicate the error" if matches!(this.machine.isolated_op, IsolatedOp::Allow) { - let miri_fd = this.read_scalar(miri_fd)?.to_i32()?; - if let Some(host_fd) = - this.machine.file_handler.handles.get(&miri_fd).and_then(|fd| fd.as_unix_host_fd()) - { - // "returns 1 if fd is an open file descriptor referring to a terminal; - // otherwise 0 is returned, and errno is set to indicate the error" - // SAFETY: isatty has no preconditions - let is_tty = unsafe { libc::isatty(host_fd) }; - if is_tty == 0 { - let errno = std::io::Error::last_os_error() - .raw_os_error() - .map(Scalar::from_i32) - .unwrap(); - this.set_last_error(errno)?; - } - return Ok(is_tty); + let fd = this.read_scalar(miri_fd)?.to_i32()?; + if this.machine.file_handler.handles.get(&fd).map(|fd| fd.is_tty()) == Some(true) { + return Ok(Scalar::from_i32(1)); } } - // We are attemping to use a Unix interface on a non-Unix platform, or we are on a Unix - // platform and the passed file descriptor is not open, or isolation is enabled - // FIXME: It should be possible to emulate this at least on Windows by using - // GetConsoleMode. + // Fallback when the FD was not found or isolation is enabled. let enotty = this.eval_libc("ENOTTY")?; this.set_last_error(enotty)?; - Ok(0) + Ok(Scalar::from_i32(0)) } fn realpath( diff --git a/tests/pass-dep/shims/libc-fs-with-isolation.rs b/tests/pass-dep/shims/libc-fs-with-isolation.rs new file mode 100644 index 0000000000..f1838cf64f --- /dev/null +++ b/tests/pass-dep/shims/libc-fs-with-isolation.rs @@ -0,0 +1,28 @@ +//@ignore-target-windows: no libc on Windows +//@compile-flags: -Zmiri-isolation-error=warn-nobacktrace +//@normalize-stderr-test: "(stat(x)?)" -> "$$STAT" + +use std::ffi::CString; +use std::fs; +use std::io::{Error, ErrorKind}; + +fn main() { + // test `fcntl` + unsafe { + assert_eq!(libc::fcntl(1, libc::F_DUPFD, 0), -1); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EPERM)); + } + + // test `readlink` + let symlink_c_str = CString::new("foo.txt").unwrap(); + let mut buf = vec![0; "foo_link.txt".len() + 1]; + unsafe { + assert_eq!(libc::readlink(symlink_c_str.as_ptr(), buf.as_mut_ptr(), buf.len()), -1); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); + } + + // test `stat` + assert_eq!(fs::metadata("foo.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); + // check that it is the right kind of `PermissionDenied` + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); +} diff --git a/tests/pass-dep/shims/libc-fs-with-isolation.stderr b/tests/pass-dep/shims/libc-fs-with-isolation.stderr new file mode 100644 index 0000000000..21fcb65243 --- /dev/null +++ b/tests/pass-dep/shims/libc-fs-with-isolation.stderr @@ -0,0 +1,6 @@ +warning: `fcntl` was made to return an error due to isolation + +warning: `readlink` was made to return an error due to isolation + +warning: `$STAT` was made to return an error due to isolation + diff --git a/tests/pass-dep/shims/libc-fs.rs b/tests/pass-dep/shims/libc-fs.rs new file mode 100644 index 0000000000..acf16ecb7e --- /dev/null +++ b/tests/pass-dep/shims/libc-fs.rs @@ -0,0 +1,137 @@ +//@ignore-target-windows: no libc on Windows +//@compile-flags: -Zmiri-disable-isolation + +#![feature(io_error_more)] +#![feature(io_error_uncategorized)] + +use std::convert::TryInto; +use std::ffi::CString; +use std::fs::{canonicalize, remove_file, File}; +use std::io::{Error, ErrorKind, Write}; +use std::os::unix::ffi::OsStrExt; +use std::path::PathBuf; + +fn main() { + test_dup_stdout_stderr(); + test_canonicalize_too_long(); + test_readlink(); + test_file_open_unix_allow_two_args(); + test_file_open_unix_needs_three_args(); + test_file_open_unix_extra_third_arg(); +} + +fn tmp() -> PathBuf { + std::env::var("MIRI_TEMP") + .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("\\", "/")); + }) + .unwrap_or_else(|_| std::env::temp_dir()) +} + +/// Prepare: compute filename and make sure the file does not exist. +fn prepare(filename: &str) -> PathBuf { + let path = tmp().join(filename); + // Clean the paths for robustness. + remove_file(&path).ok(); + path +} + +/// Prepare like above, and also write some initial content to the file. +fn prepare_with_content(filename: &str, content: &[u8]) -> PathBuf { + let path = prepare(filename); + let mut file = File::create(&path).unwrap(); + file.write(content).unwrap(); + path +} + +fn test_file_open_unix_allow_two_args() { + let path = prepare_with_content("test_file_open_unix_allow_two_args.txt", &[]); + + let mut name = path.into_os_string(); + name.push("\0"); + let name_ptr = name.as_bytes().as_ptr().cast::(); + let _fd = unsafe { libc::open(name_ptr, libc::O_RDONLY) }; +} + +fn test_file_open_unix_needs_three_args() { + let path = prepare_with_content("test_file_open_unix_needs_three_args.txt", &[]); + + let mut name = path.into_os_string(); + name.push("\0"); + let name_ptr = name.as_bytes().as_ptr().cast::(); + let _fd = unsafe { libc::open(name_ptr, libc::O_CREAT, 0o666) }; +} + +fn test_file_open_unix_extra_third_arg() { + let path = prepare_with_content("test_file_open_unix_extra_third_arg.txt", &[]); + + let mut name = path.into_os_string(); + name.push("\0"); + let name_ptr = name.as_bytes().as_ptr().cast::(); + let _fd = unsafe { libc::open(name_ptr, libc::O_RDONLY, 42) }; +} + +fn test_dup_stdout_stderr() { + let bytes = b"hello dup fd\n"; + unsafe { + let new_stdout = libc::fcntl(1, libc::F_DUPFD, 0); + let new_stderr = libc::fcntl(2, libc::F_DUPFD, 0); + libc::write(new_stdout, bytes.as_ptr() as *const libc::c_void, bytes.len()); + libc::write(new_stderr, bytes.as_ptr() as *const libc::c_void, bytes.len()); + } +} + +fn test_canonicalize_too_long() { + // Make sure we get an error for long paths. + let too_long = "x/".repeat(libc::PATH_MAX.try_into().unwrap()); + assert!(canonicalize(too_long).is_err()); +} + +fn test_readlink() { + let bytes = b"Hello, World!\n"; + let path = prepare_with_content("miri_test_fs_link_target.txt", bytes); + let expected_path = path.as_os_str().as_bytes(); + + let symlink_path = prepare("miri_test_fs_symlink.txt"); + std::os::unix::fs::symlink(&path, &symlink_path).unwrap(); + + // Test that the expected string gets written to a buffer of proper + // length, and that a trailing null byte is not written. + let symlink_c_str = CString::new(symlink_path.as_os_str().as_bytes()).unwrap(); + let symlink_c_ptr = symlink_c_str.as_ptr(); + + // Make the buf one byte larger than it needs to be, + // and check that the last byte is not overwritten. + let mut large_buf = vec![0xFF; expected_path.len() + 1]; + let res = + unsafe { libc::readlink(symlink_c_ptr, large_buf.as_mut_ptr().cast(), large_buf.len()) }; + // Check that the resovled path was properly written into the buf. + assert_eq!(&large_buf[..(large_buf.len() - 1)], expected_path); + assert_eq!(large_buf.last(), Some(&0xFF)); + assert_eq!(res, large_buf.len() as isize - 1); + + // Test that the resolved path is truncated if the provided buffer + // is too small. + let mut small_buf = [0u8; 2]; + let res = + unsafe { libc::readlink(symlink_c_ptr, small_buf.as_mut_ptr().cast(), small_buf.len()) }; + assert_eq!(small_buf, &expected_path[..small_buf.len()]); + assert_eq!(res, small_buf.len() as isize); + + // Test that we report a proper error for a missing path. + let bad_path = CString::new("MIRI_MISSING_FILE_NAME").unwrap(); + let res = unsafe { + libc::readlink(bad_path.as_ptr(), small_buf.as_mut_ptr().cast(), small_buf.len()) + }; + assert_eq!(res, -1); + assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound); +} diff --git a/tests/pass-dep/shims/fs.stderr b/tests/pass-dep/shims/libc-fs.stderr similarity index 100% rename from tests/pass-dep/shims/fs.stderr rename to tests/pass-dep/shims/libc-fs.stderr diff --git a/tests/pass-dep/shims/fs.stdout b/tests/pass-dep/shims/libc-fs.stdout similarity index 100% rename from tests/pass-dep/shims/fs.stdout rename to tests/pass-dep/shims/libc-fs.stdout diff --git a/tests/pass-dep/shims/libc-rsfs.stdout b/tests/pass-dep/shims/libc-rsfs.stdout new file mode 100644 index 0000000000..b6fa69e3d5 --- /dev/null +++ b/tests/pass-dep/shims/libc-rsfs.stdout @@ -0,0 +1 @@ +hello dup fd diff --git a/tests/pass-dep/shims/fs_with_isolation.rs b/tests/pass/shims/fs-with-isolation.rs similarity index 62% rename from tests/pass-dep/shims/fs_with_isolation.rs rename to tests/pass/shims/fs-with-isolation.rs index f5420dbc55..8fa683085b 100644 --- a/tests/pass-dep/shims/fs_with_isolation.rs +++ b/tests/pass/shims/fs-with-isolation.rs @@ -2,21 +2,14 @@ //@compile-flags: -Zmiri-isolation-error=warn-nobacktrace //@normalize-stderr-test: "(stat(x)?)" -> "$$STAT" -use std::ffi::CString; use std::fs::{self, File}; -use std::io::{Error, ErrorKind}; +use std::io::ErrorKind; use std::os::unix; fn main() { // test `open` assert_eq!(File::create("foo.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); - // test `fcntl` - unsafe { - assert_eq!(libc::fcntl(1, libc::F_DUPFD, 0), -1); - assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EPERM)); - } - // test `unlink` assert_eq!(fs::remove_file("foo.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); @@ -26,17 +19,8 @@ fn main() { ErrorKind::PermissionDenied ); - // test `readlink` - let symlink_c_str = CString::new("foo.txt").unwrap(); - let mut buf = vec![0; "foo_link.txt".len() + 1]; - unsafe { - assert_eq!(libc::readlink(symlink_c_str.as_ptr(), buf.as_mut_ptr(), buf.len()), -1); - assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); - } - // test `stat` assert_eq!(fs::metadata("foo.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); - assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); // test `rename` assert_eq!(fs::rename("a.txt", "b.txt").unwrap_err().kind(), ErrorKind::PermissionDenied); @@ -49,5 +33,4 @@ fn main() { // test `opendir` assert_eq!(fs::read_dir("foo/bar").unwrap_err().kind(), ErrorKind::PermissionDenied); - assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EACCES)); } diff --git a/tests/pass-dep/shims/fs_with_isolation.stderr b/tests/pass/shims/fs-with-isolation.stderr similarity index 79% rename from tests/pass-dep/shims/fs_with_isolation.stderr rename to tests/pass/shims/fs-with-isolation.stderr index ad75e42831..452c5b9b77 100644 --- a/tests/pass-dep/shims/fs_with_isolation.stderr +++ b/tests/pass/shims/fs-with-isolation.stderr @@ -1,13 +1,9 @@ warning: `open` was made to return an error due to isolation -warning: `fcntl` was made to return an error due to isolation - warning: `unlink` was made to return an error due to isolation warning: `symlink` was made to return an error due to isolation -warning: `readlink` was made to return an error due to isolation - warning: `$STAT` was made to return an error due to isolation warning: `rename` was made to return an error due to isolation diff --git a/tests/pass-dep/shims/fs.rs b/tests/pass/shims/fs.rs similarity index 76% rename from tests/pass-dep/shims/fs.rs rename to tests/pass/shims/fs.rs index e573d330aa..65cf6fe66b 100644 --- a/tests/pass-dep/shims/fs.rs +++ b/tests/pass/shims/fs.rs @@ -3,14 +3,15 @@ #![feature(io_error_more)] #![feature(io_error_uncategorized)] +#![feature(is_terminal)] use std::collections::HashMap; -use std::ffi::{CString, OsString}; +use std::ffi::OsString; use std::fs::{ - create_dir, read_dir, read_link, remove_dir, remove_dir_all, remove_file, rename, File, - OpenOptions, + canonicalize, create_dir, read_dir, read_link, remove_dir, remove_dir_all, remove_file, rename, + File, OpenOptions, }; -use std::io::{Error, ErrorKind, Read, Result, Seek, SeekFrom, Write}; +use std::io::{Error, ErrorKind, IsTerminal, Read, Result, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; fn main() { @@ -26,13 +27,7 @@ fn main() { test_rename(); test_directory(); test_canonicalize(); - test_dup_stdout_stderr(); test_from_raw_os_error(); - - // These all require unix, if the test is changed to no longer `ignore-windows`, move these to a unix test - test_file_open_unix_allow_two_args(); - test_file_open_unix_needs_three_args(); - test_file_open_unix_extra_third_arg(); } fn tmp() -> PathBuf { @@ -97,43 +92,12 @@ fn test_file() { file.read_to_end(&mut contents).unwrap(); assert_eq!(bytes, contents.as_slice()); + assert!(!file.is_terminal()); + // Removing file should succeed. remove_file(&path).unwrap(); } -fn test_file_open_unix_allow_two_args() { - use std::os::unix::ffi::OsStrExt; - - let path = prepare_with_content("test_file_open_unix_allow_two_args.txt", &[]); - - let mut name = path.into_os_string(); - name.push("\0"); - let name_ptr = name.as_bytes().as_ptr().cast::(); - let _fd = unsafe { libc::open(name_ptr, libc::O_RDONLY) }; -} - -fn test_file_open_unix_needs_three_args() { - use std::os::unix::ffi::OsStrExt; - - let path = prepare_with_content("test_file_open_unix_needs_three_args.txt", &[]); - - let mut name = path.into_os_string(); - name.push("\0"); - let name_ptr = name.as_bytes().as_ptr().cast::(); - let _fd = unsafe { libc::open(name_ptr, libc::O_CREAT, 0o666) }; -} - -fn test_file_open_unix_extra_third_arg() { - use std::os::unix::ffi::OsStrExt; - - let path = prepare_with_content("test_file_open_unix_extra_third_arg.txt", &[]); - - let mut name = path.into_os_string(); - name.push("\0"); - let name_ptr = name.as_bytes().as_ptr().cast::(); - let _fd = unsafe { libc::open(name_ptr, libc::O_RDONLY, 42) }; -} - fn test_file_clone() { let bytes = b"Hello, World!\n"; let path = prepare_with_content("miri_test_fs_file_clone.txt", bytes); @@ -279,46 +243,6 @@ fn test_symlink() { symlink_file.read_to_end(&mut contents).unwrap(); assert_eq!(bytes, contents.as_slice()); - #[cfg(unix)] - { - use std::os::unix::ffi::OsStrExt; - - let expected_path = path.as_os_str().as_bytes(); - - // Test that the expected string gets written to a buffer of proper - // length, and that a trailing null byte is not written. - let symlink_c_str = CString::new(symlink_path.as_os_str().as_bytes()).unwrap(); - let symlink_c_ptr = symlink_c_str.as_ptr(); - - // Make the buf one byte larger than it needs to be, - // and check that the last byte is not overwritten. - let mut large_buf = vec![0xFF; expected_path.len() + 1]; - let res = unsafe { - libc::readlink(symlink_c_ptr, large_buf.as_mut_ptr().cast(), large_buf.len()) - }; - // Check that the resovled path was properly written into the buf. - assert_eq!(&large_buf[..(large_buf.len() - 1)], expected_path); - assert_eq!(large_buf.last(), Some(&0xFF)); - assert_eq!(res, large_buf.len() as isize - 1); - - // Test that the resolved path is truncated if the provided buffer - // is too small. - let mut small_buf = [0u8; 2]; - let res = unsafe { - libc::readlink(symlink_c_ptr, small_buf.as_mut_ptr().cast(), small_buf.len()) - }; - assert_eq!(small_buf, &expected_path[..small_buf.len()]); - assert_eq!(res, small_buf.len() as isize); - - // Test that we report a proper error for a missing path. - let bad_path = CString::new("MIRI_MISSING_FILE_NAME").unwrap(); - let res = unsafe { - libc::readlink(bad_path.as_ptr(), small_buf.as_mut_ptr().cast(), small_buf.len()) - }; - assert_eq!(res, -1); - assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound); - } - // Test that metadata of a symbolic link (i.e., the file it points to) is correct. check_metadata(bytes, &symlink_path).unwrap(); // Test that the metadata of a symbolic link is correct when not following it. @@ -369,7 +293,6 @@ fn test_rename() { } 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"); @@ -379,11 +302,6 @@ fn test_canonicalize() { 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()); } fn test_directory() { @@ -440,16 +358,6 @@ fn test_directory() { remove_dir_all(&dir_path).unwrap(); } -fn test_dup_stdout_stderr() { - let bytes = b"hello dup fd\n"; - unsafe { - let new_stdout = libc::fcntl(1, libc::F_DUPFD, 0); - let new_stderr = libc::fcntl(2, libc::F_DUPFD, 0); - libc::write(new_stdout, bytes.as_ptr() as *const libc::c_void, bytes.len()); - libc::write(new_stderr, bytes.as_ptr() as *const libc::c_void, bytes.len()); - } -} - fn test_from_raw_os_error() { let code = 6; // not a code that std or Miri know let error = Error::from_raw_os_error(code); diff --git a/tests/pass/shims/io.rs b/tests/pass/shims/io.rs new file mode 100644 index 0000000000..4d43549a93 --- /dev/null +++ b/tests/pass/shims/io.rs @@ -0,0 +1,9 @@ +#![feature(is_terminal)] + +use std::io::IsTerminal; + +fn main() { + // We can't really assume that this is truly a terminal, and anyway on Windows Miri will always + // return `false` here, but we can check that the call succeeds. + std::io::stdout().is_terminal(); +}