[pulse][nullptr][objcpp] Warn on sending a message to nil when return type is non-POD

Summary: In ObjC, when a method is called on nil, there is no NPE, the method is actually not called and the return value is 0/false/nil. There is an exception in the case where the return type is non-POD. In that case it's UB and we want to report an error.

Reviewed By: jvillard

Differential Revision: D26815687

fbshipit-source-id: 8126414ab

infer/src/IR/ProcAttributes.ml

@@ -75,7 +75,8 @@ type t =
   ; objc_accessor: objc_accessor_type option  (** type of ObjC accessor, if any *)
   ; proc_name: Procname.t  (** name of the procedure *)
   ; ret_type: Typ.t  (** return type *)
-  ; has_added_return_param: bool  (** whether or not a return param was added *) }
+  ; has_added_return_param: bool  (** whether or not a return param was added *)
+  ; is_ret_type_pod: bool  (** whether or not the return type is POD *) }
 
 let get_annotated_formals {method_annotation= {params}; formals} =
   let rec zip_params ial parl =
@@ -137,7 +138,8 @@ let default translation_unit proc_name =
   ; method_annotation= Annot.Method.empty
   ; objc_accessor= None
   ; proc_name
-  ; ret_type= StdTyp.void }
+  ; ret_type= StdTyp.void
+  ; is_ret_type_pod= true }
 
 
 let pp_parameters =
@@ -183,7 +185,8 @@ let pp f
      ; method_annotation
      ; objc_accessor
      ; proc_name
-     ; ret_type }[@warning "+9"]) =
+     ; ret_type
+     ; is_ret_type_pod }[@warning "+9"]) =
   let default = default translation_unit proc_name in
   let pp_bool_default ~default title b f () =
     if not (Bool.equal default b) then F.fprintf f "; %s= %b@," title b
@@ -249,6 +252,7 @@ let pp f
     F.fprintf f "; objc_accessor= %a@," (Pp.option pp_objc_accessor_type) objc_accessor ;
   (* always print ret type *)
   F.fprintf f "; ret_type= %a @," (Typ.pp_full Pp.text) ret_type ;
+  pp_bool_default ~default:default.is_ret_type_pod "is_ret_type_pod" is_ret_type_pod f () ;
   F.fprintf f "; proc_id= %a }@]" Procname.pp_unique_id proc_name
 
 

infer/src/IR/ProcAttributes.mli

@@ -58,7 +58,8 @@ type t =
   ; objc_accessor: objc_accessor_type option  (** type of ObjC accessor, if any *)
   ; proc_name: Procname.t  (** name of the procedure *)
   ; ret_type: Typ.t  (** return type *)
-  ; has_added_return_param: bool  (** whether or not a return param was added *) }
+  ; has_added_return_param: bool  (** whether or not a return param was added *)
+  ; is_ret_type_pod: bool  (** whether or not the return type is POD *) }
 
 val default : SourceFile.t -> Procname.t -> t
 (** Create a proc_attributes with default values. *)

infer/src/IR/Procdesc.ml

@@ -531,6 +531,8 @@ let get_locals pdesc = pdesc.attributes.locals
 
 let has_added_return_param pdesc = pdesc.attributes.has_added_return_param
 
+let is_ret_type_pod pdesc = pdesc.attributes.is_ret_type_pod
+
 (** Return name and type of captured variables *)
 let get_captured pdesc = pdesc.attributes.captured
 

infer/src/IR/Procdesc.mli

@@ -262,6 +262,8 @@ val get_ret_type : t -> Typ.t
 
 val has_added_return_param : t -> bool
 
+val is_ret_type_pod : t -> bool
+
 val get_ret_var : t -> Pvar.t
 
 val get_start_node : t -> Node.t

infer/src/clang/CType_decl.ml

@@ -148,16 +148,21 @@ module BuildMethodSignature = struct
     in
     let return_typ = qual_type_to_sil_type tenv return_qual_type in
     let return_typ_annot = CAst_utils.sil_annot_of_type return_qual_type in
+    let is_ret_typ_pod = CGeneral_utils.is_type_pod return_qual_type in
     if should_add_return_param return_typ then
-      (StdTyp.void, Some (CType.add_pointer_to_typ return_typ), Annot.Item.empty, true)
-    else (return_typ, None, return_typ_annot, false)
+      ( StdTyp.void
+      , Some (CType.add_pointer_to_typ return_typ)
+      , Annot.Item.empty
+      , true
+      , is_ret_typ_pod )
+    else (return_typ, None, return_typ_annot, false, is_ret_typ_pod)
 
 
   let method_signature_of_decl qual_type_to_sil_type tenv method_decl ?block_return_type
       ?(passed_as_noescape_block_to = None) procname =
     let decl_info = Clang_ast_proj.get_decl_tuple method_decl in
     let loc = decl_info.Clang_ast_t.di_source_range in
-    let ret_type, return_param_typ, ret_typ_annot, has_added_return_param =
+    let ret_type, return_param_typ, ret_typ_annot, has_added_return_param, is_ret_type_pod =
       get_return_val_and_param_types qual_type_to_sil_type tenv ~block_return_type method_decl
     in
     let method_kind = CMethodProperties.get_method_kind method_decl in
@@ -175,6 +180,7 @@ module BuildMethodSignature = struct
     ; class_param
     ; params
     ; ret_type= (ret_type, ret_typ_annot)
+    ; is_ret_type_pod
     ; has_added_return_param
     ; attributes
     ; loc

infer/src/clang/cGeneral_utils.mli

@@ -51,3 +51,5 @@ val is_cpp_translation : CFrontend_config.translation_unit_context -> bool
 
 val is_objc_extension : CFrontend_config.translation_unit_context -> bool
 (** true if the current language is ObjC or ObjC++ *)
+
+val is_type_pod : Clang_ast_t.qual_type -> bool

infer/src/clang/cMethodSignature.ml

@@ -31,6 +31,7 @@ type t =
   ; params: param_type list
   ; ret_type: Typ.t * Annot.Item.t
   ; has_added_return_param: bool
+  ; is_ret_type_pod: bool
   ; attributes: Clang_ast_t.attribute list
   ; loc: Clang_ast_t.source_range
   ; method_kind: ClangMethodKind.t
@@ -55,15 +56,17 @@ let is_setter {pointer_to_property_opt; params} =
   Option.is_some pointer_to_property_opt && Int.equal (List.length params) 1
 
 
-let mk name class_param params ret_type ?(has_added_return_param = false) attributes loc method_kind
-    ?(is_cpp_virtual = false) ?(passed_as_noescape_block_to = None) ?(is_no_return = false)
-    ?(is_variadic = false) pointer_to_parent pointer_to_property_opt return_param_typ access =
+let mk name class_param params ret_type ?(has_added_return_param = false) ?(is_ret_type_pod = true)
+    attributes loc method_kind ?(is_cpp_virtual = false) ?(passed_as_noescape_block_to = None)
+    ?(is_no_return = false) ?(is_variadic = false) pointer_to_parent pointer_to_property_opt
+    return_param_typ access =
   { name
   ; access
   ; class_param
   ; params
   ; ret_type
   ; has_added_return_param
+  ; is_ret_type_pod
   ; attributes
   ; loc
   ; method_kind

infer/src/clang/cMethodSignature.mli

@@ -24,6 +24,7 @@ type t =
   ; params: param_type list
   ; ret_type: Typ.t * Annot.Item.t
   ; has_added_return_param: bool
+  ; is_ret_type_pod: bool
   ; attributes: Clang_ast_t.attribute list
   ; loc: Clang_ast_t.source_range
   ; method_kind: ClangMethodKind.t
@@ -46,6 +47,7 @@ val mk :
   -> param_type list
   -> Typ.t * Annot.Item.t
   -> ?has_added_return_param:bool
+  -> ?is_ret_type_pod:bool
   -> Clang_ast_t.attribute list
   -> Clang_ast_t.source_range
   -> ClangMethodKind.t

infer/src/clang/cMethod_trans.ml

@@ -264,6 +264,7 @@ let create_local_procdesc ?(set_objc_accessor_attr = false) ?(record_lambda_capt
         ; formals
         ; const_formals
         ; has_added_return_param
+        ; is_ret_type_pod= ms.CMethodSignature.is_ret_type_pod
         ; access
         ; is_defined= defined
         ; is_biabduction_model= Config.biabduction_models_mode

infer/src/pulse/PulseObjectiveCSummary.ml

@@ -31,7 +31,13 @@ let mk_objc_method_nil_summary_aux proc_desc astate =
 let mk_objc_method_nil_summary ({InterproceduralAnalysis.proc_desc} as analysis_data) initial =
   let proc_name = Procdesc.get_proc_name proc_desc in
   match (initial, proc_name) with
-  | ContinueProgram astate, Procname.ObjC_Cpp {kind= ObjCInstanceMethod} ->
+  | ContinueProgram astate, Procname.ObjC_Cpp {kind= ObjCInstanceMethod}
+    when Procdesc.is_ret_type_pod proc_desc ->
+      (* In ObjC, when a method is called on nil, there is no NPE,
+         the method is actually not called and the return value is 0/false/nil.
+         We create a nil summary to avoid reporting NPE in this case.
+         However, there is an exception in the case where the return type is non-POD.
+         In that case it's UB and we want to report an error. *)
       let result = mk_objc_method_nil_summary_aux proc_desc astate in
       Some (PulseReport.report_list_result analysis_data result)
   | ContinueProgram _, _

infer/tests/codetoanalyze/objcpp/pulse/NPEBasic.mm

@@ -21,6 +21,12 @@
 
 - (int*)getXPtr;
 
++ (SomeObject*)unknown;
+
++ (SomeObject*)returnsNil;
+
+- (SomeObject*)get;
+
 @end
 
 @implementation SomeObject
@@ -41,6 +47,15 @@
   return &_x;
 }
 
++ (SomeObject*)returnsNil {
+  return nil;
+}
+
+- (SomeObject*)get {
+  SomeObject* o = [SomeObject new];
+  return o;
+}
+
 @end
 
 int dereferenceNilBad() {
@@ -53,18 +68,30 @@ int testCallMethodReturnsPODOk() {
   return [obj returnsPOD];
 }
 
-std::shared_ptr<int> FN_testCallMethodReturnsnonPODBad() {
+std::shared_ptr<int> testCallMethodReturnsnonPODBad() {
   SomeObject* obj = nil;
   std::shared_ptr<int> d = [obj returnsnonPOD]; // UB
   return d;
 }
 
+std::shared_ptr<int> testSkippedOK() {
+  SomeObject* obj = [[SomeObject unknown] get];
+  std::shared_ptr<int> result = [obj returnsnonPOD];
+  return result;
+}
+
+std::shared_ptr<int> testNonPODTraceBad() {
+  SomeObject* obj = [[SomeObject returnsNil] get];
+  std::shared_ptr<int> result = [obj returnsnonPOD];
+  return result;
+}
+
 int testAccessPropertyAccessorOk() {
   SomeObject* obj = nil;
   return obj.x; // calls property accessor method
 }
 
-std::shared_ptr<int> FN_testAccessPropertyAccessorBad() {
+std::shared_ptr<int> testAccessPropertyAccessorBad() {
   SomeObject* obj = nil;
   return obj.ptr; // calls property accessor method, but return type is non-POD
 }

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

@@ -1,5 +1,8 @@
 codetoanalyze/objcpp/pulse/AllocPatternMemLeak.mm, A.create_no_release_leak_bad, 1, MEMORY_LEAK, no_bucket, ERROR, [allocation part of the trace starts here,allocated by call to `ABFDataCreate` (modelled),allocation part of the trace ends here,memory becomes unreachable here]
 codetoanalyze/objcpp/pulse/NPEBasic.mm, dereferenceNilBad, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,invalid access occurs here]
+codetoanalyze/objcpp/pulse/NPEBasic.mm, testAccessPropertyAccessorBad, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,when calling `SomeObject.ptr` here,parameter `self` of SomeObject.ptr,invalid access occurs here]
+codetoanalyze/objcpp/pulse/NPEBasic.mm, testCallMethodReturnsnonPODBad, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,when calling `SomeObject.returnsnonPOD` here,parameter `self` of SomeObject.returnsnonPOD,invalid access occurs here]
+codetoanalyze/objcpp/pulse/NPEBasic.mm, testNonPODTraceBad, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,when calling `SomeObject.returnsNil` here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,passed as argument to `SomeObject.returnsNil`,return from call to `SomeObject.returnsNil`,passed as argument to `SomeObject.get`,return from call to `SomeObject.get`,assigned,when calling `SomeObject.returnsnonPOD` here,parameter `self` of SomeObject.returnsnonPOD,invalid access occurs here]
 codetoanalyze/objcpp/pulse/NPEBasic.mm, testTraceBad, 3, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,passed as argument to `SomeObject.getXPtr`,return from call to `SomeObject.getXPtr`,assigned,invalid access occurs here]
 codetoanalyze/objcpp/pulse/use_after_delete.mm, PulseTest.deref_deleted_in_objc_method_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,was invalidated by `delete`,use-after-lifetime part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,when calling `Simple::Simple` here,parameter `__param_0` of Simple::Simple,invalid access occurs here]
 codetoanalyze/objcpp/pulse/use_after_delete.mm, deref_deleted_bad, 3, USE_AFTER_DELETE, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,was invalidated by `delete`,use-after-lifetime part of the trace starts here,passed as argument to `new` (modelled),return from call to `new` (modelled),assigned,when calling `Simple::Simple` here,parameter `__param_0` of Simple::Simple,invalid access occurs here]