Improvement: Check ALL foreign function calls for a sync variable

The idea is to perform the check in `link_return_value_if_sync_variable` for all foreign function calls.

This saves us having to maintain a list of "supported functions" function from the std where we actually track if a mutex/lock guard/condvar/join handle is being passed around. It also simplifies the number of cases in FunctionCall that we need to handle.

The new solution is more robust since it always performs the check, making it "future-proof" in a certain way.
In the process, we needed to handle extracting the arguments in a different way that ignores constants and skips the check if the function has zero arguments.

Author: hlisdero

src/translator/function_call.rs

@@ -21,9 +21,6 @@ pub enum FunctionCall {
     /// Abridged function call.
     /// Non-recursive call for the translation process.
     Foreign,
-    /// Abridged function call that involves a synchronization primitive.
-    /// Non-recursive call for the translation process.
-    ForeignWithSyncPrimitive,
     /// MIR function call (the "default" case).
     /// Recursive call for the translation process.
     MirFunction,
@@ -62,11 +59,6 @@ impl FunctionCall {
     /// Returns the corresponding variant for the function or `None` otherwise.
     fn is_supported_function(function_name: &str) -> Option<Self> {
         match function_name {
-            "std::clone::Clone::clone"
-            | "std::ops::Deref::deref"
-            | "std::ops::DerefMut::deref_mut"
-            | "std::result::Result::<T, E>::unwrap"
-            | "std::sync::Arc::<T>::new" => Some(Self::ForeignWithSyncPrimitive),
             "std::sync::Condvar::new" => Some(Self::CondVarNew),
             "std::sync::Condvar::notify_one" => Some(Self::CondVarNotifyOne),
             "std::sync::Condvar::wait" => Some(Self::CondVarWait),

src/translator/function_call_handler.rs

@@ -22,25 +22,27 @@ impl<'tcx> Translator<'tcx> {
         let function_name = self.tcx.def_path_str(function_def_id);
 
         match function_call {
-            FunctionCall::ForeignWithSyncPrimitive => {
-                self.call_foreign_function_with_sync_primitive(
+            FunctionCall::CondVarNew => {
+                self.call_condvar_new(&function_name, args, destination, &function_call_places);
+            }
+            FunctionCall::CondVarNotifyOne => {
+                self.call_condvar_notify_one(
                     &function_name,
                     args,
                     destination,
                     &function_call_places,
                 );
             }
-            FunctionCall::CondVarNew => {
-                self.call_condvar_new(&function_name, destination, &function_call_places);
-            }
-            FunctionCall::CondVarNotifyOne => {
-                self.call_condvar_notify_one(&function_name, args, &function_call_places);
-            }
             FunctionCall::CondVarWait => {
                 self.call_condvar_wait(args, destination, &function_call_places);
             }
             FunctionCall::Foreign => {
-                self.call_foreign_function(&function_name, &function_call_places);
+                self.call_foreign_function(
+                    &function_name,
+                    args,
+                    destination,
+                    &function_call_places,
+                );
             }
             FunctionCall::MirFunction => {
                 let (start_place, end_place, _) = function_call_places;
@@ -52,10 +54,10 @@ impl<'tcx> Translator<'tcx> {
                 self.call_mutex_lock(&function_name, args, destination, &function_call_places);
             }
             FunctionCall::MutexNew => {
-                self.call_mutex_new(&function_name, destination, &function_call_places);
+                self.call_mutex_new(&function_name, args, destination, &function_call_places);
             }
             FunctionCall::ThreadJoin => {
-                self.call_thread_join(&function_name, args, &function_call_places);
+                self.call_thread_join(&function_name, args, destination, &function_call_places);
             }
             FunctionCall::ThreadSpawn => {
                 self.call_thread_spawn(&function_name, args, destination, &function_call_places);
@@ -72,34 +74,25 @@ impl<'tcx> Translator<'tcx> {
     /// A separate counter is incremented every time that
     /// the function is called to generate a unique label.
     ///
+    /// Performs a check to keep track of synchronization primitives.
+    /// In case the first argument is a mutex, lock guard, join handle or condition variable,
+    /// it links the first argument of the function to its return value.
+    ///
     /// Returns the transition that represents the function call.
     fn call_foreign_function(
         &mut self,
         function_name: &str,
+        args: &[rustc_middle::mir::Operand<'tcx>],
+        destination: rustc_middle::mir::Place<'tcx>,
         function_call_places: &FunctionPlaces,
     ) -> TransitionRef {
         let index = self.function_counter.get_count(function_name);
         self.function_counter.increment(function_name);
-        call_foreign_function(
+        let function_transition = call_foreign_function(
             function_call_places,
             &foreign_call_transition_labels(function_name, index),
             &mut self.net,
-        )
-    }
-
-    /// Handler for the case `FunctionCall::ForeignWithSyncPrimitive`.
-    /// It is an extension of `call_foreign_function` that performs a check
-    /// to keep track of synchronization primitives.
-    /// The goal is to link the first argument of the function to its return value
-    /// in case the first argument is a mutex, lock guard, join handle or condition variable.
-    fn call_foreign_function_with_sync_primitive(
-        &mut self,
-        function_name: &str,
-        args: &[rustc_middle::mir::Operand<'tcx>],
-        destination: rustc_middle::mir::Place<'tcx>,
-        function_call_places: &FunctionPlaces,
-    ) {
-        self.call_foreign_function(function_name, function_call_places);
+        );
 
         let current_function = self.call_stack.peek_mut();
         link_return_value_if_sync_variable(
@@ -109,16 +102,19 @@ impl<'tcx> Translator<'tcx> {
             current_function.def_id,
             self.tcx,
         );
+
+        function_transition
     }
 
     /// Handler for the case `FunctionCall::CondvarNew`.
     fn call_condvar_new(
         &mut self,
         function_name: &str,
+        args: &[rustc_middle::mir::Operand<'tcx>],
         destination: rustc_middle::mir::Place<'tcx>,
         function_call_places: &FunctionPlaces,
     ) {
-        self.call_foreign_function(function_name, function_call_places);
+        self.call_foreign_function(function_name, args, destination, function_call_places);
 
         let current_function = self.call_stack.peek_mut();
         self.condvar_manager.translate_side_effects_new(
@@ -133,9 +129,11 @@ impl<'tcx> Translator<'tcx> {
         &mut self,
         function_name: &str,
         args: &[rustc_middle::mir::Operand<'tcx>],
+        destination: rustc_middle::mir::Place<'tcx>,
         function_call_places: &FunctionPlaces,
     ) {
-        let notify_one_transition = self.call_foreign_function(function_name, function_call_places);
+        let notify_one_transition =
+            self.call_foreign_function(function_name, args, destination, function_call_places);
 
         let current_function = self.call_stack.peek_mut();
         self.condvar_manager.translate_side_effects_notify_one(
@@ -177,7 +175,7 @@ impl<'tcx> Translator<'tcx> {
         function_call_places: &FunctionPlaces,
     ) {
         let transition_function_call =
-            self.call_foreign_function(function_name, function_call_places);
+            self.call_foreign_function(function_name, args, destination, function_call_places);
 
         let current_function = self.call_stack.peek_mut();
         self.mutex_manager.translate_side_effects_lock(
@@ -193,10 +191,11 @@ impl<'tcx> Translator<'tcx> {
     fn call_mutex_new(
         &mut self,
         function_name: &str,
+        args: &[rustc_middle::mir::Operand<'tcx>],
         destination: rustc_middle::mir::Place<'tcx>,
         function_call_places: &FunctionPlaces,
     ) {
-        self.call_foreign_function(function_name, function_call_places);
+        self.call_foreign_function(function_name, args, destination, function_call_places);
 
         let current_function = self.call_stack.peek_mut();
         self.mutex_manager.translate_side_effects_new(
@@ -211,10 +210,11 @@ impl<'tcx> Translator<'tcx> {
         &mut self,
         function_name: &str,
         args: &[rustc_middle::mir::Operand<'tcx>],
+        destination: rustc_middle::mir::Place<'tcx>,
         function_call_places: &FunctionPlaces,
     ) {
         let transition_function_call =
-            self.call_foreign_function(function_name, function_call_places);
+            self.call_foreign_function(function_name, args, destination, function_call_places);
 
         let current_function = self.call_stack.peek();
         self.thread_manager.translate_side_effects_join(
@@ -233,7 +233,7 @@ impl<'tcx> Translator<'tcx> {
         function_call_places: &FunctionPlaces,
     ) {
         let transition_function_call =
-            self.call_foreign_function(function_name, function_call_places);
+            self.call_foreign_function(function_name, args, destination, function_call_places);
 
         let current_function = self.call_stack.peek_mut();
         self.thread_manager.translate_side_effects_spawn(

src/translator/sync.rs

@@ -11,7 +11,7 @@ mod thread;
 mod thread_manager;
 
 use crate::translator::mir_function::Memory;
-use crate::utils::{check_substring_in_place_type, extract_nth_argument};
+use crate::utils::{check_substring_in_place_type, extract_nth_argument_as_place};
 
 pub use condvar::Condvar;
 pub use condvar_manager::{CondvarManager, CondvarRef};
@@ -145,6 +145,17 @@ fn generalized_link_place_if_sync_variable<'tcx>(
 /// Checks if the first argument for a function call contains a mutex, a lock guard,
 /// a join handle or a condition variable, i.e. a synchronization variable.
 /// If the first argument contains a synchronization variable, links it to the return value.
+/// If there is no first argument or it is a constant,
+/// then there is nothing to check, therefore the function simply returns.
+///
+/// Why check only the first argument?
+/// Because most function in the standard library involving synchronization primitives
+/// receive it through the first argument. For instance:
+///  * `std::clone::Clone::clone`
+///  * `std::ops::Deref::deref`
+///  * `std::ops::DerefMut::deref_mut`
+///  * `std::result::Result::<T, E>::unwrap`
+///  * `std::sync::Arc::<T>::new`
 ///
 /// Receives a reference to the memory of the caller function to
 /// link the return local variable to the synchronization variable.
@@ -155,7 +166,10 @@ pub fn link_return_value_if_sync_variable<'tcx>(
     caller_function_def_id: rustc_hir::def_id::DefId,
     tcx: rustc_middle::ty::TyCtxt<'tcx>,
 ) {
-    let first_argument = extract_nth_argument(args, 0);
+    let Some(first_argument) = extract_nth_argument_as_place(args, 0) else {
+         // Nothing to check: Either the first argument is not present or it is a constant.
+        return;
+    };
     link_if_sync_variable(
         &return_value,
         &first_argument,

src/utils.rs

@@ -69,6 +69,32 @@ pub fn extract_nth_argument<'tcx>(
     }
 }
 
+/// Extracts the n-th argument from the arguments for the function call.
+/// Returns the place corresponding to that argument.
+///
+/// This is also useful for obtaining the self reference for method calls.
+/// For example: The call `mutex.lock()` desugars to `std::sync::Mutex::lock(&mutex)`
+/// where `&self` is the first argument.
+///
+/// If the argument can not be found (the array is shorter than the `index` argument)
+/// or the argument is a constant (which does not have a `Place` representation),
+/// then the function returns `None`.
+pub fn extract_nth_argument_as_place<'tcx>(
+    args: &[rustc_middle::mir::Operand<'tcx>],
+    index: usize,
+) -> Option<rustc_middle::mir::Place<'tcx>> {
+    let Some(operand) = args.get(index) else {
+        return None;
+    };
+
+    match operand {
+        rustc_middle::mir::Operand::Move(place) | rustc_middle::mir::Operand::Copy(place) => {
+            Some(*place)
+        }
+        rustc_middle::mir::Operand::Constant(_) => None,
+    }
+}
+
 /// Extracts the closure passed as the 0-th argument to `std::thread::spawn`.
 /// Returns the place corresponding to that argument.
 ///