Skip to content

Commit 6ff0af3

Browse files
committed
Fix #1419.
1 parent bd97074 commit 6ff0af3

File tree

2 files changed

+45
-17
lines changed

2 files changed

+45
-17
lines changed

src/shims/sync.rs

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,31 @@ fn set_at_offset<'mir, 'tcx: 'mir>(
5858
// store an i32 in the first four bytes equal to the corresponding libc mutex kind constant
5959
// (e.g. PTHREAD_MUTEX_NORMAL).
6060

61+
/// A flag that allows to distinguish `PTHREAD_MUTEX_NORMAL` from
62+
/// `PTHREAD_MUTEX_DEFAULT`. Since in `glibc` they have the same numeric values,
63+
/// but different behaviour, we need a way to distinguish them. We do this by
64+
/// setting this bit flag to the `PTHREAD_MUTEX_NORMAL` mutexes. See the comment
65+
/// in `pthread_mutexattr_settype` function.
66+
const PTHREAD_MUTEX_NORMAL_FLAG: i32 = 0x8000000;
67+
6168
const PTHREAD_MUTEXATTR_T_MIN_SIZE: u64 = 4;
6269

70+
fn is_mutex_kind_default<'mir, 'tcx: 'mir>(
71+
ecx: &mut MiriEvalContext<'mir, 'tcx>,
72+
kind: Scalar<Tag>,
73+
) -> InterpResult<'tcx, bool> {
74+
Ok(kind == ecx.eval_libc("PTHREAD_MUTEX_DEFAULT")?)
75+
}
76+
77+
fn is_mutex_kind_normal<'mir, 'tcx: 'mir>(
78+
ecx: &mut MiriEvalContext<'mir, 'tcx>,
79+
kind: Scalar<Tag>,
80+
) -> InterpResult<'tcx, bool> {
81+
let kind = kind.to_i32()?;
82+
let mutex_normal_kind = ecx.eval_libc("PTHREAD_MUTEX_NORMAL")?.to_i32()?;
83+
Ok(kind == (mutex_normal_kind | PTHREAD_MUTEX_NORMAL_FLAG))
84+
}
85+
6386
fn mutexattr_get_kind<'mir, 'tcx: 'mir>(
6487
ecx: &MiriEvalContext<'mir, 'tcx>,
6588
attr_op: OpTy<'tcx, Tag>,
@@ -318,8 +341,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
318341
let this = self.eval_context_mut();
319342

320343
let kind = this.read_scalar(kind_op)?.not_undef()?;
321-
if kind == this.eval_libc("PTHREAD_MUTEX_DEFAULT")?
322-
|| kind == this.eval_libc("PTHREAD_MUTEX_NORMAL")?
344+
if kind == this.eval_libc("PTHREAD_MUTEX_NORMAL")? {
345+
// In `glibc` implementation, the numeric values of
346+
// `PTHREAD_MUTEX_NORMAL` and `PTHREAD_MUTEX_DEFAULT` are equal, but
347+
// they have different behaviour in some cases. Therefore, we add
348+
// this flag to ensure that we can distinguish
349+
// `PTHREAD_MUTEX_NORMAL` from `PTHREAD_MUTEX_DEFAULT`.
350+
let normal_kind = kind.to_i32()? | PTHREAD_MUTEX_NORMAL_FLAG;
351+
// Check that after setting the flag, the kind is distinguishable
352+
// from all other kinds.
353+
assert_ne!(normal_kind, this.eval_libc("PTHREAD_MUTEX_DEFAULT")?.to_i32()?);
354+
assert_ne!(normal_kind, this.eval_libc("PTHREAD_MUTEX_ERRORCHECK")?.to_i32()?);
355+
assert_ne!(normal_kind, this.eval_libc("PTHREAD_MUTEX_RECURSIVE")?.to_i32()?);
356+
mutexattr_set_kind(this, attr_op, Scalar::from_i32(normal_kind))?;
357+
} else if kind == this.eval_libc("PTHREAD_MUTEX_DEFAULT")?
323358
|| kind == this.eval_libc("PTHREAD_MUTEX_ERRORCHECK")?
324359
|| kind == this.eval_libc("PTHREAD_MUTEX_RECURSIVE")?
325360
{
@@ -378,14 +413,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
378413
Ok(0)
379414
} else {
380415
// Trying to acquire the same mutex again.
381-
if kind == this.eval_libc("PTHREAD_MUTEX_DEFAULT")? {
382-
// FIXME: Sometimes this is actually a Deadlock.
383-
// https://github.com/rust-lang/miri/issues/1419
384-
throw_ub_format!(
385-
"trying to acquire already locked PTHREAD_MUTEX_DEFAULT (see #1419)"
386-
);
387-
}
388-
if kind == this.eval_libc("PTHREAD_MUTEX_NORMAL")? {
416+
if is_mutex_kind_default(this, kind)? {
417+
throw_ub_format!("trying to acquire already locked PTHREAD_MUTEX_DEFAULT");
418+
} else if is_mutex_kind_normal(this, kind)? {
389419
throw_machine_stop!(TerminationInfo::Deadlock);
390420
} else if kind == this.eval_libc("PTHREAD_MUTEX_ERRORCHECK")? {
391421
this.eval_libc_i32("EDEADLK")
@@ -417,8 +447,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
417447
if owner_thread != active_thread {
418448
this.eval_libc_i32("EBUSY")
419449
} else {
420-
if kind == this.eval_libc("PTHREAD_MUTEX_DEFAULT")?
421-
|| kind == this.eval_libc("PTHREAD_MUTEX_NORMAL")?
450+
if is_mutex_kind_default(this, kind)?
451+
|| is_mutex_kind_normal(this, kind)?
422452
|| kind == this.eval_libc("PTHREAD_MUTEX_ERRORCHECK")?
423453
{
424454
this.eval_libc_i32("EBUSY")
@@ -452,11 +482,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
452482
// The mutex was locked by another thread or not locked at all. See
453483
// the “Unlock When Not Owner” column in
454484
// https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_unlock.html.
455-
if kind == this.eval_libc("PTHREAD_MUTEX_DEFAULT")? {
485+
if is_mutex_kind_default(this, kind)? {
456486
throw_ub_format!(
457487
"unlocked a PTHREAD_MUTEX_DEFAULT mutex that was not locked by the current thread"
458488
);
459-
} else if kind == this.eval_libc("PTHREAD_MUTEX_NORMAL")? {
489+
} else if is_mutex_kind_normal(this, kind)? {
460490
throw_ub_format!(
461491
"unlocked a PTHREAD_MUTEX_NORMAL mutex that was not locked by the current thread"
462492
);

tests/compile-fail/sync/libc_pthread_mutex_normal_deadlock.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ fn main() {
1111
let mut mutex: libc::pthread_mutex_t = std::mem::zeroed();
1212
assert_eq!(libc::pthread_mutex_init(&mut mutex as *mut _, &mutexattr as *const _), 0);
1313
assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0);
14-
// FIXME: The error should be deadlock. See issue
15-
// https://github.com/rust-lang/miri/issues/1419.
16-
libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR Undefined Behavior
14+
libc::pthread_mutex_lock(&mut mutex as *mut _); //~ ERROR deadlock: the evaluated program deadlocked
1715
}
1816
}

0 commit comments

Comments
 (0)