Auto merge of rust-lang#2349 - saethlin:isatty, r=RalfJung

Improve isatty support

Per rust-lang/miri#2292 (comment), this is an attempt at

> do something more clever with Miri's `isatty` shim

Since Unix -> Unix is very simple, I'm starting with a patch that just does that. Happy to augment/rewrite this based on feedback.

The linked file in libtest specifically only supports stdout. If we're doing this to support terminal applications, I think it would be strange to support one but not all 3 of the standard streams.

The `atty` crate contains a bunch of extra logic that libtest does not contain, in order to support MSYS terminals: softprops/atty@db8d55f so I think if we're going to do Windows support, we should probably access all that logic somehow. I think it's pretty clear that the implementation is not going to change, so I think if we want to, pasting the contents of the `atty` crate into Miri is on the table, instead of taking a dependency.

Author: bors

src/shims/unix/foreign_items.rs

@@ -440,12 +440,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             // Miscellaneous
             "isatty" => {
                 let [fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                this.read_scalar(fd)?.to_i32()?;
-                // "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"
-                // FIXME: we just say nothing is a terminal.
-                let enotty = this.eval_libc("ENOTTY")?;
-                this.set_last_error(enotty)?;
-                this.write_null(dest)?;
+                let result = this.isatty(fd)?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
             }
             "pthread_atfork" => {
                 let [prepare, parent, child] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;

src/shims/unix/fs.rs

@@ -50,6 +50,9 @@ trait FileDescriptor: std::fmt::Debug {
     ) -> InterpResult<'tcx, io::Result<i32>>;
 
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>>;
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32>;
 }
 
 impl FileDescriptor for FileHandle {
@@ -114,6 +117,12 @@ impl FileDescriptor for FileHandle {
         let duplicated = self.file.try_clone()?;
         Ok(Box::new(FileHandle { file: duplicated, writable: self.writable }))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        use std::os::unix::io::AsRawFd;
+        Some(self.file.as_raw_fd())
+    }
 }
 
 impl FileDescriptor for io::Stdin {
@@ -159,6 +168,11 @@ impl FileDescriptor for io::Stdin {
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(io::stdin()))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        Some(libc::STDIN_FILENO)
+    }
 }
 
 impl FileDescriptor for io::Stdout {
@@ -209,6 +223,11 @@ impl FileDescriptor for io::Stdout {
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(io::stdout()))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        Some(libc::STDOUT_FILENO)
+    }
 }
 
 impl FileDescriptor for io::Stderr {
@@ -252,6 +271,11 @@ impl FileDescriptor for io::Stderr {
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(io::stderr()))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        Some(libc::STDERR_FILENO)
+    }
 }
 
 #[derive(Debug)]
@@ -297,6 +321,11 @@ impl FileDescriptor for DummyOutput {
     fn dup<'tcx>(&mut self) -> io::Result<Box<dyn FileDescriptor>> {
         Ok(Box::new(DummyOutput))
     }
+
+    #[cfg(unix)]
+    fn as_unix_host_fd(&self) -> Option<i32> {
+        None
+    }
 }
 
 #[derive(Debug)]
@@ -1660,6 +1689,38 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
         }
     }
+
+    #[cfg_attr(not(unix), allow(unused))]
+    fn isatty(&mut self, miri_fd: &OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
+        let this = self.eval_context_mut();
+        #[cfg(unix)]
+        {
+            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);
+            }
+        }
+        // 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.
+        // FIXME: It should be possible to emulate this at least on Windows by using
+        // GetConsoleMode.
+        let enotty = this.eval_libc("ENOTTY")?;
+        this.set_last_error(enotty)?;
+        Ok(0)
+    }
 }
 
 /// Extracts the number of seconds and nanoseconds elapsed between `time` and the unix epoch when

tests/pass/libc.rs

@@ -311,6 +311,17 @@ fn test_posix_gettimeofday() {
     assert_eq!(is_error, -1);
 }
 
+fn test_isatty() {
+    // Testing whether our isatty shim returns the right value would require controlling whether
+    // these streams are actually TTYs, which is hard.
+    // For now, we just check that these calls are supported at all.
+    unsafe {
+        libc::isatty(libc::STDIN_FILENO);
+        libc::isatty(libc::STDOUT_FILENO);
+        libc::isatty(libc::STDERR_FILENO);
+    }
+}
+
 fn main() {
     #[cfg(any(target_os = "linux", target_os = "freebsd"))]
     test_posix_fadvise();
@@ -335,4 +346,6 @@ fn main() {
 
     #[cfg(any(target_os = "linux"))]
     test_clocks();
+
+    test_isatty();
 }