[ownership] make heuristic for reporting on lambdas more shareable

Summary: I want to use that function in Pulse too so let's share it.

Reviewed By: da319

Differential Revision: D13466897

fbshipit-source-id: 1bf3afee4

infer/src/IR/Procdesc.ml

@@ -710,7 +710,7 @@ let is_specialized pdesc =
 
 
 (* true if pvar is a captured variable of a cpp lambda or objc block *)
-let is_captured_var procdesc pvar =
+let is_captured_pvar procdesc pvar =
   let procname = get_proc_name procdesc in
   let pvar_name = Pvar.get_name pvar in
   let pvar_local_matches (var_data : ProcAttributes.var_data) =
@@ -735,6 +735,10 @@ let is_captured_var procdesc pvar =
   is_captured_var_cpp_lambda || is_captured_var_objc_block
 
 
+let is_captured_var procdesc var =
+  Var.get_pvar var |> Option.exists ~f:(fun pvar -> is_captured_pvar procdesc pvar)
+
+
 let has_modify_in_block_attr procdesc pvar =
   let pvar_name = Pvar.get_name pvar in
   let pvar_local_matches (var_data : ProcAttributes.var_data) =

infer/src/IR/Procdesc.mli

@@ -288,9 +288,12 @@ val pp_local : Format.formatter -> ProcAttributes.var_data -> unit
 
 val is_specialized : t -> bool
 
-val is_captured_var : t -> Pvar.t -> bool
+val is_captured_pvar : t -> Pvar.t -> bool
 (** true if pvar is a captured variable of a cpp lambda or obcj block *)
 
+val is_captured_var : t -> Var.t -> bool
+(** true if var is a captured variable of a cpp lambda or obcj block *)
+
 val has_modify_in_block_attr : t -> Pvar.t -> bool
 
 val is_connected : t -> (unit, [`Join | `Other]) Result.t

infer/src/IR/Var.ml

@@ -36,8 +36,6 @@ let get_ident = function ProgramVar _ -> None | LogicalVar id -> Some id
 
 let get_pvar = function ProgramVar pvar -> Some pvar | LogicalVar _ -> None
 
-let get_mangled = function ProgramVar pvar -> Some (Pvar.get_name pvar) | LogicalVar _ -> None
-
 let is_global = function ProgramVar pvar -> Pvar.is_global pvar | LogicalVar _ -> false
 
 let is_return = function ProgramVar pvar -> Pvar.is_return pvar | LogicalVar _ -> false
@@ -48,6 +46,17 @@ let is_footprint = function ProgramVar _ -> false | LogicalVar id -> Ident.is_fo
 
 let is_none = function LogicalVar id -> Ident.is_none id | _ -> false
 
+let get_declaring_function = function
+  | LogicalVar _ ->
+      None
+  | ProgramVar pvar ->
+      Pvar.get_declaring_function pvar
+
+
+let is_local_to_procedure proc_name var =
+  get_declaring_function var |> Option.exists ~f:(Typ.Procname.equal proc_name)
+
+
 let get_all_vars_in_exp e =
   let acc = Exp.free_vars e |> Sequence.map ~f:of_id in
   Exp.program_vars e |> Sequence.map ~f:of_pvar |> Sequence.append acc

infer/src/IR/Var.mli

@@ -31,10 +31,10 @@ val get_ident : t -> Ident.t option
 
 val get_pvar : t -> Pvar.t option
 
-val get_mangled : t -> Mangled.t option
-
 val is_global : t -> bool
 
+val is_local_to_procedure : Typ.Procname.t -> t -> bool
+
 val is_return : t -> bool
 
 val is_this : t -> bool

infer/src/checkers/Ownership.ml

@@ -382,13 +382,14 @@ end
 module Analyzer = LowerHil.MakeAbstractInterpreter (TransferFunctions (ProcCfg.Exceptional))
 
 let report_invalid_return post end_loc summary =
-  let locals =
+  let proc_name = Summary.get_proc_name summary in
-    Procdesc.get_locals summary.Summary.proc_desc |> List.map ~f:(fun {ProcAttributes.name} -> name)
-  in
   let is_local_to_procedure var =
-    Var.get_mangled var
+    (* In case of lambdas, only report on variables local to the lambda that are not captured from
-    |> Option.value_map ~default:false ~f:(fun mangled ->
+         the enclosing procedure to avoid false positives in case the lambda is called within the
-           List.mem ~equal:Mangled.equal locals mangled )
+         same procedure (which is frequent, at the cost of false negatives in case this lambda is
+         returned to the caller). *)
+    Var.is_local_to_procedure proc_name var
+    && not (Procdesc.is_captured_var summary.Summary.proc_desc var)
   in
   (* look for return values that are borrowed from (now-invalid) local variables *)
   let report_invalid_return base (capability : CapabilityDomain.t) =

infer/src/checkers/liveness.ml

@@ -179,7 +179,7 @@ let checker {Callbacks.tenv; summary; proc_desc} : Summary.t =
       || is_unused_tmp_var pvar
       || VarSet.mem (Var.of_pvar pvar) captured_by_ref_vars
       || Domain.mem (Var.of_pvar pvar) live_vars
-      || Procdesc.is_captured_var proc_desc pvar
+      || Procdesc.is_captured_pvar proc_desc pvar
       || is_scope_guard typ
       || Procdesc.has_modify_in_block_attr proc_desc pvar )
   in

infer/src/checkers/uninit.ml

@@ -69,7 +69,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
     match (HilExp.AccessExpression.get_typ access_expr tenv, base) with
     | Some typ, (Var.ProgramVar pv, _) ->
         (not (Pvar.is_frontend_tmp pv))
-        && (not (Procdesc.is_captured_var pdesc pv))
+        && (not (Procdesc.is_captured_pvar pdesc pv))
         && MaybeUninitVars.mem access_expr maybe_uninit_vars
         && should_report_on_type typ
     | _, _ ->

infer/tests/codetoanalyze/cpp/ownership/closures.cpp

@@ -114,7 +114,16 @@ std::function<int()> FN_ref_capture_return_lambda_bad() {
             // stack address
 }
 
-int ref_capture_return_local_lambda_ok() {
+S& lambda_return_local_bad() {
+  S x;
+  auto f = [&x](void) -> S& {
+    S y;
+    return y;
+  };
+  return f();
+}
+
+int ref_capture_return_enclosing_local_lambda_ok() {
   S x;
   auto f = [&x](void) -> S& {
     // do not report this because there is a good chance that this function will
@@ -124,7 +133,7 @@ int ref_capture_return_local_lambda_ok() {
   return f().f;
 }
 
-S& fn_ref_capture_return_local_lambda_bad() {
+S& FN_ref_capture_return_enclosing_local_lambda_bad() {
   S x;
   auto f = [&x](void) -> S& {
     // no way to know if ok here

infer/tests/codetoanalyze/cpp/ownership/issues.exp

@@ -3,6 +3,7 @@ codetoanalyze/cpp/ownership/basics.cpp, multiple_invalidations_loop_bad, 3, USE_
 codetoanalyze/cpp/ownership/basics.cpp, multiple_invalidations_loop_bad, 5, USE_AFTER_LIFETIME, no_bucket, ERROR, [End of variable lifetime,Use of invalid variable]
 codetoanalyze/cpp/ownership/basics.cpp, multiple_invalidations_loop_bad, 8, USE_AFTER_LIFETIME, no_bucket, ERROR, [End of variable lifetime,Use of invalid variable]
 codetoanalyze/cpp/ownership/closures.cpp, implicit_ref_capture_destroy_invoke_bad, 6, USE_AFTER_LIFETIME, no_bucket, ERROR, [End of variable lifetime,Use of invalid variable]
+codetoanalyze/cpp/ownership/closures.cpp, lambda_return_local_bad::lambda_closures.cpp:119:12_operator(), 3, USE_AFTER_LIFETIME, no_bucket, ERROR, [Return of stack variable,End of procedure]
 codetoanalyze/cpp/ownership/closures.cpp, ref_capture_destroy_invoke_bad, 6, USE_AFTER_LIFETIME, no_bucket, ERROR, [End of variable lifetime,Use of invalid variable]
 codetoanalyze/cpp/ownership/returns.cpp, returns::return_deleted_bad, 4, USE_AFTER_LIFETIME, no_bucket, ERROR, [End of variable lifetime,Use of invalid variable]
 codetoanalyze/cpp/ownership/returns.cpp, returns::return_literal_stack_reference_bad, 0, USE_AFTER_LIFETIME, no_bucket, ERROR, [Return of stack variable,End of procedure]