[clang][objcpp] register exported methods and treat them as private in RacerD

Summary:
In ObjC there are no access modifiers.  The strongest alternative is to put methods in the implementation but omit them from the interface declaration.

Put exported ObjC methods in their own field in the class structure and use that in RacerD to decide whether to report on the method.

Reviewed By: mbouaziz

Differential Revision: D13597504

fbshipit-source-id: c4a3d2705

infer/src/IR/Tenv.ml

@@ -34,9 +34,10 @@ let pp fmt (tenv : t) =
 let create () = TypenameHash.create 1000
 
 (** Construct a struct type in a type environment *)
-let mk_struct tenv ?default ?fields ?statics ?methods ?supers ?annots name =
+let mk_struct tenv ?default ?fields ?statics ?methods ?exported_objc_methods ?supers ?annots name =
   let struct_typ =
-    Typ.Struct.internal_mk_struct ?default ?fields ?statics ?methods ?supers ?annots ()
+    Typ.Struct.internal_mk_struct ?default ?fields ?statics ?methods ?exported_objc_methods ?supers
+      ?annots ()
   in
   TypenameHash.replace tenv name struct_typ ;
   struct_typ

infer/src/IR/Tenv.mli

@@ -35,6 +35,7 @@ val mk_struct :
   -> ?fields:Typ.Struct.fields
   -> ?statics:Typ.Struct.fields
   -> ?methods:Typ.Procname.t list
+  -> ?exported_objc_methods:Typ.Procname.t list
   -> ?supers:Typ.Name.t list
   -> ?annots:Annot.Item.t
   -> Typ.Name.t

infer/src/IR/Typ.ml

@@ -1459,6 +1459,7 @@ module Struct = struct
     ; statics: fields  (** static fields *)
     ; supers: Name.t list  (** superclasses *)
     ; methods: Procname.t list  (** methods defined *)
+    ; exported_objc_methods: Procname.t list  (** methods in ObjC interface, subset of [methods] *)
     ; annots: Annot.Item.t  (** annotations *) }
 
   type lookup = Name.t -> t option
@@ -1467,7 +1468,7 @@ module Struct = struct
     F.fprintf f "@\n\t\t%a %a %a" (pp_full pe) typ Fieldname.pp field_name Annot.Item.pp ann
 
 
-  let pp pe name f {fields; supers; methods; annots} =
+  let pp pe name f {fields; supers; methods; exported_objc_methods; annots} =
     if Config.debug_mode then
       (* change false to true to print the details of struct *)
       F.fprintf f
@@ -1478,6 +1479,8 @@ module Struct = struct
          \t}@\n\
          \tmethods: {%a@\n\
          \t}@\n\
+         \texported_obj_methods: {%a@\n\
+         \t}@\n\
          \tannots: {%a@\n\
          \t}"
         Name.pp name
@@ -1486,17 +1489,28 @@ module Struct = struct
         (Pp.seq (fun f n -> F.fprintf f "@\n\t\t%a" Name.pp n))
         supers
         (Pp.seq (fun f m -> F.fprintf f "@\n\t\t%a" Procname.pp m))
-        methods Annot.Item.pp annots
+        methods
+        (Pp.seq (fun f m -> F.fprintf f "@\n\t\t%a" Procname.pp m))
+        exported_objc_methods Annot.Item.pp annots
     else Name.pp f name
 
 
-  let internal_mk_struct ?default ?fields ?statics ?methods ?supers ?annots () =
-    let default_ = {fields= []; statics= []; methods= []; supers= []; annots= Annot.Item.empty} in
+  let internal_mk_struct ?default ?fields ?statics ?methods ?exported_objc_methods ?supers ?annots
+      () =
+    let default_ =
+      { fields= []
+      ; statics= []
+      ; methods= []
+      ; exported_objc_methods= []
+      ; supers= []
+      ; annots= Annot.Item.empty }
+    in
     let mk_struct_ ?(default = default_) ?(fields = default.fields) ?(statics = default.statics)
-        ?(methods = default.methods) ?(supers = default.supers) ?(annots = default.annots) () =
-      {fields; statics; methods; supers; annots}
+        ?(methods = default.methods) ?(exported_objc_methods = default.exported_objc_methods)
+        ?(supers = default.supers) ?(annots = default.annots) () =
+      {fields; statics; methods; exported_objc_methods; supers; annots}
     in
-    mk_struct_ ?default ?fields ?statics ?methods ?supers ?annots ()
+    mk_struct_ ?default ?fields ?statics ?methods ?exported_objc_methods ?supers ?annots ()
 
 
   (** the element typ of the final extensible array in the given typ, if any *)

infer/src/IR/Typ.mli

@@ -665,6 +665,7 @@ module Struct : sig
     ; statics: fields  (** static fields *)
     ; supers: Name.t list  (** supers *)
     ; methods: Procname.t list  (** methods defined *)
+    ; exported_objc_methods: Procname.t list  (** methods in ObjC interface, subset of [methods] *)
     ; annots: Annot.Item.t  (** annotations *) }
 
   type lookup = Name.t -> t option
@@ -679,6 +680,7 @@ module Struct : sig
     -> ?fields:fields
     -> ?statics:fields
     -> ?methods:Procname.t list
+    -> ?exported_objc_methods:Procname.t list
     -> ?supers:Name.t list
     -> ?annots:Annot.Item.t
     -> unit

infer/src/clang/objcInterface_decl.ml

@@ -121,7 +121,7 @@ let add_class_to_tenv qual_type_to_sil_type procname_from_decl tenv decl_info na
   in
   ignore
     (Tenv.mk_struct tenv ~fields:all_fields ~supers ~methods ~annots:Annot.Class.objc
-       interface_name) ;
+       ~exported_objc_methods:methods interface_name) ;
   interface_desc
 
 

infer/src/concurrency/RacerD.ml

@@ -968,16 +968,17 @@ let should_report_on_proc procdesc tenv =
       || Procdesc.get_access procdesc <> PredSymb.Private
          && (not (Typ.Procname.Java.is_autogen_method java_pname))
          && not (Annotations.pdesc_return_annot_ends_with procdesc Annotations.visibleForTesting)
-  | ObjC_Cpp objc_cpp_pname ->
-      ( match objc_cpp_pname.Typ.Procname.ObjC_Cpp.kind with
+  | ObjC_Cpp {kind; class_name} ->
+      ( match kind with
       | CPPMethod _ | CPPConstructor _ | CPPDestructor _ ->
           Procdesc.get_access procdesc <> PredSymb.Private
       | ObjCClassMethod | ObjCInstanceMethod | ObjCInternalMethod ->
-          (* ObjC has no private methods, just a convention that they start with an underscore *)
-          not (String.is_prefix objc_cpp_pname.Typ.Procname.ObjC_Cpp.method_name ~prefix:"_") )
+          Tenv.lookup tenv class_name
+          |> Option.exists ~f:(fun {Typ.Struct.exported_objc_methods} ->
+                 List.mem ~equal:Typ.Procname.equal exported_objc_methods proc_name ) )
       &&
       let matcher = ConcurrencyModels.cpp_lock_types_matcher in
-      Option.exists (Tenv.lookup tenv objc_cpp_pname.class_name) ~f:(fun class_str ->
+      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 ->

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

@@ -4,31 +4,26 @@
  * 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>
+#import <Foundation/NSObject.h>
 
 @interface Basic : NSObject
-- (int)read;
-- (void)_private_write_no_top_level_report:(int)data;
-- (void)write:(int)data;
+- (int)read_bad;
+- (void)write_bad:(int)data;
 @end
 
 @implementation Basic {
-  std::mutex mutex_;
-  int data_;
-}
-
-- (int)read {
-  return data_;
+  std::mutex _mutex;
+  int _data;
 }
 
-- (void)_private_write_no_top_level_report:(int)data {
-  data_ = data;
+- (int)read_bad {
+  return _data;
 }
 
-- (void)write:(int)data {
-  mutex_.lock();
-  [self _private_write_no_top_level_report:data];
-  mutex_.unlock();
+- (void)write_bad:(int)data {
+  _mutex.lock();
+  _data = data;
+  _mutex.unlock();
 }
 @end

infer/tests/codetoanalyze/objcpp/racerd/Private.h

@@ -0,0 +1,13 @@
+/*
+ * 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>
+
+@interface Private : NSObject
+- (void)write_ok:(int)data;
+- (int)read_other_bad;
+- (void)write_other_bad:(int)other_data;
+@end

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

@@ -0,0 +1,36 @@
+/*
+ * 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 <mutex>
+#import "Private.h"
+
+@implementation Private {
+  std::mutex _mutex;
+  int _data;
+  int _other_data;
+}
+
+// no report on _data
+- (int)_private_read_ok {
+  return _data;
+}
+
+- (void)write_ok:(int)data {
+  _mutex.lock();
+  _data = data;
+  _mutex.unlock();
+}
+
+- (int)read_other_bad {
+  return _other_data;
+}
+
+- (void)write_other_bad:(int)other_data {
+  _mutex.lock();
+  _other_data = other_data;
+  _mutex.unlock();
+}
+@end

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

@@ -1 +1,2 @@
-codetoanalyze/objcpp/racerd/Basic.mm, Basic_read, 22, LOCK_CONSISTENCY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `self.data_`,<Write trace>,call to Basic__private_write_no_top_level_report:,access to `self.data_`]
+codetoanalyze/objcpp/racerd/Basic.mm, Basic_read_bad, 21, LOCK_CONSISTENCY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `self._data`,<Write trace>,access to `self._data`]
+codetoanalyze/objcpp/racerd/Private.mm, Private_read_other_bad, 28, LOCK_CONSISTENCY_VIOLATION, no_bucket, ERROR, [<Read trace>,access to `self._other_data`,<Write trace>,access to `self._other_data`]