refactor mutex interception argument handling

Author: RalfJung

src/concurrency/genmc/intercept.rs

@@ -1,47 +1,37 @@
-use rustc_middle::throw_unsup_format;
+use rustc_middle::{throw_unsup_format, ty};
 use tracing::debug;
 
-use crate::concurrency::genmc::MAX_ACCESS_SIZE;
 use crate::concurrency::thread::EvalContextExt as _;
-use crate::{
-    BlockReason, InterpResult, MachineCallback, MiriInterpCx, OpTy, Scalar, UnblockKind,
-    VisitProvenance, VisitWith, callback, interp_ok, throw_ub_format,
-};
+use crate::*;
 
 // Handling of code intercepted by Miri in GenMC mode, such as assume statement or `std::sync::Mutex`.
 
-#[derive(Clone, Copy)]
-struct MutexMethodArgs {
-    address: u64,
-    size: u64,
-}
-
 impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
 trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
-    fn parse_mutex_method_args(
+    /// Small helper to get the arguments of an intercepted function call.
+    fn get_fn_args<const N: usize>(
         &self,
-        args: &[rustc_const_eval::interpret::FnArg<'tcx, crate::Provenance>],
-    ) -> InterpResult<'tcx, MutexMethodArgs> {
-        assert_eq!(args.len(), 1, "Mutex lock/unlock/try_lock should take exactly 1 argument.");
+        instance: ty::Instance<'tcx>,
+        args: &[FnArg<'tcx>],
+    ) -> InterpResult<'tcx, [OpTy<'tcx>; N]> {
         let this = self.eval_context_ref();
-        let arg = this.copy_fn_arg(&args[0]);
-        // FIXME(genmc): use actual size of the pointee of `arg`.
-        let size = 1;
-        // GenMC does not support large accesses, we limit the size to the maximum access size.
-        interp_ok(MutexMethodArgs {
-            address: this.read_target_usize(&arg)?,
-            size: size.min(MAX_ACCESS_SIZE),
-        })
+        let args = this.copy_fn_args(args); // FIXME: Should `InPlace` arguments be reset to uninit?
+        if let Ok(ops) = args.try_into() {
+            return interp_ok(ops);
+        }
+        panic!("{} is a diagnostic item expected to have {} arguments", instance, N);
     }
 
-    fn intercept_mutex_lock(&mut self, args: MutexMethodArgs) -> InterpResult<'tcx> {
+    fn intercept_mutex_lock(&mut self, mutex: MPlaceTy<'tcx>) -> InterpResult<'tcx> {
         debug!("GenMC: handling Mutex::lock()");
-        let MutexMethodArgs { address, size } = args;
         let this = self.eval_context_mut();
         let genmc_ctx = this.machine.data_race.as_genmc_ref().unwrap();
-        let genmc_tid = genmc_ctx.active_thread_genmc_tid(&this.machine);
-        let result =
-            genmc_ctx.handle.borrow_mut().pin_mut().handle_mutex_lock(genmc_tid, address, size);
+
+        let result = genmc_ctx.handle.borrow_mut().pin_mut().handle_mutex_lock(
+            genmc_ctx.active_thread_genmc_tid(&this.machine),
+            mutex.ptr().addr().bytes(),
+            mutex.layout.size.bytes(),
+        );
         if let Some(error) = result.error.as_ref() {
             // FIXME(genmc): improve error handling.
             throw_ub_format!("{}", error.to_string_lossy());
@@ -58,9 +48,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 None,
                 crate::callback!(
                     @capture<'tcx> {
-                        genmc_tid: i32,
-                        address: u64,
-                        size: u64,
+                        mutex: MPlaceTy<'tcx>,
                     }
                     |this, unblock: crate::UnblockKind| {
                         debug!("GenMC: handling Mutex::lock: unblocking callback called.");
@@ -69,7 +57,11 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                         let result = genmc_ctx.handle
                             .borrow_mut()
                             .pin_mut()
-                            .handle_mutex_lock(genmc_tid, address, size);
+                            .handle_mutex_lock(
+                                genmc_ctx.active_thread_genmc_tid(&this.machine),
+                                mutex.ptr().addr().bytes(),
+                                mutex.layout.size.bytes(),
+                            );
                         if let Some(error) = result.error.as_ref() {
                             // FIXME(genmc): improve error handling.
                             throw_ub_format!("{}", error.to_string_lossy());
@@ -89,16 +81,16 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
     fn intercept_mutex_try_lock(
         &mut self,
-        args: MutexMethodArgs,
+        mutex: MPlaceTy<'tcx>,
         dest: &crate::PlaceTy<'tcx>,
     ) -> InterpResult<'tcx> {
         debug!("GenMC: handling Mutex::try_lock()");
         let this = self.eval_context_mut();
         let genmc_ctx = this.machine.data_race.as_genmc_ref().unwrap();
         let result = genmc_ctx.handle.borrow_mut().pin_mut().handle_mutex_try_lock(
             genmc_ctx.active_thread_genmc_tid(&this.machine),
-            args.address,
-            args.size,
+            mutex.ptr().addr().bytes(),
+            mutex.layout.size.bytes(),
         );
         if let Some(error) = result.error.as_ref() {
             // FIXME(genmc): improve error handling.
@@ -111,14 +103,14 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         interp_ok(())
     }
 
-    fn intercept_mutex_unlock(&self, args: MutexMethodArgs) -> InterpResult<'tcx> {
+    fn intercept_mutex_unlock(&self, mutex: MPlaceTy<'tcx>) -> InterpResult<'tcx> {
         debug!("GenMC: handling Mutex::unlock()");
         let this = self.eval_context_ref();
         let genmc_ctx = this.machine.data_race.as_genmc_ref().unwrap();
         let result = genmc_ctx.handle.borrow_mut().pin_mut().handle_mutex_unlock(
             genmc_ctx.active_thread_genmc_tid(&this.machine),
-            args.address,
-            args.size,
+            mutex.ptr().addr().bytes(),
+            mutex.layout.size.bytes(),
         );
         if let Some(error) = result.error.as_ref() {
             // FIXME(genmc): improve error handling.
@@ -147,19 +139,23 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         // NOTE: When adding new intercepted functions here, they must also be added to `fn get_function_kind` in `concurrency/genmc/scheduling.rs`.
         use rustc_span::sym;
-        interp_ok(if this.tcx.is_diagnostic_item(sym::sys_mutex_lock, instance.def_id()) {
-            this.intercept_mutex_lock(this.parse_mutex_method_args(args)?)?;
-            true
+        if this.tcx.is_diagnostic_item(sym::sys_mutex_lock, instance.def_id()) {
+            let [mutex] = this.get_fn_args(instance, args)?;
+            let mutex = this.deref_pointer(&mutex)?;
+            this.intercept_mutex_lock(mutex)?;
         } else if this.tcx.is_diagnostic_item(sym::sys_mutex_try_lock, instance.def_id()) {
-            this.intercept_mutex_try_lock(this.parse_mutex_method_args(args)?, dest)?;
-            true
+            let [mutex] = this.get_fn_args(instance, args)?;
+            let mutex = this.deref_pointer(&mutex)?;
+            this.intercept_mutex_try_lock(mutex, dest)?;
         } else if this.tcx.is_diagnostic_item(sym::sys_mutex_unlock, instance.def_id()) {
-            this.intercept_mutex_unlock(this.parse_mutex_method_args(args)?)?;
-            true
+            let [mutex] = this.get_fn_args(instance, args)?;
+            let mutex = this.deref_pointer(&mutex)?;
+            this.intercept_mutex_unlock(mutex)?;
         } else {
             // Nothing to intercept.
-            false
-        })
+            return interp_ok(false);
+        }
+        interp_ok(true)
     }
 
     /// Handle an `assume` statement. This will tell GenMC to block the current thread if the `condition` is false.

src/lib.rs

@@ -110,6 +110,7 @@ pub type StrictPointer = interpret::Pointer<machine::Provenance>;
 pub type Scalar = interpret::Scalar<machine::Provenance>;
 pub type ImmTy<'tcx> = interpret::ImmTy<'tcx, machine::Provenance>;
 pub type OpTy<'tcx> = interpret::OpTy<'tcx, machine::Provenance>;
+pub type FnArg<'tcx> = interpret::FnArg<'tcx, machine::Provenance>;
 pub type PlaceTy<'tcx> = interpret::PlaceTy<'tcx, machine::Provenance>;
 pub type MPlaceTy<'tcx> = interpret::MPlaceTy<'tcx, machine::Provenance>;
 

src/machine.rs

@@ -1165,7 +1165,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
         ecx: &mut MiriInterpCx<'tcx>,
         instance: ty::Instance<'tcx>,
         abi: &FnAbi<'tcx, Ty<'tcx>>,
-        args: &[FnArg<'tcx, Provenance>],
+        args: &[FnArg<'tcx>],
         dest: &PlaceTy<'tcx>,
         ret: Option<mir::BasicBlock>,
         unwind: mir::UnwindAction,
@@ -1201,7 +1201,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
         ecx: &mut MiriInterpCx<'tcx>,
         fn_val: DynSym,
         abi: &FnAbi<'tcx, Ty<'tcx>>,
-        args: &[FnArg<'tcx, Provenance>],
+        args: &[FnArg<'tcx>],
         dest: &PlaceTy<'tcx>,
         ret: Option<mir::BasicBlock>,
         unwind: mir::UnwindAction,