[objc] Adding missing fields to env when executing generated getters and setters.

Reviewed By: mbouaziz

Differential Revision: D5923752

fbshipit-source-id: 0f3a7db

Makefile

@@ -61,7 +61,7 @@ ifneq ($(PYTHON_lxml),no)
 BUILD_SYSTEMS_TESTS += results_xml
 endif
 ifneq ($(XCODE_SELECT),no)
-BUILD_SYSTEMS_TESTS += xcodebuild_no_xcpretty
+BUILD_SYSTEMS_TESTS += xcodebuild_no_xcpretty objc_getters_setters
 DIRECT_TESTS += \
   objc_frontend objc_errors objc_linters objc_ioslints \
 	objcpp_frontend objcpp_linters objc_linters-for-test-only objcpp_linters-for-test-only \

infer/src/IR/ProcAttributes.ml

@@ -31,8 +31,8 @@ let proc_flags_find proc_flags key = Hashtbl.find proc_flags key
 
 (** Type for ObjC accessors *)
 type objc_accessor_type =
-  | Objc_getter of Typ.Fieldname.t
+  | Objc_getter of Typ.Struct.field
-  | Objc_setter of Typ.Fieldname.t
+  | Objc_setter of Typ.Struct.field
   [@@deriving compare]
 
 type t =

infer/src/IR/ProcAttributes.mli

@@ -29,8 +29,8 @@ val proc_flags_find : proc_flags -> string -> string
 (** find a value for a key in the proc flags *)
 
 type objc_accessor_type =
-  | Objc_getter of Typ.Fieldname.t
+  | Objc_getter of Typ.Struct.field
-  | Objc_setter of Typ.Fieldname.t
+  | Objc_setter of Typ.Struct.field
   [@@deriving compare]
 
 type t =

infer/src/IR/Procdesc.ml

@@ -483,10 +483,10 @@ let pp_variable_list fmt etl =
 
 let pp_objc_accessor fmt accessor =
   match accessor with
-  | Some ProcAttributes.Objc_getter name
+  | Some ProcAttributes.Objc_getter field
-   -> Format.fprintf fmt "Getter of %a, " Typ.Fieldname.pp name
+   -> Format.fprintf fmt "Getter of %a, " (Typ.Struct.pp_field Pp.text) field
-  | Some ProcAttributes.Objc_setter name
+  | Some ProcAttributes.Objc_setter field
-   -> Format.fprintf fmt "Setter of %a, " Typ.Fieldname.pp name
+   -> Format.fprintf fmt "Setter of %a, " (Typ.Struct.pp_field Pp.text) field
   | None
    -> ()
 

infer/src/IR/Tenv.ml

@@ -61,6 +61,17 @@ let lookup tenv name : Typ.Struct.t option =
 (** Add a (name,type) pair to the global type environment. *)
 let add tenv name struct_typ = TypenameHash.replace tenv name struct_typ
 
+(** Add a field to a given struct in the global type environment. *)
+let add_field tenv class_tn_name field =
+  match lookup tenv class_tn_name with
+  | Some ({fields} as struct_typ)
+   -> let field_equal (f1, _, _) (f2, _, _) = Typ.Fieldname.equal f1 f2 in
+      if not (List.mem ~equal:field_equal fields field) then
+        let new_fields = field :: fields in
+        ignore (mk_struct tenv ~default:struct_typ ~fields:new_fields ~statics:[] class_tn_name)
+  | _
+   -> ()
+
 (** Get method that is being overriden by java_pname (if any) **)
 let get_overriden_method tenv pname_java =
   let struct_typ_get_method_by_name (struct_typ: Typ.Struct.t) method_name =

infer/src/IR/Tenv.mli

@@ -38,6 +38,9 @@ val mk_struct :
   -> Typ.Struct.t
 (** Construct a struct_typ, normalizing field types *)
 
+val add_field : t -> Typ.Name.t -> Typ.Struct.field -> unit
+(** Add a field to a given struct in the global type environment. *)
+
 val mem : t -> Typ.Name.t -> bool
 (** Check if typename is found in t *)
 

infer/src/IR/Typ.ml

@@ -1235,14 +1235,16 @@ module Struct = struct
 
   type lookup = Name.t -> t option
 
+  let pp_field pe f (field_name, typ, ann) =
+    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} =
     if Config.debug_mode then
       (* change false to true to print the details of struct *)
       F.fprintf f
         "%a @\n\tfields: {%a@\n\t}@\n\tsupers: {%a@\n\t}@\n\tmethods: {%a@\n\t}@\n\tannots: {%a@\n\t}"
         Name.pp name
-        (Pp.seq (fun f (fld, t, a) ->
+        (Pp.seq (pp_field pe))
-             F.fprintf f "@\n\t\t%a %a %a" (pp_full pe) t Fieldname.pp fld Annot.Item.pp a ))
         fields
         (Pp.seq (fun f n -> F.fprintf f "@\n\t\t%a" Name.pp n))
         supers

infer/src/IR/Typ.mli

@@ -580,6 +580,8 @@ module Struct : sig
 
   type lookup = Name.t -> t option
 
+  val pp_field : Pp.env -> F.formatter -> field -> unit
+
   val pp : Pp.env -> Name.t -> F.formatter -> t -> unit
   (** Pretty print a struct type. *)
 

infer/src/backend/symExec.ml

@@ -1631,26 +1631,35 @@ and check_variadic_sentinel_if_present ({Builtin.prop_; path; proc_name} as buil
      -> let formals = callee_attributes.ProcAttributes.formals in
         check_variadic_sentinel (List.length formals) sentinel_arg builtin_args
 
-and sym_exec_objc_getter field_name ret_typ tenv ret_id pdesc pname loc args prop =
+and sym_exec_objc_getter field ret_typ tenv ret_id pdesc pname loc args prop =
+  let field_name, _, _ = field in
   L.d_strln
     ( "No custom getter found. Executing the ObjC builtin getter with ivar "
     ^ Typ.Fieldname.to_string field_name ^ "." ) ;
   let ret_id = match ret_id with Some (ret_id, _) -> ret_id | None -> assert false in
   match args with
-  | [(lexp, ({Typ.desc= Tstruct _} as typ | {desc= Tptr (({desc= Tstruct _} as typ), _)}))]
+  | [ ( lexp
-   -> let field_access_exp = Exp.Lfield (lexp, field_name, typ) in
+      , ( {Typ.desc= Tstruct struct_name} as typ
+        | {desc= Tptr (({desc= Tstruct struct_name} as typ), _)} ) ) ]
+   -> Tenv.add_field tenv struct_name field ;
+      let field_access_exp = Exp.Lfield (lexp, field_name, typ) in
       execute_load ~report_deref_errors:false pname pdesc tenv ret_id field_access_exp ret_typ loc
         prop
   | _
    -> raise (Exceptions.Wrong_argument_number __POS__)
 
-and sym_exec_objc_setter field_name _ tenv _ pdesc pname loc args prop =
+and sym_exec_objc_setter field _ tenv _ pdesc pname loc args prop =
+  let field_name, _, _ = field in
   L.d_strln
     ( "No custom setter found. Executing the ObjC builtin setter with ivar "
     ^ Typ.Fieldname.to_string field_name ^ "." ) ;
   match args with
-  | (lexp1, ({Typ.desc= Tstruct _} as typ1 | {Typ.desc= Tptr (typ1, _)})) :: (lexp2, typ2) :: _
+  | ( lexp1
-   -> let field_access_exp = Exp.Lfield (lexp1, field_name, typ1) in
+    , ( {Typ.desc= Tstruct struct_name} as typ1
+      | {Typ.desc= Tptr (({Typ.desc= Tstruct struct_name} as typ1), _)} ) )
+    :: (lexp2, typ2) :: _
+   -> Tenv.add_field tenv struct_name field ;
+      let field_access_exp = Exp.Lfield (lexp1, field_name, typ1) in
       execute_store ~report_deref_errors:false pname pdesc tenv field_access_exp typ2 lexp2 loc
         prop
   | _
@@ -1660,10 +1669,10 @@ and sym_exec_objc_accessor property_accesor ret_typ tenv ret_id pdesc _ loc args
     : Builtin.ret_typ =
   let f_accessor =
     match property_accesor with
-    | ProcAttributes.Objc_getter field_name
+    | ProcAttributes.Objc_getter field
-     -> sym_exec_objc_getter field_name
+     -> sym_exec_objc_getter field
-    | ProcAttributes.Objc_setter field_name
+    | ProcAttributes.Objc_setter field
-     -> sym_exec_objc_setter field_name
+     -> sym_exec_objc_setter field
   in
   (* we want to execute in the context of the current procedure, not in the context of callee_pname,
      since this is the procname of the setter/getter method *)

infer/src/clang/cMethod_trans.ml

@@ -491,7 +491,7 @@ let get_byval_args_indices ~shift args =
       let index' = index + shift in
       Option.some_if (is_value qual_type) index' )
 
-let get_objc_property_accessor ms =
+let get_objc_property_accessor tenv ms =
   let open Clang_ast_t in
   match CAst_utils.get_decl_opt (CMethod_signature.ms_get_pointer_to_property_opt ms) with
   | Some ObjCPropertyDecl (_, _, obj_c_property_decl_info)
@@ -499,7 +499,8 @@ let get_objc_property_accessor ms =
       let ivar_decl_ref = obj_c_property_decl_info.Clang_ast_t.opdi_ivar_decl in
       match CAst_utils.get_decl_opt_with_decl_ref ivar_decl_ref with
       | Some ObjCIvarDecl (_, {ni_name}, _, _, _)
-       -> let class_tname =
+       -> (
+          let class_tname =
             match CMethod_signature.ms_get_name ms with
             | Typ.Procname.ObjC_Cpp objc_cpp
              -> Typ.Procname.objc_cpp_get_class_type_name objc_cpp
@@ -507,10 +508,21 @@ let get_objc_property_accessor ms =
              -> assert false
           in
           let field_name = CGeneral_utils.mk_class_field_name class_tname ni_name in
-          if CMethod_signature.ms_is_getter ms then Some (ProcAttributes.Objc_getter field_name)
+          match Tenv.lookup tenv class_tname with
-          else if CMethod_signature.ms_is_setter ms then
+          | Some {fields}
-            Some (ProcAttributes.Objc_setter field_name)
+           -> (
-          else None
+              let field_opt =
+                List.find ~f:(fun (name, _, _) -> Typ.Fieldname.equal name field_name) fields
+              in
+              match field_opt with
+              | Some field when CMethod_signature.ms_is_getter ms
+               -> Some (ProcAttributes.Objc_getter field)
+              | Some field when CMethod_signature.ms_is_setter ms
+               -> Some (ProcAttributes.Objc_setter field)
+              | _
+               -> None )
+          | None
+           -> None )
       | _
        -> None )
   | _
@@ -566,7 +578,7 @@ let create_local_procdesc ?(set_objc_accessor_attr= false) trans_unit_ctx cfg te
     let loc_exit = CLocation.get_sil_location_from_range trans_unit_ctx source_range false in
     let ret_type = get_return_type tenv ms in
     let objc_property_accessor =
-      if set_objc_accessor_attr then get_objc_property_accessor ms else None
+      if set_objc_accessor_attr then get_objc_property_accessor tenv ms else None
     in
     let procdesc =
       let proc_attributes =

infer/tests/build_systems/codetoanalyze/objc_getters_setters/A.h

@@ -0,0 +1,16 @@
+/*
+ * 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.
+ */
+#import <Foundation/NSObject.h>
+#import <Foundation/NSData.h>
+
+@interface A : NSObject
+
+@property(nullable, nonatomic, copy) NSData* metadata;
+
+@end

infer/tests/build_systems/codetoanalyze/objc_getters_setters/A.m

@@ -0,0 +1,13 @@
+/*
+ * 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.
+ */
+#import "A.h"
+
+@implementation A
+
+@end

infer/tests/build_systems/codetoanalyze/objc_getters_setters/B.h

@@ -0,0 +1,13 @@
+/*
+ * 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.
+ */
+#import <Foundation/NSObject.h>
+
+@interface B : NSObject
+
+@end

infer/tests/build_systems/codetoanalyze/objc_getters_setters/B.m

@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+#import "A.h"
+#import "B.h"
+
+@implementation B
+
+- (int)npe_no_bad_footprint_in_getter:(A*)a {
+  int* p = nil;
+  NSData* metadata = a.metadata; // Doesn't crash here with Bad_footprint
+  return *p; // NPE
+}
+
+- (int)npe_no_bad_footprint_in_setter:(A*)a andMetadata:(NSData*)metadata {
+  int* p = nil;
+  a.metadata = metadata; // Doesn't crash here with Bad_footprint
+  return *p; // NPE
+}
+@end

infer/tests/build_systems/objc_getters_setters/Makefile

@@ -0,0 +1,24 @@
+# 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.
+
+TESTS_DIR = ../..
+ROOT_DIR = $(TESTS_DIR)/../..
+CODETOANALYZE_DIR = ../codetoanalyze/objc_getters_setters
+
+ANALYZER = infer
+SOURCES = $(CODETOANALYZE_DIR)/A.m $(CODETOANALYZE_DIR)/B.m
+OBJECTS = $(CODETOANALYZE_DIR)/A.o $(CODETOANALYZE_DIR)/B.o
+INFER_OPTIONS = --report-custom-error --developer-mode --project-root $(TESTS_DIR)
+INFERPRINT_OPTIONS = --project-root $(TESTS_DIR) --issues-tests
+CLEAN_EXTRA = -c
+
+include $(TESTS_DIR)/infer.make
+
+infer-out/report.json: $(CLANG_DEPS) $(SOURCES) $(MAKEFILE_LIST)
+	$(QUIET)$(REMOVE_DIR) buck-out && \
+	$(call silent_on_success,Testing analysis with Objective-C getters and setters,\
+	$(INFER_BIN) $(INFER_OPTIONS) --results-dir $(CURDIR)/infer-out -- clang $(CLEAN_EXTRA) $(SOURCES))

infer/tests/build_systems/objc_getters_setters/issues.exp

@@ -0,0 +1,2 @@
+build_systems/codetoanalyze/objc_getters_setters/B.m, B_npe_no_bad_footprint_in_getter:, 3, NULL_DEREFERENCE, [start of procedure npe_no_bad_footprint_in_getter:]
+build_systems/codetoanalyze/objc_getters_setters/B.m, B_npe_no_bad_footprint_in_setter:andMetadata:, 3, NULL_DEREFERENCE, [start of procedure npe_no_bad_footprint_in_setter:andMetadata:]