[ownership] do not warn on returning ref to outer local

Summary:
Lambdas can capture references to locals of the enclosing method as long as
they are not propagated outside the method. However to keep things simple
always allow them to capture locals of the enclosing method at the price of
some false negatives.

Reviewed By: da319

Differential Revision: D8974434

fbshipit-source-id: 957ae44bd

infer/src/IR/Var.ml

@@ -30,6 +30,8 @@ let of_formal_index formal_index = of_id (Ident.create_footprint Ident.name_spec
 
 let to_exp = function ProgramVar pvar -> Exp.Lvar pvar | LogicalVar id -> Exp.Var id
 
+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

infer/src/IR/Var.mli

@@ -27,6 +27,8 @@ val get_all_vars_in_exp : Exp.t -> t Sequence.t
 
 val to_exp : t -> Exp.t
 
+val get_mangled : t -> Mangled.t option
+
 val is_global : t -> bool
 
 val is_return : t -> bool

infer/src/checkers/Ownership.ml

@@ -353,7 +353,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
         , _
         , loc )
       when Typ.Procname.ObjC_Cpp.is_cpp_lambda callee_pname ->
-        (* invoking a lambda; check that it's captured vars are valid *)
+        (* invoking a lambda; check that its captured vars are valid *)
         Domain.check_var_lifetime lhs_base loc summary astate ;
         astate
     | Call (ret_id_typ, call_exp, actuals, _, loc) -> (
@@ -376,18 +376,24 @@ end
 
 module Analyzer = LowerHil.MakeAbstractInterpreter (ProcCfg.Exceptional) (TransferFunctions)
 
-let report_invalid_return post end_loc formal_map summary =
+let report_invalid_return post end_loc summary =
+  let locals =
+    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
+    |> Option.value_map ~default:false ~f:(fun mangled ->
+           List.mem ~equal:Mangled.equal locals mangled )
+  in
   (* look for return values that are borrowed from (now-invalid) local variables *)
   let report_invalid_return base (capability : CapabilityDomain.astate) =
     if Var.is_return (fst base) then
       match capability with
       | BorrowedFrom vars ->
           VarSet.iter
-            (fun borrowed_base ->
+            (fun ((var, _) as borrowed_base) ->
-              if
+              if is_local_to_procedure var then
-                (not (FormalMap.is_formal borrowed_base formal_map))
+                Domain.report_return_stack_var borrowed_base end_loc summary )
-                && not (Var.is_global (fst borrowed_base))
-              then Domain.report_return_stack_var borrowed_base end_loc summary )
             vars
       | InvalidatedAt invalidated_loc ->
           Domain.report_use_after_lifetime base ~use_loc:end_loc ~invalidated_loc summary
@@ -403,8 +409,7 @@ let checker {Callbacks.proc_desc; tenv; summary} =
   ( match Analyzer.compute_post proc_data ~initial with
   | Some post ->
       let end_loc = Procdesc.Node.get_loc (Procdesc.get_exit_node proc_desc) in
-      let formal_map = FormalMap.make proc_desc in
+      report_invalid_return post end_loc summary
-      report_invalid_return post end_loc formal_map summary
   | None ->
       () ) ;
   summary

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

@@ -113,3 +113,23 @@ std::function<int()> FN_ref_capture_return_lambda_bad() {
   return f; // if the caller invokes the lambda, it will try to read the invalid
             // stack address
 }
+
+int ref_capture_return_local_lambda_ok() {
+  S x;
+  auto f = [&x](void) -> S& {
+    // do not report this because there is a good chance that this function will
+    // only be used in the local scope
+    return x;
+  };
+  return f().f;
+}
+
+S& fn_ref_capture_return_local_lambda_bad() {
+  S x;
+  auto f = [&x](void) -> S& {
+    // no way to know if ok here
+    return x;
+  };
+  // woops, this returns a ref to a local!
+  return f();
+}