diff options
Diffstat (limited to 'clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp')
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp | 181 |
1 files changed, 113 insertions, 68 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 922048733c7c4..bc7a8a3b12a1d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -81,8 +81,7 @@ class NullabilityChecker : public Checker<check::Bind, check::PreCall, check::PreStmt<ReturnStmt>, check::PostCall, check::PostStmt<ExplicitCastExpr>, check::PostObjCMessage, check::DeadSymbols, - check::Event<ImplicitNullDerefEvent>> { - mutable std::unique_ptr<BugType> BT; + check::Location, check::Event<ImplicitNullDerefEvent>> { public: // If true, the checker will not diagnose nullabilility issues for calls @@ -101,25 +100,32 @@ public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const; void checkEvent(ImplicitNullDerefEvent Event) const; + void checkLocation(SVal Location, bool IsLoad, const Stmt *S, + CheckerContext &C) const; 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<BugType> BTs[CK_NumCheckKinds]; + + const std::unique_ptr<BugType> &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 @@ -151,18 +157,16 @@ private: /// /// 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<BugType> &BT = getBugType(CK); auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); if (Region) { R->markInteresting(Region); @@ -430,9 +434,10 @@ static bool checkInvariantViolation(ProgramStateRef State, ExplodedNode *N, 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)) @@ -442,7 +447,7 @@ void NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg, 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. @@ -487,34 +492,76 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const { 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); } } } +// Whenever we see a load from a typed memory region that's been annotated as +// 'nonnull', we want to trust the user on that and assume that it is is indeed +// non-null. +// +// We do so even if the value is known to have been assigned to null. +// The user should be warned on assigning the null value to a non-null pointer +// as opposed to warning on the later dereference of this pointer. +// +// \code +// int * _Nonnull var = 0; // we want to warn the user here... +// // . . . +// *var = 42; // ...and not here +// \endcode +void NullabilityChecker::checkLocation(SVal Location, bool IsLoad, + const Stmt *S, + CheckerContext &Context) const { + // We should care only about loads. + // The main idea is to add a constraint whenever we're loading a value from + // an annotated pointer type. + if (!IsLoad) + return; + + // Annotations that we want to consider make sense only for types. + const auto *Region = + dyn_cast_or_null<TypedValueRegion>(Location.getAsRegion()); + if (!Region) + return; + + ProgramStateRef State = Context.getState(); + + auto StoredVal = State->getSVal(Region).getAs<loc::MemRegionVal>(); + if (!StoredVal) + return; + + Nullability NullabilityOfTheLoadedValue = + getNullabilityAnnotation(Region->getValueType()); + + if (NullabilityOfTheLoadedValue == Nullability::Nonnull) { + // It doesn't matter what we think about this particular pointer, it should + // be considered non-null as annotated by the developer. + if (ProgramStateRef NewState = State->assume(*StoredVal, true)) { + Context.addTransition(NewState); + } + } +} + /// Find the outermost subexpression of E that is not an implicit cast. /// This looks through the implicit casts to _Nonnull that ARC adds to /// return expressions of ObjC types when the return type of the function or /// method is non-null but the express is not. static const Expr *lookThroughImplicitCasts(const Expr *E) { - assert(E); - - while (auto *ICE = dyn_cast<ImplicitCastExpr>(E)) { - E = ICE->getSubExpr(); - } - - return E; + return E->IgnoreImpCasts(); } /// This method check when nullable pointer or null value is returned from a @@ -572,11 +619,9 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, 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) @@ -587,8 +632,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, 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; } @@ -609,7 +654,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, State->get<NullabilityMap>(Region); if (TrackedNullability) { Nullability TrackedNullabValue = TrackedNullability->getValue(); - if (Filter.CheckNullableReturnedFromNonnull && + if (ChecksEnabled[CK_NullableReturnedFromNonnull] && Nullness != NullConstraint::IsNotNull && TrackedNullabValue == Nullability::Nullable && RequiredNullability == Nullability::Nonnull) { @@ -621,9 +666,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, 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; } @@ -674,7 +718,8 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, 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)) { @@ -687,9 +732,9 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, 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; } @@ -705,7 +750,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, TrackedNullability->getValue() != Nullability::Nullable) continue; - if (Filter.CheckNullablePassedToNonnull && + if (ChecksEnabled[CK_NullablePassedToNonnull] && RequiredNullability == Nullability::Nonnull && isDiagnosableCall(Call)) { ExplodedNode *N = C.addTransition(State); @@ -713,17 +758,18 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, 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; @@ -1083,8 +1129,7 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, 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)) { @@ -1102,9 +1147,8 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, 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; } @@ -1130,14 +1174,14 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, 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; } @@ -1188,15 +1232,16 @@ void ento::registerNullabilityBase(CheckerManager &mgr) { mgr.registerChecker<NullabilityChecker>(); } -bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) { +bool ento::shouldRegisterNullabilityBase(const CheckerManager &mgr) { return true; } #define REGISTER_CHECKER(name, trackingRequired) \ void ento::register##name##Checker(CheckerManager &mgr) { \ NullabilityChecker *checker = mgr.getChecker<NullabilityChecker>(); \ - 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 || \ @@ -1204,7 +1249,7 @@ bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) { checker, "NoDiagnoseCallsToSystemHeaders", true); \ } \ \ - bool ento::shouldRegister##name##Checker(const LangOptions &LO) { \ + bool ento::shouldRegister##name##Checker(const CheckerManager &mgr) { \ return true; \ } |