diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 1bcc4c7ac..a554b4828 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -362,11 +362,19 @@ module TransferFunctions (CFG : ProcCfg.S) = struct (AccessPrecondition.make_unprotected (IntSet.singleton 0)) container_access AccessDomain.empty in + (* if a container c is owned in cpp, make c[i] owned for all i *) + let return_ownership = + match callee_pname with + | Typ.Procname.ObjC_Cpp _ | C _ -> + OwnershipAbstractValue.make_owned_if 0 + | _ -> + OwnershipAbstractValue.unowned + in Some { locks= false ; threads= ThreadsDomain.empty ; accesses= callee_accesses - ; return_ownership= OwnershipAbstractValue.unowned + ; return_ownership ; return_attributes= AttributeSetDomain.empty } diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml index 284d675ce..a0dcec720 100644 --- a/infer/src/concurrency/RacerDConfig.ml +++ b/infer/src/concurrency/RacerDConfig.ml @@ -149,9 +149,18 @@ module Models = struct let get_container_access = let is_cpp_container_read = + let is_container_operator pname_qualifiers = + match QualifiedCppName.extract_last pname_qualifiers with + | Some (last, _) -> + String.equal last "operator[]" + | None -> + false + in let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::map::find"] in fun pname -> - QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) + let pname_qualifiers = Typ.Procname.get_qualifiers pname in + QualifiedCppName.Match.match_qualifiers matcher pname_qualifiers + || is_container_operator pname_qualifiers and is_cpp_container_write = let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::map::operator[]"; "std::map::erase"] @@ -198,10 +207,14 @@ module Models = struct None in PatternMatch.supertype_find_map_opt tenv get_container_access_ typename + (* The following order matters: we want to check if pname is a container write + before we check if pname is a container read. This is due to a different + treatment between std::map::operator[] and all other operator[]. *) + | Typ.Procname.ObjC_Cpp _ | C _ as pname + when is_cpp_container_write pname -> + Some ContainerWrite | Typ.Procname.ObjC_Cpp _ | C _ as pname when is_cpp_container_read pname -> Some ContainerRead - | Typ.Procname.ObjC_Cpp _ | C _ as pname when is_cpp_container_write pname -> - Some ContainerWrite | _ -> None diff --git a/infer/tests/codetoanalyze/cpp/racerd/constructor_ownership.cpp b/infer/tests/codetoanalyze/cpp/racerd/constructor_ownership.cpp new file mode 100644 index 000000000..ba801378d --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/racerd/constructor_ownership.cpp @@ -0,0 +1,90 @@ +/* + * 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 +#include +#include + +namespace constructors { + +struct dynamic { + enum T { + NULLT, + }; + + private: + struct ObjectMaker; + + public: + template + dynamic(T t); + static ObjectMaker object(); + dynamic(dynamic&&) noexcept; + dynamic& operator=(dynamic&&) noexcept; + dynamic& operator[](dynamic const&) &; + dynamic&& operator[](dynamic const&) &&; + template + void insert(K&&, V&& val); + + private: + T type_; +}; + +dynamic& dynamic::operator=(dynamic&& o) noexcept { + if (&o != this) { + if (type_ == o.type_) { + } else { + type_ = o.type_; + } + } + return *this; +} + +struct dynamic::ObjectMaker { + friend struct dynamic; + + explicit ObjectMaker() {} + + ObjectMaker(ObjectMaker&&) = default; +}; + +inline dynamic::ObjectMaker dynamic::object() { return ObjectMaker(); } + +struct BSS { + dynamic toJson_ok() const noexcept { + dynamic ret = dynamic::object(); + ret["key"] = dynamic::object(); + return ret; + } + + dynamic& toJson_race(dynamic& ret) const noexcept { + ret["key"] = dynamic::object(); + return ret; + } +}; + +struct TSL { + std::mutex mutex_; + + void not_locked_ok(dynamic& ret) { BSS().toJson_ok(); } + + void locked_ok(dynamic& ret) { + std::lock_guard lock(mutex_); + BSS().toJson_ok(); + } + + void not_locked_race(dynamic& ret) { BSS().toJson_race(ret); } + + void locked_race(dynamic& ret) { + std::lock_guard lock(mutex_); + BSS().toJson_race(ret); + } +}; + +} // namespace constructors diff --git a/infer/tests/codetoanalyze/cpp/racerd/issues.exp b/infer/tests/codetoanalyze/cpp/racerd/issues.exp index b683fac38..e100a0b49 100644 --- a/infer/tests/codetoanalyze/cpp/racerd/issues.exp +++ b/infer/tests/codetoanalyze/cpp/racerd/issues.exp @@ -1,6 +1,7 @@ codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get2, 3, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`] codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get4, 0, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get5, 0, LOCK_CONSISTENCY_VIOLATION, [,call to basics::Basic_get_private_suspiciously_read,access to `&this.suspiciously_read`,,access to `&this.suspiciously_read`] +codetoanalyze/cpp/racerd/constructor_ownership.cpp, constructors::TSL_not_locked_race, 0, LOCK_CONSISTENCY_VIOLATION, [,call to constructors::BSS_toJson_race,call to constructors::dynamic_operator=,access to `&this.type_`,,call to constructors::BSS_toJson_race,call to constructors::dynamic_operator=,access to `&this.type_`] codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test2_bad, 5, LOCK_CONSISTENCY_VIOLATION, [,access to `&x.f`,,access to `&x.f`] codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test3_bad, 5, LOCK_CONSISTENCY_VIOLATION, [,access to `&x.f`,,access to `&x.f`] codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get2, 3, LOCK_CONSISTENCY_VIOLATION, [,access to `&this.suspiciously_written`,,access to `&this.suspiciously_written`]