Skip to content

Commit 49b53d4

Browse files
committed
Auto merge of #3957 - YohDeadfall:macos-thread-name, r=RalfJung
Fixed get/set thread name implementations for macOS and FreeBSD So, the story of fixing `pthread_getname_np` and `pthread_setname_np` continues, but this time I fixed the macOS implementation. ### [`pthread_getname_np`](https://github.com/apple-oss-distributions/libpthread/blob/c032e0b076700a0a47db75528a282b8d3a06531a/src/pthread.c#L1160-L1175) The function never fails except for an invalid thread. Miri never verifies thread identifiers and uses them as indices when accessing a vector of threads. Therefore, the only possible result is `0` and a possibly trimmed output. ```c int pthread_getname_np(pthread_t thread, char *threadname, size_t len) { if (thread == pthread_self()) { strlcpy(threadname, thread->pthread_name, len); return 0; } if (!_pthread_validate_thread_and_list_lock(thread)) { return ESRCH; } strlcpy(threadname, thread->pthread_name, len); _pthread_lock_unlock(&_pthread_list_lock); return 0; } ``` #### [`strcpy`](https://www.man7.org/linux/man-pages/man7/strlcpy.7.html) ``` strlcpy(3bsd) strlcat(3bsd) Copy and catenate the input string into a destination string. If the destination buffer, limited by its size, isn't large enough to hold the copy, the resulting string is truncated (but it is guaranteed to be null-terminated). They return the length of the total string they tried to create. ``` ### [`pthread_setname_np`](https://github.com/apple-oss-distributions/libpthread/blob/c032e0b076700a0a47db75528a282b8d3a06531a/src/pthread.c#L1178-L1200) ```c pthread_setname_np(const char *name) { int res; pthread_t self = pthread_self(); size_t len = 0; if (name != NULL) { len = strlen(name); } _pthread_validate_signature(self); res = __proc_info(5, getpid(), 2, (uint64_t)0, (void*)name, (int)len); if (res == 0) { if (len > 0) { strlcpy(self->pthread_name, name, MAXTHREADNAMESIZE); } else { bzero(self->pthread_name, MAXTHREADNAMESIZE); } } return res; } ``` Where `5` is [`PROC_INFO_CALL_SETCONTROL`](https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/sys/proc_info_private.h#L274), and `2` is [`PROC_INFO_CALL_SETCONTROL`](https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/sys/proc_info.h#L821). And `__proc_info` is a syscall handled by the XNU kernel by [`proc_info_internal`](https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/kern/proc_info.c#L300-L314): ```c int proc_info_internal(int callnum, int pid, uint32_t flags, uint64_t ext_id, int flavor, uint64_t arg, user_addr_t buffer, uint32_t buffersize, int32_t * retval) { switch (callnum) { // ... case PROC_INFO_CALL_SETCONTROL: return proc_setcontrol(pid, flavor, arg, buffer, buffersize, retval); ``` And the actual logic from [`proc_setcontrol`](https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/kern/proc_info.c#L3218-L3227): ```c case PROC_SELFSET_THREADNAME: { /* * This is a bit ugly, as it copies the name into the kernel, and then * invokes bsd_setthreadname again to copy it into the uthread name * buffer. Hopefully this isn't such a hot codepath that an additional * MAXTHREADNAMESIZE copy is a big issue. */ if (buffersize > (MAXTHREADNAMESIZE - 1)) { return ENAMETOOLONG; } ``` Unrelated to the current pull request, but perhaps, there's a very ugly thing in the kernel/libc because the last thing happening in `PROC_SELFSET_THREADNAME` is `bsd_setthreadname` which sets the name in the user space. But we just saw that `pthread_setname_np` sets the name in the user space too. Guess, I need to open a ticket in one of Apple's repositories at least to clarify that :D
2 parents f47b990 + 027860e commit 49b53d4

File tree

7 files changed

+191
-61
lines changed

7 files changed

+191
-61
lines changed

src/shims/unix/freebsd/foreign_items.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
3434
"pthread_get_name_np" => {
3535
let [thread, name, len] =
3636
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
37-
// FreeBSD's pthread_get_name_np does not return anything.
37+
// FreeBSD's pthread_get_name_np does not return anything
38+
// and uses strlcpy, which truncates the resulting value,
39+
// but always adds a null terminator (except for zero-sized buffers).
40+
// https://github.com/freebsd/freebsd-src/blob/c2d93a803acef634bd0eede6673aeea59e90c277/lib/libthr/thread/thr_info.c#L119-L144
3841
this.pthread_getname_np(
3942
this.read_scalar(thread)?,
4043
this.read_scalar(name)?,
4144
this.read_scalar(len)?,
45+
/* truncate */ true,
4246
)?;
4347
}
4448

src/shims/unix/linux/foreign_items.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
8484
this.read_scalar(name)?,
8585
TASK_COMM_LEN,
8686
)?;
87+
let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") };
8788
this.write_scalar(res, dest)?;
8889
}
8990
"pthread_getname_np" => {
@@ -93,14 +94,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
9394
// In case of glibc, the length of the output buffer must
9495
// be not shorter than TASK_COMM_LEN.
9596
let len = this.read_scalar(len)?;
96-
let res = if len.to_target_usize(this)? < TASK_COMM_LEN as u64 {
97-
this.eval_libc("ERANGE")
98-
} else {
99-
this.pthread_getname_np(
97+
let res = if len.to_target_usize(this)? >= TASK_COMM_LEN as u64
98+
&& this.pthread_getname_np(
10099
this.read_scalar(thread)?,
101100
this.read_scalar(name)?,
102101
len,
103-
)?
102+
/* truncate*/ false,
103+
)? {
104+
Scalar::from_u32(0)
105+
} else {
106+
this.eval_libc("ERANGE")
104107
};
105108
this.write_scalar(res, dest)?;
106109
}

src/shims/unix/macos/foreign_items.rs

+33-5
Original file line numberDiff line numberDiff line change
@@ -164,24 +164,52 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
164164
// Threading
165165
"pthread_setname_np" => {
166166
let [name] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
167+
168+
// The real implementation has logic in two places:
169+
// * in userland at https://github.com/apple-oss-distributions/libpthread/blob/c032e0b076700a0a47db75528a282b8d3a06531a/src/pthread.c#L1178-L1200,
170+
// * in kernel at https://github.com/apple-oss-distributions/xnu/blob/8d741a5de7ff4191bf97d57b9f54c2f6d4a15585/bsd/kern/proc_info.c#L3218-L3227.
171+
//
172+
// The function in libc calls the kernel to validate
173+
// the security policies and the input. If all of the requirements
174+
// are met, then the name is set and 0 is returned. Otherwise, if
175+
// the specified name is lomnger than MAXTHREADNAMESIZE, then
176+
// ENAMETOOLONG is returned.
177+
//
178+
// FIXME: the real implementation maybe returns ESRCH if the thread ID is invalid.
167179
let thread = this.pthread_self()?;
168-
let max_len = this.eval_libc("MAXTHREADNAMESIZE").to_target_usize(this)?;
169-
let res = this.pthread_setname_np(
180+
let res = if this.pthread_setname_np(
170181
thread,
171182
this.read_scalar(name)?,
172-
max_len.try_into().unwrap(),
173-
)?;
183+
this.eval_libc("MAXTHREADNAMESIZE").to_target_usize(this)?.try_into().unwrap(),
184+
)? {
185+
Scalar::from_u32(0)
186+
} else {
187+
this.eval_libc("ENAMETOOLONG")
188+
};
174189
// Contrary to the manpage, `pthread_setname_np` on macOS still
175190
// returns an integer indicating success.
176191
this.write_scalar(res, dest)?;
177192
}
178193
"pthread_getname_np" => {
179194
let [thread, name, len] =
180195
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
181-
let res = this.pthread_getname_np(
196+
197+
// The function's behavior isn't portable between platforms.
198+
// In case of macOS, a truncated name (due to a too small buffer)
199+
// does not lead to an error.
200+
//
201+
// For details, see the implementation at
202+
// https://github.com/apple-oss-distributions/libpthread/blob/c032e0b076700a0a47db75528a282b8d3a06531a/src/pthread.c#L1160-L1175.
203+
// The key part is the strlcpy, which truncates the resulting value,
204+
// but always null terminates (except for zero sized buffers).
205+
//
206+
// FIXME: the real implementation returns ESRCH if the thread ID is invalid.
207+
let res = Scalar::from_u32(0);
208+
this.pthread_getname_np(
182209
this.read_scalar(thread)?,
183210
this.read_scalar(name)?,
184211
this.read_scalar(len)?,
212+
/* truncate */ true,
185213
)?;
186214
this.write_scalar(res, dest)?;
187215
}

src/shims/unix/solarish/foreign_items.rs

+4
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,20 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
3131
this.read_scalar(name)?,
3232
max_len,
3333
)?;
34+
let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") };
3435
this.write_scalar(res, dest)?;
3536
}
3637
"pthread_getname_np" => {
3738
let [thread, name, len] =
3839
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
40+
// https://github.com/illumos/illumos-gate/blob/c56822be04b6c157c8b6f2281e47214c3b86f657/usr/src/lib/libc/port/threads/thr.c#L2449-L2480
3941
let res = this.pthread_getname_np(
4042
this.read_scalar(thread)?,
4143
this.read_scalar(name)?,
4244
this.read_scalar(len)?,
45+
/* truncate */ false,
4346
)?;
47+
let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") };
4448
this.write_scalar(res, dest)?;
4549
}
4650

src/shims/unix/thread.rs

+19-11
Original file line numberDiff line numberDiff line change
@@ -63,38 +63,41 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
6363
interp_ok(Scalar::from_uint(thread_id.to_u32(), this.libc_ty_layout("pthread_t").size))
6464
}
6565

66-
/// Set the name of the current thread. `max_name_len` is the maximal length of the name
67-
/// including the null terminator.
66+
/// Set the name of the specified thread. If the name including the null terminator
67+
/// is longer than `name_max_len`, then `false` is returned.
6868
fn pthread_setname_np(
6969
&mut self,
7070
thread: Scalar,
7171
name: Scalar,
72-
max_name_len: usize,
73-
) -> InterpResult<'tcx, Scalar> {
72+
name_max_len: usize,
73+
) -> InterpResult<'tcx, bool> {
7474
let this = self.eval_context_mut();
7575

7676
let thread = thread.to_int(this.libc_ty_layout("pthread_t").size)?;
7777
let thread = ThreadId::try_from(thread).unwrap();
7878
let name = name.to_pointer(this)?;
79-
8079
let name = this.read_c_str(name)?.to_owned();
8180

8281
// Comparing with `>=` to account for null terminator.
83-
if name.len() >= max_name_len {
84-
return interp_ok(this.eval_libc("ERANGE"));
82+
if name.len() >= name_max_len {
83+
return interp_ok(false);
8584
}
8685

8786
this.set_thread_name(thread, name);
8887

89-
interp_ok(Scalar::from_u32(0))
88+
interp_ok(true)
9089
}
9190

91+
/// Get the name of the specified thread. If the thread name doesn't fit
92+
/// the buffer, then if `truncate` is set the truncated name is written out,
93+
/// otherwise `false` is returned.
9294
fn pthread_getname_np(
9395
&mut self,
9496
thread: Scalar,
9597
name_out: Scalar,
9698
len: Scalar,
97-
) -> InterpResult<'tcx, Scalar> {
99+
truncate: bool,
100+
) -> InterpResult<'tcx, bool> {
98101
let this = self.eval_context_mut();
99102

100103
let thread = thread.to_int(this.libc_ty_layout("pthread_t").size)?;
@@ -104,9 +107,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
104107

105108
// FIXME: we should use the program name if the thread name is not set
106109
let name = this.get_thread_name(thread).unwrap_or(b"<unnamed>").to_owned();
107-
let (success, _written) = this.write_c_str(&name, name_out, len)?;
110+
let name = match truncate {
111+
true => &name[..name.len().min(len.try_into().unwrap_or(usize::MAX).saturating_sub(1))],
112+
false => &name,
113+
};
114+
115+
let (success, _written) = this.write_c_str(name, name_out, len)?;
108116

109-
interp_ok(if success { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") })
117+
interp_ok(success)
110118
}
111119

112120
fn sched_yield(&mut self) -> InterpResult<'tcx, ()> {

tests/pass-dep/libc/pthread-threadname.rs

+109-39
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,23 @@
11
//@ignore-target: windows # No pthreads on Windows
2-
use std::ffi::CStr;
3-
#[cfg(not(target_os = "freebsd"))]
4-
use std::ffi::CString;
2+
use std::ffi::{CStr, CString};
53
use std::thread;
64

5+
const MAX_THREAD_NAME_LEN: usize = {
6+
cfg_if::cfg_if! {
7+
if #[cfg(any(target_os = "linux"))] {
8+
16
9+
} else if #[cfg(any(target_os = "illumos", target_os = "solaris"))] {
10+
32
11+
} else if #[cfg(target_os = "macos")] {
12+
libc::MAXTHREADNAMESIZE // 64, at the time of writing
13+
} else if #[cfg(target_os = "freebsd")] {
14+
usize::MAX // as far as I can tell
15+
} else {
16+
panic!()
17+
}
18+
}
19+
};
20+
721
fn main() {
822
// The short name should be shorter than 16 bytes which POSIX promises
923
// for thread names. The length includes a null terminator.
@@ -52,61 +66,117 @@ fn main() {
5266
}
5367

5468
thread::Builder::new()
55-
.name(short_name.clone())
5669
.spawn(move || {
57-
// Rust remembers the full thread name itself.
58-
assert_eq!(thread::current().name(), Some(short_name.as_str()));
70+
// Set short thread name.
71+
let cstr = CString::new(short_name.clone()).unwrap();
72+
assert!(cstr.to_bytes_with_nul().len() <= MAX_THREAD_NAME_LEN); // this should fit
73+
assert_eq!(set_thread_name(&cstr), 0);
5974

60-
// Note that glibc requires 15 bytes long buffer exculding a null terminator.
61-
// Otherwise, `pthread_getname_np` returns an error.
75+
// Now get it again, in various ways.
76+
77+
// POSIX seems to promise at least 15 chars excluding a null terminator.
6278
let mut buf = vec![0u8; short_name.len().max(15) + 1];
6379
assert_eq!(get_thread_name(&mut buf), 0);
64-
6580
let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
66-
// POSIX seems to promise at least 15 chars excluding a null terminator.
67-
assert_eq!(short_name.as_bytes(), cstr.to_bytes());
81+
assert_eq!(cstr.to_bytes(), short_name.as_bytes());
6882

69-
// Also test directly calling pthread_setname to check its return value.
70-
assert_eq!(set_thread_name(&cstr), 0);
83+
// Test what happens when the buffer is shorter than 16, but still long enough.
84+
let res = get_thread_name(&mut buf[..15]);
85+
cfg_if::cfg_if! {
86+
if #[cfg(target_os = "linux")] {
87+
// For glibc used by linux-gnu there should be a failue,
88+
// if a shorter than 16 bytes buffer is provided, even if that would be
89+
// large enough for the thread name.
90+
assert_eq!(res, libc::ERANGE);
91+
} else {
92+
// Everywhere else, this should work.
93+
assert_eq!(res, 0);
94+
// POSIX seems to promise at least 15 chars excluding a null terminator.
95+
let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
96+
assert_eq!(short_name.as_bytes(), cstr.to_bytes());
97+
}
98+
}
99+
100+
// Test what happens when the buffer is too short even for the short name.
101+
let res = get_thread_name(&mut buf[..4]);
102+
cfg_if::cfg_if! {
103+
if #[cfg(any(target_os = "freebsd", target_os = "macos"))] {
104+
// On macOS and FreeBSD it's not an error for the buffer to be
105+
// too short for the thread name -- they truncate instead.
106+
assert_eq!(res, 0);
107+
let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
108+
assert_eq!(cstr.to_bytes_with_nul().len(), 4);
109+
assert!(short_name.as_bytes().starts_with(cstr.to_bytes()));
110+
} else {
111+
// The rest should give an error.
112+
assert_eq!(res, libc::ERANGE);
113+
}
114+
}
71115

72-
// For glibc used by linux-gnu there should be a failue,
73-
// if a shorter than 16 bytes buffer is provided, even if that would be
74-
// large enough for the thread name.
75-
#[cfg(target_os = "linux")]
76-
assert_eq!(get_thread_name(&mut buf[..15]), libc::ERANGE);
116+
// Test zero-sized buffer.
117+
let res = get_thread_name(&mut []);
118+
cfg_if::cfg_if! {
119+
if #[cfg(any(target_os = "freebsd", target_os = "macos"))] {
120+
// On macOS and FreeBSD it's not an error for the buffer to be
121+
// too short for the thread name -- even with size 0.
122+
assert_eq!(res, 0);
123+
} else {
124+
// The rest should give an error.
125+
assert_eq!(res, libc::ERANGE);
126+
}
127+
}
77128
})
78129
.unwrap()
79130
.join()
80131
.unwrap();
81132

82133
thread::Builder::new()
83-
.name(long_name.clone())
84134
.spawn(move || {
85-
// Rust remembers the full thread name itself.
86-
assert_eq!(thread::current().name(), Some(long_name.as_str()));
135+
// Set full thread name.
136+
let cstr = CString::new(long_name.clone()).unwrap();
137+
let res = set_thread_name(&cstr);
138+
cfg_if::cfg_if! {
139+
if #[cfg(target_os = "freebsd")] {
140+
// Names of all size are supported.
141+
assert!(cstr.to_bytes_with_nul().len() <= MAX_THREAD_NAME_LEN);
142+
assert_eq!(res, 0);
143+
} else if #[cfg(target_os = "macos")] {
144+
// Name is too long.
145+
assert!(cstr.to_bytes_with_nul().len() > MAX_THREAD_NAME_LEN);
146+
assert_eq!(res, libc::ENAMETOOLONG);
147+
} else {
148+
// Name is too long.
149+
assert!(cstr.to_bytes_with_nul().len() > MAX_THREAD_NAME_LEN);
150+
assert_eq!(res, libc::ERANGE);
151+
}
152+
}
153+
// Set the longest name we can.
154+
let truncated_name = &long_name[..long_name.len().min(MAX_THREAD_NAME_LEN - 1)];
155+
let cstr = CString::new(truncated_name).unwrap();
156+
assert_eq!(set_thread_name(&cstr), 0);
157+
158+
// Now get it again, in various ways.
87159

88-
// But the system is limited -- make sure we successfully set a truncation.
89-
// Note that there's no specific to glibc buffer requirement, since the value
90-
// `long_name` is longer than 16 bytes including a null terminator.
160+
// This name should round-trip properly.
91161
let mut buf = vec![0u8; long_name.len() + 1];
92162
assert_eq!(get_thread_name(&mut buf), 0);
93-
94163
let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
95-
// POSIX seems to promise at least 15 chars excluding a null terminator.
96-
assert!(
97-
cstr.to_bytes().len() >= 15,
98-
"name is too short: len={}",
99-
cstr.to_bytes().len()
100-
);
101-
assert!(long_name.as_bytes().starts_with(cstr.to_bytes()));
102-
103-
// Also test directly calling pthread_setname to check its return value.
104-
assert_eq!(set_thread_name(&cstr), 0);
164+
assert_eq!(cstr.to_bytes(), truncated_name.as_bytes());
105165

106-
// But with a too long name it should fail (except on FreeBSD where the
107-
// function has no return, hence cannot indicate failure).
108-
#[cfg(not(target_os = "freebsd"))]
109-
assert_ne!(set_thread_name(&CString::new(long_name).unwrap()), 0);
166+
// Test what happens when our buffer is just one byte too small.
167+
let res = get_thread_name(&mut buf[..truncated_name.len()]);
168+
cfg_if::cfg_if! {
169+
if #[cfg(any(target_os = "freebsd", target_os = "macos"))] {
170+
// On macOS and FreeBSD it's not an error for the buffer to be
171+
// too short for the thread name -- they truncate instead.
172+
assert_eq!(res, 0);
173+
let cstr = CStr::from_bytes_until_nul(&buf).unwrap();
174+
assert_eq!(cstr.to_bytes(), &truncated_name.as_bytes()[..(truncated_name.len() - 1)]);
175+
} else {
176+
// The rest should give an error.
177+
assert_eq!(res, libc::ERANGE);
178+
}
179+
}
110180
})
111181
.unwrap()
112182
.join()

tests/pass/concurrency/threadname.rs

+13
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,19 @@ fn main() {
1616
.join()
1717
.unwrap();
1818

19+
// Long thread name.
20+
let long_name = std::iter::once("test_named_thread_truncation")
21+
.chain(std::iter::repeat(" long").take(100))
22+
.collect::<String>();
23+
thread::Builder::new()
24+
.name(long_name.clone())
25+
.spawn(move || {
26+
assert_eq!(thread::current().name().unwrap(), long_name);
27+
})
28+
.unwrap()
29+
.join()
30+
.unwrap();
31+
1932
// Also check main thread name.
2033
assert_eq!(thread::current().name().unwrap(), "main");
2134
}

0 commit comments

Comments
 (0)