Address some of the reviewers comments.

This commit is contained in:
Vytautas Astrauskas 2020-04-15 21:25:12 -07:00
parent 51b16e56cd
commit 325c31e578
15 changed files with 101 additions and 74 deletions

View File

@ -206,7 +206,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
let res: InterpResult<'_, i64> = (|| {
// Main loop.
while ecx.schedule()? {
assert!(ecx.step()?);
assert!(ecx.step()?, "Bug: a terminated thread was scheduled for execution.");
ecx.process_diagnostics();
}
// Read the return code pointer *before* we run TLS destructors, to assert

View File

@ -428,41 +428,37 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
mir::interpret::ConstValue::Scalar(Scalar::Ptr(ptr)) => {
let alloc_id = ptr.alloc_id;
let alloc = ecx.tcx.alloc_map.lock().get(alloc_id);
let tcx = ecx.tcx;
let is_thread_local = |def_id| {
tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::THREAD_LOCAL)
};
match alloc {
Some(GlobalAlloc::Static(def_id))
if ecx
.tcx
.codegen_fn_attrs(def_id)
.flags
.contains(CodegenFnAttrFlags::THREAD_LOCAL) =>
{
// We have a thread-local static.
Some(GlobalAlloc::Static(def_id)) if is_thread_local(def_id) => {
let new_alloc_id = if let Some(new_alloc_id) =
ecx.get_thread_local_alloc_id(alloc_id)
{
new_alloc_id
} else {
if ecx.tcx.is_foreign_item(def_id) {
if tcx.is_foreign_item(def_id) {
throw_unsup_format!(
"Foreign thread-local statics are not supported."
)
}
let instance = Instance::mono(ecx.tcx.tcx, def_id);
let instance = Instance::mono(tcx.tcx, def_id);
let gid = GlobalId { instance, promoted: None };
let raw_const = ecx
.tcx
let raw_const = tcx
.const_eval_raw(ty::ParamEnv::reveal_all().and(gid))
.map_err(|err| {
// no need to report anything, the const_eval call takes care of that
// for statics
assert!(ecx.tcx.is_static(def_id));
assert!(tcx.is_static(def_id));
match err {
ErrorHandled::Reported => err_inval!(ReferencedConstant),
ErrorHandled::TooGeneric => err_inval!(TooGeneric),
}
})?;
let id = raw_const.alloc_id;
let mut alloc_map = ecx.tcx.alloc_map.lock();
let mut alloc_map = tcx.alloc_map.lock();
let allocation = alloc_map.unwrap_memory(id);
let new_alloc_id = alloc_map.create_memory_alloc(allocation);
ecx.set_thread_local_alloc_id(alloc_id, new_alloc_id);

View File

@ -293,26 +293,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.write_scalar(Scalar::from_i32(result), dest)?;
}
// Miscellaneous
"isatty" => {
let _fd = this.read_scalar(args[0])?.to_i32()?;
// "returns 1 if fd is an open file descriptor referring to a terminal; otherwise 0 is returned, and errno is set to indicate the error"
// FIXME: we just say nothing is a terminal.
let enotty = this.eval_libc("ENOTTY")?;
this.set_last_error(enotty)?;
this.write_null(dest)?;
}
"pthread_atfork" => {
let _prepare = this.read_scalar(args[0])?.not_undef()?;
let _parent = this.read_scalar(args[1])?.not_undef()?;
let _child = this.read_scalar(args[1])?.not_undef()?;
// We do not support forking, so there is nothing to do here.
this.write_null(dest)?;
}
"sched_yield" => {
this.write_null(dest)?;
}
// Threading
"pthread_create" => {
assert_eq!(args.len(), 4);
@ -339,20 +319,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.write_scalar(Scalar::from_i32(result), dest)?;
}
"pthread_attr_getguardsize" => {
assert_eq!(args.len(), 2);
let guard_size = this.deref_operand(args[1])?;
let guard_size_type = args[1].layout.ty
.builtin_deref(true)
.ok_or_else(|| err_ub_format!(
"wrong signature used for `pthread_attr_getguardsize`: first argument must be a raw pointer."
))?
.ty;
let guard_size_layout = this.layout_of(guard_size_type)?;
this.write_scalar(Scalar::from_uint(crate::PAGE_SIZE, guard_size_layout.size), guard_size.into())?;
// Return success (`0`).
// Miscellaneous
"isatty" => {
let _fd = this.read_scalar(args[0])?.to_i32()?;
// "returns 1 if fd is an open file descriptor referring to a terminal; otherwise 0 is returned, and errno is set to indicate the error"
// FIXME: we just say nothing is a terminal.
let enotty = this.eval_libc("ENOTTY")?;
this.set_last_error(enotty)?;
this.write_null(dest)?;
}
"pthread_atfork" => {
let _prepare = this.read_scalar(args[0])?.not_undef()?;
let _parent = this.read_scalar(args[1])?.not_undef()?;
let _child = this.read_scalar(args[1])?.not_undef()?;
// We do not support forking, so there is nothing to do here.
this.write_null(dest)?;
}
@ -369,6 +349,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
=> {
this.write_null(dest)?;
}
"pthread_attr_getguardsize" if this.frame().instance.to_string().starts_with("std::sys::unix::")
=> {
let guard_size = this.deref_operand(args[1])?;
let guard_size_layout = this.libc_ty_layout("size_t")?;
this.write_scalar(Scalar::from_uint(crate::PAGE_SIZE, guard_size_layout.size), guard_size.into())?;
// Return success (`0`).
this.write_null(dest)?;
}
| "signal"
| "sigaction"

View File

@ -11,13 +11,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
start_routine: OpTy<'tcx, Tag>,
arg: OpTy<'tcx, Tag>,
) -> InterpResult<'tcx, i32> {
println!(
"WARNING: The thread support is experimental. \
For example, Miri does not detect data races yet."
);
let this = self.eval_context_mut();
this.tcx.sess.warn(
"The thread support is experimental. \
For example, Miri does not detect data races yet.",
);
let new_thread_id = this.create_thread()?;
let old_thread_id = this.set_active_thread(new_thread_id)?;
@ -57,6 +57,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
Ok(0)
}
fn pthread_join(
&mut self,
thread: OpTy<'tcx, Tag>,
@ -73,6 +74,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
Ok(0)
}
fn pthread_detach(&mut self, thread: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> {
let this = self.eval_context_mut();
@ -81,12 +83,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
Ok(0)
}
fn pthread_self(&mut self, dest: PlaceTy<'tcx, Tag>) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let thread_id = this.get_active_thread()?;
this.write_scalar(Scalar::from_uint(thread_id.index() as u128, dest.layout.size), dest)
}
fn prctl(
&mut self,
option: OpTy<'tcx, Tag>,

View File

@ -2,6 +2,7 @@
use std::cell::RefCell;
use std::convert::TryFrom;
use std::num::NonZeroU32;
use log::trace;
@ -42,21 +43,18 @@ impl ThreadId {
}
/// An identifier of a set of blocked threads.
///
/// Note: 0 is not a valid identifier.
#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
pub struct BlockSetId(u32);
pub struct BlockSetId(NonZeroU32);
impl From<u32> for BlockSetId {
fn from(id: u32) -> Self {
assert_ne!(id, 0, "0 is not a valid blockset id");
Self(id)
Self(NonZeroU32::new(id).expect("0 is not a valid blockset id"))
}
}
impl BlockSetId {
pub fn to_u32_scalar<'tcx>(&self) -> Scalar<Tag> {
Scalar::from_u32(self.0)
Scalar::from_u32(self.0.get())
}
}
@ -150,6 +148,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
pub fn get_thread_local_alloc_id(&self, static_alloc_id: AllocId) -> Option<AllocId> {
self.thread_local_alloc_ids.borrow().get(&(static_alloc_id, self.active_thread)).cloned()
}
/// Set the allocation id as the allocation id of the given thread local
/// static for the active thread.
pub fn set_thread_local_alloc_id(&self, static_alloc_id: AllocId, new_alloc_id: AllocId) {
@ -161,20 +160,24 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
"Bug: a thread local initialized twice for the same thread."
);
}
/// Borrow the stack of the active thread.
fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Tag, FrameData<'tcx>>] {
&self.threads[self.active_thread].stack
}
/// Mutably borrow the stack of the active thread.
fn active_thread_stack_mut(&mut self) -> &mut Vec<Frame<'mir, 'tcx, Tag, FrameData<'tcx>>> {
&mut self.threads[self.active_thread].stack
}
/// Create a new thread and returns its id.
fn create_thread(&mut self) -> ThreadId {
let new_thread_id = ThreadId::new(self.threads.len());
self.threads.push(Default::default());
new_thread_id
}
/// Set an active thread and return the id of the thread that was active before.
fn set_active_thread_id(&mut self, id: ThreadId) -> ThreadId {
let active_thread_id = self.active_thread;
@ -182,19 +185,23 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
assert!(self.active_thread.index() < self.threads.len());
active_thread_id
}
/// Get the id of the currently active thread.
fn get_active_thread_id(&self) -> ThreadId {
self.active_thread
}
/// Get the borrow of the currently active thread.
fn active_thread_mut(&mut self) -> &mut Thread<'mir, 'tcx> {
&mut self.threads[self.active_thread]
}
/// Mark the thread as detached, which means that no other thread will try
/// to join it and the thread is responsible for cleaning up.
fn detach_thread(&mut self, id: ThreadId) {
self.threads[id].detached = true;
}
/// Mark that the active thread tries to join the thread with `joined_thread_id`.
fn join_thread(&mut self, joined_thread_id: ThreadId) {
assert!(!self.threads[joined_thread_id].detached, "Bug: trying to join a detached thread.");
@ -215,23 +222,32 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
);
}
}
/// Set the name of the active thread.
fn set_thread_name(&mut self, new_thread_name: Vec<u8>) {
self.active_thread_mut().thread_name = Some(new_thread_name);
}
/// Get ids and states of all threads ever allocated.
fn get_all_thread_ids_with_states(&self) -> Vec<(ThreadId, ThreadState)> {
self.threads.iter_enumerated().map(|(id, thread)| (id, thread.state)).collect()
}
/// Allocate a new blockset id.
fn create_blockset(&mut self) -> BlockSetId {
self.blockset_counter = self.blockset_counter.checked_add(1).unwrap();
self.blockset_counter.into()
}
/// Block the currently active thread and put it into the given blockset.
fn block_active_thread(&mut self, set: BlockSetId) {
let state = &mut self.active_thread_mut().state;
assert_eq!(*state, ThreadState::Enabled);
*state = ThreadState::Blocked(set);
}
/// Unblock any one thread from the given blockset if it contains at least
/// one. Return the id of the unblocked thread.
fn unblock_random_thread(&mut self, set: BlockSetId) -> Option<ThreadId> {
for (id, thread) in self.threads.iter_enumerated_mut() {
if thread.state == ThreadState::Blocked(set) {
@ -242,6 +258,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
}
None
}
/// Decide which thread to run next.
///
/// Returns `false` if all threads terminated.
@ -278,60 +295,74 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let this = self.eval_context_ref();
this.machine.threads.get_thread_local_alloc_id(static_alloc_id)
}
fn set_thread_local_alloc_id(&self, static_alloc_id: AllocId, thread_local_alloc_id: AllocId) {
let this = self.eval_context_ref();
this.machine.threads.set_thread_local_alloc_id(static_alloc_id, thread_local_alloc_id)
}
fn create_thread(&mut self) -> InterpResult<'tcx, ThreadId> {
let this = self.eval_context_mut();
Ok(this.machine.threads.create_thread())
}
fn detach_thread(&mut self, thread_id: ThreadId) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
this.machine.threads.detach_thread(thread_id);
Ok(())
}
fn join_thread(&mut self, joined_thread_id: ThreadId) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
this.machine.threads.join_thread(joined_thread_id);
Ok(())
}
fn set_active_thread(&mut self, thread_id: ThreadId) -> InterpResult<'tcx, ThreadId> {
let this = self.eval_context_mut();
Ok(this.machine.threads.set_active_thread_id(thread_id))
}
fn get_active_thread(&self) -> InterpResult<'tcx, ThreadId> {
let this = self.eval_context_ref();
Ok(this.machine.threads.get_active_thread_id())
}
fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Tag, FrameData<'tcx>>] {
let this = self.eval_context_ref();
this.machine.threads.active_thread_stack()
}
fn active_thread_stack_mut(&mut self) -> &mut Vec<Frame<'mir, 'tcx, Tag, FrameData<'tcx>>> {
let this = self.eval_context_mut();
this.machine.threads.active_thread_stack_mut()
}
fn set_active_thread_name(&mut self, new_thread_name: Vec<u8>) -> InterpResult<'tcx, ()> {
let this = self.eval_context_mut();
Ok(this.machine.threads.set_thread_name(new_thread_name))
}
fn get_all_thread_ids_with_states(&mut self) -> Vec<(ThreadId, ThreadState)> {
let this = self.eval_context_mut();
this.machine.threads.get_all_thread_ids_with_states()
}
fn create_blockset(&mut self) -> InterpResult<'tcx, BlockSetId> {
let this = self.eval_context_mut();
Ok(this.machine.threads.create_blockset())
}
fn block_active_thread(&mut self, set: BlockSetId) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
Ok(this.machine.threads.block_active_thread(set))
}
fn unblock_random_thread(&mut self, set: BlockSetId) -> InterpResult<'tcx, Option<ThreadId>> {
let this = self.eval_context_mut();
Ok(this.machine.threads.unblock_random_thread(set))
}
/// Decide which thread to run next.
///
/// Returns `false` if all threads terminated.

View File

@ -0,0 +1,9 @@
// ignore-linux
// ignore-macos
use std::thread;
// error-pattern: Miri does not support threading
fn main() {
thread::spawn(|| {});
}

View File

@ -1,3 +1,5 @@
// ignore-windows
//! This test just calls the relevant APIs to check if Miri crashes.
use std::sync::{Arc, Mutex};

View File

@ -0,0 +1,2 @@
warning: The thread support is experimental. For example, Miri does not detect data races yet.

View File

@ -1,3 +0,0 @@
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.

View File

@ -1,3 +1,5 @@
// ignore-windows
use std::thread;
fn create_and_detach() {

View File

@ -0,0 +1,2 @@
warning: The thread support is experimental. For example, Miri does not detect data races yet.

View File

@ -1,10 +0,0 @@
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.

View File

@ -1,3 +1,5 @@
// ignore-windows
#![feature(thread_local)]
use std::thread;

View File

@ -0,0 +1,2 @@
warning: The thread support is experimental. For example, Miri does not detect data races yet.

View File

@ -1 +0,0 @@
WARNING: The thread support is experimental. For example, Miri does not detect data races yet.