[racerd] iOS first steps

Reviewed By: mbouaziz, jvillard

Differential Revision: D10508880

fbshipit-source-id: 04e994468

Makefile

@@ -82,7 +82,8 @@ BUILD_SYSTEMS_TESTS += objc_getters_setters objc_missing_fld objc_retain_cycles
 DIRECT_TESTS += \
   objc_frontend objc_errors objc_linters objc_ioslints objcpp_errors objcpp_nullable objcpp_retain-cycles \
   objc_linters-def-folder objc_nullable objc_liveness objcpp_liveness objc_uninit \
-  objcpp_frontend objcpp_linters cpp_linters  objc_linters-for-test-only objcpp_linters-for-test-only
+  objcpp_frontend objcpp_linters cpp_linters  objc_linters-for-test-only objcpp_linters-for-test-only \
+	objcpp_racerd
 ifneq ($(XCODE_SELECT),no)
 BUILD_SYSTEMS_TESTS += xcodebuild_no_xcpretty
 endif

infer/src/IR/Mangled.ml

@@ -38,6 +38,10 @@ let this = from_string "this"
 
 let is_this = function {plain= "this"} -> true | _ -> false
 
+let self = from_string "self"
+
+let is_self = function {plain= "self"} -> true | _ -> false
+
 module Set = Caml.Set.Make (struct
   type nonrec t = t
 

infer/src/IR/Mangled.mli

@@ -35,6 +35,10 @@ val this : t
 
 val is_this : t -> bool
 
+val self : t [@@warning "-32"]
+
+val is_self : t -> bool
+
 (** Set of Mangled. *)
 module Set : Caml.Set.S with type elt = t
 

infer/src/IR/Pvar.ml

@@ -47,7 +47,7 @@ let compare_modulo_this x y =
     else
       let cmp = Mangled.compare x.pv_name y.pv_name in
       if not (Int.equal 0 cmp) then cmp
-      else if Mangled.is_this x.pv_name then 0
+      else if Mangled.is_this x.pv_name || Mangled.is_self x.pv_name then 0
       else compare_pvar_kind x.pv_kind y.pv_kind
 
 
@@ -134,7 +134,7 @@ let is_static_local pv = match pv.pv_kind with Global_var (_, _, _, true, _) ->
 let is_this pvar = Mangled.is_this (get_name pvar)
 
 (** Check if a pvar is the special "self" var *)
-let is_self pvar = Mangled.equal (get_name pvar) (Mangled.from_string "self")
+let is_self pvar = Mangled.is_self (get_name pvar)
 
 (** Check if the pvar is a return var *)
 let is_return pv = Mangled.equal (get_name pv) Ident.name_return

infer/src/IR/Pvar.mli

@@ -22,7 +22,7 @@ type translation_unit = SourceFile.t option [@@deriving compare]
 type t [@@deriving compare]
 
 val compare_modulo_this : t -> t -> int
-(** Comparison considering all pvars named 'this' to be equal *)
+(** Comparison considering all pvars named 'this'/'self' to be equal *)
 
 val equal : t -> t -> bool
 (** Equality for pvar's *)

infer/src/biabduction/Prover.ml

@@ -999,8 +999,7 @@ let check_inconsistency_base tenv prop =
         let language = Typ.Procname.get_language (Procdesc.get_proc_name pdesc) in
         let is_java_this pvar = Language.equal language Java && Pvar.is_this pvar in
         let is_objc_instance_self pvar =
-          Language.equal language Clang
+          Language.equal language Clang && Pvar.is_self pvar
-          && Mangled.equal (Pvar.get_name pvar) (Mangled.from_string "self")
           && ClangMethodKind.equal procedure_attr.ProcAttributes.clang_method_kind
                ClangMethodKind.OBJC_INSTANCE
         in

infer/src/biabduction/SymExec.ml

@@ -781,8 +781,7 @@ let receiver_self receiver prop =
     ~f:(fun hpred ->
       match hpred with
       | Sil.Hpointsto (Exp.Lvar pv, Sil.Eexp (e, _), _) ->
-          Exp.equal e receiver && Pvar.is_seed pv
+          Exp.equal e receiver && Pvar.is_seed pv && Pvar.is_self pv
-          && Mangled.equal (Pvar.get_name pv) (Mangled.from_string "self")
       | _ ->
           false )
     prop.Prop.sigma

infer/src/clang/cPredicates.ml

@@ -983,7 +983,7 @@ let is_receiver_self an =
   | Ctl_parser_types.Stmt (Clang_ast_t.ObjCMessageExpr (_, fst_param :: _, _, _)) ->
       CAst_utils.exists_eventually_st
         (decl_ref_name ~kind:`ImplicitParam)
-        (ALVar.Const "self") fst_param
+        (ALVar.Const CFrontend_config.self) fst_param
   | _ ->
       false
 

infer/src/concurrency/RacerD.ml

@@ -1254,31 +1254,20 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) =
       { reported_acc with
         reported_writes= Typ.Procname.Set.empty; reported_reads= Typ.Procname.Set.empty }
     in
-    let class_has_mutex_member objc_cpp tenv =
-      let class_name = Typ.Procname.ObjC_Cpp.get_class_type_name objc_cpp in
-      let matcher = ConcurrencyModels.cpp_lock_types_matcher in
-      Option.exists (Tenv.lookup tenv class_name) ~f:(fun class_str ->
-          (* check if the class contains a lock member *)
-          List.exists class_str.Typ.Struct.fields ~f:(fun (_, ft, _) ->
-              Option.exists (Typ.name ft) ~f:(fun name ->
-                  QualifiedCppName.Match.match_qualifiers matcher (Typ.Name.qual_name name) ) ) )
-    in
     let should_report {tenv; procdesc} =
+      List.exists grouped_accesses ~f:(fun ({threads} : reported_access) ->
+          ThreadsDomain.is_any threads )
+      &&
       match Procdesc.get_proc_name procdesc with
       | Java _ ->
-          List.exists grouped_accesses ~f:(fun ({threads} : reported_access) ->
+          should_report_in_java procdesc tenv
-              ThreadsDomain.is_any threads )
-          && should_report_on_proc procdesc tenv
       | ObjC_Cpp objc_cpp ->
-          (* do not report if a procedure is private  *)
+          should_report_in_objcpp procdesc objc_cpp tenv
-          Procdesc.get_access procdesc <> PredSymb.Private
-          && (* report if the class has a mutex member  *)
-             class_has_mutex_member objc_cpp tenv
       | _ ->
           false
     in
     let reportable_accesses = List.filter ~f:should_report grouped_accesses in
-    List.fold ~f:(report_unsafe_access reportable_accesses) reportable_accesses ~init:reported
+    List.fold reportable_accesses ~init:reported ~f:(report_unsafe_access reportable_accesses)
   in
   ReportMap.fold report_accesses_on_location aggregated_access_map empty_reported |> ignore
 
@@ -1291,43 +1280,37 @@ let make_results_table file_env =
   let open RacerDDomain in
   let aggregate_post tenv procdesc acc {threads; accesses; wobbly_paths} =
     AccessDomain.fold
-      (fun snapshot acc ->
+      (fun snapshot acc -> ReportMap.add {threads; snapshot; tenv; procdesc; wobbly_paths} acc)
-        let reported_access : reported_access =
-          {threads; snapshot; tenv; procdesc; wobbly_paths}
-        in
-        ReportMap.add reported_access acc )
       accesses acc
   in
   let aggregate_posts acc (tenv, proc_desc) =
     Payload.read proc_desc (Procdesc.get_proc_name proc_desc)
     |> Option.fold ~init:acc ~f:(aggregate_post tenv proc_desc)
   in
-  List.fold ~f:aggregate_posts file_env ~init:ReportMap.empty
+  List.fold file_env ~init:ReportMap.empty ~f:aggregate_posts
 
 
 (* aggregate all of the procedures in the file env by their declaring
    class. this lets us analyze each class individually *)
 let aggregate_by_class file_env =
-  List.fold file_env
+  List.fold file_env ~init:String.Map.empty ~f:(fun acc ((_, pdesc) as proc) ->
-    ~f:(fun acc ((_, pdesc) as proc) ->
-      let pname = Procdesc.get_proc_name pdesc in
       let classname =
-        match pname with
+        match Procdesc.get_proc_name pdesc with
         | Typ.Procname.Java java_pname ->
-            Typ.Procname.Java.get_class_name java_pname
+            Some (Typ.Procname.Java.get_class_name java_pname)
+        | ObjC_Cpp objc_cpp_pname ->
+            Some (Typ.Procname.ObjC_Cpp.get_class_name objc_cpp_pname)
         | _ ->
-            "unknown"
+            None
       in
-      String.Map.update acc classname ~f:(function None -> [proc] | Some bucket -> proc :: bucket)
+      Option.fold classname ~init:acc ~f:(fun acc classname ->
-      )
+          String.Map.add_multi acc ~key:classname ~data:proc ) )
-    ~init:String.Map.empty
 
 
 (* Gathers results by analyzing all the methods in a file, then
    post-processes the results to check an (approximation of) thread
    safety *)
 let file_analysis {Callbacks.procedures; source_file} =
-  String.Map.iter
+  String.Map.iter (aggregate_by_class procedures) ~f:(fun class_env ->
-    ~f:(fun class_env -> report_unsafe_accesses (make_results_table class_env))
+      report_unsafe_accesses (make_results_table class_env) ) ;
-    (aggregate_by_class procedures) ;
   IssueLog.store Config.racerd_issues_dir_name source_file

infer/src/concurrency/RacerDModels.ml

@@ -371,8 +371,8 @@ let is_marked_thread_safe pdesc tenv =
 
 
 (* return true if procedure is at an abstraction boundary or reporting has been explicitly
-            requested via @ThreadSafe *)
+            requested via @ThreadSafe in java *)
-let should_report_on_proc proc_desc tenv =
+let should_report_in_java proc_desc tenv =
   let proc_name = Procdesc.get_proc_name proc_desc in
   is_thread_safe_method proc_name tenv
   || (not
@@ -383,3 +383,15 @@ let should_report_on_proc proc_desc tenv =
             false ))
      && Procdesc.get_access proc_desc <> PredSymb.Private
      && not (Annotations.pdesc_return_annot_ends_with proc_desc Annotations.visibleForTesting)
+
+
+let should_report_in_objcpp proc_desc objc_cpp tenv =
+  Procdesc.get_access proc_desc <> PredSymb.Private
+  &&
+  let class_name = Typ.Procname.ObjC_Cpp.get_class_type_name objc_cpp in
+  let matcher = ConcurrencyModels.cpp_lock_types_matcher in
+  Option.exists (Tenv.lookup tenv class_name) ~f:(fun class_str ->
+      (* check if the class contains a lock member *)
+      List.exists class_str.Typ.Struct.fields ~f:(fun (_, ft, _) ->
+          Option.exists (Typ.name ft) ~f:(fun name ->
+              QualifiedCppName.Match.match_qualifiers matcher (Typ.Name.qual_name name) ) ) )

infer/src/concurrency/RacerDModels.mli

@@ -51,6 +51,9 @@ val is_thread_safe_method : Typ.Procname.t -> Tenv.t -> bool
 
 val is_marked_thread_safe : Procdesc.t -> Tenv.t -> bool
 
-val should_report_on_proc : Procdesc.t -> Tenv.t -> bool
+val should_report_in_java : Procdesc.t -> Tenv.t -> bool
 (** return true if procedure is at an abstraction boundary or reporting has been explicitly
    requested via @ThreadSafe *)
+
+val should_report_in_objcpp : Procdesc.t -> Typ.Procname.ObjC_Cpp.t -> Tenv.t -> bool
+(** check if the class contains a lock member *)

infer/tests/codetoanalyze/objcpp/racerd/Basic.mm

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2018-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+#import <Foundation/NSObject.h>
+#import <mutex>
+
+@interface Basic : NSObject
+- (int)read;
+- (void)write:(int)data;
+@end
+
+@implementation Basic {
+  std::mutex mutex_;
+  int data_;
+}
+
+- (int)read {
+  return data_;
+}
+
+- (void)write:(int)data {
+  mutex_.lock();
+  data_ = data;
+  mutex_.unlock();
+}
+@end

infer/tests/codetoanalyze/objcpp/racerd/Makefile

@@ -0,0 +1,19 @@
+# Copyright (c) 2016-present, Facebook, Inc.
+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+
+TESTS_DIR = ../../..
+
+CLANG_OPTIONS = -c $(OBJCPP_CLANG_OPTIONS)
+
+INFER_OPTIONS = --racerd-only --debug-exceptions --project-root $(TESTS_DIR)
+
+INFERPRINT_OPTIONS = --issues-tests
+
+SOURCES = $(wildcard *.mm)
+
+include $(TESTS_DIR)/clang.make
+include $(TESTS_DIR)/objc.make
+
+infer-out/report.json: $(MAKEFILE_LIST)

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

@@ -0,0 +1 @@
+codetoanalyze/objcpp/racerd/Basic.mm, Basic_read, 21, LOCK_CONSISTENCY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `self.data_`,<Write trace>,access to `self.data_`]