Skip to content

Commit 7bcb8c1

Browse files
RalfJungRalfJung
authored
Merge pull request #5001 from RalfJung/tid
tid handling: cleanup and unification
2 parents 85298e9 + 64ab201 commit 7bcb8c1

8 files changed

Lines changed: 38 additions & 31 deletions

File tree

‎src/tools/miri/src/concurrency/blocking_io.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> {
219219
let this = self.eval_context_mut();
220220
this.machine.blocking_io.register(
221221
source_fd,
222-
InterestReceiver::UnblockThread(this.machine.threads.active_thread()),
222+
InterestReceiver::UnblockThread(this.active_thread()),
223223
interests,
224224
);
225225
this.block_thread(BlockReason::IO, timeout, callback);

‎src/tools/miri/src/shims/env.rs‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::assert_matches;
12
use std::ffi::{OsStr, OsString};
23

34
use rustc_data_structures::fx::FxHashMap;
@@ -108,32 +109,22 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
108109
if this.machine.communicate() { std::process::id() } else { 1000 }
109110
}
110111

111-
/// Get an "OS" thread ID for the current thread.
112-
fn get_current_tid(&self) -> u32 {
112+
/// Get an "OS" thread ID for any thread.
113+
fn get_tid(&self, thread: ThreadId) -> u32 {
113114
let this = self.eval_context_ref();
114-
self.get_tid(this.machine.threads.active_thread())
115+
assert!(this.target_os_is_unix());
116+
// On Linux, the main thread has PID == TID so we uphold this. For simplicity we do it
117+
// everywhere. That also ensures this ID is different from what is returned by
118+
// `pthread_self`.
119+
this.get_pid().strict_add(thread.to_u32())
115120
}
116121

117122
/// Convert TID back to a `ThreadId`, or `None` if it is invalid or the thread has terminated.
118123
fn get_thread_id_from_linux_tid(&self, tid: u32) -> Option<ThreadId> {
119124
let this = self.eval_context_ref();
120-
debug_assert!(matches!(this.tcx.sess.target.os, Os::Linux | Os::Android));
125+
assert_matches!(this.tcx.sess.target.os, Os::Linux | Os::Android);
121126
// TID = PID + thread_index => index = TID - PID.
122127
let id = tid.checked_sub(this.get_pid())?;
123128
this.machine.threads.thread_id_try_from(id).ok()
124129
}
125-
126-
/// Get an "OS" thread ID for any thread.
127-
fn get_tid(&self, thread: ThreadId) -> u32 {
128-
let this = self.eval_context_ref();
129-
let index = thread.to_u32();
130-
let target_os = &this.tcx.sess.target.os;
131-
if matches!(target_os, Os::Linux | Os::Android) {
132-
// On Linux, the main thread has PID == TID so we uphold this.
133-
this.get_pid().strict_add(index)
134-
} else {
135-
// Other platforms do not display any relationship between PID and TID.
136-
index
137-
}
138-
}
139130
}

‎src/tools/miri/src/shims/unix/env.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
269269

270270
// For most platforms the return type is an `i32`, but some are unsigned. The TID
271271
// will always be positive so we don't need to differentiate.
272-
interp_ok(Scalar::from_u32(this.get_current_tid()))
272+
interp_ok(Scalar::from_u32(this.get_tid(this.active_thread())))
273273
}
274274

275275
/// `fields_size`, if present, says how large each field of the struct is.
@@ -323,7 +323,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
323323
/// allows querying the ID for arbitrary threads, identified by their pthread_t.
324324
///
325325
/// API documentation: <https://www.manpagez.com/man/3/pthread_threadid_np/>.
326-
fn apple_pthread_threadip_np(
326+
fn apple_pthread_threadid_np(
327327
&mut self,
328328
thread_op: &OpTy<'tcx>,
329329
tid_op: &OpTy<'tcx>,
@@ -349,6 +349,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
349349
thread
350350
};
351351

352+
// This returns an `int`, not a `pthread_t`, so we treat it like we treat `gettid` on Linux.
352353
let tid = this.get_tid(thread);
353354
let tid_dest = this.deref_pointer_as(tid_op, this.machine.layouts.u64)?;
354355
this.write_int(tid, &tid_dest)?;

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
180180
&[Os::Linux, Os::Android, Os::MacOs, Os::Solaris, Os::Illumos],
181181
link_name,
182182
)?;
183+
183184
let [uname] = this.check_shim_sig(
184185
shim_sig!(extern "C" fn(*mut _) -> i32),
185186
link_name,
@@ -296,6 +297,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
296297
"flock" => {
297298
// Currently this function does not exist on all Unixes, e.g. on Solaris.
298299
this.check_target_os(&[Os::Linux, Os::FreeBsd, Os::MacOs, Os::Illumos], link_name)?;
300+
299301
let [fd, op] = this.check_shim_sig(
300302
shim_sig!(extern "C" fn(i32, i32) -> i32),
301303
link_name,
@@ -495,6 +497,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
495497
&[Os::Linux, Os::FreeBsd, Os::Solaris, Os::Illumos, Os::Android],
496498
link_name,
497499
)?;
500+
498501
let [fd, offset, len] = this.check_shim_sig(
499502
shim_sig!(extern "C" fn(i32, libc::off_t, libc::off_t) -> i32),
500503
link_name,
@@ -560,6 +563,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
560563
&[Os::Linux, Os::Android, Os::FreeBsd, Os::Solaris, Os::Illumos],
561564
link_name,
562565
)?;
566+
563567
let [pipefd, flags] = this.check_shim_sig(
564568
shim_sig!(extern "C" fn(*mut _, i32) -> i32),
565569
link_name,
@@ -743,6 +747,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
743747
"reallocarray" => {
744748
// Currently this function does not exist on all Unixes, e.g. on macOS.
745749
this.check_target_os(&[Os::Linux, Os::FreeBsd, Os::Android], link_name)?;
750+
746751
let [ptr, nmemb, size] =
747752
this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
748753
let ptr = this.read_pointer(ptr)?;
@@ -1020,6 +1025,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
10201025
&[Os::FreeBsd, Os::Linux, Os::Android, Os::Solaris, Os::Illumos],
10211026
link_name,
10221027
)?;
1028+
10231029
let [clock_id, flags, req, rem] =
10241030
this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
10251031
let result = this.clock_nanosleep(clock_id, flags, req, rem)?;
@@ -1028,6 +1034,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
10281034
"sched_getaffinity" => {
10291035
// Currently this function does not exist on all Unixes, e.g. on macOS.
10301036
this.check_target_os(&[Os::Linux, Os::FreeBsd, Os::Android], link_name)?;
1037+
10311038
let [pid, cpusetsize, mask] =
10321039
this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
10331040
let pid = this.read_scalar(pid)?.to_u32()?;
@@ -1072,6 +1079,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
10721079
"sched_setaffinity" => {
10731080
// Currently this function does not exist on all Unixes, e.g. on macOS.
10741081
this.check_target_os(&[Os::Linux, Os::FreeBsd, Os::Android], link_name)?;
1082+
10751083
let [pid, cpusetsize, mask] =
10761084
this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
10771085
let pid = this.read_scalar(pid)?.to_u32()?;
@@ -1140,6 +1148,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
11401148
&[Os::Linux, Os::MacOs, Os::FreeBsd, Os::Illumos, Os::Solaris, Os::Android],
11411149
link_name,
11421150
)?;
1151+
11431152
let [buf, bufsize] =
11441153
this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
11451154
let buf = this.read_pointer(buf)?;
@@ -1172,6 +1181,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
11721181
&[Os::Linux, Os::FreeBsd, Os::Illumos, Os::Solaris, Os::Android],
11731182
link_name,
11741183
)?;
1184+
11751185
let [ptr, len, flags] =
11761186
this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
11771187
let ptr = this.read_pointer(ptr)?;
@@ -1185,6 +1195,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
11851195
// This function is non-standard but exists with the same signature and
11861196
// same behavior (eg never fails) on FreeBSD and Solaris/Illumos.
11871197
this.check_target_os(&[Os::FreeBsd, Os::Illumos, Os::Solaris], link_name)?;
1198+
11881199
let [ptr, len] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
11891200
let ptr = this.read_pointer(ptr)?;
11901201
let len = this.read_target_usize(len)?;
@@ -1208,6 +1219,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
12081219
&[Os::Linux, Os::FreeBsd, Os::Illumos, Os::Solaris, Os::Android, Os::MacOs],
12091220
link_name,
12101221
)?;
1222+
12111223
// This function looks and behaves exactly like miri_start_unwind.
12121224
let [payload] = this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
12131225
this.handle_miri_start_unwind(payload)?;

‎src/tools/miri/src/shims/unix/macos/foreign_items.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
244244
"pthread_threadid_np" => {
245245
let [thread, tid_ptr] =
246246
this.check_shim_sig_lenient(abi, CanonAbi::C, link_name, args)?;
247-
let res = this.apple_pthread_threadip_np(thread, tid_ptr)?;
247+
let res = this.apple_pthread_threadid_np(thread, tid_ptr)?;
248248
this.write_scalar(res, dest)?;
249249
}
250250

‎src/tools/miri/src/shims/windows/foreign_items.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -953,8 +953,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
953953
Handle::Pseudo(PseudoHandle::CurrentThread) => this.active_thread(),
954954
_ => this.invalid_handle("GetThreadDescription")?,
955955
};
956-
let tid = this.get_tid(thread);
957-
this.write_scalar(Scalar::from_u32(tid), dest)?;
956+
this.write_scalar(Scalar::from_u32(thread.to_u32()), dest)?;
958957
}
959958
"GetCurrentThreadId" => {
960959
let [] = this.check_shim_sig(
@@ -963,9 +962,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
963962
abi,
964963
args,
965964
)?;
966-
let thread = this.active_thread();
967-
let tid = this.get_tid(thread);
968-
this.write_scalar(Scalar::from_u32(tid), dest)?;
965+
this.write_scalar(Scalar::from_u32(this.active_thread().to_u32()), dest)?;
969966
}
970967

971968
// Miscellaneous

‎src/tools/miri/src/shims/windows/handle.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ impl Handle {
184184

185185
match Self::from_packed(handle) {
186186
Some(Self::Thread(thread)) => {
187-
// Validate the thread id. Windows handles remain valid even after thread termination
187+
// Validate the thread id. Windows handles remain valid even after thread
188+
// termination.
188189
use crate::concurrency::thread::ThreadLookupError;
189190
match cx.machine.threads.thread_id_try_from(thread.to_u32()) {
190191
Ok(id) | Err(ThreadLookupError::Terminated(id)) =>

‎src/tools/miri/tests/pass-dep/shims/gettid.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,16 +165,21 @@ fn main() {
165165
// The value is not important, we only care that whatever the value is,
166166
// won't change from execution to execution.
167167
if cfg!(with_isolation) {
168-
if cfg!(any(target_os = "linux", target_os = "android")) {
169-
// Linux starts the TID at the PID, which is 1000.
168+
if cfg!(any(
169+
target_os = "linux",
170+
target_os = "android",
171+
target_os = "freebsd",
172+
target_os = "macos"
173+
)) {
174+
// On these OSes we start the TID at the PID, which is 1000.
170175
assert_eq!(tid, 1000);
171176
} else {
172177
// Other platforms start counting from 0.
173178
assert_eq!(tid, 0);
174179
}
175180
}
176181

177-
// On Linux, the first TID is the PID.
182+
// On Linux, the main thread TID is the PID.
178183
#[cfg(any(target_os = "linux", target_os = "android"))]
179184
assert_eq!(tid, unsafe { libc::getpid() } as u64);
180185

0 commit comments

Comments
 (0)