Auto merge of #3899 - YohDeadfall:prctl-thread-name, r=RalfJung

bors · bors · commit 9a228e5dba57 · 2024-10-21T14:25:26.000Z
Android: Added support for prctl handling thread names Addresses the first part of #3618.
diff --git a/ci/ci.sh b/ci/ci.sh
@@ -154,7 +154,7 @@ case $HOST_TARGET in
     TEST_TARGET=i686-unknown-freebsd   run_tests_minimal $BASIC $UNIX time hashmap random threadname pthread fs libc-pipe
     TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls libc-pipe
     TEST_TARGET=x86_64-pc-solaris      run_tests_minimal $BASIC $UNIX time hashmap random thread sync available-parallelism tls libc-pipe
-    TEST_TARGET=aarch64-linux-android  run_tests_minimal $BASIC $UNIX time hashmap pthread --skip threadname
+    TEST_TARGET=aarch64-linux-android  run_tests_minimal $BASIC $UNIX time hashmap threadname pthread
     TEST_TARGET=wasm32-wasip2          run_tests_minimal $BASIC wasm
     TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std
     TEST_TARGET=thumbv7em-none-eabihf  run_tests_minimal no_std
diff --git a/src/shims/unix/android/foreign_items.rs b/src/shims/unix/android/foreign_items.rs
@@ -1,6 +1,7 @@
 use rustc_span::Symbol;
 use rustc_target::spec::abi::Abi;
 
+use crate::shims::unix::android::thread::prctl;
 use crate::*;
 
 pub fn is_dyn_sym(_name: &str) -> bool {
@@ -25,6 +26,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.write_scalar(errno_place.to_ref(this).to_scalar(), dest)?;
             }
 
+            // Threading
+            "prctl" => prctl(this, link_name, abi, args, dest)?,
+
             _ => return interp_ok(EmulateItemResult::NotSupported),
         }
         interp_ok(EmulateItemResult::NeedsReturn)
diff --git a/src/shims/unix/android/mod.rs b/src/shims/unix/android/mod.rs
@@ -1 +1,2 @@
 pub mod foreign_items;
+pub mod thread;
diff --git a/src/shims/unix/android/thread.rs b/src/shims/unix/android/thread.rs
@@ -0,0 +1,57 @@
+use rustc_span::Symbol;
+use rustc_target::abi::Size;
+use rustc_target::spec::abi::Abi;
+
+use crate::helpers::check_min_arg_count;
+use crate::shims::unix::thread::EvalContextExt as _;
+use crate::*;
+
+const TASK_COMM_LEN: usize = 16;
+
+pub fn prctl<'tcx>(
+    this: &mut MiriInterpCx<'tcx>,
+    link_name: Symbol,
+    abi: Abi,
+    args: &[OpTy<'tcx>],
+    dest: &MPlaceTy<'tcx>,
+) -> InterpResult<'tcx> {
+    // We do not use `check_shim` here because `prctl` is variadic. The argument
+    // count is checked bellow.
+    this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?;
+
+    // FIXME: Use constants once https://github.com/rust-lang/libc/pull/3941 backported to the 0.2 branch.
+    let pr_set_name = 15;
+    let pr_get_name = 16;
+
+    let [op] = check_min_arg_count("prctl", args)?;
+    let res = match this.read_scalar(op)?.to_i32()? {
+        op if op == pr_set_name => {
+            let [_, name] = check_min_arg_count("prctl(PR_SET_NAME, ...)", args)?;
+            let name = this.read_scalar(name)?;
+            let thread = this.pthread_self()?;
+            // The Linux kernel silently truncates long names.
+            // https://www.man7.org/linux/man-pages/man2/PR_SET_NAME.2const.html
+            let res =
+                this.pthread_setname_np(thread, name, TASK_COMM_LEN, /* truncate */ true)?;
+            assert!(res);
+            Scalar::from_u32(0)
+        }
+        op if op == pr_get_name => {
+            let [_, name] = check_min_arg_count("prctl(PR_GET_NAME, ...)", args)?;
+            let name = this.read_scalar(name)?;
+            let thread = this.pthread_self()?;
+            let len = Scalar::from_target_usize(TASK_COMM_LEN as u64, this);
+            this.check_ptr_access(
+                name.to_pointer(this)?,
+                Size::from_bytes(TASK_COMM_LEN),
+                CheckInAllocMsg::MemoryAccessTest,
+            )?;
+            let res = this.pthread_getname_np(thread, name, len, /* truncate*/ false)?;
+            assert!(res);
+            Scalar::from_u32(0)
+        }
+        op => throw_unsup_format!("Miri does not support `prctl` syscall with op={}", op),
+    };
+    this.write_scalar(res, dest)?;
+    interp_ok(())
+}
diff --git a/src/shims/unix/freebsd/foreign_items.rs b/src/shims/unix/freebsd/foreign_items.rs
@@ -29,6 +29,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     this.read_scalar(thread)?,
                     this.read_scalar(name)?,
                     max_len,
+                    /* truncate */ false,
                 )?;
             }
             "pthread_get_name_np" => {
diff --git a/src/shims/unix/linux/foreign_items.rs b/src/shims/unix/linux/foreign_items.rs
@@ -84,6 +84,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     this.read_scalar(thread)?,
                     this.read_scalar(name)?,
                     TASK_COMM_LEN,
+                    /* truncate */ false,
                 )?;
                 let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") };
                 this.write_scalar(res, dest)?;
diff --git a/src/shims/unix/macos/foreign_items.rs b/src/shims/unix/macos/foreign_items.rs
@@ -181,6 +181,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     thread,
                     this.read_scalar(name)?,
                     this.eval_libc("MAXTHREADNAMESIZE").to_target_usize(this)?.try_into().unwrap(),
+                    /* truncate */ false,
                 )? {
                     Scalar::from_u32(0)
                 } else {
diff --git a/src/shims/unix/solarish/foreign_items.rs b/src/shims/unix/solarish/foreign_items.rs
@@ -30,6 +30,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     this.read_scalar(thread)?,
                     this.read_scalar(name)?,
                     max_len,
+                    /* truncate */ false,
                 )?;
                 let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") };
                 this.write_scalar(res, dest)?;
diff --git a/src/shims/unix/thread.rs b/src/shims/unix/thread.rs
@@ -64,23 +64,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     }
 
     /// Set the name of the specified thread. If the name including the null terminator
-    /// is longer than `name_max_len`, then `false` is returned.
+    /// is longer or equals to `name_max_len`, then if `truncate` is set the truncated name
+    /// is used as the thread name, otherwise `false` is returned.
     fn pthread_setname_np(
         &mut self,
         thread: Scalar,
         name: Scalar,
         name_max_len: usize,
+        truncate: bool,
     ) -> InterpResult<'tcx, bool> {
         let this = self.eval_context_mut();
 
         let thread = thread.to_int(this.libc_ty_layout("pthread_t").size)?;
         let thread = ThreadId::try_from(thread).unwrap();
         let name = name.to_pointer(this)?;
-        let name = this.read_c_str(name)?.to_owned();
+        let mut name = this.read_c_str(name)?.to_owned();
 
         // Comparing with `>=` to account for null terminator.
         if name.len() >= name_max_len {
-            return interp_ok(false);
+            if truncate {
+                name.truncate(name_max_len.saturating_sub(1));
+            } else {
+                return interp_ok(false);
+            }
         }
 
         this.set_thread_name(thread, name);
diff --git a/test_dependencies/Cargo.lock b/test_dependencies/Cargo.lock
diff --git a/tests/fail-dep/libc/prctl-get-name-buffer-too-small.rs b/tests/fail-dep/libc/prctl-get-name-buffer-too-small.rs
@@ -0,0 +1,10 @@
+//! Ensure we report UB when the buffer is smaller than 16 bytes (even if the thread
+//! name would fit in the smaller buffer).
+//@only-target: android  # Miri supports prctl for Android only
+
+fn main() {
+    let mut buf = vec![0u8; 15];
+    unsafe {
+        libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr().cast::<libc::c_char>()); //~ ERROR: memory access failed: expected a pointer to 16 bytes of memory, but got alloc952 which is only 15 bytes from the end of the allocation
+    }
+}
diff --git a/tests/fail-dep/libc/prctl-get-name-buffer-too-small.stderr b/tests/fail-dep/libc/prctl-get-name-buffer-too-small.stderr
@@ -0,0 +1,21 @@
+error: Undefined Behavior: memory access failed: expected a pointer to 16 bytes of memory, but got ALLOC which is only 15 bytes from the end of the allocation
+  --> tests/fail-dep/libc/prctl-threadname.rs:LL:CC
+   |
+LL |         libc::prctl(libc::PR_GET_NAME, buf.as_mut_ptr().cast::<libc::c_char>());
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: expected a pointer to 16 bytes of memory, but got ALLOC which is only 15 bytes from the end of the allocation
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+help: ALLOC was allocated here:
+  --> tests/fail-dep/libc/prctl-threadname.rs:LL:CC
+   |
+LL |     let mut buf = vec![0u8; 15];
+   |                   ^^^^^^^^^^^^^
+   = note: BACKTRACE (of the first span):
+   = note: inside `main` at tests/fail-dep/libc/prctl-threadname.rs:LL:CC
+   = note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+
diff --git a/tests/pass-dep/libc/prctl-threadname.rs b/tests/pass-dep/libc/prctl-threadname.rs
@@ -0,0 +1,74 @@
+//@only-target: android  # Miri supports prctl for Android only
+use std::ffi::{CStr, CString};
+use std::thread;
+
+// The Linux kernel all names 16 bytes long including the null terminator.
+const MAX_THREAD_NAME_LEN: usize = 16;
+
+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>();
+
+    fn set_thread_name(name: &CStr) -> i32 {
+        unsafe { libc::prctl(libc::PR_SET_NAME, name.as_ptr().cast::<libc::c_char>()) }
+    }
+
+    fn get_thread_name(name: &mut [u8]) -> i32 {
+        assert!(name.len() >= MAX_THREAD_NAME_LEN);
+        unsafe { libc::prctl(libc::PR_GET_NAME, name.as_mut_ptr().cast::<libc::c_char>()) }
+    }
+
+    // Set name via Rust API, get it via prctl.
+    let long_name2 = long_name.clone();
+    thread::Builder::new()
+        .name(long_name.clone())
+        .spawn(move || {
+            let mut buf = vec![0u8; MAX_THREAD_NAME_LEN];
+            assert_eq!(get_thread_name(&mut buf), 0);
+            let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
+            let truncated_name = &long_name2[..long_name2.len().min(MAX_THREAD_NAME_LEN - 1)];
+            assert_eq!(cstr.to_bytes(), truncated_name.as_bytes());
+        })
+        .unwrap()
+        .join()
+        .unwrap();
+
+    // Set name via prctl and get it again (short name).
+    thread::Builder::new()
+        .spawn(move || {
+            // Set short thread name.
+            let cstr = CString::new(short_name.clone()).unwrap();
+            assert!(cstr.to_bytes_with_nul().len() <= MAX_THREAD_NAME_LEN); // this should fit
+            assert_eq!(set_thread_name(&cstr), 0);
+
+            let mut buf = vec![0u8; MAX_THREAD_NAME_LEN];
+            assert_eq!(get_thread_name(&mut buf), 0);
+            let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
+            assert_eq!(cstr.to_bytes(), short_name.as_bytes());
+        })
+        .unwrap()
+        .join()
+        .unwrap();
+
+    // Set name via prctl and get it again (long name).
+    thread::Builder::new()
+        .spawn(move || {
+            // Set full thread name.
+            let cstr = CString::new(long_name.clone()).unwrap();
+            assert!(cstr.to_bytes_with_nul().len() > MAX_THREAD_NAME_LEN);
+            // Names are truncated by the Linux kernel.
+            assert_eq!(set_thread_name(&cstr), 0);
+
+            let mut buf = vec![0u8; MAX_THREAD_NAME_LEN];
+            assert_eq!(get_thread_name(&mut buf), 0);
+            let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
+            assert_eq!(cstr.to_bytes(), &long_name.as_bytes()[..(MAX_THREAD_NAME_LEN - 1)]);
+        })
+        .unwrap()
+        .join()
+        .unwrap();
+}
diff --git a/tests/pass-dep/libc/pthread-threadname.rs b/tests/pass-dep/libc/pthread-threadname.rs
@@ -1,4 +1,5 @@
 //@ignore-target: windows # No pthreads on Windows
+//@ignore-target: android # No pthread_{get,set}_name on Android
 use std::ffi::{CStr, CString};
 use std::thread;
 

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`use rustc_span::Symbol;`
`2`	`2`	`use rustc_target::spec::abi::Abi;`
`3`	`3`
	`4`	`+use crate::shims::unix::android::thread::prctl;`
`4`	`5`	`use crate::*;`
`5`	`6`
`6`	`7`	`pub fn is_dyn_sym(_name: &str) -> bool {`
`@@ -25,6 +26,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {`
`25`	`26`	`this.write_scalar(errno_place.to_ref(this).to_scalar(), dest)?;`
`26`	`27`	`}`
`27`	`28`
	`29`	`+ // Threading`
	`30`	`+ "prctl" => prctl(this, link_name, abi, args, dest)?,`
	`31`	`+`
`28`	`32`	`_ => return interp_ok(EmulateItemResult::NotSupported),`
`29`	`33`	`}`
`30`	`34`	`interp_ok(EmulateItemResult::NeedsReturn)`
Original file line number	Diff line number	Diff line change
`@@ -1 +1,2 @@`
`1`	`1`	`pub mod foreign_items;`
	`2`	`+pub mod thread;`
Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {`
`29`	`29`	`this.read_scalar(thread)?,`
`30`	`30`	`this.read_scalar(name)?,`
`31`	`31`	`max_len,`
	`32`	`+ /* truncate */ false,`
`32`	`33`	`)?;`
`33`	`34`	`}`
`34`	`35`	`"pthread_get_name_np" => {`