From 9026be9976f795154f23c46bb49d0bb72bf03c92 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 6 Oct 2023 11:06:16 +0200
Subject: [PATCH] add a direct dlsym test

---
 .../miri/src/shims/unix/foreign_items.rs      |  8 +++--
 src/tools/miri/src/shims/unix/fs.rs           | 35 ++++++++++---------
 .../miri/tests/pass-dep/shims/libc-misc.rs    | 14 ++++++++
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs
index f18fa1cbbb7ab..c013d27502927 100644
--- a/src/tools/miri/src/shims/unix/foreign_items.rs
+++ b/src/tools/miri/src/shims/unix/foreign_items.rs
@@ -22,8 +22,10 @@ use shims::unix::macos::foreign_items as macos;
 
 fn is_dyn_sym(name: &str, target_os: &str) -> bool {
     match name {
-        // `signal` is set up as a weak symbol in `init_extern_statics` so we might as well allow it
-        // in `dlsym` as well.
+        // Used for tests.
+        "isatty" => true,
+        // `signal` is set up as a weak symbol in `init_extern_statics` (on Android) so we might as
+        // well allow it in `dlsym`.
         "signal" => true,
         // Give specific OSes a chance to allow their symbols.
         _ =>
@@ -588,7 +590,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "getuid"
             if this.frame_in_std() => {
                 let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                // FOr now, just pretend we always have this fixed UID.
+                // For now, just pretend we always have this fixed UID.
                 this.write_int(super::UID, dest)?;
             }
 
diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs
index 8963dfb97a6e5..1014a61b75ed2 100644
--- a/src/tools/miri/src/shims/unix/fs.rs
+++ b/src/tools/miri/src/shims/unix/fs.rs
@@ -63,7 +63,7 @@ pub trait FileDescriptor: std::fmt::Debug + Any {
 
     fn dup(&mut self) -> io::Result<Box<dyn FileDescriptor>>;
 
-    fn is_tty(&self) -> bool {
+    fn is_tty(&self, _communicate_allowed: bool) -> bool {
         false
     }
 
@@ -156,8 +156,8 @@ impl FileDescriptor for FileHandle {
         Some(self.file.as_raw_fd())
     }
 
-    fn is_tty(&self) -> bool {
-        self.file.is_terminal()
+    fn is_tty(&self, communicate_allowed: bool) -> bool {
+        communicate_allowed && self.file.is_terminal()
     }
 }
 
@@ -188,8 +188,8 @@ impl FileDescriptor for io::Stdin {
         Some(libc::STDIN_FILENO)
     }
 
-    fn is_tty(&self) -> bool {
-        self.is_terminal()
+    fn is_tty(&self, communicate_allowed: bool) -> bool {
+        communicate_allowed && self.is_terminal()
     }
 }
 
@@ -225,8 +225,8 @@ impl FileDescriptor for io::Stdout {
         Some(libc::STDOUT_FILENO)
     }
 
-    fn is_tty(&self) -> bool {
-        self.is_terminal()
+    fn is_tty(&self, communicate_allowed: bool) -> bool {
+        communicate_allowed && self.is_terminal()
     }
 }
 
@@ -255,8 +255,8 @@ impl FileDescriptor for io::Stderr {
         Some(libc::STDERR_FILENO)
     }
 
-    fn is_tty(&self) -> bool {
-        self.is_terminal()
+    fn is_tty(&self, communicate_allowed: bool) -> bool {
+        communicate_allowed && self.is_terminal()
     }
 }
 
@@ -1721,15 +1721,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let this = self.eval_context_mut();
         // "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 fd = this.read_scalar(miri_fd)?.to_i32()?;
-            if this.machine.file_handler.handles.get(&fd).map(|fd| fd.is_tty()) == Some(true) {
+        let fd = this.read_scalar(miri_fd)?.to_i32()?;
+        let error = if let Some(fd) = this.machine.file_handler.handles.get(&fd) {
+            if fd.is_tty(this.machine.communicate()) {
                 return Ok(Scalar::from_i32(1));
+            } else {
+                this.eval_libc("ENOTTY")
             }
-        }
-        // Fallback when the FD was not found or isolation is enabled.
-        let enotty = this.eval_libc("ENOTTY");
-        this.set_last_error(enotty)?;
+        } else {
+            // FD does not exist
+            this.eval_libc("EBADF")
+        };
+        this.set_last_error(error)?;
         Ok(Scalar::from_i32(0))
     }
 
diff --git a/src/tools/miri/tests/pass-dep/shims/libc-misc.rs b/src/tools/miri/tests/pass-dep/shims/libc-misc.rs
index ebfeb863abfd9..82c49cfb17c5a 100644
--- a/src/tools/miri/tests/pass-dep/shims/libc-misc.rs
+++ b/src/tools/miri/tests/pass-dep/shims/libc-misc.rs
@@ -3,6 +3,7 @@
 #![feature(io_error_more)]
 
 use std::fs::{remove_file, File};
+use std::mem::transmute;
 use std::os::unix::io::AsRawFd;
 use std::path::PathBuf;
 
@@ -375,6 +376,18 @@ fn test_sigrt() {
     assert!(max - min >= 8)
 }
 
+fn test_dlsym() {
+    let addr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, b"notasymbol\0".as_ptr().cast()) };
+    assert!(addr as usize == 0);
+
+    let addr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, b"isatty\0".as_ptr().cast()) };
+    assert!(addr as usize != 0);
+    let isatty: extern "C" fn(i32) -> i32 = unsafe { transmute(addr) };
+    assert_eq!(isatty(999), 0);
+    let errno = std::io::Error::last_os_error().raw_os_error().unwrap();
+    assert_eq!(errno, libc::EBADF);
+}
+
 fn main() {
     test_posix_gettimeofday();
     test_posix_mkstemp();
@@ -387,6 +400,7 @@ fn main() {
 
     test_isatty();
     test_clocks();
+    test_dlsym();
 
     test_memcpy();
     test_strcpy();