[thread-safety] assume formals of DI constructors are owned

Reviewed By: jeremydubreil

Differential Revision: D4699862

fbshipit-source-id: 1cf23b0

infer/src/checkers/ThreadSafety.ml

@@ -762,16 +762,25 @@ let analyze_procedure callback =
         let initial =
           if is_initializer tenv (Procdesc.get_proc_name pdesc)
           then
-            (* express that the constructor owns [this] *)
-            match FormalMap.get_formal_base 0 extras with
-            | Some base ->
-                let attribute_map =
-                  AttributeMapDomain.add_attribute
-                    (base, [])
-                    Attribute.unconditionally_owned
-                    ThreadSafetyDomain.empty.attribute_map in
-                { ThreadSafetyDomain.empty with attribute_map; }
-            | None -> ThreadSafetyDomain.empty
+            let add_owned_formal acc formal_index =
+              match FormalMap.get_formal_base formal_index extras with
+              | Some base ->
+                  AttributeMapDomain.add_attribute (base, []) Attribute.unconditionally_owned acc
+              | None ->
+                  acc in
+            let owned_formals =
+              (* if a constructer is called via DI, all of its formals will be freshly allocated
+                 and therefore owned. we assume that constructors annotated with @Inject will only
+                 be called via DI or using fresh parameters. *)
+              if Annotations.pdesc_has_return_annot pdesc Annotations.ia_is_inject
+              then List.mapi ~f:(fun i _ -> i)  (Procdesc.get_formals pdesc)
+              else [0] (* express that the constructor owns [this] *) in
+            let attribute_map =
+              List.fold
+                ~f:add_owned_formal
+                owned_formals
+                ~init:ThreadSafetyDomain.empty.attribute_map in
+            { ThreadSafetyDomain.empty with attribute_map; }
           else
             ThreadSafetyDomain.empty in
 

infer/src/checkers/annotations.ml

@@ -201,6 +201,9 @@ let ia_is_no_allocation ia =
 let ia_is_ignore_allocations ia =
   ia_ends_with ia ignore_allocations
 
+let ia_is_inject ia =
+  ia_ends_with ia inject
+
 let ia_is_suppress_lint ia =
   ia_ends_with ia suppress_lint
 

infer/src/checkers/annotations.mli

@@ -63,6 +63,7 @@ val ia_is_functional : Annot.Item.t -> bool
 val ia_is_performance_critical : Annot.Item.t -> bool
 val ia_is_no_allocation : Annot.Item.t -> bool
 val ia_is_ignore_allocations : Annot.Item.t -> bool
+val ia_is_inject : Annot.Item.t -> bool
 val ia_is_suppress_lint : Annot.Item.t -> bool
 val ia_is_on_event : Annot.Item.t -> bool
 val ia_is_on_bind : Annot.Item.t -> bool

infer/tests/codetoanalyze/java/threadsafety/Ownership.java

@@ -44,6 +44,21 @@ public class Ownership {
     owned.f = new Object(); // should not report
   }
 
+  Obj mInjectedField1;
+  Obj mInjectedField2;
+
+  // because this constructor is meant to be called via DI, we assume that injectedField and other
+  // parameters passed to the constructor will always be freshly allocated
+  @Inject Ownership(Obj injectedField1, Obj injectedField2) {
+    mInjectedField1 = injectedField1;
+    mInjectedField2 = injectedField2;
+    mInjectedField1.f = new Object(); // should not warn
+    mInjectedField2.f = new Object(); // should not warn
+  }
+
+  Ownership(Obj obj, Object o) {
+    obj.f = o; // not annotated @Inject; should warn
+  }
 
   native void leakToAnotherThread(Object o);
 

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

@@ -41,6 +41,7 @@ codetoanalyze/java/threadsafety/Locks.java, void Locks.afterReentrantLockUnlockB
 codetoanalyze/java/threadsafety/Locks.java, void Locks.afterUnlockBad(), 3, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Locks.f]
 codetoanalyze/java/threadsafety/Locks.java, void Locks.afterWriteLockUnlockBad(), 3, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Locks.f]
 codetoanalyze/java/threadsafety/Locks.java, void Locks.lockInOneBranchBad(boolean), 4, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Locks.f]
+codetoanalyze/java/threadsafety/Ownership.java, Ownership.<init>(Obj,Object), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Obj.f]
 codetoanalyze/java/threadsafety/Ownership.java, void Ownership.FP_ownAndConditionalOwnOk(), 3, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Obj.f]
 codetoanalyze/java/threadsafety/Ownership.java, void Ownership.FP_twoDifferentConditionalOwnsOk(), 4, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Obj.f]
 codetoanalyze/java/threadsafety/Ownership.java, void Ownership.cantOwnThisBad(), 1, THREAD_SAFETY_VIOLATION, [call to void Ownership.setField(Obj),access to codetoanalyze.java.checkers.Ownership.field]