Auto merge of #2620 - RalfJung:getname, r=RalfJung

bors · bors · commit 6ce5226b4b2b · 2022-10-26T08:58:53.000Z
add pthread_getname_np

A new libstd test needs this, and there doesn't seem to be a good reason not to have this.
diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs
@@ -870,6 +870,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.machine.threads.active_thread_stack_mut()
     }
 
+    /// Set the name of the current thread. The buffer must not include the null terminator.
     #[inline]
     fn set_thread_name(&mut self, thread: ThreadId, new_thread_name: Vec<u8>) {
         let this = self.eval_context_mut();
diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs
@@ -1,6 +1,7 @@
 pub mod convert;
 
 use std::cmp;
+use std::iter;
 use std::mem;
 use std::num::NonZeroUsize;
 use std::time::Duration;
@@ -735,6 +736,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         })
     }
 
+    /// Read a sequence of bytes until the first null terminator.
     fn read_c_str<'a>(&'a self, ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx, &'a [u8]>
     where
         'tcx: 'a,
@@ -761,6 +763,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         this.read_bytes_ptr_strip_provenance(ptr, len)
     }
 
+    /// Helper function to write a sequence of bytes with an added null-terminator, which is what
+    /// the Unix APIs usually handle. This function returns `Ok((false, length))` without trying
+    /// to write if `size` is not large enough to fit the contents of `c_str` plus a null
+    /// terminator. It returns `Ok((true, length))` if the writing process was successful. The
+    /// string length returned does include the null terminator.
+    fn write_c_str(
+        &mut self,
+        c_str: &[u8],
+        ptr: Pointer<Option<Provenance>>,
+        size: u64,
+    ) -> InterpResult<'tcx, (bool, u64)> {
+        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
+        // terminator to memory using the `ptr` pointer would cause an out-of-bounds access.
+        let string_length = u64::try_from(c_str.len()).unwrap();
+        let string_length = string_length.checked_add(1).unwrap();
+        if size < string_length {
+            return Ok((false, string_length));
+        }
+        self.eval_context_mut()
+            .write_bytes_ptr(ptr, c_str.iter().copied().chain(iter::once(0u8)))?;
+        Ok((true, string_length))
+    }
+
+    /// Read a sequence of u16 until the first null terminator.
     fn read_wide_str(&self, mut ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx, Vec<u16>> {
         let this = self.eval_context_ref();
         let size2 = Size::from_bytes(2);
@@ -783,6 +809,39 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Ok(wchars)
     }
 
+    /// Helper function to write a sequence of u16 with an added 0x0000-terminator, which is what
+    /// the Windows APIs usually handle. This function returns `Ok((false, length))` without trying
+    /// to write if `size` is not large enough to fit the contents of `os_string` plus a null
+    /// terminator. It returns `Ok((true, length))` if the writing process was successful. The
+    /// string length returned does include the null terminator. Length is measured in units of
+    /// `u16.`
+    fn write_wide_str(
+        &mut self,
+        wide_str: &[u16],
+        ptr: Pointer<Option<Provenance>>,
+        size: u64,
+    ) -> InterpResult<'tcx, (bool, u64)> {
+        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required
+        // 0x0000 terminator to memory would cause an out-of-bounds access.
+        let string_length = u64::try_from(wide_str.len()).unwrap();
+        let string_length = string_length.checked_add(1).unwrap();
+        if size < string_length {
+            return Ok((false, string_length));
+        }
+
+        // Store the UTF-16 string.
+        let size2 = Size::from_bytes(2);
+        let this = self.eval_context_mut();
+        let mut alloc = this
+            .get_ptr_alloc_mut(ptr, size2 * string_length, Align::from_bytes(2).unwrap())?
+            .unwrap(); // not a ZST, so we will get a result
+        for (offset, wchar) in wide_str.iter().copied().chain(iter::once(0x0000)).enumerate() {
+            let offset = u64::try_from(offset).unwrap();
+            alloc.write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar))?;
+        }
+        Ok((true, string_length))
+    }
+
     /// Check that the ABI is what we expect.
     fn check_abi<'a>(&self, abi: Abi, exp_abi: Abi) -> InterpResult<'a, ()> {
         if self.eval_context_ref().machine.enforce_abi && abi != exp_abi {
diff --git a/src/tools/miri/src/shims/os_str.rs b/src/tools/miri/src/shims/os_str.rs
@@ -1,6 +1,5 @@
 use std::borrow::Cow;
 use std::ffi::{OsStr, OsString};
-use std::iter;
 use std::path::{Path, PathBuf};
 
 #[cfg(unix)]
@@ -9,7 +8,6 @@ use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::os::windows::ffi::{OsStrExt, OsStringExt};
 
 use rustc_middle::ty::layout::LayoutOf;
-use rustc_target::abi::{Align, Size};
 
 use crate::*;
 
@@ -100,16 +98,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         size: u64,
     ) -> InterpResult<'tcx, (bool, u64)> {
         let bytes = os_str_to_bytes(os_str)?;
-        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null
-        // terminator to memory using the `ptr` pointer would cause an out-of-bounds access.
-        let string_length = u64::try_from(bytes.len()).unwrap();
-        let string_length = string_length.checked_add(1).unwrap();
-        if size < string_length {
-            return Ok((false, string_length));
-        }
-        self.eval_context_mut()
-            .write_bytes_ptr(ptr, bytes.iter().copied().chain(iter::once(0u8)))?;
-        Ok((true, string_length))
+        self.eval_context_mut().write_c_str(bytes, ptr, size)
     }
 
     /// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what
@@ -140,25 +129,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         }
 
         let u16_vec = os_str_to_u16vec(os_str)?;
-        // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required
-        // 0x0000 terminator to memory would cause an out-of-bounds access.
-        let string_length = u64::try_from(u16_vec.len()).unwrap();
-        let string_length = string_length.checked_add(1).unwrap();
-        if size < string_length {
-            return Ok((false, string_length));
-        }
-
-        // Store the UTF-16 string.
-        let size2 = Size::from_bytes(2);
-        let this = self.eval_context_mut();
-        let mut alloc = this
-            .get_ptr_alloc_mut(ptr, size2 * string_length, Align::from_bytes(2).unwrap())?
-            .unwrap(); // not a ZST, so we will get a result
-        for (offset, wchar) in u16_vec.into_iter().chain(iter::once(0x0000)).enumerate() {
-            let offset = u64::try_from(offset).unwrap();
-            alloc.write_scalar(alloc_range(size2 * offset, size2), Scalar::from_u16(wchar))?;
-        }
-        Ok((true, string_length))
+        self.eval_context_mut().write_wide_str(&u16_vec, ptr, size)
     }
 
     /// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of bytes.
diff --git a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs
@@ -26,8 +26,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "pthread_set_name_np" => {
                 let [thread, name] =
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                let res =
-                    this.pthread_setname_np(this.read_scalar(thread)?, this.read_scalar(name)?)?;
+                let max_len = usize::MAX; // freebsd does not seem to have a limit.
+                let res = this.pthread_setname_np(
+                    this.read_scalar(thread)?,
+                    this.read_scalar(name)?,
+                    max_len,
+                )?;
                 this.write_scalar(res, dest)?;
             }
 
diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs
@@ -68,8 +68,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "pthread_setname_np" => {
                 let [thread, name] =
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                let res =
-                    this.pthread_setname_np(this.read_scalar(thread)?, this.read_scalar(name)?)?;
+                let max_len = 16;
+                let res = this.pthread_setname_np(
+                    this.read_scalar(thread)?,
+                    this.read_scalar(name)?,
+                    max_len,
+                )?;
+                this.write_scalar(res, dest)?;
+            }
+            "pthread_getname_np" => {
+                let [thread, name, len] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let res = this.pthread_getname_np(
+                    this.read_scalar(thread)?,
+                    this.read_scalar(name)?,
+                    this.read_scalar(len)?,
+                )?;
                 this.write_scalar(res, dest)?;
             }
 
diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs
@@ -176,7 +176,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "pthread_setname_np" => {
                 let [name] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let thread = this.pthread_self()?;
-                this.pthread_setname_np(thread, this.read_scalar(name)?)?;
+                let max_len = this.eval_libc("MAXTHREADNAMESIZE")?.to_machine_usize(this)?;
+                this.pthread_setname_np(
+                    thread,
+                    this.read_scalar(name)?,
+                    max_len.try_into().unwrap(),
+                )?;
+            }
+            "pthread_getname_np" => {
+                let [thread, name, len] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let res = this.pthread_getname_np(
+                    this.read_scalar(thread)?,
+                    this.read_scalar(name)?,
+                    this.read_scalar(len)?,
+                )?;
+                this.write_scalar(res, dest)?;
             }
 
             // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
diff --git a/src/tools/miri/src/shims/unix/thread.rs b/src/tools/miri/src/shims/unix/thread.rs
@@ -67,22 +67,49 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Ok(Scalar::from_machine_usize(thread_id.into(), this))
     }
 
+    /// Set the name of the current thread. `max_name_len` is the maximal length of the name
+    /// including the null terminator.
     fn pthread_setname_np(
         &mut self,
         thread: Scalar<Provenance>,
         name: Scalar<Provenance>,
+        max_name_len: usize,
     ) -> InterpResult<'tcx, Scalar<Provenance>> {
         let this = self.eval_context_mut();
 
         let thread = ThreadId::try_from(thread.to_machine_usize(this)?).unwrap();
         let name = name.to_pointer(this)?;
 
         let name = this.read_c_str(name)?.to_owned();
+
+        // Comparing with `>=` to account for null terminator.
+        if name.len() >= max_name_len {
+            return this.eval_libc("ERANGE");
+        }
+
         this.set_thread_name(thread, name);
 
         Ok(Scalar::from_u32(0))
     }
 
+    fn pthread_getname_np(
+        &mut self,
+        thread: Scalar<Provenance>,
+        name_out: Scalar<Provenance>,
+        len: Scalar<Provenance>,
+    ) -> InterpResult<'tcx, Scalar<Provenance>> {
+        let this = self.eval_context_mut();
+
+        let thread = ThreadId::try_from(thread.to_machine_usize(this)?).unwrap();
+        let name_out = name_out.to_pointer(this)?;
+        let len = len.to_machine_usize(this)?;
+
+        let name = this.get_thread_name(thread).to_owned();
+        let (success, _written) = this.write_c_str(&name, name_out, len)?;
+
+        if success { Ok(Scalar::from_u32(0)) } else { this.eval_libc("ERANGE") }
+    }
+
     fn sched_yield(&mut self) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
diff --git a/src/tools/miri/tests/pass-dep/shims/pthreads.rs b/src/tools/miri/tests/pass-dep/shims/pthreads.rs
@@ -1,10 +1,14 @@
 //@ignore-target-windows: No libc on Windows
+#![feature(cstr_from_bytes_until_nul)]
+use std::ffi::CStr;
+use std::thread;
 
 fn main() {
     test_mutex_libc_init_recursive();
     test_mutex_libc_init_normal();
     test_mutex_libc_init_errorcheck();
     test_rwlock_libc_static_initializer();
+    test_named_thread_truncation();
 
     #[cfg(any(target_os = "linux"))]
     test_mutex_libc_static_initializer_recursive();
@@ -125,3 +129,24 @@ fn test_rwlock_libc_static_initializer() {
         assert_eq!(libc::pthread_rwlock_destroy(rw.get()), 0);
     }
 }
+
+fn test_named_thread_truncation() {
+    let long_name = std::iter::once("test_named_thread_truncation")
+        .chain(std::iter::repeat(" yada").take(100))
+        .collect::<String>();
+
+    let result = thread::Builder::new().name(long_name.clone()).spawn(move || {
+        // Rust remembers the full thread name itself.
+        assert_eq!(thread::current().name(), Some(long_name.as_str()));
+
+        // But the system is limited -- make sure we successfully set a truncation.
+        let mut buf = vec![0u8; long_name.len() + 1];
+        unsafe {
+            libc::pthread_getname_np(libc::pthread_self(), buf.as_mut_ptr().cast(), buf.len());
+        }
+        let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
+        assert!(cstr.to_bytes().len() >= 15); // POSIX seems to promise at least 15 chars
+        assert!(long_name.as_bytes().starts_with(cstr.to_bytes()));
+    });
+    result.unwrap().join().unwrap();
+}

Original file line number	Diff line number	Diff line change
`@@ -870,6 +870,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {`
`870`	`870`	`this.machine.threads.active_thread_stack_mut()`
`871`	`871`	`}`
`872`	`872`
	`873`	`+ /// Set the name of the current thread. The buffer must not include the null terminator.`
`873`	`874`	`#[inline]`
`874`	`875`	`fn set_thread_name(&mut self, thread: ThreadId, new_thread_name: Vec<u8>) {`
`875`	`876`	`let this = self.eval_context_mut();`