diff --git a/infer/src/checkers/NullabilityCheck.ml b/infer/src/checkers/NullabilityCheck.ml index cff88697b..f4dc844f7 100644 --- a/infer/src/checkers/NullabilityCheck.ml +++ b/infer/src/checkers/NullabilityCheck.ml @@ -10,7 +10,9 @@ module L = Logging module MF = MarkupFormatter module CallSites = AbstractDomain.FiniteSet (CallSite) -module Domain = AbstractDomain.Map (AccessPath) (CallSites) +module NullableAP = AbstractDomain.Map (AccessPath) (CallSites) +module NullCheckedPname = AbstractDomain.InvertedSet (Typ.Procname) +module Domain = AbstractDomain.Pair (NullableAP) (NullCheckedPname) module TransferFunctions (CFG : ProcCfg.S) = struct module CFG = CFG @@ -80,19 +82,41 @@ module TransferFunctions (CFG : ProcCfg.S) = struct Reporting.log_error summary ~loc ~ltr:trace exn - let rec longest_nullable_prefix ap astate = - try Some (ap, Domain.find ap astate) + let add_nullable_ap ap call_sites (aps, pnames) = (NullableAP.add ap call_sites aps, pnames) + + let remove_nullable_ap ap (aps, pnames) = (NullableAP.remove ap aps, pnames) + + let find_nullable_ap ap (aps, _) = NullableAP.find ap aps + + let assume_pnames_notnull ap (aps, checked_pnames) : Domain.astate = + let updated_pnames = + try + let call_sites = NullableAP.find ap aps in + CallSites.fold + (fun call_site s -> NullCheckedPname.add (CallSite.pname call_site) s) + call_sites checked_pnames + with Not_found -> checked_pnames + in + (NullableAP.remove ap aps, updated_pnames) + + + let rec longest_nullable_prefix ap ((nulable_aps, _) as astate) = + try Some (ap, NullableAP.find ap nulable_aps) with Not_found -> match ap with _, [] -> None | p -> longest_nullable_prefix (AccessPath.truncate p) astate - let exec_instr (astate: Domain.astate) proc_data _ (instr: HilInstr.t) : Domain.astate = + let exec_instr ((_, checked_pnames) as astate) proc_data _ (instr: HilInstr.t) : Domain.astate = match instr with + | Call (Some ret_var, Direct callee_pname, _, _, _) + when NullCheckedPname.mem callee_pname checked_pnames -> + (* Do not report nullable when the method has already been checked for null *) + remove_nullable_ap (ret_var, []) 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 -> let call_site = CallSite.make callee_pname loc in - Domain.add (ret_var, []) (CallSites.singleton call_site) astate + add_nullable_ap (ret_var, []) (CallSites.singleton call_site) astate | Call (_, Direct callee_pname, (HilExp.AccessPath receiver) :: _, _, loc) when is_instance_method callee_pname -> ( match longest_nullable_prefix receiver astate with @@ -100,9 +124,9 @@ module TransferFunctions (CFG : ProcCfg.S) = struct astate | Some (nullable_ap, call_sites) -> report_nullable_dereference nullable_ap call_sites proc_data loc ; - Domain.remove receiver astate ) + remove_nullable_ap receiver astate ) | Call (Some ret_var, _, _, _, _) -> - Domain.remove (ret_var, []) astate + remove_nullable_ap (ret_var, []) astate | Assign (lhs, rhs, loc) -> ( Option.iter @@ -113,22 +137,22 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | HilExp.AccessPath ap -> ( try (* Add the lhs to the list of nullable values if the rhs is nullable *) - Domain.add lhs (Domain.find ap astate) astate + add_nullable_ap lhs (find_nullable_ap ap astate) astate with Not_found -> (* Remove the lhs from the list of nullable values if the rhs is not nullable *) - Domain.remove lhs astate ) + remove_nullable_ap lhs astate ) | _ -> (* Remove the lhs from the list of nullable values if the rhs is not an access path *) - Domain.remove lhs astate ) + remove_nullable_ap lhs astate ) | Assume (HilExp.AccessPath ap, _, _, _) -> - Domain.remove ap astate + assume_pnames_notnull ap astate | Assume ( ( HilExp.BinaryOperator (Binop.Ne, HilExp.AccessPath ap, exp) | HilExp.BinaryOperator (Binop.Ne, exp, HilExp.AccessPath ap) ) , _ , _ , _ ) -> - if HilExp.is_null_literal exp then Domain.remove ap astate else astate + if HilExp.is_null_literal exp then assume_pnames_notnull ap astate else astate | _ -> astate @@ -137,7 +161,7 @@ end module Analyzer = LowerHil.MakeAbstractInterpreter (ProcCfg.Exceptional) (TransferFunctions) let checker {Callbacks.summary; proc_desc; tenv} = - let initial = Domain.empty in + let initial = (NullableAP.empty, NullCheckedPname.empty) in let proc_data = ProcData.make proc_desc tenv summary in ignore (Analyzer.compute_post proc_data ~initial) ; summary diff --git a/infer/tests/codetoanalyze/cpp/nullable/issues.exp b/infer/tests/codetoanalyze/cpp/nullable/issues.exp index cf424bd9b..6c4c75713 100644 --- a/infer/tests/codetoanalyze/cpp/nullable/issues.exp +++ b/infer/tests/codetoanalyze/cpp/nullable/issues.exp @@ -20,3 +20,22 @@ codetoanalyze/cpp/nullable/method.cpp, dereferenceFieldOfNullableObjectBad, 2, N 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, methodCallOnFieldOfNullableObjectBad, 2, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullObject] codetoanalyze/cpp/nullable/method.cpp, methodCallOnFieldOfNullableObjectBad, 2, NULL_DEREFERENCE, [start of procedure methodCallOnFieldOfNullableObjectBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, methodCheckedForNullOkay, 1, NULL_TEST_AFTER_DEREFERENCE, [start of procedure methodCheckedForNullOkay(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] +codetoanalyze/cpp/nullable/method.cpp, methodCheckedForNullOkay, 2, NULL_DEREFERENCE, [start of procedure methodCheckedForNullOkay(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, methodNotAlwaysCheckedForNullBad, 1, NULL_TEST_AFTER_DEREFERENCE, [start of procedure methodNotAlwaysCheckedForNullBad(),Condition is false,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] +codetoanalyze/cpp/nullable/method.cpp, methodNotAlwaysCheckedForNullBad, 2, NULLABLE_DEREFERENCE, [deference of n$6,definition of mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, methodNotAlwaysCheckedForNullBad, 2, NULL_DEREFERENCE, [start of procedure methodNotAlwaysCheckedForNullBad(),Condition is false,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, methodNotAlwaysCheckedForNullOkay, 1, NULL_TEST_AFTER_DEREFERENCE, [start of procedure methodNotAlwaysCheckedForNullOkay(),Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] +codetoanalyze/cpp/nullable/method.cpp, methodNotAlwaysCheckedForNullOkay, 2, NULL_DEREFERENCE, [start of procedure methodNotAlwaysCheckedForNullOkay(),Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, nullableAssignmentInOneBranchBad, 7, NULLABLE_DEREFERENCE, [deference of &p,assignment of the nullable value,definition of mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, nullableAssignmentInOneBranchBad, 7, NULL_DEREFERENCE, [start of procedure nullableAssignmentInOneBranchBad(),Condition is true,start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject] +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] +codetoanalyze/cpp/nullable/method.cpp, reportsViolationInNullBranchBad, 1, NULL_TEST_AFTER_DEREFERENCE, [start of procedure reportsViolationInNullBranchBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] +codetoanalyze/cpp/nullable/method.cpp, reportsViolationInNullBranchBad, 2, NULLABLE_DEREFERENCE, [deference of n$5,definition of mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, reportsViolationInNullBranchBad, 2, NULL_DEREFERENCE, [start of procedure reportsViolationInNullBranchBad(),start of procedure mayReturnNullObject,Condition is true,return from a call to T_mayReturnNullObject,Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, reportsViolationOutsideOfNullCheckBad, 1, NULL_TEST_AFTER_DEREFERENCE, [start of procedure reportsViolationOutsideOfNullCheckBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is false] +codetoanalyze/cpp/nullable/method.cpp, reportsViolationOutsideOfNullCheckBad, 2, NULL_DEREFERENCE, [start of procedure reportsViolationOutsideOfNullCheckBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is true,start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, reportsViolationOutsideOfNullCheckBad, 4, NULLABLE_DEREFERENCE, [deference of n$2,definition of mayReturnNullObject] +codetoanalyze/cpp/nullable/method.cpp, reportsViolationOutsideOfNullCheckBad, 4, NULL_DEREFERENCE, [start of procedure reportsViolationOutsideOfNullCheckBad(),start of procedure mayReturnNullObject,Condition is false,return from a call to T_mayReturnNullObject,Condition is true,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] diff --git a/infer/tests/codetoanalyze/cpp/nullable/method.cpp b/infer/tests/codetoanalyze/cpp/nullable/method.cpp index d487ffa1f..e2d01d2ce 100644 --- a/infer/tests/codetoanalyze/cpp/nullable/method.cpp +++ b/infer/tests/codetoanalyze/cpp/nullable/method.cpp @@ -31,6 +31,9 @@ class T { } } + public: + T* doesNotReturnNullObject() { return new T(); } + public: void doSomething() {} }; @@ -72,3 +75,51 @@ void avoidDoubleReportingBad(T* t) { p->doSomething(); // should report here p->doSomething(); // should not report here } + +void nullableAssignmentInOneBranchBad(T* t) { + T* p; + if (star()) { + p = t->mayReturnNullObject(); + } else { + p = t->doesNotReturnNullObject(); + } + p->doSomething(); // reports here +} + +void methodCheckedForNullOkay(T* t) { + if (t->mayReturnNullObject() != nullptr) { + t->mayReturnNullObject()->doSomething(); // does not report here + } +} + +void reportsViolationOutsideOfNullCheckBad(T* t) { + if (t->mayReturnNullObject() != nullptr) { + t->mayReturnNullObject()->doSomething(); // does not report here + } + t->mayReturnNullObject()->doSomething(); // reports here +} + +void reportsViolationInNullBranchBad(T* t) { + if (t->mayReturnNullObject() == nullptr) { + t->mayReturnNullObject()->doSomething(); // reports here + } +} + +void reportsViolationInNotNullElseBranchBad(T* t) { + if (t->mayReturnNullObject() != nullptr) { + } else { + t->mayReturnNullObject()->doSomething(); // reports here + } +} + +void methodNotAlwaysCheckedForNullOkay(T* t) { + if (star() && t->mayReturnNullObject() != nullptr) { + t->mayReturnNullObject()->doSomething(); // does not report here + } +} + +void methodNotAlwaysCheckedForNullBad(T* t) { + if (star() || t->mayReturnNullObject() != nullptr) { + t->mayReturnNullObject()->doSomething(); // reports here + } +}