[ThreadSafety] Treating unique_lock.

Reviewed By: jberdine

Differential Revision: D5301590

fbshipit-source-id: db5e42d

infer/src/checkers/ThreadSafety.ml

@@ -84,16 +84,24 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
 
   let get_lock_model =
     let is_cpp_lock =
-      let matcher = QualifiedCppName.Match.of_fuzzy_qual_names [
+      let matcher_lock = QualifiedCppName.Match.of_fuzzy_qual_names [
-          "std::mutex::lock"; "std::lock_guard::lock_guard"] in
+          "std::mutex::lock"; "std::unique_lock::lock"] in
-      fun pname ->
+      let matcher_lock_constructor = QualifiedCppName.Match.of_fuzzy_qual_names [
-        QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname)
+          "std::lock_guard::lock_guard"; "std::unique_lock::unique_lock"] in
-    and is_std_mutex_unlock =
+      fun pname actuals ->
-      let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::mutex::unlock"] in
+        QualifiedCppName.Match.match_qualifiers matcher_lock (Typ.Procname.get_qualifiers pname)
+        || (QualifiedCppName.Match.match_qualifiers matcher_lock_constructor
+              (Typ.Procname.get_qualifiers pname)
+            (* Passing additional parameter allows to defer the lock *)
+            && (Int.equal 2 (List.length actuals)))
+    and is_cpp_unlock =
+      let matcher = QualifiedCppName.Match.of_fuzzy_qual_names
+          ["std::mutex::unlock"; "std::unique_lock::unlock"] in
       fun pname ->
         QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname)
     in
-    function
+    fun pname actuals ->
+      match pname with
       | Typ.Procname.Java java_pname ->
           if is_thread_utils_method "assertHoldsLock" (Typ.Procname.Java java_pname) then Lock
           else
@@ -124,9 +132,9 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
               | _ ->
                   NoEffect
             end
-    | (Typ.Procname.ObjC_Cpp _ as pname) when is_cpp_lock pname ->
+      | (Typ.Procname.ObjC_Cpp _ as pname) when is_cpp_lock pname actuals ->
           Lock
-    | (Typ.Procname.ObjC_Cpp _ as pname) when is_std_mutex_unlock pname ->
+      | (Typ.Procname.ObjC_Cpp _ as pname) when is_cpp_unlock pname ->
           Unlock
       | pname when Typ.Procname.equal pname BuiltinDecl.__set_locked_attribute ->
           Lock
@@ -557,7 +565,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
           then
             { astate with threads = true; }
           else
-            match get_lock_model callee_pname with
+            match get_lock_model callee_pname actuals with
             | Lock ->
                 { astate with locks = true; }
             | Unlock ->
@@ -960,8 +968,10 @@ let analyze_procedure { Callbacks.proc_desc; tenv; summary; } =
              lock at the end of the procedure, as destructors are not modeled yet *)
           let update_locks = match Procdesc.get_proc_name proc_desc with
             | ObjC_Cpp _ when locks ->
-                let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::lock_guard"] in
+                let matcher = QualifiedCppName.Match.of_fuzzy_qual_names
-                (* Unlock, if the procedure contains a local field of type std::lock_guard *)
+                    ["std::lock_guard"; "std::unique_lock"] in
+                (* Unlock, if the procedure contains a local field
+                   of type std::lock_guard or std::unique_lock *)
                 not (List.exists (Procdesc.get_locals proc_desc) ~f:(fun (_, ft) ->
                     Option.exists (Typ.name ft) ~f:(fun name ->
                         QualifiedCppName.Match.match_qualifiers matcher (Typ.Name.qual_name name))

infer/tests/codetoanalyze/cpp/threadsafety/issues.exp

@@ -5,3 +5,7 @@ codetoanalyze/cpp/threadsafety/lock_guard.cpp, basics::LockGuard_get4, 0, THREAD
 codetoanalyze/cpp/threadsafety/lock_guard.cpp, basics::LockGuard_test1, 2, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_read`,<Beginning of write trace>,access to `suspiciously_read`]
 codetoanalyze/cpp/threadsafety/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get3, 0, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `not_guarded`,<Beginning of write trace>,access to `not_guarded`]
 codetoanalyze/cpp/threadsafety/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get4, 0, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_read`,<Beginning of write trace>,access to `suspiciously_read`]
+codetoanalyze/cpp/threadsafety/unique_lock.cpp, basics::UniqueLock_get2, 3, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_written1`,<Beginning of write trace>,access to `suspiciously_written1`]
+codetoanalyze/cpp/threadsafety/unique_lock.cpp, basics::UniqueLock_get2, 4, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_written2`,<Beginning of write trace>,access to `suspiciously_written2`]
+codetoanalyze/cpp/threadsafety/unique_lock.cpp, basics::UniqueLock_get4, 1, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_read1`,<Beginning of write trace>,access to `suspiciously_read1`]
+codetoanalyze/cpp/threadsafety/unique_lock.cpp, basics::UniqueLock_get4, 2, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_read2`,<Beginning of write trace>,access to `suspiciously_read2`]

infer/tests/codetoanalyze/cpp/threadsafety/unique_lock.cpp

@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+#include <mutex>
+
+namespace basics {
+
+class UniqueLock {
+ public:
+  UniqueLock() {}
+
+  void set(int new_value) {
+    std::unique_lock<std::mutex> g(mutex_);
+    well_guarded1 = new_value;
+    suspiciously_read1 = new_value;
+    g.unlock();
+    not_guarded1 = new_value;
+    suspiciously_written1 = new_value;
+  }
+
+  void set2(int new_value) {
+    std::unique_lock<std::mutex> g(mutex_, std::defer_lock);
+    not_guarded2 = new_value;
+    suspiciously_written2 = new_value;
+    g.lock();
+    well_guarded2 = new_value;
+    suspiciously_read2 = new_value;
+    g.unlock();
+  }
+
+  int get1() {
+    int result;
+    std::lock_guard<std::mutex> lock(mutex_);
+    result = well_guarded1;
+    return result + well_guarded2;
+  }
+
+  int get2() {
+    int result;
+    std::lock_guard<std::mutex> lock(mutex_);
+    result = suspiciously_written1;
+    return result + suspiciously_written2;
+  }
+
+  int get3() {
+    int result = not_guarded1;
+    return result + not_guarded2;
+  }
+
+  int get4() {
+    int result = suspiciously_read1;
+    return result + suspiciously_read2;
+  }
+
+ private:
+  int well_guarded1;
+  int suspiciously_read1;
+  int suspiciously_written1;
+  int not_guarded1;
+  int well_guarded2;
+  int suspiciously_read2;
+  int suspiciously_written2;
+  int not_guarded2;
+  std::mutex mutex_;
+};
+}