diff --git a/infer/src/cost/cost.ml b/infer/src/cost/cost.ml index 184411f28..6a12d2d3d 100644 --- a/infer/src/cost/cost.ml +++ b/infer/src/cost/cost.ml @@ -62,12 +62,22 @@ module InstrBasicCostWithReason = struct && Procdesc.is_objc_arc_on callee_pdesc ) + let get_modeled_cost_unless_top ~default modeled_cost = + if BasicCost.is_top modeled_cost then + (* If we get Top modeled cost, this could cause + Top-poisoning up the call chain, so instead, we keep + underestimating the cost as we do with the case where we + have no summary below.*) + default + else BasicCostWithReason.of_basic_cost modeled_cost + + let dispatch_operation tenv callee_pname callee_cost_opt fun_arg_list get_summary model_env ret inferbo_mem = match CostModels.Call.dispatch tenv callee_pname fun_arg_list with | Some model -> - BasicCostWithReason.of_basic_cost - (model {get_summary; model_env= Lazy.force model_env} ~ret inferbo_mem) + let modeled_cost = model {get_summary; model_env= Lazy.force model_env} ~ret inferbo_mem in + get_modeled_cost_unless_top ~default:(BasicCostWithReason.one ()) modeled_cost | None -> ( match callee_cost_opt with | Some callee_cost -> @@ -83,31 +93,33 @@ module InstrBasicCostWithReason = struct let dispatch_autoreleasepool tenv callee_pname callee_cost_opt fun_arg_list ({get_summary} as extras) model_env ((_, ret_typ) as ret) cfg loc inferbo_mem : BasicCostWithReason.t = + let fun_cost = + if + is_objc_call_from_no_arc_to_arc extras cfg callee_pname + && Typ.is_pointer_to_objc_non_tagged_class ret_typ + && not (return_object_owned_by_caller callee_pname) + then + let autoreleasepool_trace = + Bounds.BoundTrace.of_arc_from_non_arc (Procname.to_string callee_pname) loc + in + BasicCostWithReason.one ~autoreleasepool_trace () + else BasicCostWithReason.zero + in match CostAutoreleaseModels.Call.dispatch tenv callee_pname fun_arg_list with | Some model -> let autoreleasepool_size = model {get_summary; model_env= Lazy.force model_env} ~ret inferbo_mem in - BasicCostWithReason.of_basic_cost autoreleasepool_size + get_modeled_cost_unless_top ~default:fun_cost autoreleasepool_size | None -> - let fun_cost = - if - is_objc_call_from_no_arc_to_arc extras cfg callee_pname - && Typ.is_pointer_to_objc_non_tagged_class ret_typ - && not (return_object_owned_by_caller callee_pname) - then - let autoreleasepool_trace = - Bounds.BoundTrace.of_arc_from_non_arc (Procname.to_string callee_pname) loc - in - BasicCostWithReason.one ~autoreleasepool_trace () - else BasicCostWithReason.zero - in Option.value_map ~default:fun_cost ~f:(BasicCostWithReason.plus fun_cost) callee_cost_opt let dispatch_allocation tenv callee_pname callee_cost_opt : BasicCostWithReason.t = CostAllocationModels.ProcName.dispatch tenv callee_pname - |> Option.value ~default:(Option.value callee_cost_opt ~default:BasicCostWithReason.zero) + |> Option.value_map ~default:(Option.value callee_cost_opt ~default:BasicCostWithReason.zero) + ~f:(fun modeled_cost -> + get_modeled_cost_unless_top ~default:BasicCostWithReason.zero modeled_cost ) let dispatch_func_ptr_call {inferbo_invariant_map; integer_type_widths} instr_node fun_exp diff --git a/infer/src/cost/costAllocationModels.ml b/infer/src/cost/costAllocationModels.ml index 1f75e3866..5c0921b4c 100644 --- a/infer/src/cost/costAllocationModels.ml +++ b/infer/src/cost/costAllocationModels.ml @@ -6,16 +6,16 @@ *) open! IStd -module BasicCostWithReason = CostDomain.BasicCostWithReason +module BasicCost = CostDomain.BasicCost module ProcName = struct - let dispatch : (Tenv.t, BasicCostWithReason.t, unit) ProcnameDispatcher.ProcName.dispatcher = + let dispatch : (Tenv.t, BasicCost.t, unit) ProcnameDispatcher.ProcName.dispatcher = let open ProcnameDispatcher.ProcName in let match_builtin builtin _ s = String.equal s (Procname.get_method builtin) in make_dispatcher - [ +match_builtin BuiltinDecl.__new <>--> BasicCostWithReason.one () - ; +match_builtin BuiltinDecl.__new_array <>--> BasicCostWithReason.one () - ; +match_builtin BuiltinDecl.__objc_alloc_no_fail <>--> BasicCostWithReason.one () - ; +match_builtin BuiltinDecl.malloc <>--> BasicCostWithReason.one () - ; +match_builtin BuiltinDecl.malloc_no_fail <>--> BasicCostWithReason.one () ] + [ +match_builtin BuiltinDecl.__new <>--> BasicCost.one () + ; +match_builtin BuiltinDecl.__new_array <>--> BasicCost.one () + ; +match_builtin BuiltinDecl.__objc_alloc_no_fail <>--> BasicCost.one () + ; +match_builtin BuiltinDecl.malloc <>--> BasicCost.one () + ; +match_builtin BuiltinDecl.malloc_no_fail <>--> BasicCost.one () ] end diff --git a/infer/src/cost/costAllocationModels.mli b/infer/src/cost/costAllocationModels.mli index 33db4f627..5df794c6b 100644 --- a/infer/src/cost/costAllocationModels.mli +++ b/infer/src/cost/costAllocationModels.mli @@ -8,6 +8,5 @@ open! IStd module ProcName : sig - val dispatch : - (Tenv.t, CostDomain.BasicCostWithReason.t, unit) ProcnameDispatcher.ProcName.dispatcher + val dispatch : (Tenv.t, CostDomain.BasicCost.t, unit) ProcnameDispatcher.ProcName.dispatcher end diff --git a/infer/tests/codetoanalyze/objc/performance/NSString.m b/infer/tests/codetoanalyze/objc/performance/NSString.m index 25be4dab7..63517e683 100644 --- a/infer/tests/codetoanalyze/objc/performance/NSString.m +++ b/infer/tests/codetoanalyze/objc/performance/NSString.m @@ -129,7 +129,7 @@ bool string_has_prefix_linear(NSString* str, NSString* prefix) { @implementation DummyClass -+ (void)call_string_by_appending_string_constant_FP { ++ (void)call_string_by_appending_string_constant { NSString* s = [NSStringFromClass(self) stringByAppendingString:@"abc"]; } diff --git a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp index 8d710c4f7..9d24d2354 100644 --- a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp +++ b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp @@ -90,7 +90,7 @@ codetoanalyze/objc/performance/NSSet.m, nsset_init_with_array_linear, 6 + 3 ⋅ codetoanalyze/objc/performance/NSSet.m, nsset_init_with_set_constant, 5, OnUIThread:false, [] codetoanalyze/objc/performance/NSSet.m, nsset_iterate_linear, 5 + 8 ⋅ (set->elements.length.ub + 1), OnUIThread:false, [{set->elements.length.ub + 1},Loop] codetoanalyze/objc/performance/NSSet.m, nsset_next_object_linear, 4 + 5 ⋅ (set->elements.length.ub + 1), OnUIThread:false, [{set->elements.length.ub + 1},Loop] -codetoanalyze/objc/performance/NSString.m, DummyClass.call_string_by_appending_string_constant_FP, ⊤, OnUIThread:false, [Unbounded loop,Modeled call to NSString.stringByAppendingString:] +codetoanalyze/objc/performance/NSString.m, DummyClass.call_string_by_appending_string_constant, 7, OnUIThread:false, [] codetoanalyze/objc/performance/NSString.m, DummyClass.dealloc, 0, OnUIThread:false, [] codetoanalyze/objc/performance/NSString.m, call_component_separated_by_char_constant, 44, OnUIThread:false, [] codetoanalyze/objc/performance/NSString.m, call_init_with_string_constant, 13, OnUIThread:false, [] diff --git a/infer/tests/codetoanalyze/objc/performance/issues.exp b/infer/tests/codetoanalyze/objc/performance/issues.exp index 7f07153e3..93b3487e1 100644 --- a/infer/tests/codetoanalyze/objc/performance/issues.exp +++ b/infer/tests/codetoanalyze/objc/performance/issues.exp @@ -11,7 +11,6 @@ codetoanalyze/objc/performance/NSMutableArray.m, nsmarray_set_linear, 2, INTEGER codetoanalyze/objc/performance/NSSet.m, nsset_enumerator_linear, 7, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [,Assignment,,Assignment,Binary operation: ([0, +oo] + [1, set->elements.length.ub + 1]):signed64] codetoanalyze/objc/performance/NSSet.m, nsset_init_constant, 3, CONDITION_ALWAYS_FALSE, no_bucket, WARNING, [Here] codetoanalyze/objc/performance/NSSet.m, nsset_iterate_linear, 3, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [,Assignment,,Assignment,Binary operation: ([0, +oo] + [1, set->elements.length.ub + 1]):signed64] -codetoanalyze/objc/performance/NSString.m, DummyClass.call_string_by_appending_string_constant_FP, 0, INFINITE_EXECUTION_TIME, no_bucket, ERROR, [Unbounded loop,Modeled call to NSString.stringByAppendingString:] codetoanalyze/objc/performance/NSString.m, init_string_constant, 2, CONDITION_ALWAYS_FALSE, no_bucket, WARNING, [Here] codetoanalyze/objc/performance/NSString.m, replace_linear_FP, 0, INFINITE_EXECUTION_TIME, no_bucket, ERROR, [Unbounded loop,Loop] codetoanalyze/objc/performance/NSString.m, replace_linear_FP, 2, INTEGER_OVERFLOW_U5, no_bucket, ERROR, [,Unknown value from: NSString.stringByReplacingOccurrencesOfString:withString:,Binary operation: ([0, +oo] + 1):signed32]