diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 8c3f29b9d..b2d682142 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/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 diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index 745424d6b..0dfd3e5a3 100644 --- a/infer/src/checkers/annotations.ml +++ b/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 diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index 74024bf52..5b2506d55 100644 --- a/infer/src/checkers/annotations.mli +++ b/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 diff --git a/infer/tests/codetoanalyze/java/threadsafety/Ownership.java b/infer/tests/codetoanalyze/java/threadsafety/Ownership.java index c860768a3..51cb91473 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/Ownership.java +++ b/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); diff --git a/infer/tests/codetoanalyze/java/threadsafety/issues.exp b/infer/tests/codetoanalyze/java/threadsafety/issues.exp index e5ce4d1f2..31f7a333c 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/issues.exp +++ b/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.(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]