From 89ec1effde749c5aca0a42df385c9755084e594e Mon Sep 17 00:00:00 2001 From: Jeremy Dubreil Date: Mon, 12 Feb 2018 11:07:39 -0800 Subject: [PATCH] [infer][nullable checker] lookup the attributes in the CFG Summary: 1) Fixes some false negatives when a method annotated with `nullable` in the header is not annotated in the implementation and the attribute lookup returns the implementation. In that case, we should follow the information given in the header. 2) Fixes some false positives when annotations are in the other way around, i.e. annotated in the implementation but not in the headers. For now, there should be no report in this case, but the analysis should be extended to report the inconsistency between the header and the implementation 3) Fixes some cases of weird reports caused by name conflicts where the method in the include has the same name has another method annotated differently. Reviewed By: jvillard Differential Revision: D6935379 fbshipit-source-id: 3577eb0 --- infer/src/checkers/NullabilityCheck.ml | 19 ++++++++++++++++--- .../codetoanalyze/objc/nullable/Examples.m | 13 +++++++++++++ .../codetoanalyze/objc/nullable/Library.h | 15 +++++++++++++++ .../codetoanalyze/objc/nullable/issues.exp | 1 + 4 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 infer/tests/codetoanalyze/objc/nullable/Library.h diff --git a/infer/src/checkers/NullabilityCheck.ml b/infer/src/checkers/NullabilityCheck.ml index 15c61d637..4cf3e2561 100644 --- a/infer/src/checkers/NullabilityCheck.ml +++ b/infer/src/checkers/NullabilityCheck.ml @@ -91,6 +91,19 @@ module TransferFunctions (CFG : ProcCfg.S) = struct (Specs.get_err_log summary) false + (* On Clang languages, the annotations like _Nullabe can be found on the declaration + or on the implementation without the two being necessarily consistent. + Here, we explicitely want to lookup the annotations locally: either form + the implementation when defined locally, or from the included headers *) + let lookup_local_attributes = function + | Typ.Procname.Java _ as pname -> + (* Looking up the attribute according to the classpath *) + Specs.proc_resolve_attributes pname + | pname -> + (* Looking up the attributes locally, i.e. either from the file of from the includes *) + Option.map ~f:Procdesc.get_attributes (Ondemand.get_proc_desc pname) + + let report_nullable_dereference ap call_sites {ProcData.pdesc; extras} loc = let summary = extras in if is_conflicting_report summary loc then () @@ -134,7 +147,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let trace = let with_origin_site = let callee_pname = CallSite.pname call_site in - match Specs.proc_resolve_attributes callee_pname with + match lookup_local_attributes callee_pname with | None -> [] | Some attributes -> @@ -252,8 +265,8 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | 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 - ~attrs_of_pname:Specs.proc_resolve_attributes Annotations.ia_is_nullable -> + when Annotations.pname_has_return_annot callee_pname ~attrs_of_pname:lookup_local_attributes + Annotations.ia_is_nullable -> let call_site = CallSite.make callee_pname loc in add_nullable_ap (ret_var, []) (CallSites.singleton call_site) astate | Call (_, Direct callee_pname, (HilExp.AccessExpression receiver) :: _, _, loc) diff --git a/infer/tests/codetoanalyze/objc/nullable/Examples.m b/infer/tests/codetoanalyze/objc/nullable/Examples.m index eed1acee9..9b237d4ae 100644 --- a/infer/tests/codetoanalyze/objc/nullable/Examples.m +++ b/infer/tests/codetoanalyze/objc/nullable/Examples.m @@ -7,6 +7,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ #import +#import "Library.h" int* __nullable returnsNull(); @@ -262,6 +263,18 @@ typedef struct s_ { return array; } +- (NSArray*)dereferenceLibraryMethodOk:(L*)object { + NSObject* nullableObject = [object libraryMethod]; + NSArray* array = @[ nullableObject ]; // does not report here + return array; +} + +- (NSArray*)dereferenceNullableLibraryMethodBad:(L*)object { + NSObject* nullableObject = [object nullableLibraryMethod]; + NSArray* array = @[ nullableObject ]; // reports here + return array; +} + @end @protocol P diff --git a/infer/tests/codetoanalyze/objc/nullable/Library.h b/infer/tests/codetoanalyze/objc/nullable/Library.h new file mode 100644 index 000000000..7a59fc443 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/nullable/Library.h @@ -0,0 +1,15 @@ +/* + * Copyright (c) 2018 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import + +@interface L : NSObject +- (NSObject*)libraryMethod; +- (NSObject* _Nullable)nullableLibraryMethod; +@end diff --git a/infer/tests/codetoanalyze/objc/nullable/issues.exp b/infer/tests/codetoanalyze/objc/nullable/issues.exp index dc870cc1a..c06497d5a 100644 --- a/infer/tests/codetoanalyze/objc/nullable/issues.exp +++ b/infer/tests/codetoanalyze/objc/nullable/issues.exp @@ -4,6 +4,7 @@ codetoanalyze/objc/nullable/Examples.m, T_assignNonnullFieldToNullBad, 1, FIELD_ codetoanalyze/objc/nullable/Examples.m, T_assignUnnanotatedFieldToNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is assigned null here] codetoanalyze/objc/nullable/Examples.m, T_createArrayByAddingNilBad, 2, NULLABLE_DEREFERENCE, [dereferencing the return of nullableMethod,assignment of the nullable value,definition of nullableMethod] codetoanalyze/objc/nullable/Examples.m, T_dereferenceNullableFunctionBad, 2, NULLABLE_DEREFERENCE, [dereference of &p,assignment of the nullable value,definition of returnsNull] +codetoanalyze/objc/nullable/Examples.m, T_dereferenceNullableLibraryMethodBad:, 2, NULLABLE_DEREFERENCE, [dereference of &nullableObject,assignment of the nullable value,definition of nullableLibraryMethod] codetoanalyze/objc/nullable/Examples.m, T_dereferenceUnnanotatedFieldAfterTestForNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is compared to null here] codetoanalyze/objc/nullable/Examples.m, T_indirectNullableKeyInNSDictionaryBad, 3, NULLABLE_DEREFERENCE, [dereference of &nullableKeyString,assignment of the nullable value,definition of nullableMethod] codetoanalyze/objc/nullable/Examples.m, T_insertNullableObjectInMutableArrayBad, 2, NULLABLE_DEREFERENCE, [dereferencing the return of nullableMethod,assignment of the nullable value,definition of nullableMethod]