diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 672988f7a..e4ee27f0a 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -929,6 +929,9 @@ let analyze_procedure {Callbacks.proc_desc; get_proc_desc; tenv; summary} = | _ -> OwnershipDomain.empty in + let add_conditional_owned_formal acc (formal, formal_index) = + OwnershipDomain.add (formal, []) (OwnershipAbstractValue.make_owned_if formal_index) acc + in if is_initializer tenv (Procdesc.get_proc_name proc_desc) then let add_owned_formal acc formal_index = match FormalMap.get_formal_base formal_index formal_map with @@ -937,25 +940,27 @@ let analyze_procedure {Callbacks.proc_desc; get_proc_desc; tenv; summary} = | None -> acc in - let owned_formals = + let ownership = (* 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 proc_desc Annotations.ia_is_inject then List.mapi ~f:(fun i _ -> i) (Procdesc.get_formals proc_desc) - else [0] - (* express that the constructor owns [this] *) + |> List.fold ~f:add_owned_formal ~init:own_locals_in_cpp + else + (* express that the constructor owns [this] *) + let init = add_owned_formal own_locals_in_cpp 0 in + List.fold ~f:add_conditional_owned_formal ~init + (List.filter + ~f:(fun (_, index) -> not (Int.equal index 0)) + (FormalMap.get_formals_indexes formal_map)) in - let ownership = List.fold ~f:add_owned_formal owned_formals ~init:own_locals_in_cpp in {RacerDDomain.empty with ownership; threads} else (* add Owned(formal_index) predicates for each formal to indicate that each one is owned if it is owned in the caller *) - let add_owned_formal acc (formal, formal_index) = - OwnershipDomain.add (formal, []) (OwnershipAbstractValue.make_owned_if formal_index) acc - in let ownership = - List.fold ~f:add_owned_formal + List.fold ~f:add_conditional_owned_formal (FormalMap.get_formals_indexes formal_map) ~init:own_locals_in_cpp in diff --git a/infer/tests/codetoanalyze/cpp/racerd/constructor_formals.cpp b/infer/tests/codetoanalyze/cpp/racerd/constructor_formals.cpp new file mode 100644 index 000000000..1ce72a793 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/racerd/constructor_formals.cpp @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2017 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#include + +namespace constructor_formals { + +struct Y { + Y() : rawStatus_(-3) {} + Y(const Y& p) = default; + Y& operator=(const Y& p) = default; + Y(Y&& p) noexcept : rawStatus_(p.rawStatus_) { p.rawStatus_ = -3; } + + int rawStatus_; +}; + +struct S { + Y w() { return returnCode; } + + Y returnCode; +}; + +struct SU { + void p() { + S s; + auto result = s.w(); + } +}; + +class Basic { + public: + Basic() {} + + int test_locked() { + SU su; + mutex_.lock(); + su.p(); + } + + int test_not_locked() { + SU su; + su.p(); // FP fixed after adding ownership to formal parameters of + // constructors + } + + private: + std::mutex mutex_; +}; +} // namespace constructor_formals diff --git a/infer/tests/codetoanalyze/java/racerd/Ownership.java b/infer/tests/codetoanalyze/java/racerd/Ownership.java index 4971c6748..bcb493a38 100644 --- a/infer/tests/codetoanalyze/java/racerd/Ownership.java +++ b/infer/tests/codetoanalyze/java/racerd/Ownership.java @@ -517,3 +517,14 @@ class Subclass extends Obj { } } + +@ThreadSafe +class OtherObj { + private OtherObj(Obj o) { + o.f = new Object(); + } + + void mutateInConstructorOk() { + new OtherObj(new Obj()); + } +}