From ec281799d56404bfe7796f11b2b153f8fcffbe2d Mon Sep 17 00:00:00 2001
From: Josh Berdine <jjb@fb.com>
Date: Fri, 3 Nov 2017 06:36:48 -0700
Subject: [PATCH] [lock-consistency] Skip more, and in should_analyze_proc
 instead of get_summary

Summary:
Seems it should have been done there all along.

The analyzer does not currently understand the implementation of
atomicity in folly::AtomicStruct.

The analyzer does not currently understand when std::atomic operations
are are used correctly versus incorrectly.

The analyzer does not currently understand that the representation of
folly::ThreadLocal is, ah, thread-local, leading to false alarms.

The analyzer does not currently understand the control flow /
scheduling constraints imposed by the implementation of Future.

It seems that the implementation of folly::Optional is more C++
template magic than the analyzer can currently understand.

The model of std::vector contains bogus memory accesses, leading to
false alarms.

Reviewed By: sblackshear

Differential Revision: D6226199

fbshipit-source-id: 8cb083b
---
 infer/src/concurrency/RacerD.ml        | 18 +++---------------
 infer/src/concurrency/RacerDConfig.ml  | 24 ++++++++++++++++++++++++
 infer/src/concurrency/RacerDConfig.mli |  4 ++++
 3 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml
index 87ea82639..3d77505d5 100644
--- a/infer/src/concurrency/RacerD.ml
+++ b/infer/src/concurrency/RacerD.ml
@@ -370,17 +370,6 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
       ; return_attributes= AttributeSetDomain.empty }
 
 
-  let cpp_force_skipped =
-    let matcher =
-      lazy
-        (QualifiedCppName.Match.of_fuzzy_qual_names
-           ["folly::AtomicStruct::load"; "folly::detail::SingletonHolder::createInstance"])
-    in
-    fun pname ->
-      QualifiedCppName.Match.match_qualifiers (Lazy.force matcher)
-        (Typ.Procname.get_qualifiers pname)
-
-
   let get_summary caller_pdesc callee_pname actuals callee_loc tenv =
     let open RacerDConfig in
     let get_receiver_ap actuals =
@@ -398,8 +387,6 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
     | Some ContainerRead, _ ->
         make_container_access callee_pname ~is_write:false (get_receiver_ap actuals) callee_loc
           tenv
-    | None, Typ.Procname.ObjC_Cpp _ when cpp_force_skipped callee_pname ->
-        None
     | None, _ ->
         Summary.read_summary caller_pdesc callee_pname
 
@@ -864,8 +851,8 @@ let pdesc_is_assumed_thread_safe pdesc tenv =
    find more bugs. this is just a temporary measure to avoid obvious false positives *)
 let should_analyze_proc pdesc tenv =
   let pn = Procdesc.get_proc_name pdesc in
-  not (Typ.Procname.is_destructor pn) && not (Typ.Procname.is_class_initializer pn)
-  && not (FbThreadSafety.is_logging_method pn) && not (pdesc_is_assumed_thread_safe pdesc tenv)
+  not (Typ.Procname.is_class_initializer pn) && not (FbThreadSafety.is_logging_method pn)
+  && not (pdesc_is_assumed_thread_safe pdesc tenv) && not (RacerDConfig.Models.should_skip pn)
 
 
 let get_current_class_and_threadsafe_superclasses tenv pname =
@@ -1733,3 +1720,4 @@ let file_analysis {Callbacks.procedures} =
            else (module MayAliasQuotientedAccessListMap) )
            class_env))
     (aggregate_by_class procedures)
+
diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml
index 3cd5629e2..f4f6b9059 100644
--- a/infer/src/concurrency/RacerDConfig.ml
+++ b/infer/src/concurrency/RacerDConfig.ml
@@ -205,4 +205,28 @@ module Models = struct
       | _ ->
           None
 
+
+  let should_skip =
+    let matcher =
+      lazy
+        (QualifiedCppName.Match.of_fuzzy_qual_names
+           [ "folly::AtomicStruct::AtomicStruct"
+           ; "folly::Future::Future"
+           ; "folly::LockedPtr::LockedPtr"
+           ; "folly::Optional::Optional"
+           ; "folly::Optional::hasValue"
+           ; "folly::Promise::Promise"
+           ; "folly::ThreadLocal::ThreadLocal"
+           ; "folly::detail::SingletonHolder::createInstance"
+           ; "std::atomic"
+           ; "std::vector::vector" ])
+    in
+    function
+      | Typ.Procname.ObjC_Cpp _ | C _ as pname ->
+          Typ.Procname.is_destructor pname
+          || QualifiedCppName.Match.match_qualifiers (Lazy.force matcher)
+               (Typ.Procname.get_qualifiers pname)
+      | _ ->
+          false
+
 end
diff --git a/infer/src/concurrency/RacerDConfig.mli b/infer/src/concurrency/RacerDConfig.mli
index 8abceb27c..d3c6ecd80 100644
--- a/infer/src/concurrency/RacerDConfig.mli
+++ b/infer/src/concurrency/RacerDConfig.mli
@@ -35,4 +35,8 @@ module Models : sig
 
   val get_container_access : Typ.Procname.t -> Tenv.t -> container_access option
   (** return Some (access) if this procedure accesses the contents of a container (e.g., Map.get) *)
+
+  val should_skip : Typ.Procname.t -> bool
+  (** holds of procedure names which should not be analyzed in order to avoid known sources of
+      inaccuracy *)
 end