diff --git a/infer/src/IR/Var.ml b/infer/src/IR/Var.ml index bb079034c..90f427fce 100644 --- a/infer/src/IR/Var.ml +++ b/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 diff --git a/infer/src/IR/Var.mli b/infer/src/IR/Var.mli index 6fd30d728..3bddad91f 100644 --- a/infer/src/IR/Var.mli +++ b/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 diff --git a/infer/src/checkers/Ownership.ml b/infer/src/checkers/Ownership.ml index 5c81b23de..10da35b88 100644 --- a/infer/src/checkers/Ownership.ml +++ b/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 -> - if - (not (FormalMap.is_formal borrowed_base formal_map)) - && not (Var.is_global (fst borrowed_base)) - then Domain.report_return_stack_var borrowed_base end_loc summary ) + (fun ((var, _) as borrowed_base) -> + if is_local_to_procedure var 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 formal_map summary + report_invalid_return post end_loc summary | None -> () ) ; summary diff --git a/infer/tests/codetoanalyze/cpp/ownership/closures.cpp b/infer/tests/codetoanalyze/cpp/ownership/closures.cpp index 22fde6185..b64b4b294 100644 --- a/infer/tests/codetoanalyze/cpp/ownership/closures.cpp +++ b/infer/tests/codetoanalyze/cpp/ownership/closures.cpp @@ -113,3 +113,23 @@ std::function 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(); +}