diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -82,7 +82,6 @@ check::PostCall, check::PostStmt, check::PostObjCMessage, check::DeadSymbols, check::Location, check::Event> { - mutable std::unique_ptr BT; public: // If true, the checker will not diagnose nullabilility issues for calls @@ -107,21 +106,26 @@ void printState(raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep) const override; - struct NullabilityChecksFilter { - DefaultBool CheckNullPassedToNonnull; - DefaultBool CheckNullReturnedFromNonnull; - DefaultBool CheckNullableDereferenced; - DefaultBool CheckNullablePassedToNonnull; - DefaultBool CheckNullableReturnedFromNonnull; - - CheckerNameRef CheckNameNullPassedToNonnull; - CheckerNameRef CheckNameNullReturnedFromNonnull; - CheckerNameRef CheckNameNullableDereferenced; - CheckerNameRef CheckNameNullablePassedToNonnull; - CheckerNameRef CheckNameNullableReturnedFromNonnull; + enum CheckKind { + CK_NullPassedToNonnull, + CK_NullReturnedFromNonnull, + CK_NullableDereferenced, + CK_NullablePassedToNonnull, + CK_NullableReturnedFromNonnull, + CK_NumCheckKinds }; - NullabilityChecksFilter Filter; + DefaultBool ChecksEnabled[CK_NumCheckKinds]; + CheckerNameRef CheckNames[CK_NumCheckKinds]; + mutable std::unique_ptr BTs[CK_NumCheckKinds]; + + const std::unique_ptr &getBugType(CheckKind Kind) const { + if (!BTs[Kind]) + BTs[Kind].reset(new BugType(CheckNames[Kind], "Nullability", + categories::MemoryError)); + return BTs[Kind]; + } + // When set to false no nullability information will be tracked in // NullabilityMap. It is possible to catch errors like passing a null pointer // to a callee that expects nonnull argument without the information that is @@ -153,18 +157,16 @@ /// /// When \p SuppressPath is set to true, no more bugs will be reported on this /// path by this checker. - void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error, + void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N, const MemRegion *Region, CheckerContext &C, const Stmt *ValueExpr = nullptr, - bool SuppressPath = false) const; + bool SuppressPath = false) const; - void reportBug(StringRef Msg, ErrorKind Error, ExplodedNode *N, + void reportBug(StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N, const MemRegion *Region, BugReporter &BR, const Stmt *ValueExpr = nullptr) const { - if (!BT) - BT.reset(new BugType(this, "Nullability", categories::MemoryError)); - + const std::unique_ptr &BT = getBugType(CK); auto R = std::make_unique(*BT, Msg, N); if (Region) { R->markInteresting(Region); @@ -432,9 +434,10 @@ return false; } -void NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg, - ErrorKind Error, ExplodedNode *N, const MemRegion *Region, - CheckerContext &C, const Stmt *ValueExpr, bool SuppressPath) const { +void NullabilityChecker::reportBugIfInvariantHolds( + StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N, + const MemRegion *Region, CheckerContext &C, const Stmt *ValueExpr, + bool SuppressPath) const { ProgramStateRef OriginalState = N->getState(); if (checkInvariantViolation(OriginalState, N, C)) @@ -444,7 +447,7 @@ N = C.addTransition(OriginalState, N); } - reportBug(Msg, Error, N, Region, C.getBugReporter(), ValueExpr); + reportBug(Msg, Error, CK, N, Region, C.getBugReporter(), ValueExpr); } /// Cleaning up the program state. @@ -489,17 +492,19 @@ if (!TrackedNullability) return; - if (Filter.CheckNullableDereferenced && + if (ChecksEnabled[CK_NullableDereferenced] && TrackedNullability->getValue() == Nullability::Nullable) { BugReporter &BR = *Event.BR; // Do not suppress errors on defensive code paths, because dereferencing // a nullable pointer is always an error. if (Event.IsDirectDereference) reportBug("Nullable pointer is dereferenced", - ErrorKind::NullableDereferenced, Event.SinkNode, Region, BR); + ErrorKind::NullableDereferenced, CK_NullableDereferenced, + Event.SinkNode, Region, BR); else { reportBug("Nullable pointer is passed to a callee that requires a " - "non-null", ErrorKind::NullablePassedToNonnull, + "non-null", + ErrorKind::NullablePassedToNonnull, CK_NullableDereferenced, Event.SinkNode, Region, BR); } } @@ -614,11 +619,9 @@ bool NullReturnedFromNonNull = (RequiredNullability == Nullability::Nonnull && Nullness == NullConstraint::IsNull); - if (Filter.CheckNullReturnedFromNonnull && - NullReturnedFromNonNull && + if (ChecksEnabled[CK_NullReturnedFromNonnull] && NullReturnedFromNonNull && RetExprTypeLevelNullability != Nullability::Nonnull && - !InSuppressedMethodFamily && - C.getLocationContext()->inTopFrame()) { + !InSuppressedMethodFamily && C.getLocationContext()->inTopFrame()) { static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull"); ExplodedNode *N = C.generateErrorNode(State, &Tag); if (!N) @@ -629,8 +632,8 @@ OS << (RetExpr->getType()->isObjCObjectPointerType() ? "nil" : "Null"); OS << " returned from a " << C.getDeclDescription(D) << " that is expected to return a non-null value"; - reportBugIfInvariantHolds(OS.str(), - ErrorKind::NilReturnedToNonnull, N, nullptr, C, + reportBugIfInvariantHolds(OS.str(), ErrorKind::NilReturnedToNonnull, + CK_NullReturnedFromNonnull, N, nullptr, C, RetExpr); return; } @@ -651,7 +654,7 @@ State->get(Region); if (TrackedNullability) { Nullability TrackedNullabValue = TrackedNullability->getValue(); - if (Filter.CheckNullableReturnedFromNonnull && + if (ChecksEnabled[CK_NullableReturnedFromNonnull] && Nullness != NullConstraint::IsNotNull && TrackedNullabValue == Nullability::Nullable && RequiredNullability == Nullability::Nonnull) { @@ -663,9 +666,8 @@ OS << "Nullable pointer is returned from a " << C.getDeclDescription(D) << " that is expected to return a non-null value"; - reportBugIfInvariantHolds(OS.str(), - ErrorKind::NullableReturnedToNonnull, N, - Region, C); + reportBugIfInvariantHolds(OS.str(), ErrorKind::NullableReturnedToNonnull, + CK_NullableReturnedFromNonnull, N, Region, C); } return; } @@ -716,7 +718,8 @@ unsigned ParamIdx = Param->getFunctionScopeIndex() + 1; - if (Filter.CheckNullPassedToNonnull && Nullness == NullConstraint::IsNull && + if (ChecksEnabled[CK_NullPassedToNonnull] && + Nullness == NullConstraint::IsNull && ArgExprTypeLevelNullability != Nullability::Nonnull && RequiredNullability == Nullability::Nonnull && isDiagnosableCall(Call)) { @@ -729,9 +732,9 @@ OS << (Param->getType()->isObjCObjectPointerType() ? "nil" : "Null"); OS << " passed to a callee that requires a non-null " << ParamIdx << llvm::getOrdinalSuffix(ParamIdx) << " parameter"; - reportBugIfInvariantHolds(OS.str(), ErrorKind::NilPassedToNonnull, N, - nullptr, C, - ArgExpr, /*SuppressPath=*/false); + reportBugIfInvariantHolds(OS.str(), ErrorKind::NilPassedToNonnull, + CK_NullPassedToNonnull, N, nullptr, C, ArgExpr, + /*SuppressPath=*/false); return; } @@ -747,7 +750,7 @@ TrackedNullability->getValue() != Nullability::Nullable) continue; - if (Filter.CheckNullablePassedToNonnull && + if (ChecksEnabled[CK_NullablePassedToNonnull] && RequiredNullability == Nullability::Nonnull && isDiagnosableCall(Call)) { ExplodedNode *N = C.addTransition(State); @@ -755,17 +758,18 @@ llvm::raw_svector_ostream OS(SBuf); OS << "Nullable pointer is passed to a callee that requires a non-null " << ParamIdx << llvm::getOrdinalSuffix(ParamIdx) << " parameter"; - reportBugIfInvariantHolds(OS.str(), - ErrorKind::NullablePassedToNonnull, N, - Region, C, ArgExpr, /*SuppressPath=*/true); + reportBugIfInvariantHolds(OS.str(), ErrorKind::NullablePassedToNonnull, + CK_NullablePassedToNonnull, N, Region, C, + ArgExpr, /*SuppressPath=*/true); return; } - if (Filter.CheckNullableDereferenced && + if (ChecksEnabled[CK_NullableDereferenced] && Param->getType()->isReferenceType()) { ExplodedNode *N = C.addTransition(State); reportBugIfInvariantHolds("Nullable pointer is dereferenced", - ErrorKind::NullableDereferenced, N, Region, - C, ArgExpr, /*SuppressPath=*/true); + ErrorKind::NullableDereferenced, + CK_NullableDereferenced, N, Region, C, + ArgExpr, /*SuppressPath=*/true); return; } continue; @@ -1125,8 +1129,7 @@ bool NullAssignedToNonNull = (LocNullability == Nullability::Nonnull && RhsNullness == NullConstraint::IsNull); - if (Filter.CheckNullPassedToNonnull && - NullAssignedToNonNull && + if (ChecksEnabled[CK_NullPassedToNonnull] && NullAssignedToNonNull && ValNullability != Nullability::Nonnull && ValueExprTypeLevelNullability != Nullability::Nonnull && !isARCNilInitializedLocal(C, S)) { @@ -1144,9 +1147,8 @@ llvm::raw_svector_ostream OS(SBuf); OS << (LocType->isObjCObjectPointerType() ? "nil" : "Null"); OS << " assigned to a pointer which is expected to have non-null value"; - reportBugIfInvariantHolds(OS.str(), - ErrorKind::NilAssignedToNonnull, N, nullptr, C, - ValueStmt); + reportBugIfInvariantHolds(OS.str(), ErrorKind::NilAssignedToNonnull, + CK_NullPassedToNonnull, N, nullptr, C, ValueStmt); return; } @@ -1172,14 +1174,14 @@ if (RhsNullness == NullConstraint::IsNotNull || TrackedNullability->getValue() != Nullability::Nullable) return; - if (Filter.CheckNullablePassedToNonnull && + if (ChecksEnabled[CK_NullablePassedToNonnull] && LocNullability == Nullability::Nonnull) { static CheckerProgramPointTag Tag(this, "NullablePassedToNonnull"); ExplodedNode *N = C.addTransition(State, C.getPredecessor(), &Tag); reportBugIfInvariantHolds("Nullable pointer is assigned to a pointer " "which is expected to have non-null value", - ErrorKind::NullableAssignedToNonnull, N, - ValueRegion, C); + ErrorKind::NullableAssignedToNonnull, + CK_NullablePassedToNonnull, N, ValueRegion, C); } return; } @@ -1237,8 +1239,9 @@ #define REGISTER_CHECKER(name, trackingRequired) \ void ento::register##name##Checker(CheckerManager &mgr) { \ NullabilityChecker *checker = mgr.getChecker(); \ - checker->Filter.Check##name = true; \ - checker->Filter.CheckName##name = mgr.getCurrentCheckerName(); \ + checker->ChecksEnabled[NullabilityChecker::CK_##name] = true; \ + checker->CheckNames[NullabilityChecker::CK_##name] = \ + mgr.getCurrentCheckerName(); \ checker->NeedTracking = checker->NeedTracking || trackingRequired; \ checker->NoDiagnoseCallsToSystemHeaders = \ checker->NoDiagnoseCallsToSystemHeaders || \ @@ -1246,7 +1249,7 @@ checker, "NoDiagnoseCallsToSystemHeaders", true); \ } \ \ - bool ento::shouldRegister##name##Checker(const CheckerManager &mgr) { \ + bool ento::shouldRegister##name##Checker(const CheckerManager &mgr) { \ return true; \ } diff --git a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist --- a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist +++ b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist @@ -206,9 +206,9 @@ descriptionNullable pointer is passed to a callee that requires a non-null 1st parameter categoryMemory error typeNullability - check_namenullability.NullabilityBase + check_namenullability.NullablePassedToNonnull - issue_hash_content_of_line_in_contextff735bea0eb12d4d172b139143c32365 + issue_hash_content_of_line_in_contextcff860dfd96389c0fc88f9c04b609f87 issue_context_kindObjective-C method issue_contextmethod issue_hash_function_offset6 diff --git a/clang/test/Analysis/incorrect-checker-names.mm b/clang/test/Analysis/incorrect-checker-names.mm --- a/clang/test/Analysis/incorrect-checker-names.mm +++ b/clang/test/Analysis/incorrect-checker-names.mm @@ -48,16 +48,16 @@ // Make every dereference a different path to avoid sinks after errors. switch (getRandom()) { case 0: { - Dummy &r = *p; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}} + Dummy &r = *p; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}} } break; case 1: { - int b = p->val; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}} + int b = p->val; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}} } break; case 2: { - int stuff = *ptr; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}} + int stuff = *ptr; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}} } break; case 3: - takesNonnull(p); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter [nullability.NullabilityBase]}} + takesNonnull(p); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter [nullability.NullablePassedToNonnull]}} break; case 4: { Dummy d; @@ -65,8 +65,10 @@ Dummy dd(d); break; } - case 5: takesAttrNonnull(p); break; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null [nullability.NullabilityBase]}} - default: { Dummy d = *p; } break; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}} + case 5: + takesAttrNonnull(p); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null [nullability.NullableDereferenced]}} + break; + default: { Dummy d = *p; } break; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}} } if (p) { takesNonnull(p); @@ -80,12 +82,12 @@ if (getRandom()) { takesNullable(q); // FIXME: This shouldn't be tied to a modeling checker. - takesNonnull(q); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter [nullability.NullabilityBase]}} + takesNonnull(q); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter [nullability.NullPassedToNonnull]}} } Dummy a; Dummy *_Nonnull nonnull = &a; // FIXME: This shouldn't be tied to a modeling checker. - nonnull = q; // expected-warning {{Null assigned to a pointer which is expected to have non-null value [nullability.NullabilityBase]}} + nonnull = q; // expected-warning {{Null assigned to a pointer which is expected to have non-null value [nullability.NullPassedToNonnull]}} q = &a; takesNullable(q); takesNonnull(q);