From cd27695524ef6d70266a17a0a540dbe28869eb36 Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Tue, 20 Oct 2020 07:49:35 -0700 Subject: [PATCH] [cost] Add model: NSFileManager.contentsOfDirectoryAtURL Summary: This diff adds a model for NSFileManager.contentsOfDirectoryAtURL as returning a constant-length collection. The analyzer cannot know files in a directory. We have some options to handle such unknown data. 1. Use `Unknown` value, ie `top` 2. Use a symbolic value 3. Use a constant value We had been used the first option. An upside of this is that the analyzer can remain as sound. However, a downside of this is the top value can be propagated to other procedures, making their costs top, thus we may miss some cost changes of them. The second option is to introduce a symbolic value, ie. that for the number of files. A problem is that the symbolic value will never be concretized. As a result, the symbol can be propagated to other procedures, increasing the coefficient of the complexity or making top costs. Note that handling multiple symbols is somewhat limited in Inferbo's interval domain. The last option is to introduce a constant value. I think this is the best approach we can take among above. Even though we may have FNs when there are a lot of files in a directory, we cannot reason or expect about that at the analysis time anyway. Reviewed By: ezgicicek Differential Revision: D24418099 fbshipit-source-id: bf8cf3538 --- .../src/bufferoverrun/bufferOverrunModels.ml | 3 +++ .../objc/performance/NSFileManager.m | 20 +++++++++++++++++++ .../objc/performance/cost-issues.exp | 1 + .../codetoanalyze/objc/performance/issues.exp | 1 + 4 files changed, 25 insertions(+) create mode 100644 infer/tests/codetoanalyze/objc/performance/NSFileManager.m diff --git a/infer/src/bufferoverrun/bufferOverrunModels.ml b/infer/src/bufferoverrun/bufferOverrunModels.ml index 4326d2793..e25b69099 100644 --- a/infer/src/bufferoverrun/bufferOverrunModels.ml +++ b/infer/src/bufferoverrun/bufferOverrunModels.ml @@ -1713,6 +1713,9 @@ module Call = struct $--> NSCollection.new_collection_by_add_all ; +PatternMatch.ObjectiveC.implements "NSEnumerator" &:: "nextObject" <>$ capt_exp $--> NSCollection.next_object + ; +PatternMatch.ObjectiveC.implements "NSFileManager" + &:: "contentsOfDirectoryAtURL:includingPropertiesForKeys:options:error:" + &--> NSCollection.new_collection ; +PatternMatch.ObjectiveC.implements "NSKeyedUnarchiver" &:: "decodeObjectForKey:" $ capt_var_exn $+...$--> NSCollection.get_any_index ; +PatternMatch.ObjectiveC.implements "NSMutableArray" diff --git a/infer/tests/codetoanalyze/objc/performance/NSFileManager.m b/infer/tests/codetoanalyze/objc/performance/NSFileManager.m new file mode 100644 index 000000000..d9011b107 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/performance/NSFileManager.m @@ -0,0 +1,20 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +#import + +/* We model the file lookup as returning a constant-length collection + * (T77961166). */ +void contents_of_directory_at_url_constant(NSFileManager* file_manager, + NSURL* url, + NSArray* keys) { + NSArray* array = [file_manager contentsOfDirectoryAtURL:url + includingPropertiesForKeys:keys + options:0 + error:nil]; + for (int i = 0; i < array.count; i++) { + } +} diff --git a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp index cdf263eaf..f01847a57 100644 --- a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp +++ b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp @@ -43,6 +43,7 @@ codetoanalyze/objc/performance/NSDictionary.m, nsdictionary_find_key_constant, 1 codetoanalyze/objc/performance/NSDictionary.m, nsdictionary_init_dictionary_constant, 3, OnUIThread:false, [] codetoanalyze/objc/performance/NSDictionary.m, nsdictionary_init_literal_constant, 45, OnUIThread:false, [] codetoanalyze/objc/performance/NSDictionary.m, nsdictionary_init_with_dictionary_linear, 7 + 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/NSFileManager.m, contents_of_directory_at_url_constant, 11, OnUIThread:false, [] codetoanalyze/objc/performance/NSInteger.m, nsinteger_value_linear, 3 + 3 ⋅ integer + 2 ⋅ (1+max(0, integer)), OnUIThread:false, [{1+max(0, integer)},Loop,{integer},Loop] codetoanalyze/objc/performance/NSInteger.m, nsnumber_number_with_int_integer_value_constant, 34, OnUIThread:false, [] codetoanalyze/objc/performance/NSKeyedUnarchiver.m, decode_object_for_key_linear, 9 + 3 ⋅ keyed_unarchiver->elements[*].ub + 2 ⋅ (1+max(0, keyed_unarchiver->elements[*].ub)), OnUIThread:false, [{1+max(0, keyed_unarchiver->elements[*].ub)},Loop,{keyed_unarchiver->elements[*].ub},Loop] diff --git a/infer/tests/codetoanalyze/objc/performance/issues.exp b/infer/tests/codetoanalyze/objc/performance/issues.exp index 78f9630d3..bb7fa4dd9 100644 --- a/infer/tests/codetoanalyze/objc/performance/issues.exp +++ b/infer/tests/codetoanalyze/objc/performance/issues.exp @@ -1,5 +1,6 @@ codetoanalyze/objc/performance/NSArray.m, nsarray_empty_array_constant, 3, CONDITION_ALWAYS_FALSE, no_bucket, WARNING, [Here] codetoanalyze/objc/performance/NSArray.m, nsarray_init_constant, 3, CONDITION_ALWAYS_FALSE, no_bucket, WARNING, [Here] +codetoanalyze/objc/performance/NSFileManager.m, contents_of_directory_at_url_constant, 7, CONDITION_ALWAYS_FALSE, no_bucket, WARNING, [Here] codetoanalyze/objc/performance/NSMutableArray.m, nsarray_new_constant, 2, CONDITION_ALWAYS_FALSE, no_bucket, WARNING, [Here] codetoanalyze/objc/performance/NSMutableArray.m, nsmarray_remove_constant, 6, BUFFER_OVERRUN_L1, no_bucket, ERROR, [,Array declaration,Through,Through,Through,Through,Array access: Offset: 0 Size: 0] codetoanalyze/objc/performance/NSMutableArray.m, nsmarray_remove_in_loop_constant, 6, BUFFER_OVERRUN_L3, no_bucket, ERROR, [,Set array size,,Assignment,Set array size,Array access: Offset: [0, 9] Size: [1, 10]]