From ce9472f4518786f0965aa953b9b72eb0685bab5b Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Wed, 18 Nov 2020 07:43:59 -0800 Subject: [PATCH] [cost] Add trace of closure in autoreleasepool checker Summary: This diff adds trace of closure in autoreleasepool checker. We introduce a symbolic trace value for closure variable. * It is added to the trace when closure variable is called * and is substituted to concrete one when actual closure is given later. Reviewed By: ezgicicek Differential Revision: D25025883 fbshipit-source-id: a6e246be7 --- infer/src/bufferoverrun/bounds.ml | 39 +++++++++++++++++-- infer/src/bufferoverrun/bounds.mli | 4 ++ infer/src/bufferoverrun/polynomials.ml | 30 ++++++++++---- infer/src/bufferoverrun/polynomials.mli | 2 +- infer/src/cost/cost.ml | 9 +++-- .../objc/autoreleasepool/cost-issues.exp | 4 +- .../objc/autoreleasepool/issues.exp | 4 +- 7 files changed, 72 insertions(+), 20 deletions(-) diff --git a/infer/src/bufferoverrun/bounds.ml b/infer/src/bufferoverrun/bounds.ml index d5d93c63d..78e1be83c 100644 --- a/infer/src/bufferoverrun/bounds.ml +++ b/infer/src/bufferoverrun/bounds.ml @@ -1292,10 +1292,11 @@ module BoundTrace = struct | Call of {callee_pname: Procname.t; callee_trace: t; location: Location.t} | ModeledFunction of {pname: string; location: Location.t} | ArcFromNonArc of {pname: string; location: Location.t} + | FuncPtr of {path: Symb.SymbolPath.partial; location: Location.t} [@@deriving compare] let rec length = function - | Loop _ | ModeledFunction _ | ArcFromNonArc _ -> + | Loop _ | ModeledFunction _ | ArcFromNonArc _ | FuncPtr _ -> 1 | Call {callee_trace} -> 1 + length callee_trace @@ -1315,24 +1316,42 @@ module BoundTrace = struct | Call {callee_pname; callee_trace; location} -> F.fprintf f "%a -> Call `%a` (%a)" pp callee_trace Procname.pp callee_pname Location.pp location + | FuncPtr {path; location} -> + F.fprintf f "FuncPtr `%a` (%a)" Symb.SymbolPath.pp_partial path Location.pp location let call ~callee_pname ~location callee_trace = Call {callee_pname; callee_trace; location} - let rec make_err_trace ~depth trace = + let rec is_func_ptr = function + | Call {callee_trace} -> + is_func_ptr callee_trace + | FuncPtr _ -> + true + | Loop _ | ModeledFunction _ | ArcFromNonArc _ -> + false + + + let rec make_err_trace_of_non_func_ptr ~depth trace = match trace with | Loop loop_head_loc -> [Errlog.make_trace_element depth loop_head_loc "Loop" []] | Call {callee_pname; location; callee_trace} -> let desc = F.asprintf "Call to %a" Procname.pp callee_pname in Errlog.make_trace_element depth location desc [] - :: make_err_trace ~depth:(depth + 1) callee_trace + :: make_err_trace_of_non_func_ptr ~depth:(depth + 1) callee_trace | ModeledFunction {pname; location} -> let desc = F.asprintf "Modeled call to %s" pname in [Errlog.make_trace_element depth location desc []] | ArcFromNonArc {pname; location} -> let desc = F.asprintf "ARC function call to %s from non-ARC caller" pname in [Errlog.make_trace_element depth location desc []] + | FuncPtr _ -> + assert false + + + let make_err_trace ~depth trace = + (* Function pointer trace is suppressed. *) + if is_func_ptr trace then [] else make_err_trace_of_non_func_ptr ~depth trace let of_loop location = Loop location @@ -1340,6 +1359,20 @@ module BoundTrace = struct let of_modeled_function pname location = ModeledFunction {pname; location} let of_arc_from_non_arc pname location = ArcFromNonArc {pname; location} + + let of_function_ptr path location = FuncPtr {path; location} + + let rec subst ~get_autoreleasepool_trace x = + match x with + | Call {callee_pname; callee_trace; location} -> + subst ~get_autoreleasepool_trace callee_trace + |> Option.map ~f:(fun callee_trace' -> + if phys_equal callee_trace callee_trace' then x + else Call {callee_pname; callee_trace= callee_trace'; location} ) + | FuncPtr {path} -> + get_autoreleasepool_trace path + | Loop _ | ModeledFunction _ | ArcFromNonArc _ -> + Some x end (** A NonNegativeBound is a Bound that is either non-negative or symbolic but will be evaluated to a diff --git a/infer/src/bufferoverrun/bounds.mli b/infer/src/bufferoverrun/bounds.mli index 4c975e0ab..9efcf9eb3 100644 --- a/infer/src/bufferoverrun/bounds.mli +++ b/infer/src/bufferoverrun/bounds.mli @@ -158,6 +158,10 @@ module BoundTrace : sig val of_modeled_function : string -> Location.t -> t val of_arc_from_non_arc : string -> Location.t -> t + + val of_function_ptr : Symb.SymbolPath.partial -> Location.t -> t + + val subst : get_autoreleasepool_trace:(Symb.SymbolPath.partial -> t option) -> t -> t option end type ('c, 's, 't) valclass = Constant of 'c | Symbolic of 's | ValTop of 't diff --git a/infer/src/bufferoverrun/polynomials.ml b/infer/src/bufferoverrun/polynomials.ml index d867b67d1..c8a2a757d 100644 --- a/infer/src/bufferoverrun/polynomials.ml +++ b/infer/src/bufferoverrun/polynomials.ml @@ -347,12 +347,13 @@ module NonNegativeNonTopPolynomial = struct body ) - let singleton key = - { poly= {const= NonNegativeInt.zero; terms= M.singleton key one_poly} - ; autoreleasepool_trace= None } + let singleton ?autoreleasepool_trace key = + {poly= {const= NonNegativeInt.zero; terms= M.singleton key one_poly}; autoreleasepool_trace} - let of_func_ptr path = singleton (FuncPtr path) + let of_func_ptr path location = + singleton ~autoreleasepool_trace:(BoundTrace.of_function_ptr path location) (FuncPtr path) + let rec of_valclass : (NonNegativeInt.t, Key.t, 't) valclass -> ('t, t, 't) below_above = function | ValTop trace -> @@ -508,11 +509,24 @@ module NonNegativeNonTopPolynomial = struct plus_poly acc funcptr_p ) terms (poly_of_non_negative_int const) in + let subst_autoreleasepool_trace autoreleasepool_trace = + let trace_of_path path = + match FuncPtr.Set.is_singleton_or_more (eval_func_ptrs path) with + | Singleton (Closure {name}) -> + get_closure_callee_cost name + |> Option.bind ~f:(fun {autoreleasepool_trace} -> autoreleasepool_trace) + | Singleton (Path path) -> + Some (BoundTrace.of_function_ptr path location) + | Empty | More -> + None + in + autoreleasepool_trace + |> Option.bind ~f:(BoundTrace.subst ~get_autoreleasepool_trace:trace_of_path) + |> Option.map ~f:(BoundTrace.call ~callee_pname ~location) + in match subst_poly poly with | poly -> - let autoreleasepool_trace = - Option.map autoreleasepool_trace ~f:(BoundTrace.call ~callee_pname ~location) - in + let autoreleasepool_trace = subst_autoreleasepool_trace autoreleasepool_trace in Val {poly; autoreleasepool_trace} | exception ReturnTop s_trace -> Above s_trace @@ -816,7 +830,7 @@ module NonNegativePolynomial = struct |> make_trace_set ~map_above:TopTrace.unbounded_loop - let of_func_ptr path = Val (NonNegativeNonTopPolynomial.of_func_ptr path) + let of_func_ptr path location = Val (NonNegativeNonTopPolynomial.of_func_ptr path location) let is_symbolic = function | Below _ | Above _ -> diff --git a/infer/src/bufferoverrun/polynomials.mli b/infer/src/bufferoverrun/polynomials.mli index cf553e183..8cf384316 100644 --- a/infer/src/bufferoverrun/polynomials.mli +++ b/infer/src/bufferoverrun/polynomials.mli @@ -73,7 +73,7 @@ module NonNegativePolynomial : sig val of_non_negative_bound : ?degree_kind:DegreeKind.t -> Bounds.NonNegativeBound.t -> t - val of_func_ptr : Symb.SymbolPath.partial -> t + val of_func_ptr : Symb.SymbolPath.partial -> Location.t -> t val plus : t -> t -> t diff --git a/infer/src/cost/cost.ml b/infer/src/cost/cost.ml index e1ebc0084..9d7610d8b 100644 --- a/infer/src/cost/cost.ml +++ b/infer/src/cost/cost.ml @@ -108,7 +108,8 @@ module InstrBasicCostWithReason = struct |> Option.value ~default:(Option.value callee_cost_opt ~default:BasicCostWithReason.zero) - let dispatch_func_ptr_call {inferbo_invariant_map; integer_type_widths} instr_node fun_exp = + let dispatch_func_ptr_call {inferbo_invariant_map; integer_type_widths} instr_node fun_exp + location = BufferOverrunAnalysis.extract_pre (InstrCFG.Node.id instr_node) inferbo_invariant_map |> Option.bind ~f:(fun inferbo_mem -> let func_ptrs = @@ -118,7 +119,7 @@ module InstrBasicCostWithReason = struct match FuncPtr.Set.is_singleton_or_more func_ptrs with | Singleton (Path path) -> let symbolic_cost = - BasicCost.of_func_ptr path |> BasicCostWithReason.of_basic_cost + BasicCost.of_func_ptr path location |> BasicCostWithReason.of_basic_cost in Some (CostDomain.construct ~f:(fun _ -> symbolic_cost)) | _ -> @@ -190,8 +191,8 @@ module InstrBasicCostWithReason = struct model_env ret cfg location inferbo_mem ) ) | Sil.Call (_, Exp.Const (Const.Cfun _), _, _, _) -> CostDomain.zero_record - | Sil.Call (_, fun_exp, _, _location, _) -> - dispatch_func_ptr_call extras instr_node fun_exp + | Sil.Call (_, fun_exp, _, location, _) -> + dispatch_func_ptr_call extras instr_node fun_exp location | Sil.Load {id= lhs_id} when Ident.is_none lhs_id -> (* dummy deref inserted by frontend--don't count as a step. In JDK 11, dummy deref disappears and causes cost differences diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp b/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp index 039620e21..6711e27c1 100644 --- a/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp @@ -32,8 +32,8 @@ codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callGiveMeObject_line codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callMutableCopyObject_zero:x:, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callNewObject_zero:, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.dealloc, 0, OnUIThread:false, [] -codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_linear_FP:, (x->elements.length.ub + 1) × (x->elements.length.ub + 1) + (x->elements.length.ub + 1), OnUIThread:false, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Loop] -codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_nextObject_linear:, (x->elements.length.ub + 1), OnUIThread:false, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop] +codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_linear_FP:, (x->elements.length.ub + 1) × (x->elements.length.ub + 1) + (x->elements.length.ub + 1), OnUIThread:false, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Loop,autorelease,Call to MyEnumerator.nextObject,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] +codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_nextObject_linear:, (x->elements.length.ub + 1), OnUIThread:false, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,autorelease,Call to MyEnumerator.nextObject,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.dealloc, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.makeMyEnumerator_zero:, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_enumerator.m, MyEnumerator.dealloc, 0, OnUIThread:false, [] diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp b/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp index f0d704c2e..62868f210 100644 --- a/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/issues.exp @@ -3,8 +3,8 @@ codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callIndexOfObjectPassin codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_CallBlock_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{k},Loop,autorelease,Call to ArcBlock.callBlock:[objc_blockArcBlock.call_CallBlock_linear:_4],Call to objc_blockArcBlock.call_CallBlock_linear:_4,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.call_ConditionalRunBlock_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{k},Loop,autorelease,Call to ArcBlock.conditionalRunBlock:[objc_blockArcBlock.call_ConditionalRunBlock_linear:_3],Call to objc_blockArcBlock.call_ConditionalRunBlock_linear:_3,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callGiveMeObject_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{n},Loop,autorelease,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] -codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_linear_FP:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Loop] -codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_nextObject_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop] +codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_linear_FP:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,{x->elements.length.ub + 1},Loop,autorelease,Call to MyEnumerator.nextObject,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] +codetoanalyze/objc/autoreleasepool/arc_enumerator.m, ArcEnumerator.callMyEnumerator_nextObject_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{x->elements.length.ub + 1},Call to MyEnumerator.nextObject,Loop,autorelease,Call to MyEnumerator.nextObject,Call to NoArcCallee.giveMeObject,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/basic.m, Basic.autoreleased_in_loop_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{n},Loop,autorelease,Call to Basic.call_autorelease_constant,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/basic.m, Basic.autoreleased_in_loop_sequential_linear:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{n},Loop,autorelease,Call to Basic.call_autorelease_constant,Modeled call to NSObject.autorelease] codetoanalyze/objc/autoreleasepool/basic.m, Basic.multiple_autorelease_constants:, 0, EXPENSIVE_AUTORELEASEPOOL_SIZE, no_bucket, ERROR, [{n},Loop,autorelease,Call to Basic.call_autorelease_constant,Modeled call to NSObject.autorelease]