[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

@@ -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"

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

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 ->
+  | ReadWriteRace conflict ->
-      make_with_conflicts conflict_sink original_trace ~label1:"<Read trace>"
+      make_with_conflicts conflict original_trace ~label1:"<Read trace>" ~label2:"<Write trace>"
-        ~label2:"<Write trace>"
+  | WriteWriteRace (Some conflict) ->
-  | WriteWriteRace (Some conflict_sink) ->
+      make_with_conflicts conflict original_trace ~label1:"<Write on unknown thread>"
-      make_with_conflicts conflict_sink original_trace ~label1:"<Write on unknown thread>"
         ~label2:"<Write on background thread>"
-  | 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
+type reported_access =
-    (access : RacerDDomain.TraceElem.t) thread =
+  { 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
+      report_thread_safety_violation ~make_description ~report_kind:UnannotatedInterface
-        access thread
+        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 =
+let make_guardedby_violation_description pname final_sink_site initial_sink_site final_sink =
-  { threads: RacerDDomain.ThreadsDomain.t
+  Format.asprintf
-  ; snapshot: RacerDDomain.AccessSnapshot.t
+    "GuardedBy violation. Non-private method %a%s accesses %a outside of synchronization."
-  ; tenv: Tenv.t
+    (MF.wrap_monospaced Typ.Procname.pp)
-  ; procdesc: Procdesc.t }
+    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
+          report_unannotated_interface_violation reported_access ;
-            unannoted_call_pname ;
+          update_reported reported_access reported_acc
-          update_reported snapshot.access pname 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
+            report_thread_safety_violation ~make_description:make_unprotected_write_description
-              ~make_description:make_unprotected_write_description
+              ~report_kind:(WriteWriteRace conflict) reported_access ;
-              ~report_kind:(WriteWriteRace conflict) snapshot.access threads ;
+            update_reported reported_access reported_acc )
-            update_reported snapshot.access pname 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
+                 report_thread_safety_violation ~make_description ~report_kind reported_access ;
-                   snapshot.access threads ;
+                 update_reported reported_access reported_acc )
-                 update_reported snapshot.access pname 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
+                 report_thread_safety_violation ~make_description ~report_kind reported_access ;
-                   snapshot.access threads ;
+                 update_reported reported_access reported_acc )
-                 update_reported snapshot.access pname 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
+    then init
-    else
+    else List.fold reportable_accesses ~init ~f:(report_unsafe_access reportable_accesses)
-      (* reset the reported reads and writes for each memory location *)
+  in
-      let init =
+  let report_guardedby_violations_on_location grouped_accesses init =
-        { reported_acc with
+    List.fold grouped_accesses ~init ~f:(fun acc r ->
-          reported_writes= Typ.Procname.Set.empty; reported_reads= Typ.Procname.Set.empty }
+        if should_report_guardedby_violation r && not (is_duplicate_report r acc) then (
-      in
+          report_thread_safety_violation ~make_description:make_guardedby_violation_description
-      let reportable_accesses = List.filter ~f:should_report_on_proc grouped_accesses in
+            ~report_kind:GuardedByViolation r ;
-      List.fold reportable_accesses ~init ~f:(report_unsafe_access reportable_accesses)
+          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) ->
+  List.fold file_env ~init:String.Map.empty ~f:(fun acc ((tenv, pdesc) as proc) ->
-      Procdesc.get_proc_name pdesc |> Typ.Procname.get_class_name
+      if should_report_on_proc tenv pdesc then
-      |> Option.fold ~init:acc ~f:(fun acc classname ->
+        Procdesc.get_proc_name pdesc |> Typ.Procname.get_class_name
-             String.Map.add_multi acc ~key:classname ~data:proc ) )
+        |> 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

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, [<Write on unknown thread>,access to `this.xForSub`,<Write on background thread>,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, [<Write on unknown thread>,access to `this.guardedbynl`,<Write on background thread>,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, [<Write on unknown thread>,access to `this.guardedbyrel`,<Write on background thread>,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, [<Read trace>,access to `this.g`,<Write trace>,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, [<Write on unknown thread>,access to `this.guardedByLock1`,<Write on background thread>,access to `this.guardedByLock1`]
+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, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Write on unknown thread>,access to `this.guardedByLock2`,<Write on background thread>,access to `this.guardedByLock2`]
+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, [<Read trace>,access to `this.f`,<Write trace>,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, [<Read trace>,access to `this.f`,<Write trace>,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, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,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, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,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, [<Read trace>,access to `this.f`,<Write trace>,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, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
-codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readFOkMethodAnnotated():void, 114, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.f`,<Write trace>,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, [<Read trace>,access to `this.f`,<Write trace>,access to `this.f`]
 codetoanalyze/java/infer/GuardedByExample.java, codetoanalyze.java.infer.GuardedByExample.readGFromCopyOk():void, 267, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mCopyOfG`,<Write trace>,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, [<Read trace>,access to `this.f`,<Write trace>,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, [<Read trace>,access to `infer.GuardedByExample.sGuardedByClass`,<Write trace>,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, [<Write on unknown thread>,access to `this.f`,<Write on background thread>,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()]

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;
+  }
+}

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, [<Read trace>,call to int GuardedByTests.privateUnlockedReadOk(),access to `this.f`,<Write trace>,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, [<Write on unknown thread>,call to void GuardedByTests.privateUnlockedWriteOk(),access to `this.f`,<Write on background thread>,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, [<Read trace>,access to `this.f`,<Write trace>,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, [<Write on unknown thread>,access to `this.f`,<Write on background thread>,access to `this.f`]
 codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.read4OutsideSyncBad():int, 64, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mField4`,<Write trace>,access to `this.mField4`]
 codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead1Bad():int, 21, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mField1`,<Write trace>,access to `this.mField1`]
 codetoanalyze/java/racerd/Inference.java, codetoanalyze.java.checkers.Inference.unprotectedRead2Bad():int, 34, THREAD_SAFETY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `this.mField2`,<Write trace>,access to `this.mField2`]