[thread-safety][c++] Change to LOCK_CONSISTENCY_VIOLATION issue type

Summary:
Use a distinct issue type for the Java and C++ concurrency analyses,
as the properties they are checking are significantly different.

Reviewed By: sblackshear

Differential Revision: D6151682

fbshipit-source-id: 00e00eb

infer/src/base/IssueType.ml

@@ -241,6 +241,8 @@ let leak_after_array_abstraction = from_string "Leak_after_array_abstraction"
 
 let leak_in_footprint = from_string "Leak_in_footprint"
 
+let lock_consistency_violation = from_string "LOCK_CONSISTENCY_VIOLATION"
+
 let memory_leak = from_string "MEMORY_LEAK"
 
 let missing_fld = from_string "Missing_fld" ~hum:"Missing Field"

infer/src/base/IssueType.mli

@@ -154,6 +154,8 @@ val leak_after_array_abstraction : t
 
 val leak_in_footprint : t
 
+val lock_consistency_violation : t
+
 val memory_leak : t
 
 val missing_fld : t

infer/src/concurrency/RacerD.ml

@@ -1174,8 +1174,8 @@ type report_kind =
   | ReadWriteRace of conflicts  (** non-empty list of conflicting accesses *)
   | UnannotatedInterface
 
-(** Explain why we are reporting this access *)
-let get_reporting_explanation report_kind tenv pname thread =
+(** Explain why we are reporting this access, in Java *)
+let get_reporting_explanation_java report_kind tenv pname thread =
   (* best explanation is always that the current class or method is annotated thread-safe. try for
      that first. *)
   let annotation_explanation_opt =
@@ -1199,25 +1199,39 @@ let get_reporting_explanation report_kind tenv pname thread =
   in
   match (report_kind, annotation_explanation_opt) with
   | UnannotatedInterface, Some threadsafe_explanation ->
-      F.asprintf "%s." threadsafe_explanation
+      (IssueType.interface_not_thread_safe, F.asprintf "%s." threadsafe_explanation)
   | UnannotatedInterface, None ->
       Logging.die InternalError
         "Reporting non-threadsafe interface call, but can't find a @ThreadSafe annotation"
   | _, Some threadsafe_explanation when RacerDDomain.ThreadsDomain.is_any thread ->
-      F.asprintf
-        "%s, so we assume that this method can run in parallel with other non-private methods in the class (incuding itself)."
-        threadsafe_explanation
+      ( IssueType.thread_safety_violation
+      , F.asprintf
+          "%s, so we assume that this method can run in parallel with other non-private methods in the class (incuding itself)."
+          threadsafe_explanation )
   | _, Some threadsafe_explanation ->
-      F.asprintf
-        "%s. Although this access is not known to run on a background thread, it may happen in parallel with another access that does."
-        threadsafe_explanation
+      ( IssueType.thread_safety_violation
+      , F.asprintf
+          "%s. Although this access is not known to run on a background thread, it may happen in parallel with another access that does."
+          threadsafe_explanation )
   | _, None ->
       (* failed to explain based on @ThreadSafe annotation; have to justify using background thread *)
       if RacerDDomain.ThreadsDomain.is_any thread then
-        F.asprintf "@\n Reporting because this access may occur on a background thread."
+        ( IssueType.thread_safety_violation
+        , F.asprintf "@\n Reporting because this access may occur on a background thread." )
       else
-        F.asprintf
-          "@\n Reporting because another access to the same memory occurs on a background thread, although this access may not."
+        ( IssueType.thread_safety_violation
+        , F.asprintf
+            "@\n Reporting because another access to the same memory occurs on a background thread, although this access may not."
+        )
+
+
+(** Explain why we are reporting this access, in C++ *)
+let get_reporting_explanation_cpp = (IssueType.lock_consistency_violation, "")
+
+(** Explain why we are reporting this access *)
+let get_reporting_explanation report_kind tenv pname thread =
+  if Typ.Procname.is_java pname then get_reporting_explanation_java report_kind tenv pname thread
+  else get_reporting_explanation_cpp
 
 
 let filter_by_access access_filter trace =
@@ -1320,8 +1334,7 @@ let make_trace ~report_kind original_path pdesc =
       original_trace
 
 
-let report_thread_safety_violation tenv pdesc issue_type ~make_description ~report_kind access
-    thread =
+let report_thread_safety_violation tenv pdesc ~make_description ~report_kind access thread =
   let open RacerDDomain in
   let pname = Procdesc.get_proc_name pdesc in
   let report_one_path ((_, sinks) as path) =
@@ -1338,7 +1351,7 @@ let report_thread_safety_violation tenv pdesc issue_type ~make_description ~repo
       (* what the potential bug is *)
       let description = make_description pname final_sink_site initial_sink_site final_sink in
       (* why we are reporting it *)
-      let explanation = get_reporting_explanation report_kind tenv pname thread in
+      let issue_type, explanation = get_reporting_explanation report_kind tenv pname thread in
       let error_message = F.sprintf "%s%s" description explanation in
       let exn =
         Exceptions.Checkers (issue_type.IssueType.unique_id, Localise.verbatim_desc error_message)
@@ -1358,8 +1371,8 @@ let report_unannotated_interface_violation tenv pdesc access thread reported_pna
           "Unprotected call to method of un-annotated interface %s. Consider annotating the class with %a, adding a lock, or using an interface that is known to be thread-safe."
           class_name MF.pp_monospaced "@ThreadSafe"
       in
-      report_thread_safety_violation tenv pdesc IssueType.interface_not_thread_safe
-        ~make_description ~report_kind:UnannotatedInterface access thread
+      report_thread_safety_violation tenv pdesc ~make_description ~report_kind:UnannotatedInterface
+        access thread
   | _ ->
       (* skip reporting on C++ *)
       ()
@@ -1535,7 +1548,7 @@ let report_unsafe_accesses
             if List.is_empty writes_on_background_thread && not (ThreadsDomain.is_any thread) then
               reported_acc
             else (
-              report_thread_safety_violation tenv pdesc IssueType.thread_safety_violation
+              report_thread_safety_violation tenv pdesc
                 ~make_description:make_unprotected_write_description
                 ~report_kind:(WriteWriteRace writes_on_background_thread) access thread ;
               update_reported access pname reported_acc )
@@ -1572,7 +1585,7 @@ let report_unsafe_accesses
           in
           if List.is_empty all_writes then reported_acc
           else (
-            report_thread_safety_violation tenv pdesc IssueType.thread_safety_violation
+            report_thread_safety_violation tenv pdesc
               ~make_description:(make_read_write_race_description ~read_is_sync:false all_writes)
               ~report_kind:
                 (ReadWriteRace (List.map ~f:(fun (access, _, _, _, _) -> access) all_writes))
@@ -1604,7 +1617,7 @@ let report_unsafe_accesses
           if List.is_empty conflicting_writes then reported_acc
           else (
             (* protected read with conflicting unprotected write(s). warn. *)
-            report_thread_safety_violation tenv pdesc IssueType.thread_safety_violation
+            report_thread_safety_violation tenv pdesc
               ~make_description:
                 (make_read_write_race_description ~read_is_sync:true conflicting_writes)
               ~report_kind:

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

@@ -1,14 +1,14 @@
-codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get2, 3, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]
-codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get4, 0, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
-codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get5, 0, THREAD_SAFETY_VIOLATION, [<Read trace>,call to basics::Basic_get_private_suspiciously_read,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
-codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test2_bad, 5, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&x.f`,<Write trace>,access to `&x.f`]
-codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test3_bad, 5, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&x.f`,<Write trace>,access to `&x.f`]
-codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get2, 3, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]
-codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get4, 0, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
-codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_test1, 2, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
-codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get2, 3, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]
-codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get4, 0, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
-codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 3, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written1`,<Write trace>,access to `&this.suspiciously_written1`]
-codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 4, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written2`,<Write trace>,access to `&this.suspiciously_written2`]
-codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 1, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read1`,<Write trace>,access to `&this.suspiciously_read1`]
-codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 2, THREAD_SAFETY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read2`,<Write trace>,access to `&this.suspiciously_read2`]
+codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get2, 3, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]
+codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get4, 0, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
+codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get5, 0, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,call to basics::Basic_get_private_suspiciously_read,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
+codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test2_bad, 5, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&x.f`,<Write trace>,access to `&x.f`]
+codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test3_bad, 5, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&x.f`,<Write trace>,access to `&x.f`]
+codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get2, 3, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]
+codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get4, 0, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
+codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_test1, 2, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
+codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get2, 3, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]
+codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get4, 0, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
+codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 3, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written1`,<Write trace>,access to `&this.suspiciously_written1`]
+codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 4, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written2`,<Write trace>,access to `&this.suspiciously_written2`]
+codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 1, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read1`,<Write trace>,access to `&this.suspiciously_read1`]
+codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 2, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read2`,<Write trace>,access to `&this.suspiciously_read2`]