Fixed pthread_getname_np impl for glibc

Author: YohDeadfall

src/tools/miri/src/shims/unix/linux/foreign_items.rs

@@ -9,6 +9,11 @@ use crate::machine::{SIGRTMAX, SIGRTMIN};
 use crate::shims::unix::*;
 use crate::*;
 
+// The documentation of glibc complains that the kernel never exposes
+// TASK_COMM_LEN through the headers, so it's assumed to always be 16 bytes
+// long including a null terminator.
+const TASK_COMM_LEN: usize = 16;
+
 pub fn is_dyn_sym(name: &str) -> bool {
     matches!(name, "statx")
 }
@@ -74,22 +79,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             "pthread_setname_np" => {
                 let [thread, name] =
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
-                let max_len = 16;
                 let res = this.pthread_setname_np(
                     this.read_scalar(thread)?,
                     this.read_scalar(name)?,
-                    max_len,
+                    TASK_COMM_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)?,
-                )?;
+                // The function's behavior isn't portable between platforms.
+                // In case of glibc, the length of the output buffer must
+                // be not shorter than TASK_COMM_LEN.
+                let len = this.read_scalar(len)?;
+                let res = if len.to_target_usize(this)? < TASK_COMM_LEN as u64 {
+                    this.eval_libc("ERANGE")
+                } else {
+                    this.pthread_getname_np(
+                        this.read_scalar(thread)?,
+                        this.read_scalar(name)?,
+                        len,
+                    )?
+                };
                 this.write_scalar(res, dest)?;
             }
             "gettid" => {

src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs

@@ -5,6 +5,9 @@ use std::ffi::CString;
 use std::thread;
 
 fn main() {
+    // The short name should be shorter than 16 bytes which POSIX promises
+    // for thread names. The length includes a null terminator.
+    let short_name = "test_named".to_owned();
     let long_name = std::iter::once("test_named_thread_truncation")
         .chain(std::iter::repeat(" yada").take(100))
         .collect::<String>();
@@ -48,23 +51,64 @@ fn main() {
         }
     }
 
-    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()));
+    thread::Builder::new()
+        .name(short_name.clone())
+        .spawn(move || {
+            // Rust remembers the full thread name itself.
+            assert_eq!(thread::current().name(), Some(short_name.as_str()));
 
-        // But the system is limited -- make sure we successfully set a truncation.
-        let mut buf = vec![0u8; long_name.len() + 1];
-        assert_eq!(get_thread_name(&mut buf), 0);
-        let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
-        assert!(cstr.to_bytes().len() >= 15, "name is too short: len={}", cstr.to_bytes().len()); // POSIX seems to promise at least 15 chars
-        assert!(long_name.as_bytes().starts_with(cstr.to_bytes()));
+            // Note that glibc requires 15 bytes long buffer exculding a null terminator.
+            // Otherwise, `pthread_getname_np` returns an error.
+            let mut buf = vec![0u8; short_name.len().max(15) + 1];
+            assert_eq!(get_thread_name(&mut buf), 0);
 
-        // Also test directly calling pthread_setname to check its return value.
-        assert_eq!(set_thread_name(&cstr), 0);
-        // But with a too long name it should fail (except on FreeBSD where the
-        // function has no return, hence cannot indicate failure).
-        #[cfg(not(target_os = "freebsd"))]
-        assert_ne!(set_thread_name(&CString::new(long_name).unwrap()), 0);
-    });
-    result.unwrap().join().unwrap();
+            let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
+            // POSIX seems to promise at least 15 chars excluding a null terminator.
+            assert_eq!(short_name.as_bytes(), cstr.to_bytes());
+
+            // Also test directly calling pthread_setname to check its return value.
+            assert_eq!(set_thread_name(&cstr), 0);
+
+            // For glibc used by linux-gnu there should be a failue,
+            // if a shorter than 16 bytes buffer is provided, even if that would be
+            // large enough for the thread name.
+            #[cfg(target_os = "linux")]
+            assert_eq!(get_thread_name(&mut buf[..15]), libc::ERANGE);
+        })
+        .unwrap()
+        .join()
+        .unwrap();
+
+    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.
+            // Note that there's no specific to glibc buffer requirement, since the value
+            // `long_name` is longer than 16 bytes including a null terminator.
+            let mut buf = vec![0u8; long_name.len() + 1];
+            assert_eq!(get_thread_name(&mut buf), 0);
+
+            let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
+            // POSIX seems to promise at least 15 chars excluding a null terminator.
+            assert!(
+                cstr.to_bytes().len() >= 15,
+                "name is too short: len={}",
+                cstr.to_bytes().len()
+            );
+            assert!(long_name.as_bytes().starts_with(cstr.to_bytes()));
+
+            // Also test directly calling pthread_setname to check its return value.
+            assert_eq!(set_thread_name(&cstr), 0);
+
+            // But with a too long name it should fail (except on FreeBSD where the
+            // function has no return, hence cannot indicate failure).
+            #[cfg(not(target_os = "freebsd"))]
+            assert_ne!(set_thread_name(&CString::new(long_name).unwrap()), 0);
+        })
+        .unwrap()
+        .join()
+        .unwrap();
 }