[ThreadSafety] Treating lock_guard destructor as Unlock.

Reviewed By: sblackshear

Differential Revision: D5381097

fbshipit-source-id: 0001949

infer/src/checkers/ThreadSafety.ml

@@ -73,7 +73,10 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
     and is_cpp_unlock =
       let matcher =
         QualifiedCppName.Match.of_fuzzy_qual_names
-          ["std::mutex::unlock"; "std::unique_lock::unlock"]
+          [ "std::mutex::unlock"
+          ; "std::unique_lock::unlock"
+          ; "std::lock_guard::~lock_guard"
+          ; "std::unique_lock::~unique_lock" ]
       in
       fun pname ->
         QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname)
@@ -1002,28 +1005,8 @@ let analyze_procedure {Callbacks.proc_desc; tenv; summary} =
           try AttributeMapDomain.find return_var_ap attribute_map
           with Not_found -> AttributeSetDomain.empty
         in
-        (* A hack for modeling lock_guard by releasing a
-             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"; "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)
-                     ) ))
-          | _
-           -> locks
-        in
         let escapee_formals = FormalsDomain.of_escapees escapees in
-        let post =
+        let post = (thumbs_up, threads, locks, accesses, return_attributes, escapee_formals) in
-          (thumbs_up, threads, update_locks, accesses, return_attributes, escapee_formals)
-        in
         Summary.update_summary post summary
     | None
      -> summary )

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

@@ -3,7 +3,7 @@ codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get4, 0, THREAD_SAFETY_
 codetoanalyze/cpp/threadsafety/lock_guard.cpp, basics::LockGuard_get2, 3, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_written`,<Beginning of write trace>,access to `suspiciously_written`]
 codetoanalyze/cpp/threadsafety/lock_guard.cpp, basics::LockGuard_get4, 0, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_read`,<Beginning of write trace>,access to `suspiciously_read`]
 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_get2, 3, THREAD_SAFETY_VIOLATION, [<Beginning of read trace>,access to `suspiciously_written`,<Beginning of write trace>,access to `suspiciously_written`]
 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`]

infer/tests/codetoanalyze/cpp/threadsafety/lock_guard_with_scope.cpp

@@ -22,8 +22,6 @@ class LockGuardWithScope {
       suspiciously_read = new_value;
     }
 
-    // FIXME: missing unlocks in destructors make the following accesses
-    // to be treated as protected
     not_guarded = new_value;
     suspiciously_written = new_value;
   }
@@ -38,12 +36,10 @@ class LockGuardWithScope {
   int get2() {
     int result;
     std::lock_guard<std::mutex> lock(mutex_);
-    // FIXME: It does not report due to missing unlocks in destructors
     result = suspiciously_written;
     return result;
   }
 
-  // FIXME: It reports due to missing unlocks in destructors
   int get3() { return not_guarded; }
 
   int get4() { return suspiciously_read; }