[thread-safety] support fields annotated with @ThreadConfined

Summary:
Similar to marking classes ThreadConfined, we want to support marking fields as well.
The intended semantics are: don't warn on writes to the marked field outside of syncrhonization, but continue to warn on accesses to subfields.

Reviewed By: peterogithub

Differential Revision: D4406890

fbshipit-source-id: af8a114

infer/annotations/com/facebook/infer/annotation/ThreadConfined.java

@@ -15,7 +15,7 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 
-@Target(ElementType.TYPE)
+@Target({ ElementType.TYPE, ElementType.FIELD })
 @Retention(RetentionPolicy.CLASS)
 public @interface ThreadConfined {
 }

infer/src/checkers/ThreadSafety.ml

@@ -64,18 +64,29 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
     try Some (IdAccessPathMapDomain.find id id_map)
     with Not_found -> None
 
-  let add_path_to_state exp typ loc path_state id_map owned =
+  let add_path_to_state exp typ loc path_state id_map owned tenv =
     (* remove the last field of the access path, if it has any *)
     let truncate = function
       | base, []
       | base, _ :: [] -> base, []
       | base, accesses -> base, IList.rev (IList.tl (IList.rev accesses)) in
+    let is_thread_confined (_, accesses) = match IList.rev accesses with
+      | AccessPath.FieldAccess (fieldname, Typ.Tstruct typename) :: _ ->
+          begin
+            match Tenv.lookup tenv typename with
+            | Some struct_typ ->
+                Annotations.field_has_annot fieldname struct_typ Annotations.ia_is_thread_confined
+            | None ->
+                false
+          end
+      | _ ->
+          false in
     let f_resolve_id = resolve_id id_map in
 
     IList.fold_left
       (fun acc rawpath ->
-         let base_path = truncate rawpath in
-         if not (ThreadSafetyDomain.OwnershipDomain.mem base_path owned)
+         if not (ThreadSafetyDomain.OwnershipDomain.mem (truncate rawpath) owned) &&
+            not (is_thread_confined rawpath)
          then
            ThreadSafetyDomain.PathDomain.add_sink (ThreadSafetyDomain.make_access rawpath loc) acc
          else
@@ -114,7 +125,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         Ident.create_fieldname (Mangled.from_string (Procname.get_method pn)) 0 in
       let dummy_access_exp = Exp.Lfield (exp, dummy_fieldname, typ) in
       let writes =
-        add_path_to_state dummy_access_exp typ loc astate.writes astate.id_map astate.owned in
+        add_path_to_state dummy_access_exp typ loc astate.writes astate.id_map astate.owned tenv in
       { astate with writes; } in
     let is_unprotected is_locked =
       not is_locked && not (Procdesc.is_java_synchronized pdesc) in
@@ -182,7 +193,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         let writes' =
           match lhs_exp with
           | Lfield ( _, _, typ) when is_unprotected locks -> (* abstracts no lock being held *)
-              add_path_to_state lhs_exp typ loc writes id_map owned
+              add_path_to_state lhs_exp typ loc writes id_map owned tenv
           | _ -> writes in
         (* if rhs is owned, propagate ownership to lhs. otherwise, remove lhs from ownerhsip set
            (since it may have previously held an owned memory loc and is now being reassigned *)
@@ -203,7 +214,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         let reads' =
           match rhs_exp with
           | Lfield ( _, _, typ) when is_unprotected locks ->
-              add_path_to_state rhs_exp typ loc reads id_map owned
+              add_path_to_state rhs_exp typ loc reads id_map owned tenv
           | _ ->
               reads in
         (* if rhs is owned, propagate ownership to lhs *)

infer/src/checkers/annotations.ml

@@ -81,6 +81,12 @@ let ma_contains ma ann_names =
 let pdesc_has_annot pdesc annot =
   ma_contains (Procdesc.get_attributes pdesc).ProcAttributes.method_annotation [annot]
 
+let field_has_annot fieldname (struct_typ : StructTyp.t) f =
+  let fld_has_taint_annot (fname, _, annot) =
+    Ident.equal_fieldname fieldname fname && f annot in
+  IList.exists fld_has_taint_annot struct_typ.fields ||
+  IList.exists fld_has_taint_annot struct_typ.statics
+
 let initializer_ = "Initializer"
 let inject = "Inject"
 let inject_view = "InjectView"

infer/src/checkers/annotations.mli

@@ -109,6 +109,8 @@ val ma_has_annotation_with : Annot.Method.t -> (Annot.t -> bool) -> bool
 
 val pdesc_has_annot : Procdesc.t -> string -> bool
 
+val field_has_annot : Ident.fieldname -> StructTyp.t -> (Annot.Item.t -> bool) -> bool
+
 (** Mark the return of the method_annotation with the given annotation. *)
 val method_annotation_mark_return :
   annotation -> Annot.Method.t -> Annot.Method.t

infer/tests/codetoanalyze/java/threadsafety/Annotations.java

@@ -52,4 +52,22 @@ class Annotations {
     }
   }
 
+  static class Obj {
+    Object fld;
+  }
+
+  @ThreadConfined Obj encapsulatedField;
+
+  public void mutateConfinedFieldDirectlyOk() {
+    this.encapsulatedField = new Obj();
+  }
+
+  public static void mutateConfinedFieldIndirectlyOk(Annotations a) {
+    a.encapsulatedField = new Obj();
+  }
+
+  public void mutateSubfieldOfConfinedBad() {
+    this.encapsulatedField.fld = new Object();
+  }
+
 }

infer/tests/codetoanalyze/java/threadsafety/issues.exp

@@ -1,4 +1,5 @@
 codetoanalyze/java/threadsafety/Annotations.java, void Annotations.mutateOffUiThreadBad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Annotations.f]
+codetoanalyze/java/threadsafety/Annotations.java, void Annotations.mutateSubfieldOfConfinedBad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Annotations.encapsulatedField.codetoanalyze.java.checkers.Annotations$Obj.fld]
 codetoanalyze/java/threadsafety/Builders.java, Builders$Obj Builders.buildThenMutateBad(Builders$Obj), 3, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Builders$Obj.g]
 codetoanalyze/java/threadsafety/Builders.java, Builders$Obj Builders.mutateBad(Builders$Obj), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Builders$Obj.g]
 codetoanalyze/java/threadsafety/Containers.java, void Containers.mapClearBad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mMap.clear]