From d713b98a94c44cac44135aed3980046021d1c892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Tue, 4 May 2021 12:54:24 -0700 Subject: [PATCH] [cost] Fix cost models that accept blocks to take captured vars into account Summary: Function calls that accept blocks as arguments may have additional arguments for the captured variables of the block. Cost models for these functions only considered the case where the block arguments didn't capture variables. This diff extends the model so that we handle captured variable case. This fixes some FNs in the analysis. Reviewed By: skcho Differential Revision: D28183071 fbshipit-source-id: 6a045e80e --- infer/src/cost/costModels.ml | 42 ++++++++++--------- .../codetoanalyze/objc/performance/NSArray.m | 7 ++-- .../objc/performance/cost-issues.exp | 10 ++--- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/infer/src/cost/costModels.ml b/infer/src/cost/costModels.ml index fbdf66e58..6dcb476b8 100644 --- a/infer/src/cost/costModels.ml +++ b/infer/src/cost/costModels.ml @@ -10,6 +10,7 @@ module BasicCost = CostDomain.BasicCost module BasicCostWithReason = CostDomain.BasicCostWithReason open BufferOverrunUtils.ModelEnv open CostUtils.CostModelEnv +open ProcnameDispatcher.Call.FuncArg let unit_cost_model _model_env ~ret:_ _inferbo_mem = BasicCost.one () @@ -173,25 +174,29 @@ module NSCollection = struct cost_op (get_length coll1) (get_length coll2) - let enumerate_using_block array ({get_summary; model_env} as cost_model_env) ~ret inferbo_mem = + let enumerate_using_block args ({get_summary; model_env} as cost_model_env) ~ret inferbo_mem = let pname = model_env.pname in - match pname with - | WithBlockParameters (_, [block_name]) -> ( - match get_summary (Procname.Block block_name) with - | Some {CostDomain.post= callee_summary} -> - let {BasicCostWithReason.cost= callee_cost} = - CostDomain.get_cost_kind OperationCost callee_summary - in - let length = - BoundsOfNSCollection.linear_length - ~of_function:(Procname.to_simplified_string pname) - array cost_model_env ~ret inferbo_mem - in - BasicCost.mult_loop ~iter:length ~body:callee_cost - | None -> - BasicCost.zero ) + match List.rev args with + | _block :: {exp= array} :: _captured_args -> ( + let length = + BoundsOfNSCollection.linear_length + ~of_function:(Procname.to_simplified_string pname) + array cost_model_env ~ret inferbo_mem + in + match pname with + | WithBlockParameters (_, [block_name]) -> ( + match get_summary (Procname.Block block_name) with + | Some {CostDomain.post= callee_summary} -> + let {BasicCostWithReason.cost= callee_cost} = + CostDomain.get_cost_kind OperationCost callee_summary + in + BasicCost.mult_loop ~iter:length ~body:callee_cost + | None -> + length ) + | _ -> + length ) | _ -> - BasicCost.zero + BasicCost.one () end module ImmutableSet = struct @@ -278,8 +283,7 @@ module Call = struct &:: "addObjectsFromArray:" <>$ any_arg $+ capt_exp $--> BoundsOfNSCollection.linear_length ~of_function:"NSArray.addObjectsFromArray:" ; +PatternMatch.ObjectiveC.implements_collection - &:: "enumerateObjectsUsingBlock:" <>$ capt_exp $+ any_arg - $--> NSCollection.enumerate_using_block + &:: "enumerateObjectsUsingBlock:" &::.*++> NSCollection.enumerate_using_block ; +PatternMatch.Java.implements_collections &:: "sort" $ capt_exp $+...$--> BoundsOfCollection.n_log_n_length ~of_function:"Collections.sort" diff --git a/infer/tests/codetoanalyze/objc/performance/NSArray.m b/infer/tests/codetoanalyze/objc/performance/NSArray.m index c5d7a5980..bd79ba409 100644 --- a/infer/tests/codetoanalyze/objc/performance/NSArray.m +++ b/infer/tests/codetoanalyze/objc/performance/NSArray.m @@ -230,7 +230,7 @@ void loop_with_my_enumerator_next_object_linear_FP(MyEnumerator* enumerator) { void enumerate_via_block_linear(NSArray* array) { [array enumerateObjectsUsingBlock:^(id obj, NSUInteger index, BOOL* stop){ - // do something with obj + NSLog(@"index: %@", index); }]; } @@ -239,15 +239,14 @@ void enumerate_via_block_linear(NSArray* array) { @implementation MyBlock -+ (void)call_enumerate_via_block_param_linear_FN:(NSArray*)x:(int)size { ++ (void)call_enumerate_via_block_param_quadratic:(NSArray*)x:(int)size { void (^b)(id, NSUInteger, BOOL*) = ^(id object, NSUInteger indexPath, BOOL* stop) { for (int i = 0; i < size; i++) { } }; // Here, captured arg size is passed as the first argument to the block, - // not the array x. Hence, currently we don't recognize the mode which expects - // only two args. + // not the array x. [x enumerateObjectsUsingBlock:b]; } diff --git a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp index ee723e960..e30b19fa9 100644 --- a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp +++ b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp @@ -1,13 +1,13 @@ -${XCODE_ISYSROOT}/System/Library/Frameworks/Foundation.framework/Headers/NSArray.h, NSArray.enumerateObjectsUsingBlock:[objc_blockMyBlock.call_enumerate_via_block_param_linear_FN::_3], 0, OnUIThread:false, [] +${XCODE_ISYSROOT}/System/Library/Frameworks/Foundation.framework/Headers/NSArray.h, NSArray.enumerateObjectsUsingBlock:[objc_blockMyBlock.call_enumerate_via_block_param_quadratic::_3], 0, OnUIThread:false, [] ${XCODE_ISYSROOT}/System/Library/Frameworks/Foundation.framework/Headers/NSArray.h, NSArray.enumerateObjectsUsingBlock:[objc_blockenumerate_via_block_linear_2], 0, OnUIThread:false, [] ${XCODE_ISYSROOT}/System/Library/Frameworks/Foundation.framework/Headers/NSArray.h, NSArray.indexOfObject:inSortedRange:options:usingComparator:[objc_blocknsarray_binary_search_log_FN_1], 0, OnUIThread:false, [] codetoanalyze/objc/performance/MyEnumerator.m, MyEnumerator.dealloc, 0, OnUIThread:false, [] codetoanalyze/objc/performance/MyEnumerator.m, MyEnumerator.nextObject, 5 + 3 ⋅ self->n.ub + 3 ⋅ (1+max(0, self->n.ub)), OnUIThread:false, [{1+max(0, self->n.ub)},Loop,{self->n.ub},Loop] -codetoanalyze/objc/performance/NSArray.m, MyBlock.call_enumerate_via_block_param_linear_FN::, 5, OnUIThread:false, [] +codetoanalyze/objc/performance/NSArray.m, MyBlock.call_enumerate_via_block_param_quadratic::, 4 + 3 ⋅ size × x->elements.length.ub + 2 ⋅ x->elements.length.ub + 2 ⋅ x->elements.length.ub × (1+max(0, size)), OnUIThread:false, [{1+max(0, size)},Loop,{x->elements.length.ub},Modeled call to enumerateObjectsUsingBlock:,{x->elements.length.ub},Modeled call to enumerateObjectsUsingBlock:,{size},Loop] codetoanalyze/objc/performance/NSArray.m, MyBlock.dealloc, 0, OnUIThread:false, [] codetoanalyze/objc/performance/NSArray.m, call_my_enumerator_next_object_linear, 7 + 3 ⋅ enumerator->n.ub + 3 ⋅ (1+max(0, enumerator->n.ub)), OnUIThread:false, [{1+max(0, enumerator->n.ub)},Call to MyEnumerator.nextObject,Loop,{enumerator->n.ub},Call to MyEnumerator.nextObject,Loop] codetoanalyze/objc/performance/NSArray.m, call_nsarray_enumerator_param_linear, 5, OnUIThread:false, [] -codetoanalyze/objc/performance/NSArray.m, enumerate_via_block_linear, 1, OnUIThread:false, [] +codetoanalyze/objc/performance/NSArray.m, enumerate_via_block_linear, 1 + 11 ⋅ array->elements.length.ub, OnUIThread:false, [{array->elements.length.ub},Modeled call to enumerateObjectsUsingBlock:] codetoanalyze/objc/performance/NSArray.m, loop_with_my_enumerator_next_object_linear_FP, 1 + 3 ⋅ enumerator->n.ub × (enumerator.length + 1) + 9 ⋅ (enumerator.length + 1) + 3 ⋅ (enumerator.length + 1) × (1+max(0, enumerator->n.ub)), OnUIThread:false, [{1+max(0, enumerator->n.ub)},Call to MyEnumerator.nextObject,Loop,{enumerator.length + 1},Loop,{enumerator.length + 1},Loop,{enumerator->n.ub},Call to MyEnumerator.nextObject,Loop] codetoanalyze/objc/performance/NSArray.m, multiple_nsarray_enumerators_param_linear, 9 + 5 ⋅ (enumerator2.length + enumerator1.length + 2), OnUIThread:false, [{enumerator2.length + enumerator1.length + 2},Loop] codetoanalyze/objc/performance/NSArray.m, nsarray_access_constant, 49, OnUIThread:false, [] @@ -38,8 +38,8 @@ codetoanalyze/objc/performance/NSArray.m, nsarray_next_object_linear, 4 + 5 ⋅ codetoanalyze/objc/performance/NSArray.m, nsarray_object_at_indexed_constant, 33, OnUIThread:false, [] codetoanalyze/objc/performance/NSArray.m, nsarray_sort_using_descriptors_constant, 38, OnUIThread:false, [] codetoanalyze/objc/performance/NSArray.m, nsarray_sort_using_descriptors_nlogn, 8 + array->elements.length.ub × log(array->elements.length.ub), OnUIThread:false, [{array->elements.length.ub},Modeled call to NSArray.sortedArrayUsingDescriptors:,{array->elements.length.ub},Modeled call to NSArray.sortedArrayUsingDescriptors:] -codetoanalyze/objc/performance/NSArray.m, objc_blockMyBlock.call_enumerate_via_block_param_linear_FN::_3, 2 + 3 ⋅ size + 2 ⋅ (1+max(0, size)), OnUIThread:false, [{1+max(0, size)},Loop,{size},Loop] -codetoanalyze/objc/performance/NSArray.m, objc_blockenumerate_via_block_linear_2, 0, OnUIThread:false, [] +codetoanalyze/objc/performance/NSArray.m, objc_blockMyBlock.call_enumerate_via_block_param_quadratic::_3, 2 + 3 ⋅ size + 2 ⋅ (1+max(0, size)), OnUIThread:false, [{1+max(0, size)},Loop,{size},Loop] +codetoanalyze/objc/performance/NSArray.m, objc_blockenumerate_via_block_linear_2, 11, OnUIThread:false, [] codetoanalyze/objc/performance/NSArray.m, objc_blocknsarray_binary_search_log_FN_1, 4, OnUIThread:false, [] codetoanalyze/objc/performance/NSDictionary.m, nsdictionary_all_keys_linear1, 2 + 3 ⋅ dict->elements.length.ub + 4 ⋅ (dict->elements.length.ub + 1), OnUIThread:false, [{dict->elements.length.ub + 1},Loop,{dict->elements.length.ub},Loop] codetoanalyze/objc/performance/NSDictionary.m, nsdictionary_all_keys_linear2, 5 + 3 ⋅ dict->elements.length.ub + 3 ⋅ (dict->elements.length.ub + 1), OnUIThread:false, [{dict->elements.length.ub + 1},Loop,{dict->elements.length.ub},Loop]