-
Notifications
You must be signed in to change notification settings - Fork 355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
detect when pthread_cond_t is moved #3884
Conversation
62cff38
to
d1d15a4
Compare
@rustbot ready |
/// Additional data that we attach with each cond instance. | ||
struct AdditionalCondData { | ||
/// The address of the cond. | ||
address: u64, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also store the clock id in here instead of in-memory, similar to what you did with the mutex kind.
src/shims/unix/sync.rs
Outdated
"PTHREAD_COND_INITIALIZER is incompatible with our pthread_cond layout: clock is not CLOCK_REALTIME" | ||
); | ||
} | ||
|
||
offset | ||
} | ||
|
||
#[derive(Debug, Clone, Copy)] | ||
enum ClockType { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We call it the clock "ID" in most places, and the C type name for this is clockid_t
. So ClockId
seems like a better name for this type?
src/shims/unix/sync.rs
Outdated
@@ -902,13 +912,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { | |||
return Ok(()); | |||
} | |||
}; | |||
let timeout_clock = if is_cond_clock_realtime(this, clock_id) { | |||
let timeout_clock = if matches!(clock_type, ClockType::Realtime) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make this a match clock_id { ... }
src/shims/unix/sync.rs
Outdated
@@ -933,7 +943,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { | |||
|
|||
// Destroying an uninit pthread_cond is UB, so check to make sure it's not uninit. | |||
cond_get_id(this, cond_op)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize that's pre-existing, but -- this function is already called just above, so this call can be removed. Please preserve the comment though.
Looks good, thanks. :) Can you squash this into a single commit? |
1bfc69e
to
b1aaf1a
Compare
Thanks for the blazingly fast reviews! |
@bors r+ |
☀️ Test successful - checks-actions |
Closes #3749