[Ownership] Add ownership to return param

Summary: C++17 introduce guaranteed copy elision which omits constructor calls. In ownership analysis, we depended on these constructor calls to acquire ownership. In particular, when a method returns struct, previously, a constructor was used to acquire ownership. In this diff, we acquire ownership of the returned structs directly.

Reviewed By: mbouaziz

Differential Revision: D9244302

fbshipit-source-id: ae8261b99

infer/src/IR/ProcAttributes.ml

@@ -110,7 +110,8 @@ type t =
   ; objc_accessor: objc_accessor_type option  (** type of ObjC accessor, if any *)
   ; proc_flags: proc_flags  (** flags of the procedure *)
   ; proc_name: Typ.Procname.t  (** name of the procedure *)
-  ; ret_type: Typ.t  (** return type *) }
+  ; ret_type: Typ.t  (** return type *)
+  ; has_added_return_param: bool  (** whether or not a return param was added *) }
 [@@deriving compare]
 
 let default translation_unit proc_name =
@@ -136,6 +137,7 @@ let default translation_unit proc_name =
   ; loc= Location.dummy
   ; translation_unit
   ; locals= []
+  ; has_added_return_param= false
   ; method_annotation= Annot.Method.empty
   ; objc_accessor= None
   ; proc_flags= proc_flags_empty ()
@@ -170,6 +172,7 @@ let pp f
      ; loc
      ; translation_unit
      ; locals
+     ; has_added_return_param
      ; method_annotation
      ; objc_accessor
      ; proc_flags
@@ -229,6 +232,8 @@ let pp f
     F.fprintf f "; locals= [@[%a@]]@,"
       (Pp.semicolon_seq ~print_env:Pp.text_break pp_var_data)
       locals ;
+  pp_bool_default ~default:default.has_added_return_param "has_added_return_param"
+    has_added_return_param f () ;
   if not (Annot.Method.equal default.method_annotation method_annotation) then
     F.fprintf f "; method_annotation= %a@," (Annot.Method.pp "") method_annotation ;
   if not ([%compare.equal : objc_accessor_type option] default.objc_accessor objc_accessor) then

infer/src/IR/ProcAttributes.mli

@@ -67,7 +67,8 @@ type t =
   ; objc_accessor: objc_accessor_type option  (** type of ObjC accessor, if any *)
   ; proc_flags: proc_flags  (** flags of the procedure *)
   ; proc_name: Typ.Procname.t  (** name of the procedure *)
-  ; ret_type: Typ.t  (** return type *) }
+  ; ret_type: Typ.t  (** return type *)
+  ; has_added_return_param: bool  (** whether or not a return param was added *) }
 [@@deriving compare]
 
 val default : SourceFile.t -> Typ.Procname.t -> t

infer/src/IR/Procdesc.ml

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

infer/src/IR/Procdesc.mli

@@ -215,6 +215,8 @@ val get_proc_name : t -> Typ.Procname.t
 val get_ret_type : t -> Typ.t
 (** Return the return type of the procedure and type string *)
 
+val has_added_return_param : t -> bool
+
 val get_ret_var : t -> Pvar.t
 
 val get_start_node : t -> Node.t

infer/src/checkers/Ownership.ml

@@ -246,7 +246,25 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         Option.some_if (is_aggregate base) base
 
 
+  let returns_struct pname =
+    (* Assumption: we add an extra return param for structs *)
+    Ondemand.get_proc_desc pname
+    |> Option.value_map ~default:false ~f:Procdesc.has_added_return_param
+
+
   let acquire_ownership call_exp return_base actuals loc summary astate =
+    let aquire_ownership_of_first_param actuals =
+      match actuals with
+      | HilExp.AccessExpression (AccessExpression.AddressOf access_expression) :: other_actuals -> (
+        match get_assigned_base access_expression with
+        | Some constructed_base ->
+            let astate' = Domain.actuals_add_reads other_actuals loc summary astate in
+            Some (Domain.add constructed_base CapabilityDomain.Owned astate')
+        | None ->
+            Some astate )
+      | _ ->
+          Some astate
+    in
     match call_exp with
     | HilInstr.Direct pname ->
         (* TODO: support new[], malloc, others? *)
@@ -268,18 +286,8 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
           | _ ->
               L.die InternalError "Placement new without placement in %a %a" Typ.Procname.pp pname
                 Location.pp loc
-        else if Typ.Procname.is_constructor pname then
+        else if Typ.Procname.is_constructor pname then aquire_ownership_of_first_param actuals
-          match actuals with
+        else if returns_struct pname then aquire_ownership_of_first_param (List.rev actuals)
-          | HilExp.AccessExpression (AccessExpression.AddressOf access_expression) :: other_actuals
-                -> (
-            match get_assigned_base access_expression with
-            | Some constructed_base ->
-                let astate' = Domain.actuals_add_reads other_actuals loc summary astate in
-                Some (Domain.add constructed_base CapabilityDomain.Owned astate')
-            | None ->
-                Some astate )
-          | _ ->
-              Some astate
         else None
     | HilInstr.Indirect _ ->
         None

infer/src/clang/CType_decl.ml

@@ -154,14 +154,14 @@ module BuildMethodSignature = struct
     let return_typ_annot = CAst_utils.sil_annot_of_type return_qual_type in
     let is_objc_method = CMethodProperties.is_objc_method method_decl in
     if should_add_return_param return_typ ~is_objc_method then
-      (Typ.void, Some (CType.add_pointer_to_typ return_typ), Annot.Item.empty)
+      (Typ.void, Some (CType.add_pointer_to_typ return_typ), Annot.Item.empty, true)
-    else (return_typ, None, return_typ_annot)
+    else (return_typ, None, return_typ_annot, false)
 
 
   let method_signature_of_decl qual_type_to_sil_type tenv method_decl ?block_return_type 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 =
+    let ret_type, return_param_typ, ret_typ_annot, has_added_return_param =
       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
@@ -179,6 +179,7 @@ module BuildMethodSignature = struct
     ; class_param
     ; params
     ; ret_type= (ret_type, ret_typ_annot)
+    ; has_added_return_param
     ; attributes
     ; loc
     ; method_kind

infer/src/clang/cMethodSignature.ml

@@ -26,6 +26,7 @@ type t =
   ; class_param: param_type option
   ; params: param_type list
   ; ret_type: Typ.t * Annot.Item.t
+  ; has_added_return_param: bool
   ; attributes: Clang_ast_t.attribute list
   ; loc: Clang_ast_t.source_range
   ; method_kind: ProcAttributes.clang_method_kind
@@ -49,16 +50,15 @@ 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 attributes loc method_kind ?is_cpp_virtual ?is_cpp_nothrow
+let mk name class_param params ret_type ?(has_added_return_param= false) attributes loc method_kind
-    ?is_variadic pointer_to_parent pointer_to_property_opt return_param_typ access =
+    ?(is_cpp_virtual= false) ?(is_cpp_nothrow= false) ?(is_variadic= false) pointer_to_parent
-  let is_cpp_virtual = Option.value is_cpp_virtual ~default:false in
+    pointer_to_property_opt return_param_typ access =
-  let is_cpp_nothrow = Option.value is_cpp_nothrow ~default:false in
-  let is_variadic = Option.value is_variadic ~default:false in
   { name
   ; access
   ; class_param
   ; params
   ; ret_type
+  ; has_added_return_param
   ; attributes
   ; loc
   ; method_kind

infer/src/clang/cMethodSignature.mli

@@ -19,6 +19,7 @@ type t =
   ; class_param: param_type option
   ; params: param_type list
   ; ret_type: Typ.t * Annot.Item.t
+  ; has_added_return_param: bool
   ; attributes: Clang_ast_t.attribute list
   ; loc: Clang_ast_t.source_range
   ; method_kind: ProcAttributes.clang_method_kind
@@ -36,9 +37,9 @@ val is_setter : t -> bool
 
 val mk :
   Typ.Procname.t -> param_type option -> param_type list -> Typ.t * Annot.Item.t
-  -> Clang_ast_t.attribute list -> Clang_ast_t.source_range -> ProcAttributes.clang_method_kind
+  -> ?has_added_return_param:bool -> Clang_ast_t.attribute list -> Clang_ast_t.source_range
-  -> ?is_cpp_virtual:bool -> ?is_cpp_nothrow:bool -> ?is_variadic:bool
+  -> ProcAttributes.clang_method_kind -> ?is_cpp_virtual:bool -> ?is_cpp_nothrow:bool
-  -> Clang_ast_t.pointer option -> Clang_ast_t.pointer option -> Typ.t option
+  -> ?is_variadic:bool -> Clang_ast_t.pointer option -> Clang_ast_t.pointer option -> Typ.t option
   -> Clang_ast_t.access_specifier -> t
 
 val pp : Format.formatter -> t -> unit

infer/src/clang/cMethod_trans.ml

@@ -237,6 +237,7 @@ let create_local_procdesc ?(set_objc_accessor_attr= false) trans_unit_ctx cfg te
       List.map ~f:(fun ({annot}: CMethodSignature.param_type) -> annot) all_params
     in
     let return_annot = snd ms.CMethodSignature.ret_type in
+    let has_added_return_param = ms.CMethodSignature.has_added_return_param in
     let method_annotation = (return_annot, params_annots) in
     let formals =
       List.map ~f:(fun ({name; typ}: CMethodSignature.param_type) -> (name, typ)) all_params
@@ -273,6 +274,7 @@ let create_local_procdesc ?(set_objc_accessor_attr= false) trans_unit_ctx cfg te
         ; formals
         ; const_formals
         ; by_vals
+        ; has_added_return_param
         ; access
         ; func_attributes= attributes
         ; is_defined= defined

infer/tests/codetoanalyze/cpp/ownership/basics.cpp

@@ -84,3 +84,19 @@ void iterator_pointer_arithmetic_ok(std::vector<Aggregate> v) {
     it->~Aggregate();
   }
 }
+
+struct A {
+  ~A();
+  int f(int i);
+};
+
+A getA();
+
+int struct_inside_loop_ok(std::vector<int> numbers) {
+  int sum;
+  for (auto number : numbers) {
+    A a = getA();
+    sum += a.f(number);
+  }
+  return sum;
+}