From 59a10d00d461885b9e6cb23841258cd2693d62db Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Thu, 21 Mar 2019 04:24:33 -0700 Subject: [PATCH] [racerd] guardedby Summary: Add support for GuardedBy: we deviate from the spec as follows: - No warnings issued for any access within a private method, unless that method is called from a public method and the lock isn't held when the access occurs. - Warnings are suppressed with the general RacerD mechanism, ie `ThreadSafe(enableChecks=false)` - GuardedBy warnings override thread-safety violation warnings on the same access, because GuardedBy has a clearer and simpler contract. Also, some simplifications, cleanups and perf improvements (eg avoid unreportable procs at the top level as opposed to on each of their accesses). Reviewed By: jeremydubreil Differential Revision: D14506161 fbshipit-source-id: b7d794051 --- infer/src/base/IssueType.ml | 2 + infer/src/base/IssueType.mli | 2 + infer/src/concurrency/RacerD.ml | 151 +++++++++++------- infer/tests/build_systems/ant/issues.exp | 41 +++-- .../java/racerd/GuardedByTests.java | 92 +++++++++++ .../codetoanalyze/java/racerd/issues.exp | 8 + 6 files changed, 223 insertions(+), 73 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/racerd/GuardedByTests.java diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 6fa86b5e6..ae2a6ebaa 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -271,6 +271,8 @@ let _global_variable_initialized_with_function_or_method_call = let graphql_field_access = from_string "GRAPHQL_FIELD_ACCESS" +let guardedby_violation_racerd = from_string "GUARDEDBY_VIOLATION" ~hum:"GuardedBy Violation" + let inferbo_alloc_is_big = from_string "INFERBO_ALLOC_IS_BIG" let inferbo_alloc_is_negative = from_string "INFERBO_ALLOC_IS_NEGATIVE" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index c7469a168..31a6d43d2 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -177,6 +177,8 @@ val field_not_null_checked : t val graphql_field_access : t +val guardedby_violation_racerd : t + val inferbo_alloc_is_big : t val inferbo_alloc_is_negative : t diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 4ecd4919a..4e0a979b7 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -609,6 +609,7 @@ let analyze_procedure {Callbacks.proc_desc; tenv; summary} = type conflict = RacerDDomain.TraceElem.t type report_kind = + | GuardedByViolation | WriteWriteRace of conflict option (** one of conflicting access, if there are any *) | ReadWriteRace of conflict (** one of several conflicting accesses *) | UnannotatedInterface @@ -644,6 +645,9 @@ let get_reporting_explanation_java report_kind tenv pname thread = None ) in match (report_kind, annotation_explanation_opt) with + | GuardedByViolation, _ -> + ( IssueType.guardedby_violation_racerd + , F.asprintf "@\n Reporting because field is annotated %a" MF.pp_monospaced "@GuardedBy" ) | UnannotatedInterface, Some threadsafe_explanation -> (IssueType.interface_not_thread_safe, F.asprintf "%s." threadsafe_explanation) | UnannotatedInterface, None -> @@ -715,13 +719,12 @@ let make_trace ~report_kind original_path = , conflict_end ) in match report_kind with - | ReadWriteRace conflict_sink -> - make_with_conflicts conflict_sink original_trace ~label1:"" - ~label2:"" - | WriteWriteRace (Some conflict_sink) -> - make_with_conflicts conflict_sink original_trace ~label1:"" + | ReadWriteRace conflict -> + make_with_conflicts conflict original_trace ~label1:"" ~label2:"" + | WriteWriteRace (Some conflict) -> + make_with_conflicts conflict original_trace ~label1:"" ~label2:"" - | WriteWriteRace None | UnannotatedInterface -> + | GuardedByViolation | WriteWriteRace None | UnannotatedInterface -> (original_trace, original_end, None) @@ -730,10 +733,17 @@ let log_issue current_pname ~loc ~ltr ~access issue_type error_message = error_message -let report_thread_safety_violation tenv pdesc ~make_description ~report_kind - (access : RacerDDomain.TraceElem.t) thread = +type reported_access = + { threads: RacerDDomain.ThreadsDomain.t + ; snapshot: RacerDDomain.AccessSnapshot.t + ; tenv: Tenv.t + ; procdesc: Procdesc.t } + +let report_thread_safety_violation ~make_description ~report_kind + ({threads; snapshot; tenv; procdesc} : reported_access) = let open RacerDDomain in - let pname = Procdesc.get_proc_name pdesc in + let pname = Procdesc.get_proc_name procdesc in + let access = snapshot.access in let final_pname = List.last access.trace |> Option.value_map ~default:pname ~f:CallSite.pname in let final_sink_site = CallSite.make final_pname access.loc in let initial_sink_site = CallSite.make pname (TraceElem.get_loc access) in @@ -742,14 +752,15 @@ let report_thread_safety_violation tenv pdesc ~make_description ~report_kind (* what the potential bug is *) let description = make_description pname final_sink_site initial_sink_site access in (* why we are reporting it *) - let issue_type, explanation = get_reporting_explanation report_kind tenv pname thread in + let issue_type, explanation = get_reporting_explanation report_kind tenv pname threads in let error_message = F.sprintf "%s%s" description explanation in let end_locs = Option.to_list original_end @ Option.to_list conflict_end in let access = IssueAuxData.encode end_locs in log_issue pname ~loc ~ltr ~access issue_type error_message -let report_unannotated_interface_violation tenv pdesc access thread reported_pname = +let report_unannotated_interface_violation (reported_access : reported_access) = + let reported_pname = Procdesc.get_proc_name reported_access.procdesc in match reported_pname with | Typ.Procname.Java java_pname -> let class_name = Typ.Procname.Java.get_class_name java_pname in @@ -759,8 +770,8 @@ let report_unannotated_interface_violation tenv pdesc access thread reported_pna class with %a, adding a lock, or using an interface that is known to be thread-safe." Typ.Procname.pp reported_pname class_name MF.pp_monospaced "@ThreadSafe" in - report_thread_safety_violation tenv pdesc ~make_description ~report_kind:UnannotatedInterface - access thread + report_thread_safety_violation ~make_description ~report_kind:UnannotatedInterface + reported_access | _ -> (* skip reporting on C++ *) () @@ -775,11 +786,14 @@ let make_unprotected_write_description pname final_sink_site initial_sink_site f pp_access final_sink -type reported_access = - { threads: RacerDDomain.ThreadsDomain.t - ; snapshot: RacerDDomain.AccessSnapshot.t - ; tenv: Tenv.t - ; procdesc: Procdesc.t } +let make_guardedby_violation_description pname final_sink_site initial_sink_site final_sink = + Format.asprintf + "GuardedBy violation. Non-private method %a%s accesses %a outside of synchronization." + (MF.wrap_monospaced Typ.Procname.pp) + pname + (if CallSite.equal final_sink_site initial_sink_site then "" else " indirectly") + pp_access final_sink + let make_read_write_race_description ~read_is_sync (conflict : reported_access) pname final_sink_site initial_sink_site final_sink = @@ -828,7 +842,7 @@ let should_filter_access path_opt = | AccessPath.FieldAccess fld -> String.is_substring ~substring:"$SwitchMap" (Typ.Fieldname.to_string fld) in - Option.value_map path_opt ~default:false ~f:(fun (_, path) -> List.exists path ~f:check_access) + Option.exists path_opt ~f:(fun (_, path) -> List.exists path ~f:check_access) (** @@ -893,7 +907,7 @@ end = struct M.fold f map a end -let should_report_on_proc {procdesc; tenv} = +let should_report_on_proc tenv procdesc = let proc_name = Procdesc.get_proc_name procdesc in match proc_name with | Java java_pname -> @@ -922,6 +936,16 @@ let should_report_on_proc {procdesc; tenv} = false +let should_report_guardedby_violation ({snapshot; tenv} : reported_access) = + (not snapshot.lock) + && RacerDDomain.Access.get_access_path snapshot.access.elem + |> Option.exists ~f:(fun path -> + AccessPath.get_field_and_annotation path tenv + |> Option.exists ~f:(fun (_, annotlist) -> + List.exists annotlist ~f:(fun (annot, _) -> + Annotations.annot_ends_with annot Annotations.guarded_by ) ) ) + + (** Report accesses that may race with each other. Principles for race reporting. @@ -953,13 +977,14 @@ let should_report_on_proc {procdesc; tenv} = let report_unsafe_accesses (aggregated_access_map : ReportMap.t) = let open RacerDDomain in let open RacerDModels in - let is_duplicate_report access pname + let is_duplicate_report ({snapshot; procdesc} : reported_access) {reported_sites; reported_writes; reported_reads; reported_unannotated_calls} = - let call_site = CallSite.make pname (TraceElem.get_loc access) in + let pname = Procdesc.get_proc_name procdesc in + let call_site = CallSite.make pname (TraceElem.get_loc snapshot.access) in if Config.filtering then CallSite.Set.mem call_site reported_sites || - match access.TraceElem.elem with + match snapshot.access.TraceElem.elem with | Access.Write _ | Access.ContainerWrite _ -> Typ.Procname.Set.mem pname reported_writes | Access.Read _ | Access.ContainerRead _ -> @@ -968,11 +993,12 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) = Typ.Procname.Set.mem pname reported_unannotated_calls else false in - let update_reported access pname reported = + let update_reported ({snapshot; procdesc} : reported_access) reported = if Config.filtering then - let call_site = CallSite.make pname (TraceElem.get_loc access) in + let pname = Procdesc.get_proc_name procdesc in + let call_site = CallSite.make pname (TraceElem.get_loc snapshot.access) in let reported_sites = CallSite.Set.add call_site reported.reported_sites in - match access.TraceElem.elem with + match snapshot.access.TraceElem.elem with | Access.Write _ | Access.ContainerWrite _ -> let reported_writes = Typ.Procname.Set.add pname reported.reported_writes in {reported with reported_writes; reported_sites} @@ -986,19 +1012,19 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) = {reported with reported_unannotated_calls; reported_sites} else reported in - let report_unsafe_access accesses reported_acc {snapshot; threads; tenv; procdesc} = + let report_unsafe_access accesses reported_acc + ({snapshot; threads; tenv; procdesc} as reported_access) = let pname = Procdesc.get_proc_name procdesc in - if is_duplicate_report snapshot.access pname reported_acc then reported_acc + if is_duplicate_report reported_access reported_acc then reported_acc else match snapshot.access.elem with - | Access.InterfaceCall unannoted_call_pname + | Access.InterfaceCall _ when AccessSnapshot.is_unprotected snapshot && ThreadsDomain.is_any threads && is_marked_thread_safe procdesc tenv -> (* un-annotated interface call + no lock in method marked thread-safe. warn *) - report_unannotated_interface_violation tenv procdesc snapshot.access threads - unannoted_call_pname ; - update_reported snapshot.access pname reported_acc + report_unannotated_interface_violation reported_access ; + update_reported reported_access reported_acc | Access.InterfaceCall _ -> reported_acc | (Access.Write _ | ContainerWrite _) when Typ.Procname.is_java pname -> @@ -1019,10 +1045,9 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) = AccessSnapshot.is_unprotected snapshot && (Option.is_some conflict || ThreadsDomain.is_any threads) then ( - report_thread_safety_violation tenv procdesc - ~make_description:make_unprotected_write_description - ~report_kind:(WriteWriteRace conflict) snapshot.access threads ; - update_reported snapshot.access pname reported_acc ) + report_thread_safety_violation ~make_description:make_unprotected_write_description + ~report_kind:(WriteWriteRace conflict) reported_access ; + update_reported reported_access reported_acc ) else reported_acc | Access.Write _ | ContainerWrite _ -> (* Do not report unprotected writes for ObjC_Cpp *) @@ -1043,9 +1068,8 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) = make_read_write_race_description ~read_is_sync:false conflict in let report_kind = ReadWriteRace conflict.snapshot.access in - report_thread_safety_violation tenv procdesc ~make_description ~report_kind - snapshot.access threads ; - update_reported snapshot.access pname reported_acc ) + report_thread_safety_violation ~make_description ~report_kind reported_access ; + update_reported reported_access reported_acc ) | Access.Read _ | ContainerRead _ -> (* protected read. report unprotected writes and opposite protected writes as conflicts *) let can_conflict (snapshot1 : AccessSnapshot.t) (snapshot2 : AccessSnapshot.t) = @@ -1064,26 +1088,35 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) = make_read_write_race_description ~read_is_sync:true conflict in let report_kind = ReadWriteRace conflict.snapshot.access in - report_thread_safety_violation tenv procdesc ~make_description ~report_kind - snapshot.access threads ; - update_reported snapshot.access pname reported_acc ) + report_thread_safety_violation ~make_description ~report_kind reported_access ; + update_reported reported_access reported_acc ) in - let report_accesses_on_location (grouped_accesses : reported_access list) reported_acc = + let report_accesses_on_location (reportable_accesses : reported_access list) init = (* Don't report on location if all accesses are on non-concurrent contexts *) if - List.for_all grouped_accesses ~f:(fun ({threads} : reported_access) -> + List.for_all reportable_accesses ~f:(fun ({threads} : reported_access) -> ThreadsDomain.is_any threads |> not ) - then reported_acc - else - (* reset the reported reads and writes for each memory location *) - let init = - { reported_acc with - reported_writes= Typ.Procname.Set.empty; reported_reads= Typ.Procname.Set.empty } - in - let reportable_accesses = List.filter ~f:should_report_on_proc grouped_accesses in - List.fold reportable_accesses ~init ~f:(report_unsafe_access reportable_accesses) + then init + else List.fold reportable_accesses ~init ~f:(report_unsafe_access reportable_accesses) + in + let report_guardedby_violations_on_location grouped_accesses init = + List.fold grouped_accesses ~init ~f:(fun acc r -> + if should_report_guardedby_violation r && not (is_duplicate_report r acc) then ( + report_thread_safety_violation ~make_description:make_guardedby_violation_description + ~report_kind:GuardedByViolation r ; + update_reported r acc ) + else acc ) + in + let report grouped_accesses reported_acc = + (* reset the reported reads and writes for each memory location *) + let reported_acc = + { reported_acc with + reported_writes= Typ.Procname.Set.empty; reported_reads= Typ.Procname.Set.empty } + in + report_guardedby_violations_on_location grouped_accesses reported_acc + |> report_accesses_on_location grouped_accesses in - ReportMap.fold report_accesses_on_location aggregated_access_map empty_reported |> ignore + ReportMap.fold report aggregated_access_map empty_reported |> ignore (* create a map from [abstraction of a memory loc] -> accesses that @@ -1105,10 +1138,12 @@ let make_results_table file_env = (* aggregate all of the procedures in the file env by their declaring class. this lets us analyze each class individually *) let aggregate_by_class file_env = - List.fold file_env ~init:String.Map.empty ~f:(fun acc ((_, pdesc) as proc) -> - Procdesc.get_proc_name pdesc |> Typ.Procname.get_class_name - |> Option.fold ~init:acc ~f:(fun acc classname -> - String.Map.add_multi acc ~key:classname ~data:proc ) ) + List.fold file_env ~init:String.Map.empty ~f:(fun acc ((tenv, pdesc) as proc) -> + if should_report_on_proc tenv pdesc then + Procdesc.get_proc_name pdesc |> Typ.Procname.get_class_name + |> Option.fold ~init:acc ~f:(fun acc classname -> + String.Map.add_multi acc ~key:classname ~data:proc ) + else acc ) (* Gathers results by analyzing all the methods in a file, then diff --git a/infer/tests/build_systems/ant/issues.exp b/infer/tests/build_systems/ant/issues.exp index d1c7cbfed..869b2581e 100644 --- a/infer/tests/build_systems/ant/issues.exp +++ b/infer/tests/build_systems/ant/issues.exp @@ -53,42 +53,53 @@ codetoanalyze/java/infer/FilterOutputStreamLeaks.java, codetoanalyze.java.infer. codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample$3.readFromInnerClassBad1():java.lang.String, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFromInnerClassBad1()] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample$4.readFromInnerClassBad2():java.lang.String, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFromInnerClassBad2()] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample$Sub.badSub():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure badSub()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample$Sub.badSub():void, 491, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.xForSub`,,access to `this.xForSub`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample$Sub.badSub():void, 491, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.xForSub`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 307, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread1`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 308, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread1`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 309, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread2`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 310, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread2`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 311, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread3`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 312, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.uiThread3`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 313, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.nonExpression`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.accessUnrecognizedGuardedByFieldsOk():void, 314, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.nonExpression`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByNormalLock():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure badGuardedByNormalLock()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByNormalLock():void, 526, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.guardedbynl`,,access to `this.guardedbynl`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByNormalLock():void, 526, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedbynl`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByReentrantLock():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure badGuardedByReentrantLock()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByReentrantLock():void, 530, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.guardedbyrel`,,access to `this.guardedbyrel`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.badGuardedByReentrantLock():void, 530, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedbyrel`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.byRefTrickyBad():java.lang.Object, 5, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure byRefTrickyBad()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.byRefTrickyBad():java.lang.Object, 281, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.g`,,access to `this.g`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.byRefTrickyBad():java.lang.Object, 281, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.g`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedBySelfReferenceOK():void, 409, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.self_reference`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure guardedByTypeSyntaxBad()] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure guardedByTypeSyntaxBad()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 573, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.guardedByLock1`,,access to `this.guardedByLock1`] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 574, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.guardedByLock2`,,access to `this.guardedByLock2`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 573, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedByLock1`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.guardedByTypeSyntaxBad():void, 574, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.guardedByLock2`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.itselfOK():void, 425, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.itself_fld`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFAfterBlockBad():void, 3, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFAfterBlockBad()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFAfterBlockBad():void, 98, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFAfterBlockBad():void, 98, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFBad()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBad():void, 66, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressed():void, 71, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressedOther():void, 76, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBad():void, 66, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressed():void, 71, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadButSuppressedOther():void, 76, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongAnnotation():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFBadWrongAnnotation()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongAnnotation():void, 109, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongAnnotation():void, 109, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongLock():void, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readFBadWrongLock()] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFBadWrongLock():void, 85, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkMethodAnnotated():void, 114, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkMethodAnnotated():void, 114, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkSynchronized():void, 127, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readGFromCopyOk():void, 267, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mCopyOfG`,,access to `this.mCopyOfG`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readHBad():void, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readHBad()] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readHBadSynchronizedMethodShouldntHelp():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure readHBadSynchronizedMethodShouldntHelp()] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readWriteLockOk():void, 189, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.i`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.reassignCopyOk():void, 149, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.mCopyOfG`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedMethodReadBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure synchronizedMethodReadBad()] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedMethodReadBad():void, 138, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedMethodWriteBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure synchronizedMethodWriteBad()] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedOnThisBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure synchronizedOnThisBad()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedOnThisBad():void, 205, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `infer.GuardedByExample.sGuardedByClass`,,access to `infer.GuardedByExample.sGuardedByClass`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.synchronizedOnThisBad():void, 205, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `infer.GuardedByExample.sGuardedByClass`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFAfterBlockBad():void, 3, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure writeFAfterBlockBad()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFAfterBlockBad():void, 104, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFAfterBlockBad():void, 104, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFBad():void, 1, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure writeFBad()] -codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFBad():void, 80, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFBad():void, 80, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.writeFBadWrongLock():void, 2, UNSAFE_GUARDED_BY_ACCESS, no_bucket, ERROR, [start of procedure writeFBadWrongLock()] codetoanalyze/java/infer/HashMapExample.java, codetoanalyze.java.infer.HashMapExample.getAfterClearBad():void, 5, NULL_DEREFERENCE, no_bucket, ERROR, [start of procedure getAfterClearBad()] codetoanalyze/java/infer/HashMapExample.java, codetoanalyze.java.infer.HashMapExample.getAfterRemovingTheKeyBad():void, 5, NULL_DEREFERENCE, no_bucket, ERROR, [start of procedure getAfterRemovingTheKeyBad()] diff --git a/infer/tests/codetoanalyze/java/racerd/GuardedByTests.java b/infer/tests/codetoanalyze/java/racerd/GuardedByTests.java new file mode 100644 index 000000000..e67e5dc6a --- /dev/null +++ b/infer/tests/codetoanalyze/java/racerd/GuardedByTests.java @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2016-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package codetoanalyze.java.infer; + +import com.facebook.infer.annotation.ThreadSafe; +import com.google.common.annotations.VisibleForTesting; +import javax.annotation.concurrent.GuardedBy; + +public class GuardedByTests { + private Object mlock = new Object(); + + @GuardedBy("mLock") + private int f; + + public GuardedByTests() { + // don't warn on reads or writes of Guarded fields in constructor + f = 0; + } + + public void unlockedWriteBad() { + f = 0; + } + + public void lockedWriteOk() { + synchronized (mlock) { + f = 0; + } + } + + public int unlockedReadBad() { + return f; + } + + public int lockedReadOk() { + synchronized (mlock) { + return f; + } + } + + private void privateUnlockedWriteOk() { + f = 0; + } + + private int privateUnlockedReadOk() { + return f; + } + + public void interprocUnlockedWriteBad() { + privateUnlockedWriteOk(); + } + + public int interprocUnlockedReadBad() { + return privateUnlockedReadOk(); + } + + // NB ThreadSafe annotation disables GuardedBy check too + @ThreadSafe(enableChecks = false) + int suppressedRead() { + return f; + } + + @VisibleForTesting + public void visibleForTestingOk() { + f = 0; + } + + static Object slock = new Object(); + + @GuardedBy("slock") + static int sf; + + static { + // don't warn on class initializer + sf = 0; + } + + @GuardedBy("this") + int g; + + synchronized void syncWriteOk() { + g = 5; + } + + synchronized int syncReadOk() { + return g; + } +} diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index bb0487851..472abf5ef 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/infer/tests/codetoanalyze/java/racerd/issues.exp @@ -51,6 +51,14 @@ codetoanalyze/java/racerd/DeepOwnership.java, DeepOwnership.reassignBaseToGlobal codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceBad(codetoanalyze.java.checkers.UnannotatedInterface):void, 49, INTERFACE_NOT_THREAD_SAFE, no_bucket, ERROR, [Call to un-annotated interface method void UnannotatedInterface.foo()] codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceIndirectBad(codetoanalyze.java.checkers.NotThreadSafe,codetoanalyze.java.checkers.UnannotatedInterface):void, 53, INTERFACE_NOT_THREAD_SAFE, no_bucket, ERROR, [call to void NotThreadSafe.notThreadSafeOk(UnannotatedInterface),Call to un-annotated interface method void UnannotatedInterface.foo()] codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.ThreadConfinedField.interfaceCallOnNormalFieldBad():void, 102, INTERFACE_NOT_THREAD_SAFE, no_bucket, ERROR, [Call to un-annotated interface method void UnannotatedInterface.foo()] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedReadBad():int, 58, GUARDEDBY_VIOLATION, no_bucket, ERROR, [call to int GuardedByTests.privateUnlockedReadOk(),access to `this.f`] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedReadBad():int, 58, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,call to int GuardedByTests.privateUnlockedReadOk(),access to `this.f`,,access to `this.f`] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedWriteBad():void, 54, GUARDEDBY_VIOLATION, no_bucket, ERROR, [call to void GuardedByTests.privateUnlockedWriteOk(),access to `this.f`] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.interprocUnlockedWriteBad():void, 54, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,call to void GuardedByTests.privateUnlockedWriteOk(),access to `this.f`,,access to `this.f`] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedReadBad():int, 36, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedReadBad():int, 36, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedWriteBad():void, 26, GUARDEDBY_VIOLATION, no_bucket, ERROR, [access to `this.f`] +codetoanalyze/java/racerd/GuardedByTests.java, codetoanalyze.java.infer.GuardedByTests.unlockedWriteBad():void, 26, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.f`,,access to `this.f`] codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.read4OutsideSyncBad():int, 64, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mField4`,,access to `this.mField4`] codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead1Bad():int, 21, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mField1`,,access to `this.mField1`] codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead2Bad():int, 34, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [,access to `this.mField2`,,access to `this.mField2`]