From b3e8e972d64b0fdc4f9126a3c05852c69b47e16e Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Thu, 21 Sep 2017 08:00:57 -0700 Subject: [PATCH] [thread-safety] separate warning type for interfaces called in non-threadsafe context Reviewed By: jeremydubreil Differential Revision: D5878497 fbshipit-source-id: ccb644b --- infer/src/base/IssueType.ml | 2 ++ infer/src/base/IssueType.mli | 2 ++ infer/src/checkers/ThreadSafety.ml | 18 ++++++++++-------- .../codetoanalyze/java/threadsafety/issues.exp | 4 ++-- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 4a95c383a..1c79fb7bf 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -202,6 +202,8 @@ let _global_variable_initialized_with_function_or_method_call = let inherently_dangerous_function = from_string "INHERENTLY_DANGEROUS_FUNCTION" +let interface_not_thread_safe = from_string "INTERFACE_NOT_THREAD_SAFE" + let internal_error = from_string "Internal_error" let leak_after_array_abstraction = from_string "Leak_after_array_abstraction" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index fe71adace..81b1cf96d 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -133,6 +133,8 @@ val field_not_null_checked : t val inherently_dangerous_function : t +val interface_not_thread_safe : t + val internal_error : t val leak_after_array_abstraction : t diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 4aa96d7ec..2af25d7f4 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -1189,7 +1189,7 @@ let make_trace_with_conflicts conflicts original_path pdesc = | [] -> original_trace -let report_thread_safety_violation tenv pdesc ~make_description ~conflicts access = +let report_thread_safety_violation tenv pdesc issue_type ~make_description ~conflicts access = let open ThreadSafetyDomain in let pname = Procdesc.get_proc_name pdesc in let report_one_path (_, sinks as path) = @@ -1199,9 +1199,10 @@ let report_thread_safety_violation tenv pdesc ~make_description ~conflicts acces let final_sink_site = PathDomain.Sink.call_site final_sink in let loc = CallSite.loc initial_sink_site in let ltr = make_trace_with_conflicts conflicts path pdesc in - let msg = IssueType.thread_safety_violation.unique_id in let description = make_description tenv pname final_sink_site initial_sink_site final_sink in - let exn = Exceptions.Checkers (msg, Localise.verbatim_desc description) in + let exn = + Exceptions.Checkers (issue_type.IssueType.unique_id, Localise.verbatim_desc description) + in Reporting.log_error_deprecated ~store_summary:true pname ~loc ~ltr exn in let trace_of_pname = trace_of_pname access pdesc in @@ -1213,10 +1214,11 @@ let report_unannotated_interface_violation tenv pdesc access reported_pname = -> let class_name = Typ.Procname.java_get_class_name java_pname in let make_description _ _ _ _ _ = F.asprintf - "Unprotected call to method of un-annotated interface %s. Consider annotating the class with %a or adding a lock" + "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 ~make_description ~conflicts:[] access + report_thread_safety_violation tenv pdesc IssueType.interface_not_thread_safe + ~make_description ~conflicts:[] access | _ -> (* skip reporting on C++ *) () @@ -1361,7 +1363,7 @@ let report_unsafe_accesses aggregated_access_map = -> if threaded then reported_acc else ( (* unprotected write. warn. *) - report_thread_safety_violation tenv pdesc + report_thread_safety_violation tenv pdesc IssueType.thread_safety_violation ~make_description:make_unprotected_write_description ~conflicts:[] access ; update_reported access pname reported_acc ) | _ @@ -1390,7 +1392,7 @@ let report_unsafe_accesses aggregated_access_map = in if List.is_empty all_writes then reported_acc else ( - report_thread_safety_violation tenv pdesc + report_thread_safety_violation tenv pdesc IssueType.thread_safety_violation ~make_description:(make_read_write_race_description all_writes) ~conflicts:(List.map ~f:(fun (access, _, _, _, _) -> access) all_writes) access ; @@ -1423,7 +1425,7 @@ let report_unsafe_accesses aggregated_access_map = if List.is_empty conflicting_writes then reported_acc else ( (* protected read with conflicting unprotected write(s). warn. *) - report_thread_safety_violation tenv pdesc + report_thread_safety_violation tenv pdesc IssueType.thread_safety_violation ~make_description:(make_read_write_race_description conflicting_writes) ~conflicts:(List.map ~f:(fun (access, _, _, _, _) -> access) conflicting_writes) access ; diff --git a/infer/tests/codetoanalyze/java/threadsafety/issues.exp b/infer/tests/codetoanalyze/java/threadsafety/issues.exp index 21ed2d67a..89c303f67 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/issues.exp +++ b/infer/tests/codetoanalyze/java/threadsafety/issues.exp @@ -45,8 +45,8 @@ codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutBad(Strin codetoanalyze/java/threadsafety/Containers.java, void Containers.mapRemoveBad(String), 1, THREAD_SAFETY_VIOLATION, [Write to container `&this.codetoanalyze.java.checkers.Containers.mMap` via call to `remove`] codetoanalyze/java/threadsafety/Containers.java, void Containers.mapSubclassWriteBad(HashMap,String), 1, THREAD_SAFETY_VIOLATION, [Write to container `&m` via call to `remove`] codetoanalyze/java/threadsafety/Containers.java, void Containers.poolBad(), 5, THREAD_SAFETY_VIOLATION, [Write to container `&this.codetoanalyze.java.checkers.Containers.simplePool` via call to `release`] -codetoanalyze/java/threadsafety/Dispatch.java, void Dispatch.callUnannotatedInterfaceBad(UnannotatedInterface), 1, THREAD_SAFETY_VIOLATION, [Call to un-annotated interface method void UnannotatedInterface.foo()1] -codetoanalyze/java/threadsafety/Dispatch.java, void Dispatch.callUnannotatedInterfaceIndirectBad(NotThreadSafe,UnannotatedInterface), 1, THREAD_SAFETY_VIOLATION, [call to void NotThreadSafe.notThreadSafeOk(UnannotatedInterface),Call to un-annotated interface method void UnannotatedInterface.foo()1] +codetoanalyze/java/threadsafety/Dispatch.java, void Dispatch.callUnannotatedInterfaceBad(UnannotatedInterface), 1, INTERFACE_NOT_THREAD_SAFE, [Call to un-annotated interface method void UnannotatedInterface.foo()1] +codetoanalyze/java/threadsafety/Dispatch.java, void Dispatch.callUnannotatedInterfaceIndirectBad(NotThreadSafe,UnannotatedInterface), 1, INTERFACE_NOT_THREAD_SAFE, [call to void NotThreadSafe.notThreadSafeOk(UnannotatedInterface),Call to un-annotated interface method void UnannotatedInterface.foo()1] codetoanalyze/java/threadsafety/Locks.java, void Locks.FP_unlockOneLock(), 4, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.Locks.f`] codetoanalyze/java/threadsafety/Locks.java, void Locks.afterReentrantLockUnlockBad(), 3, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.Locks.f`] codetoanalyze/java/threadsafety/Locks.java, void Locks.afterUnlockBad(), 3, THREAD_SAFETY_VIOLATION, [access to `codetoanalyze.java.checkers.Locks.f`]