Auto merge of #1157 - divergentdave:shim-pthread-try-lock, r=RalfJung

Add shims for RwLock::try_read/RwLock::try_write

This implements proper locking so that we can check for reentrancy and implement the `try_*` methods.

Fixes #781

Author: bors

src/diagnostics.rs

@@ -12,7 +12,8 @@ pub enum TerminationInfo {
     Exit(i64),
     Abort(Option<String>),
     UnsupportedInIsolation(String),
-    ExperimentalUb { msg: String, url: String }
+    ExperimentalUb { msg: String, url: String },
+    Deadlock,
 }
 
 impl fmt::Debug for TerminationInfo {
@@ -29,6 +30,8 @@ impl fmt::Debug for TerminationInfo {
                 write!(f, "{}", msg),
             ExperimentalUb { msg, .. } =>
                 write!(f, "{}", msg),
+            Deadlock =>
+                write!(f, "the evaluated program deadlocked"),
         }
     }
 }
@@ -60,6 +63,7 @@ pub fn report_error<'tcx, 'mir>(
                     "unsupported operation",
                 ExperimentalUb { .. } =>
                     "Undefined Behavior",
+                Deadlock => "deadlock",
             };
             let helps = match info {
                 UnsupportedInIsolation(_) =>

src/eval.rs

@@ -1,14 +1,14 @@
 //! Main evaluator loop and setting up the initial stack frame.
 
-use std::ffi::OsStr;
 use std::convert::TryFrom;
+use std::ffi::OsStr;
 
 use rand::rngs::StdRng;
 use rand::SeedableRng;
 
-use rustc_target::abi::LayoutOf;
-use rustc_middle::ty::{self, TyCtxt};
 use rustc_hir::def_id::DefId;
+use rustc_middle::ty::{self, layout::LayoutCx, TyCtxt};
+use rustc_target::abi::LayoutOf;
 
 use crate::*;
 
@@ -60,10 +60,13 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     main_id: DefId,
     config: MiriConfig,
 ) -> InterpResult<'tcx, (InterpCx<'mir, 'tcx, Evaluator<'tcx>>, MPlaceTy<'tcx, Tag>)> {
+    let tcx_at = tcx.at(rustc_span::source_map::DUMMY_SP);
+    let param_env = ty::ParamEnv::reveal_all();
+    let layout_cx = LayoutCx { tcx, param_env };
     let mut ecx = InterpCx::new(
-        tcx.at(rustc_span::source_map::DUMMY_SP),
-        ty::ParamEnv::reveal_all(),
-        Evaluator::new(config.communicate, config.validate),
+        tcx_at,
+        param_env,
+        Evaluator::new(config.communicate, config.validate, layout_cx),
         MemoryExtra::new(
             StdRng::seed_from_u64(config.seed.unwrap_or(0)),
             config.stacked_borrows,

src/lib.rs

@@ -39,6 +39,7 @@ pub use crate::shims::fs::{DirHandler, EvalContextExt as FileEvalContextExt, Fil
 pub use crate::shims::intrinsics::EvalContextExt as IntrinsicsEvalContextExt;
 pub use crate::shims::os_str::EvalContextExt as OsStrEvalContextExt;
 pub use crate::shims::panic::{CatchUnwindData, EvalContextExt as PanicEvalContextExt};
+pub use crate::shims::sync::{EvalContextExt as SyncEvalContextExt};
 pub use crate::shims::time::EvalContextExt as TimeEvalContextExt;
 pub use crate::shims::tls::{EvalContextExt as TlsEvalContextExt, TlsData};
 pub use crate::shims::EvalContextExt as ShimsEvalContextExt;

src/machine.rs

@@ -10,11 +10,18 @@ use std::time::Instant;
 use log::trace;
 use rand::rngs::StdRng;
 
-use rustc_data_structures::fx::FxHashMap;
-use rustc_middle::{mir, ty};
-use rustc_target::abi::{LayoutOf, Size};
 use rustc_ast::attr;
+use rustc_data_structures::fx::FxHashMap;
+use rustc_middle::{
+    mir,
+    ty::{
+        self,
+        layout::{LayoutCx, LayoutError, TyAndLayout},
+        TyCtxt,
+    },
+};
 use rustc_span::symbol::{sym, Symbol};
+use rustc_target::abi::{LayoutOf, Size};
 
 use crate::*;
 
@@ -146,6 +153,21 @@ impl MemoryExtra {
     }
 }
 
+/// Precomputed layouts of primitive types
+pub(crate) struct PrimitiveLayouts<'tcx> {
+    pub(crate) i32: TyAndLayout<'tcx>,
+    pub(crate) u32: TyAndLayout<'tcx>,
+}
+
+impl<'mir, 'tcx: 'mir> PrimitiveLayouts<'tcx> {
+    fn new(layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) -> Result<Self, LayoutError<'tcx>> {
+        Ok(Self {
+            i32: layout_cx.layout_of(layout_cx.tcx.types.i32)?,
+            u32: layout_cx.layout_of(layout_cx.tcx.types.u32)?,
+        })
+    }
+}
+
 /// The machine itself.
 pub struct Evaluator<'tcx> {
     /// Environment variables set by `setenv`.
@@ -182,10 +204,21 @@ pub struct Evaluator<'tcx> {
 
     /// The "time anchor" for this machine's monotone clock (for `Instant` simulation).
     pub(crate) time_anchor: Instant,
+
+    /// Precomputed `TyLayout`s for primitive data types that are commonly used inside Miri.
+    /// FIXME: Search through the rest of the codebase for more layout_of() calls that
+    /// could be stored here.
+    pub(crate) layouts: PrimitiveLayouts<'tcx>,
 }
 
 impl<'tcx> Evaluator<'tcx> {
-    pub(crate) fn new(communicate: bool, validate: bool) -> Self {
+    pub(crate) fn new(
+        communicate: bool,
+        validate: bool,
+        layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
+    ) -> Self {
+        let layouts = PrimitiveLayouts::new(layout_cx)
+            .expect("Couldn't get layouts of primitive types");
         Evaluator {
             // `env_vars` could be initialized properly here if `Memory` were available before
             // calling this method.
@@ -201,6 +234,7 @@ impl<'tcx> Evaluator<'tcx> {
             dir_handler: Default::default(),
             panic_payload: None,
             time_anchor: Instant::now(),
+            layouts,
         }
     }
 }

src/shims/foreign_items/posix.rs

@@ -233,6 +233,64 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_null(dest)?;
             }
 
+            // Synchronization primitives
+            "pthread_mutexattr_init" => {
+                let result = this.pthread_mutexattr_init(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_mutexattr_settype" => {
+                let result = this.pthread_mutexattr_settype(args[0], args[1])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_mutexattr_destroy" => {
+                let result = this.pthread_mutexattr_destroy(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_mutex_init" => {
+                let result = this.pthread_mutex_init(args[0], args[1])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_mutex_lock" => {
+                let result = this.pthread_mutex_lock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_mutex_trylock" => {
+                let result = this.pthread_mutex_trylock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_mutex_unlock" => {
+                let result = this.pthread_mutex_unlock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_mutex_destroy" => {
+                let result = this.pthread_mutex_destroy(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_rwlock_rdlock" => {
+                let result = this.pthread_rwlock_rdlock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_rwlock_tryrdlock" => {
+                let result = this.pthread_rwlock_tryrdlock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_rwlock_wrlock" => {
+                let result = this.pthread_rwlock_wrlock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_rwlock_trywrlock" => {
+                let result = this.pthread_rwlock_trywrlock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_rwlock_unlock" => {
+                let result = this.pthread_rwlock_unlock(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+            "pthread_rwlock_destroy" => {
+                let result = this.pthread_rwlock_destroy(args[0])?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
+
             // Better error for attempts to create a thread
             "pthread_create" => {
                 throw_unsup_format!("Miri does not support threading");
@@ -255,25 +313,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_null(dest)?;
             }
 
-            // Incomplete shims that we "stub out" just to get pre-main initialziation code to work.
+            // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
             // These shims are enabled only when the caller is in the standard library.
             | "pthread_attr_init"
             | "pthread_attr_destroy"
             | "pthread_self"
-            | "pthread_attr_setstacksize" if this.frame().instance.to_string().starts_with("std::sys::unix::") => {
-                this.write_null(dest)?;
-            }
-            | "pthread_mutexattr_init"
-            | "pthread_mutexattr_settype"
-            | "pthread_mutex_init"
-            | "pthread_mutexattr_destroy"
-            | "pthread_mutex_lock"
-            | "pthread_mutex_unlock"
-            | "pthread_mutex_destroy"
-            | "pthread_rwlock_rdlock"
-            | "pthread_rwlock_unlock"
-            | "pthread_rwlock_wrlock"
-            | "pthread_rwlock_destroy"
+            | "pthread_attr_setstacksize"
             | "pthread_condattr_init"
             | "pthread_condattr_setclock"
             | "pthread_cond_init"
@@ -282,6 +327,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             => {
                 this.write_null(dest)?;
             }
+
             | "signal"
             | "sigaction"
             | "sigaltstack"

src/shims/foreign_items/posix/linux.rs

@@ -113,7 +113,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_scalar(Scalar::from_i32(-1), dest)?;
             }
 
-            // Incomplete shims that we "stub out" just to get pre-main initialziation code to work.
+            // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
             // These shims are enabled only when the caller is in the standard library.
             "pthread_getattr_np" if this.frame().instance.to_string().starts_with("std::sys::unix::") => {
                 this.write_null(dest)?;

src/shims/foreign_items/posix/macos.rs

@@ -88,7 +88,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_scalar(stack_size, dest)?;
             }
 
-            // Incomplete shims that we "stub out" just to get pre-main initialziation code to work.
+            // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
             // These shims are enabled only when the caller is in the standard library.
             "mmap" if this.frame().instance.to_string().starts_with("std::sys::unix::") => {
                 // This is a horrible hack, but since the guard page mechanism calls mmap and expects a particular return value, we just give it that value.

src/shims/foreign_items/windows.rs

@@ -207,7 +207,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 throw_unsup_format!("Miri does not support threading");
             }
 
-            // Incomplete shims that we "stub out" just to get pre-main initialziation code to work.
+            // Incomplete shims that we "stub out" just to get pre-main initialization code to work.
             // These shims are enabled only when the caller is in the standard library.
             "GetProcessHeap" if this.frame().instance.to_string().starts_with("std::sys::windows::") => {
                 // Just fake a HANDLE
@@ -233,6 +233,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 // (Windows locks are reentrant, and we have only 1 thread,
                 // so not doing any futher checks here is at least not incorrect.)
             }
+            "TryEnterCriticalSection" if this.frame().instance.to_string().starts_with("std::sys::windows::")
+            => {
+                // There is only one thread, so this always succeeds and returns TRUE
+                this.write_scalar(Scalar::from_i32(1), dest)?;
+            }
 
             _ => throw_unsup_format!("can't call foreign function: {}", link_name),
         }

src/shims/mod.rs

@@ -5,6 +5,7 @@ pub mod fs;
 pub mod intrinsics;
 pub mod os_str;
 pub mod panic;
+pub mod sync;
 pub mod time;
 pub mod tls;