diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 6dafd8f61..ca2d89df1 100644 --- a/infer/src/base/IssueType.ml +++ b/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" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 56d1255a0..a93b48914 100644 --- a/infer/src/base/IssueType.mli +++ b/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 diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 0ab37dca4..6d35b0786 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/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: diff --git a/infer/tests/codetoanalyze/cpp/racerd/issues.exp b/infer/tests/codetoanalyze/cpp/racerd/issues.exp index fabcc4f7a..2c8ea044a 100644 --- a/infer/tests/codetoanalyze/cpp/racerd/issues.exp +++ b/infer/tests/codetoanalyze/cpp/racerd/issues.exp @@ -1,14 +1,14 @@ -codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get2, 3, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`] -codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get4, 0, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] -codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get5, 0, THREAD_SAFETY_VIOLATION, [,call to basics::Basic_get_private_suspiciously_read,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] -codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test2_bad, 5, THREAD_SAFETY_VIOLATION, [,access to `&x.f`,,access to `&x.f`] -codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test3_bad, 5, THREAD_SAFETY_VIOLATION, [,access to `&x.f`,,access to `&x.f`] -codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get2, 3, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`] -codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get4, 0, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] -codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_test1, 2, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] -codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get2, 3, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`] -codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get4, 0, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 3, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_written1`,,access to `&this.suspiciously_written1`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 4, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_written2`,,access to `&this.suspiciously_written2`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 1, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_read1`,,access to `&this.suspiciously_read1`] -codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 2, THREAD_SAFETY_VIOLATION, [,access to `&this.suspiciously_read2`,,access to `&this.suspiciously_read2`] +codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get2, 3, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`] +codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get4, 0, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] +codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get5, 0, LOCK_CONSISTENCY_VIOLATION, [,call to basics::Basic_get_private_suspiciously_read,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] +codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test2_bad, 5, LOCK_CONSISTENCY_VIOLATION, [,access to `&x.f`,,access to `&x.f`] +codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test3_bad, 5, LOCK_CONSISTENCY_VIOLATION, [,access to `&x.f`,,access to `&x.f`] +codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get2, 3, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`] +codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get4, 0, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] +codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_test1, 2, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] +codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get2, 3, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`] +codetoanalyze/cpp/racerd/lock_guard_with_scope.cpp, basics::LockGuardWithScope_get4, 0, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 3, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_written1`,,access to `&this.suspiciously_written1`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get2, 4, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_written2`,,access to `&this.suspiciously_written2`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 1, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_read1`,,access to `&this.suspiciously_read1`] +codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock_get4, 2, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_read2`,,access to `&this.suspiciously_read2`]