From a991b98d0262b14d2bf773cc8e04cf9862e3d866 Mon Sep 17 00:00:00 2001 From: Jeremy Dubreil Date: Mon, 20 Nov 2017 21:53:56 -0800 Subject: [PATCH] [infer][nullable checker] also report a null dereference error when adding nil key or value to NSDictionary Summary: Adding a null key or a null value will cause a runtime exception. Reviewed By: sblackshear Differential Revision: D6378618 fbshipit-source-id: 8bd27c6 --- infer/src/checkers/NullabilityCheck.ml | 23 +++++++++++++------ .../codetoanalyze/objc/checkers/Nullable.m | 22 ++++++++++++++---- .../codetoanalyze/objc/checkers/issues.exp | 4 ++++ 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/infer/src/checkers/NullabilityCheck.ml b/infer/src/checkers/NullabilityCheck.ml index 940eacee8..e27db7a7e 100644 --- a/infer/src/checkers/NullabilityCheck.ml +++ b/infer/src/checkers/NullabilityCheck.ml @@ -29,10 +29,20 @@ module TransferFunctions (CFG : ProcCfg.S) = struct (Specs.proc_resolve_attributes callee_pname) - let is_blacklisted callee_pname = - let blacklist = ["URLWithString:"] - and simplified_callee_pname = Typ.Procname.to_simplified_string callee_pname in - List.exists ~f:(String.equal simplified_callee_pname) blacklist + let is_blacklisted_method : Typ.Procname.t -> bool = + let blacklist = ["URLWithString:"] in + fun proc_name -> + let simplified_callee_pname = Typ.Procname.to_simplified_string proc_name in + List.exists ~f:(String.equal simplified_callee_pname) blacklist + + + let is_objc_container_add_method : Typ.Procname.t -> bool = + let add_methods = + ["arrayWithObjects:"; "arrayWithObjects:count:"; "dictionaryWithObjectsAndKeys:"] + in + fun proc_name -> + let simplified_callee_pname = Typ.Procname.to_simplified_string proc_name in + List.exists ~f:(String.equal simplified_callee_pname) add_methods let report_nullable_dereference ap call_sites {ProcData.pdesc; extras} loc = @@ -168,7 +178,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | Call (_, Direct callee_pname, (HilExp.AccessPath receiver) :: _, _, _) when Models.is_check_not_null callee_pname -> assume_pnames_notnull receiver astate - | Call (_, Direct callee_pname, _, _, _) when is_blacklisted callee_pname -> + | Call (_, Direct callee_pname, _, _, _) when is_blacklisted_method callee_pname -> astate | Call (Some ret_var, Direct callee_pname, _, _, loc) when Annotations.pname_has_return_annot callee_pname @@ -178,8 +188,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | Call (_, Direct callee_pname, (HilExp.AccessPath receiver) :: _, _, loc) when is_instance_method callee_pname -> check_ap proc_data loc receiver astate - | Call (_, Direct callee_pname, args, _, loc) - when Typ.Procname.equal callee_pname BuiltinDecl.nsArray_arrayWithObjectsCount -> + | Call (_, Direct callee_pname, args, _, loc) when is_objc_container_add_method callee_pname -> check_nil_in_nsarray proc_data loc args astate | Call (Some ret_var, _, _, _, _) -> remove_nullable_ap (ret_var, []) astate diff --git a/infer/tests/codetoanalyze/objc/checkers/Nullable.m b/infer/tests/codetoanalyze/objc/checkers/Nullable.m index 1432c0226..31ccf2f9c 100644 --- a/infer/tests/codetoanalyze/objc/checkers/Nullable.m +++ b/infer/tests/codetoanalyze/objc/checkers/Nullable.m @@ -118,14 +118,14 @@ int* __nullable returnsNull(); - (NSArray*)nullableObjectInNSArrayBad { NSObject* nullableObject = [self nullableMethod]; - NSArray* array = @[ nullableObject ]; + NSArray* array = @[ nullableObject ]; // reports here return array; } - (NSArray*)secondElementNullableObjectInNSArrayBad { NSObject* allocatedObject = [NSObject alloc]; NSObject* nullableObject = [self nullableMethod]; - NSArray* array = @[ allocatedObject, nullableObject ]; + NSArray* array = @[ allocatedObject, nullableObject ]; // reports here return array; } @@ -133,7 +133,7 @@ int* __nullable returnsNull(); NSObject* nullableObject = [self nullableMethod]; NSArray* array; if (nullableObject) { - array = @[ nullableObject ]; + array = @[ nullableObject ]; // reports here } else { array = @[ @"String" ]; } @@ -142,7 +142,21 @@ int* __nullable returnsNull(); - (NSArray*)URLWithStringOkay { NSURL* url = [NSURL URLWithString:@"some/url/string"]; - NSArray* array = @[ url ]; + NSArray* array = @[ url ]; // reports here +} + +- (NSDictionary*)nullableValueInNSDictionary { + NSObject* nullableValue = [self nullableMethod]; + NSMutableDictionary* dict = [NSMutableDictionary + dictionaryWithObjectsAndKeys:@"key", nullableValue, nil]; // reports here + return dict; +} + +- (NSDictionary*)nullableKeyInNSDictionary { + NSObject* nullableKey = [self nullableMethod]; + NSMutableDictionary* dict = [NSMutableDictionary + dictionaryWithObjectsAndKeys:nullableKey, @"value", nil]; // reports here + return dict; } @end diff --git a/infer/tests/codetoanalyze/objc/checkers/issues.exp b/infer/tests/codetoanalyze/objc/checkers/issues.exp index fa65d2635..ae0a051fd 100644 --- a/infer/tests/codetoanalyze/objc/checkers/issues.exp +++ b/infer/tests/codetoanalyze/objc/checkers/issues.exp @@ -9,8 +9,12 @@ codetoanalyze/objc/checkers/Nullable.m, T_dereferenceNullableFunctionBad, 2, NUL codetoanalyze/objc/checkers/Nullable.m, T_dereferenceNullableFunctionBad, 2, NULL_DEREFERENCE, [start of procedure dereferenceNullableFunctionBad,Skipping returnsNull(): function or method not found] codetoanalyze/objc/checkers/Nullable.m, T_dereferenceUnnanotatedFieldAfterTestForNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is compared to null here] codetoanalyze/objc/checkers/Nullable.m, T_dereferenceUnnanotatedFieldAfterTestForNullBad, 2, NULL_DEREFERENCE, [start of procedure dereferenceUnnanotatedFieldAfterTestForNullBad,Condition is true] +codetoanalyze/objc/checkers/Nullable.m, T_nullableKeyInNSDictionary, 2, NULLABLE_DEREFERENCE, [dereference of &nullableKey,assignment of the nullable value,definition of nullableMethod] +codetoanalyze/objc/checkers/Nullable.m, T_nullableKeyInNSDictionary, 2, PREMATURE_NIL_TERMINATION_ARGUMENT, [start of procedure nullableKeyInNSDictionary,start of procedure nullableMethod,return from a call to T_nullableMethod] codetoanalyze/objc/checkers/Nullable.m, T_nullableObjectInNSArrayBad, 2, NULLABLE_DEREFERENCE, [dereference of &nullableObject,assignment of the nullable value,definition of nullableMethod] codetoanalyze/objc/checkers/Nullable.m, T_nullableObjectInNSArrayBad, 2, NULL_DEREFERENCE, [start of procedure nullableObjectInNSArrayBad,start of procedure nullableMethod,return from a call to T_nullableMethod] +codetoanalyze/objc/checkers/Nullable.m, T_nullableValueInNSDictionary, 2, NULLABLE_DEREFERENCE, [dereference of &nullableValue,assignment of the nullable value,definition of nullableMethod] +codetoanalyze/objc/checkers/Nullable.m, T_nullableValueInNSDictionary, 2, PREMATURE_NIL_TERMINATION_ARGUMENT, [start of procedure nullableValueInNSDictionary,start of procedure nullableMethod,return from a call to T_nullableMethod] codetoanalyze/objc/checkers/Nullable.m, T_reassigningNullableObjectOkay, 1, DEAD_STORE, [Write of unused value] codetoanalyze/objc/checkers/Nullable.m, T_secondElementNullableObjectInNSArrayBad, 3, NULLABLE_DEREFERENCE, [dereference of &nullableObject,assignment of the nullable value,definition of nullableMethod] codetoanalyze/objc/checkers/Nullable.m, T_secondElementNullableObjectInNSArrayBad, 3, NULL_DEREFERENCE, [start of procedure secondElementNullableObjectInNSArrayBad,start of procedure nullableMethod,return from a call to T_nullableMethod]