Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit d34242e

Browse files
beepster4096beepster4096
committed
fix various issues
1 parent 9f69c41 commit d34242e

19 files changed

+306
-71
lines changed

‎src/shims/os_str.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
7474
'mir: 'a,
7575
{
7676
#[cfg(windows)]
77-
pub fn u16vec_to_osstring<'tcx, 'a>(u16_vec: Vec<u16>) -> InterpResult<'tcx, OsString> {
77+
pub fn u16vec_to_osstring<'tcx>(u16_vec: Vec<u16>) -> InterpResult<'tcx, OsString> {
7878
Ok(OsString::from_wide(&u16_vec[..]))
7979
}
8080
#[cfg(not(windows))]

‎src/shims/tls.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,13 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
244244
this.eval_windows("thread_local_key", "p_thread_callback")?.to_pointer(this)?;
245245
let thread_callback = this.get_ptr_fn(thread_callback)?.as_instance()?;
246246

247-
// Technically, the reason should be `DLL_PROCESS_DETACH` when the main thread exits but std ignores it.
247+
// FIXME: Technically, the reason should be `DLL_PROCESS_DETACH` when the main thread exits
248+
// but std treats both the same.
248249
let reason = this.eval_windows("c", "DLL_THREAD_DETACH")?;
249250

250251
// The signature of this function is `unsafe extern "system" fn(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID)`.
252+
// FIXME: `h` should be a handle to the current module and what `pv` should be is unknown
253+
// but both are ignored by std
251254
this.call_function(
252255
thread_callback,
253256
Abi::System { unwind: false },

‎src/shims/unix/thread.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
1313
) -> InterpResult<'tcx, i32> {
1414
let this = self.eval_context_mut();
1515

16+
let thread_info_place = this.deref_operand(thread)?;
17+
18+
let start_routine = this.read_pointer(start_routine)?;
19+
20+
let func_arg = this.read_immediate(arg)?;
21+
1622
this.start_thread(
17-
Some(thread),
23+
Some(thread_info_place),
1824
start_routine,
1925
Abi::C { unwind: false },
20-
arg,
26+
func_arg,
2127
this.layout_of(this.tcx.types.usize)?,
2228
)?;
2329

@@ -46,7 +52,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
4652
let this = self.eval_context_mut();
4753

4854
let thread_id = this.read_scalar(thread)?.to_machine_usize(this)?;
49-
this.detach_thread(thread_id.try_into().expect("thread ID should fit in u32"), false)?;
55+
this.detach_thread(
56+
thread_id.try_into().expect("thread ID should fit in u32"),
57+
/*allow_terminated_joined*/ false,
58+
)?;
5059

5160
Ok(0)
5261
}

‎src/shims/windows/dlsym.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
122122
_ => this.invalid_handle("SetThreadDescription")?,
123123
};
124124

125-
this.set_thread_name_wide(thread, name);
125+
this.set_thread_name_wide(thread, &name);
126126

127127
this.write_null(dest)?;
128128
}

‎src/shims/windows/foreign_items.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -323,22 +323,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
323323
// FIXME: we should set last_error, but to what?
324324
this.write_null(dest)?;
325325
}
326-
// this is only callable from std because we know that std ignores the return value
327-
"SwitchToThread" if this.frame_in_std() => {
328-
let [] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
329-
330-
this.yield_active_thread();
331-
332-
// FIXME: this should return a nonzero value if this call does result in switching to another thread.
333-
this.write_null(dest)?;
334-
}
335326
"GetStdHandle" => {
336327
let [which] =
337328
this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
338329
let which = this.read_scalar(which)?.to_i32()?;
339330
// We just make this the identity function, so we know later in `NtWriteFile` which
340331
// one it is. This is very fake, but libtest needs it so we cannot make it a
341332
// std-only shim.
333+
// FIXME: this should return real HANDLEs when io support is added
342334
this.write_scalar(Scalar::from_machine_isize(which.into(), this), dest)?;
343335
}
344336
"CloseHandle" => {
@@ -364,9 +356,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
364356
let [handle, timeout] =
365357
this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
366358

367-
this.WaitForSingleObject(handle, timeout)?;
368-
369-
this.write_scalar(Scalar::from_u32(0), dest)?;
359+
let ret = this.WaitForSingleObject(handle, timeout)?;
360+
this.write_scalar(Scalar::from_u32(ret), dest)?;
370361
}
371362
"GetCurrentThread" => {
372363
let [] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
@@ -382,6 +373,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
382373
"GetProcessHeap" if this.frame_in_std() => {
383374
let [] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
384375
// Just fake a HANDLE
376+
// It's fine to not use the Handle type here because its a stub
385377
this.write_scalar(Scalar::from_machine_isize(1, this), dest)?;
386378
}
387379
"GetModuleHandleA" if this.frame_in_std() => {
@@ -417,6 +409,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
417409
let result = this.GetCurrentProcessId()?;
418410
this.write_scalar(Scalar::from_u32(result), dest)?;
419411
}
412+
// this is only callable from std because we know that std ignores the return value
413+
"SwitchToThread" if this.frame_in_std() => {
414+
let [] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
415+
416+
this.yield_active_thread();
417+
418+
// FIXME: this should return a nonzero value if this call does result in switching to another thread.
419+
this.write_null(dest)?;
420+
}
420421

421422
_ => return Ok(EmulateByNameResult::NotSupported),
422423
}

‎src/shims/windows/handle.rs

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@ pub enum PseudoHandle {
88
CurrentThread,
99
}
1010

11+
/// Miri representation of a Windows `HANDLE`
12+
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
13+
pub enum Handle {
14+
Null,
15+
Pseudo(PseudoHandle),
16+
Thread(ThreadId),
17+
}
18+
1119
impl PseudoHandle {
1220
const CURRENT_THREAD_VALUE: u32 = 0;
1321

@@ -25,14 +33,6 @@ impl PseudoHandle {
2533
}
2634
}
2735

28-
/// Miri representation of a Windows `HANDLE`
29-
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
30-
pub enum Handle {
31-
Null,
32-
Pseudo(PseudoHandle),
33-
Thread(ThreadId),
34-
}
35-
3636
impl Handle {
3737
const NULL_DISCRIMINANT: u32 = 0;
3838
const PSEUDO_DISCRIMINANT: u32 = 1;
@@ -62,9 +62,7 @@ impl Handle {
6262
let floor_log2 = variant_count.ilog2();
6363

6464
// we need to add one for non powers of two to compensate for the difference
65-
let ceil_log2 = if variant_count.is_power_of_two() { floor_log2 } else { floor_log2 + 1 };
66-
67-
ceil_log2
65+
if variant_count.is_power_of_two() { floor_log2 } else { floor_log2 + 1 }
6866
}
6967

7068
/// Converts a handle into its machine representation.
@@ -120,6 +118,7 @@ impl Handle {
120118
pub fn to_scalar(self, cx: &impl HasDataLayout) -> Scalar<Provenance> {
121119
// 64-bit handles are sign extended 32-bit handles
122120
// see https://docs.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication
121+
#[allow(clippy::cast_possible_wrap)] // we want it to wrap
123122
let signed_handle = self.to_packed() as i32;
124123
Scalar::from_machine_isize(signed_handle.into(), cx)
125124
}
@@ -130,6 +129,7 @@ impl Handle {
130129
) -> InterpResult<'tcx, Option<Self>> {
131130
let sign_extended_handle = handle.to_machine_isize(cx)?;
132131

132+
#[allow(clippy::cast_sign_loss)] // we want to lose the sign
133133
let handle = if let Ok(signed_handle) = i32::try_from(sign_extended_handle) {
134134
signed_handle as u32
135135
} else {
@@ -154,8 +154,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
154154
fn CloseHandle(&mut self, handle_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> {
155155
let this = self.eval_context_mut();
156156

157-
match Handle::from_scalar(this.read_scalar(handle_op)?.check_init()?, this)? {
158-
Some(Handle::Thread(thread)) => this.detach_thread(thread, true)?,
157+
let handle = this.read_scalar(handle_op)?.check_init()?;
158+
159+
match Handle::from_scalar(handle, this)? {
160+
Some(Handle::Thread(thread)) =>
161+
this.detach_thread(thread, /*allow_terminated_joined*/ true)?,
159162
_ => this.invalid_handle("CloseHandle")?,
160163
}
161164

‎src/shims/windows/thread.rs

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,55 +19,71 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
1919
) -> InterpResult<'tcx, ThreadId> {
2020
let this = self.eval_context_mut();
2121

22-
if !this.ptr_is_null(this.read_pointer(security_op)?)? {
23-
throw_unsup_format!("non-null `lpThreadAttributes` in `CreateThread`")
24-
}
22+
let security = this.read_pointer(security_op)?;
2523

2624
// stacksize is ignored, but still needs to be a valid usize
27-
let _ = this.read_scalar(stacksize_op)?.to_machine_usize(this)?;
25+
this.read_scalar(stacksize_op)?.to_machine_usize(this)?;
26+
27+
let start_routine = this.read_pointer(start_op)?;
28+
29+
let func_arg = this.read_immediate(arg_op)?;
2830

2931
let flags = this.read_scalar(flags_op)?.to_u32()?;
3032

33+
let thread = if this.ptr_is_null(this.read_pointer(thread_op)?)? {
34+
None
35+
} else {
36+
let thread_info_place = this.deref_operand(thread_op)?;
37+
Some(thread_info_place)
38+
};
39+
3140
let stack_size_param_is_a_reservation =
3241
this.eval_windows("c", "STACK_SIZE_PARAM_IS_A_RESERVATION")?.to_u32()?;
3342

43+
// We ignore the stack size, so we also ignore the
44+
// `STACK_SIZE_PARAM_IS_A_RESERVATION` flag.
3445
if flags != 0 && flags != stack_size_param_is_a_reservation {
3546
throw_unsup_format!("unsupported `dwCreationFlags` {} in `CreateThread`", flags)
3647
}
3748

38-
let thread =
39-
if this.ptr_is_null(this.read_pointer(thread_op)?)? { None } else { Some(thread_op) };
49+
if !this.ptr_is_null(security)? {
50+
throw_unsup_format!("non-null `lpThreadAttributes` in `CreateThread`")
51+
}
4052

4153
this.start_thread(
4254
thread,
43-
start_op,
55+
start_routine,
4456
Abi::System { unwind: false },
45-
arg_op,
57+
func_arg,
4658
this.layout_of(this.tcx.types.u32)?,
4759
)
4860
}
4961

5062
fn WaitForSingleObject(
5163
&mut self,
52-
handle: &OpTy<'tcx, Provenance>,
53-
timeout: &OpTy<'tcx, Provenance>,
54-
) -> InterpResult<'tcx> {
64+
handle_op: &OpTy<'tcx, Provenance>,
65+
timeout_op: &OpTy<'tcx, Provenance>,
66+
) -> InterpResult<'tcx, u32> {
5567
let this = self.eval_context_mut();
5668

57-
let thread = match Handle::from_scalar(this.read_scalar(handle)?.check_init()?, this)? {
69+
let handle = this.read_scalar(handle_op)?.check_init()?;
70+
71+
let timeout = this.read_scalar(timeout_op)?.to_u32()?;
72+
73+
let thread = match Handle::from_scalar(handle, this)? {
5874
Some(Handle::Thread(thread)) => thread,
59-
// Unlike on posix, joining the current thread is not UB on windows.
60-
// It will just deadlock.
75+
// Unlike on posix, the outcome of joining the current thread is not documented.
76+
// On current Windows, it just deadlocks.
6177
Some(Handle::Pseudo(PseudoHandle::CurrentThread)) => this.get_active_thread(),
6278
_ => this.invalid_handle("WaitForSingleObject")?,
6379
};
6480

65-
if this.read_scalar(timeout)?.to_u32()? != this.eval_windows("c", "INFINITE")?.to_u32()? {
81+
if timeout != this.eval_windows("c", "INFINITE")?.to_u32()? {
6682
throw_unsup_format!("`WaitForSingleObject` with non-infinite timeout");
6783
}
6884

6985
this.join_thread(thread)?;
7086

71-
Ok(())
87+
Ok(0)
7288
}
7389
}

‎src/thread.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> {
181181
}
182182

183183
/// A specific moment in time.
184-
#[derive(Debug, Copy, Clone)]
184+
#[derive(Debug)]
185185
pub enum Time {
186186
Monotonic(Instant),
187187
RealTime(SystemTime),
@@ -357,16 +357,19 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
357357
///
358358
/// `allow_terminated_joined` allows detaching joined threads that have already terminated.
359359
/// This matches Windows's behavior for `CloseHandle`.
360+
///
361+
/// See <https://docs.microsoft.com/en-us/windows/win32/procthread/thread-handles-and-identifiers>:
362+
/// > The handle is valid until closed, even after the thread it represents has been terminated.
360363
fn detach_thread(&mut self, id: ThreadId, allow_terminated_joined: bool) -> InterpResult<'tcx> {
361364
trace!("detaching {:?}", id);
362365

363366
let is_ub = if allow_terminated_joined && self.threads[id].state == ThreadState::Terminated
364367
{
368+
// "Detached" in particular means "not yet joined". Redundant detaching is still UB.
365369
self.threads[id].join_status == ThreadJoinStatus::Detached
366370
} else {
367371
self.threads[id].join_status != ThreadJoinStatus::Joinable
368372
};
369-
370373
if is_ub {
371374
throw_ub_format!("trying to detach thread that was already detached or joined");
372375
}
@@ -406,7 +409,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
406409
}
407410

408411
/// Mark that the active thread tries to exclusively join the thread with `joined_thread_id`.
409-
/// If the thread is already joined by another thread
412+
/// If the thread is already joined by another thread, it will throw UB
410413
fn join_thread_exclusive(
411414
&mut self,
412415
joined_thread_id: ThreadId,
@@ -424,7 +427,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
424427
self.threads
425428
.iter()
426429
.all(|thread| thread.state != ThreadState::BlockedOnJoin(joined_thread_id)),
427-
"a joinable thread already has threads waiting for its termination"
430+
"this thread already has threads waiting for its termination"
428431
);
429432

430433
self.join_thread(joined_thread_id, data_race)
@@ -803,12 +806,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
803806
}
804807

805808
#[inline]
806-
fn set_thread_name_wide(&mut self, thread: ThreadId, new_thread_name: Vec<u16>) {
809+
fn set_thread_name_wide(&mut self, thread: ThreadId, new_thread_name: &[u16]) {
807810
let this = self.eval_context_mut();
808-
this.machine.threads.set_thread_name(
809-
thread,
810-
new_thread_name.into_iter().flat_map(u16::to_ne_bytes).collect(),
811-
);
811+
812+
// The Windows `GetThreadDescription` shim to get the thread name isn't implemented, so being lossy is okay.
813+
// This is only read by diagnostics, which already use `from_utf8_lossy`.
814+
this.machine
815+
.threads
816+
.set_thread_name(thread, String::from_utf16_lossy(new_thread_name).into_bytes());
812817
}
813818

814819
#[inline]

‎tests/fail/concurrency/libc_pthread_join_detached.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: trying to join a detached or already joined thread
1+
error: Undefined Behavior: trying to join a detached thread
22
--> $DIR/libc_pthread_join_detached.rs:LL:CC
33
|
4-
LL | ... assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to join a detached thread
4+
LL | assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to join a detached thread
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

‎tests/fail/concurrency/libc_pthread_join_joined.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ fn main() {
1515
// assert_eq!(libc::pthread_attr_init(&mut attr), 0); FIXME: this function is not yet implemented.
1616
assert_eq!(libc::pthread_create(&mut native, &attr, thread_start, ptr::null_mut()), 0);
1717
assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
18-
assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0); //~ ERROR: Undefined Behavior: trying to join a detached or already joined thread
18+
assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0); //~ ERROR: Undefined Behavior: trying to join an already joined thread
1919
}
2020
}

‎tests/fail/concurrency/libc_pthread_join_joined.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: trying to join a detached or already joined thread
1+
error: Undefined Behavior: trying to join an already joined thread
22
--> $DIR/libc_pthread_join_joined.rs:LL:CC
33
|
4-
LL | ... assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to join a detached or already joined thread
4+
LL | assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to join an already joined thread
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

‎tests/fail/concurrency/libc_pthread_join_main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ fn main() {
88
let thread_id: libc::pthread_t = unsafe { libc::pthread_self() };
99
let handle = thread::spawn(move || {
1010
unsafe {
11-
assert_eq!(libc::pthread_join(thread_id, ptr::null_mut()), 0); //~ ERROR: Undefined Behavior: trying to join a detached or already joined thread
11+
assert_eq!(libc::pthread_join(thread_id, ptr::null_mut()), 0); //~ ERROR: Undefined Behavior: trying to join a detached thread
1212
}
1313
});
1414
thread::yield_now();

0 commit comments

Comments
 (0)