[LockConsistency] Add ownership to container accesses for cpp

Summary: To avoid false positives, we treat `operator[]` in cpp as container read. Moreover, if a container `c` is owned, we make all accesses `c[i]` to be also owned.

Reviewed By: sblackshear

Differential Revision: D6396574

fbshipit-source-id: 94aabff

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 }
 
 

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
 

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 <mutex>
+#include <string>
+#include <unordered_map>
+
+namespace constructors {
+
+struct dynamic {
+  enum T {
+    NULLT,
+  };
+
+ private:
+  struct ObjectMaker;
+
+ public:
+  template <class T>
+  dynamic(T t);
+  static ObjectMaker object();
+  dynamic(dynamic&&) noexcept;
+  dynamic& operator=(dynamic&&) noexcept;
+  dynamic& operator[](dynamic const&) &;
+  dynamic&& operator[](dynamic const&) &&;
+  template <class K, class V>
+  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<std::mutex> lock(mutex_);
+    BSS().toJson_ok();
+  }
+
+  void not_locked_race(dynamic& ret) { BSS().toJson_race(ret); }
+
+  void locked_race(dynamic& ret) {
+    std::lock_guard<std::mutex> lock(mutex_);
+    BSS().toJson_race(ret);
+  }
+};
+
+} // namespace constructors

infer/tests/codetoanalyze/cpp/racerd/issues.exp

@@ -1,6 +1,7 @@
 codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get2, 3, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]
 codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get4, 0, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
 codetoanalyze/cpp/racerd/basics.cpp, basics::Basic_get5, 0, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,call to basics::Basic_get_private_suspiciously_read,access to `&this.suspiciously_read`,<Write trace>,access to `&this.suspiciously_read`]
+codetoanalyze/cpp/racerd/constructor_ownership.cpp, constructors::TSL_not_locked_race, 0, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,call to constructors::BSS_toJson_race,call to constructors::dynamic_operator=,access to `&this.type_`,<Write trace>,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, [<Read trace>,access to `&x.f`,<Write trace>,access to `&x.f`]
 codetoanalyze/cpp/racerd/locals_ownership.cpp, locals::Ownership_test3_bad, 5, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&x.f`,<Write trace>,access to `&x.f`]
 codetoanalyze/cpp/racerd/lock_guard.cpp, basics::LockGuard_get2, 3, LOCK_CONSISTENCY_VIOLATION, [<Read trace>,access to `&this.suspiciously_written`,<Write trace>,access to `&this.suspiciously_written`]