From ebea2a6ba16a8a8f65ed4699503508ce0e9d250e Mon Sep 17 00:00:00 2001 From: Jeremy Dubreil Date: Sun, 5 Nov 2017 15:22:56 -0800 Subject: [PATCH] [infer][clang] the nullable checker should not warn on pointer re-assignment Summary: This is a hack to removes most of the false positives of this checker in Objective C. Reviewed By: sblackshear Differential Revision: D6239914 fbshipit-source-id: 1cf05de --- infer/src/checkers/NullabilityCheck.ml | 12 +++++++++++- .../tests/codetoanalyze/cpp/nullable/issues.exp | 6 +++--- .../tests/codetoanalyze/cpp/nullable/method.cpp | 17 +++++++++++------ .../codetoanalyze/objc/checkers/Nullable.m | 7 ++++++- .../codetoanalyze/objc/checkers/issues.exp | 1 + 5 files changed, 32 insertions(+), 11 deletions(-) diff --git a/infer/src/checkers/NullabilityCheck.ml b/infer/src/checkers/NullabilityCheck.ml index 5476b68ee..4e67162cd 100644 --- a/infer/src/checkers/NullabilityCheck.ml +++ b/infer/src/checkers/NullabilityCheck.ml @@ -105,6 +105,13 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let exec_instr ((_, checked_pnames) as astate) proc_data _ (instr: HilInstr.t) : Domain.astate = + let is_pointer_assignment tenv lhs rhs = + HilExp.is_null_literal rhs + (* the rhs has type int when assigning the lhs to null *) + || Option.equal Typ.equal (AccessPath.get_typ lhs tenv) (HilExp.get_typ tenv rhs) + (* the lhs and rhs have the same type in the case of pointer assignment + but the types are different when assigning the pointee *) + in match instr with | Call (Some ret_var, Direct callee_pname, _, _, _) when NullCheckedPname.mem callee_pname checked_pnames -> @@ -132,7 +139,10 @@ module TransferFunctions (CFG : ProcCfg.S) = struct -> ( Option.iter ~f:(fun (nullable_ap, call_sites) -> - report_nullable_dereference nullable_ap call_sites proc_data loc) + if not (is_pointer_assignment proc_data.ProcData.tenv nullable_ap rhs) then + (* TODO (T22426288): Undertand why the pointer derference and the pointer + assignment have the same HIL representation *) + report_nullable_dereference nullable_ap call_sites proc_data loc) (longest_nullable_prefix lhs astate) ; match rhs with | HilExp.AccessPath ap -> ( diff --git a/infer/tests/codetoanalyze/cpp/nullable/issues.exp b/infer/tests/codetoanalyze/cpp/nullable/issues.exp index 85afffa69..a8c2923d7 100644 --- a/infer/tests/codetoanalyze/cpp/nullable/issues.exp +++ b/infer/tests/codetoanalyze/cpp/nullable/issues.exp @@ -7,15 +7,13 @@ codetoanalyze/cpp/nullable/example.cpp, T_dereference_unnanotated_field_after_te codetoanalyze/cpp/nullable/example.cpp, T_dereference_unnanotated_field_after_test_for_null_bad, 2, NULL_DEREFERENCE, [start of procedure dereference_unnanotated_field_after_test_for_null_bad,Condition is true] codetoanalyze/cpp/nullable/example.cpp, T_test_nonnull_field_for_null_bad, 1, FIELD_SHOULD_BE_NULLABLE, [Field nonnull_field is compared to null here] codetoanalyze/cpp/nullable/example.cpp, T_test_unnanotated_field_for_null_bad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotated_field is compared to null here] -codetoanalyze/cpp/nullable/method.cpp, FP_reAssigningNullableValueOk, 1, DEAD_STORE, [Write of unused value] -codetoanalyze/cpp/nullable/method.cpp, FP_reAssigningNullableValueOk, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullPointer] codetoanalyze/cpp/nullable/method.cpp, assignNullableValueBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullPointer] codetoanalyze/cpp/nullable/method.cpp, assignNullableValueBad, 2, NULL_DEREFERENCE, [start of procedure assignNullableValueBad(),start of procedure mayReturnNullPointer,Condition is true,return from a call to T_mayReturnNullPointer] codetoanalyze/cpp/nullable/method.cpp, avoidDoubleReportingBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, avoidDoubleReportingBad, 2, NULL_DEREFERENCE, [start of procedure avoidDoubleReportingBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, callMethodOnNullableObjectBad, 1, NULLABLE_DEREFERENCE, [deference of n$2,definition of mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, callMethodOnNullableObjectBad, 1, NULL_DEREFERENCE, [start of procedure callMethodOnNullableObjectBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] -codetoanalyze/cpp/nullable/method.cpp, callMethodOnNullableObjectOk, 2, NULL_TEST_AFTER_DEREFERENCE, [start of procedure callMethodOnNullableObjectOk(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] +codetoanalyze/cpp/nullable/method.cpp, callMethodOnNullableObjectOkay, 2, NULL_TEST_AFTER_DEREFERENCE, [start of procedure callMethodOnNullableObjectOkay(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] codetoanalyze/cpp/nullable/method.cpp, dereferenceFieldOfNullableObjectBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, dereferenceFieldOfNullableObjectBad, 2, NULL_DEREFERENCE, [start of procedure dereferenceFieldOfNullableObjectBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, methodAlwaysCheckedForNullOkay, 1, NULL_TEST_AFTER_DEREFERENCE, [start of procedure methodAlwaysCheckedForNullOkay(),Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] @@ -32,6 +30,8 @@ codetoanalyze/cpp/nullable/method.cpp, nullableAssignmentInOneBranchBad, 7, NULL codetoanalyze/cpp/nullable/method.cpp, onlyReportOnceBad, 1, NULLABLE_DEREFERENCE, [deference of n$6,definition of mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, onlyReportOnceBad, 1, NULL_DEREFERENCE, [start of procedure onlyReportOnceBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, onlyReportOnceBad, 3, NULL_DEREFERENCE, [start of procedure onlyReportOnceBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,start of procedure doSomething,return from a call to T_doSomething,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, reassigningNullablePointerOkay, 1, DEAD_STORE, [Write of unused value] +codetoanalyze/cpp/nullable/method.cpp, reassigningNullablePointerToNullOkay, 1, DEAD_STORE, [Write of unused value] codetoanalyze/cpp/nullable/method.cpp, reportsViolationInNotNullElseBranchBad, 1, NULL_TEST_AFTER_DEREFERENCE, [start of procedure reportsViolationInNotNullElseBranchBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] codetoanalyze/cpp/nullable/method.cpp, reportsViolationInNotNullElseBranchBad, 3, NULLABLE_DEREFERENCE, [deference of n$5,definition of mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, reportsViolationInNotNullElseBranchBad, 3, NULL_DEREFERENCE, [start of procedure reportsViolationInNotNullElseBranchBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject,Condition is false,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] diff --git a/infer/tests/codetoanalyze/cpp/nullable/method.cpp b/infer/tests/codetoanalyze/cpp/nullable/method.cpp index 8f6d9059a..2aef93620 100644 --- a/infer/tests/codetoanalyze/cpp/nullable/method.cpp +++ b/infer/tests/codetoanalyze/cpp/nullable/method.cpp @@ -43,17 +43,22 @@ void assignNullableValueBad(T* t) { *p = 42; } -void FP_reAssigningNullableValueOk(T* t) { +void reassigningNullablePointerOkay(T* t) { int* p = t->mayReturnNullPointer(); - p = new int; - *p = 42; + p = new int; // does not report here + *p = 42; // does not report here +} + +void reassigningNullablePointerToNullOkay(T* t) { + int* p = t->mayReturnNullPointer(); + p = nullptr; // does not report here } void callMethodOnNullableObjectBad(T* t) { t->mayReturnNullObject()->doSomething(); } -void callMethodOnNullableObjectOk(T* t) { +void callMethodOnNullableObjectOkay(T* t) { T* p = t->mayReturnNullObject(); if (p != nullptr) { p->doSomething(); @@ -72,8 +77,8 @@ void methodCallOnFieldOfNullableObjectBad(T* t) { void avoidDoubleReportingBad(T* t) { T* p = t->mayReturnNullObject(); - p->doSomething(); // should report here - p->doSomething(); // should not report here + p->doSomething(); // reports here + p->doSomething(); // does not report here } void nullableAssignmentInOneBranchBad(T* t) { diff --git a/infer/tests/codetoanalyze/objc/checkers/Nullable.m b/infer/tests/codetoanalyze/objc/checkers/Nullable.m index 5fa9267ee..13639d9a2 100644 --- a/infer/tests/codetoanalyze/objc/checkers/Nullable.m +++ b/infer/tests/codetoanalyze/objc/checkers/Nullable.m @@ -108,7 +108,12 @@ int* __nullable returnsNull(); - (NSString*)dereferenceNullableMethodOkay { NSObject* object = [self nullableMethod]; - return [object description]; + return [object description]; // does not report here +} + +- (void)reassigningNullableObjectOkay { + NSObject* object = [self nullableMethod]; + object = nil; // does not report here } @end diff --git a/infer/tests/codetoanalyze/objc/checkers/issues.exp b/infer/tests/codetoanalyze/objc/checkers/issues.exp index edadaab80..4063d3f21 100644 --- a/infer/tests/codetoanalyze/objc/checkers/issues.exp +++ b/infer/tests/codetoanalyze/objc/checkers/issues.exp @@ -7,6 +7,7 @@ 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_reassigningNullableObjectOkay, 1, DEAD_STORE, [Write of unused value] codetoanalyze/objc/checkers/Nullable.m, T_testNonnullFieldForNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field nonnullField is compared to null here] codetoanalyze/objc/checkers/Nullable.m, T_testUnnanotatedFieldForNullBad, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is compared to null here] codetoanalyze/objc/checkers/Nullable.m, objc_blockT_DeadStoreFP_testUnnanotatedFieldInClosureBad_1, 1, FIELD_SHOULD_BE_NULLABLE, [Field unnanotatedField is compared to null here]