diff options
Diffstat (limited to 'clang/lib/StaticAnalyzer/Checkers')
5 files changed, 129 insertions, 262 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 88feb6a..e682c4e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -99,7 +99,7 @@ class NilArgChecker : public Checker<check::PreObjCMessage, check::PostStmt<ObjCDictionaryLiteral>, check::PostStmt<ObjCArrayLiteral>, EventDispatcher<ImplicitNullDerefEvent>> { - mutable std::unique_ptr<APIMisuse> BT; + const APIMisuse BT{this, "nil argument"}; mutable llvm::SmallDenseMap<Selector, unsigned, 16> StringSelectors; mutable Selector ArrayWithObjectSel; @@ -218,10 +218,7 @@ void NilArgChecker::generateBugReport(ExplodedNode *N, SourceRange Range, const Expr *E, CheckerContext &C) const { - if (!BT) - BT.reset(new APIMisuse(this, "nil argument")); - - auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); R->addRange(Range); bugreporter::trackExpressionValue(N, E, *R); C.emitReport(std::move(R)); @@ -350,7 +347,7 @@ void NilArgChecker::checkPostStmt(const ObjCDictionaryLiteral *DL, namespace { class CFNumberChecker : public Checker< check::PreStmt<CallExpr> > { - mutable std::unique_ptr<APIMisuse> BT; + const APIMisuse BT{this, "Bad use of CFNumber APIs"}; mutable IdentifierInfo *ICreate = nullptr, *IGetValue = nullptr; public: CFNumberChecker() = default; @@ -524,10 +521,7 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE, << " bits of the integer value will be " << (isCreate ? "lost." : "garbage."); - if (!BT) - BT.reset(new APIMisuse(this, "Bad use of CFNumber APIs")); - - auto report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); report->addRange(CE->getArg(2)->getSourceRange()); C.emitReport(std::move(report)); } @@ -539,7 +533,7 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE, namespace { class CFRetainReleaseChecker : public Checker<check::PreCall> { - mutable APIMisuse BT{this, "null passed to CF memory management function"}; + const APIMisuse BT{this, "null passed to CF memory management function"}; const CallDescriptionSet ModelledCalls = { {CDM::CLibrary, {"CFRetain"}, 1}, {CDM::CLibrary, {"CFRelease"}, 1}, @@ -600,7 +594,8 @@ class ClassReleaseChecker : public Checker<check::PreObjCMessage> { mutable Selector retainS; mutable Selector autoreleaseS; mutable Selector drainS; - mutable std::unique_ptr<BugType> BT; + const APIMisuse BT{ + this, "message incorrectly sent to class instead of class instance"}; public: void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; @@ -609,10 +604,7 @@ public: void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { - if (!BT) { - BT.reset(new APIMisuse( - this, "message incorrectly sent to class instead of class instance")); - + if (releaseS.isNull()) { ASTContext &Ctx = C.getASTContext(); releaseS = GetNullarySelector("release", Ctx); retainS = GetNullarySelector("retain", Ctx); @@ -639,7 +631,7 @@ void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg, "of class '" << Class->getName() << "' and not the class directly"; - auto report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); report->addRange(msg.getSourceRange()); C.emitReport(std::move(report)); } @@ -658,7 +650,8 @@ class VariadicMethodTypeChecker : public Checker<check::PreObjCMessage> { mutable Selector orderedSetWithObjectsS; mutable Selector initWithObjectsS; mutable Selector initWithObjectsAndKeysS; - mutable std::unique_ptr<BugType> BT; + const APIMisuse BT{this, "Arguments passed to variadic method aren't all " + "Objective-C pointer types"}; bool isVariadicMessage(const ObjCMethodCall &msg) const; @@ -717,11 +710,7 @@ VariadicMethodTypeChecker::isVariadicMessage(const ObjCMethodCall &msg) const { void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { - if (!BT) { - BT.reset(new APIMisuse(this, - "Arguments passed to variadic method aren't all " - "Objective-C pointer types")); - + if (arrayWithObjectsS.isNull()) { ASTContext &Ctx = C.getASTContext(); arrayWithObjectsS = GetUnarySelector("arrayWithObjects", Ctx); dictionaryWithObjectsAndKeysS = @@ -792,8 +781,7 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg, ArgTy.print(os, C.getLangOpts()); os << "'"; - auto R = - std::make_unique<PathSensitiveBugReport>(*BT, os.str(), *errorNode); + auto R = std::make_unique<PathSensitiveBugReport>(BT, os.str(), *errorNode); R->addRange(msg.getArgSourceRange(I)); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 31cb150..fd0a398 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -78,35 +78,30 @@ static QualType getCharPtrType(ASTContext &Ctx, CharKind CK) { : Ctx.WideCharTy); } -class CStringChecker : public Checker< eval::Call, - check::PreStmt<DeclStmt>, - check::LiveSymbols, - check::DeadSymbols, - check::RegionChanges - > { - mutable std::unique_ptr<BugType> BT_Null, BT_Bounds, BT_Overlap, - BT_NotCString, BT_AdditionOverflow, BT_UninitRead; - +class CStringChecker + : public CheckerFamily<eval::Call, check::PreStmt<DeclStmt>, + check::LiveSymbols, check::DeadSymbols, + check::RegionChanges> { mutable const char *CurrentFunctionDescription = nullptr; public: - /// The filter is used to filter out the diagnostics which are not enabled by - /// the user. - struct CStringChecksFilter { - bool CheckCStringNullArg = false; - bool CheckCStringOutOfBounds = false; - bool CheckCStringBufferOverlap = false; - bool CheckCStringNotNullTerm = false; - bool CheckCStringUninitializedRead = false; - - CheckerNameRef CheckNameCStringNullArg; - CheckerNameRef CheckNameCStringOutOfBounds; - CheckerNameRef CheckNameCStringBufferOverlap; - CheckerNameRef CheckNameCStringNotNullTerm; - CheckerNameRef CheckNameCStringUninitializedRead; - }; - - CStringChecksFilter Filter; + // FIXME: The bug types emitted by this checker family have confused garbage + // in their Description and Category fields (e.g. `categories::UnixAPI` is + // passed as the description in several cases and `uninitialized` is mistyped + // as `unitialized`). This should be cleaned up. + CheckerFrontendWithBugType NullArg{categories::UnixAPI}; + CheckerFrontendWithBugType OutOfBounds{"Out-of-bound array access"}; + CheckerFrontendWithBugType BufferOverlap{categories::UnixAPI, + "Improper arguments"}; + CheckerFrontendWithBugType NotNullTerm{categories::UnixAPI}; + CheckerFrontendWithBugType UninitializedRead{ + "Accessing unitialized/garbage values"}; + + // FIXME: This bug type should be removed because it is only emitted in a + // situation that is practically impossible. + const BugType AdditionOverflow{&OutOfBounds, "API"}; + + StringRef getDebugTag() const override { return "MallocChecker"; } static void *getTag() { static int tag; return &tag; } @@ -384,7 +379,7 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, assumeZero(C, State, l, Arg.Expression->getType()); if (stateNull && !stateNonNull) { - if (Filter.CheckCStringNullArg) { + if (NullArg.isEnabled()) { SmallString<80> buf; llvm::raw_svector_ostream OS(buf); assert(CurrentFunctionDescription); @@ -468,7 +463,7 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C, return State; // Ensure that we wouldn't read uninitialized value. - if (Filter.CheckCStringUninitializedRead && + if (UninitializedRead.isEnabled() && State->getSVal(*FirstElementVal).isUndef()) { llvm::SmallString<258> Buf; llvm::raw_svector_ostream OS(Buf); @@ -524,7 +519,7 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C, if (!isa<Loc>(LastElementVal)) return State; - if (Filter.CheckCStringUninitializedRead && + if (UninitializedRead.isEnabled() && State->getSVal(LastElementVal.castAs<Loc>()).isUndef()) { const llvm::APSInt *IdxInt = LastIdx.getAsInteger(); // If we can't get emit a sensible last element index, just bail out -- @@ -581,13 +576,9 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, auto [StInBound, StOutBound] = state->assumeInBoundDual(*Idx, Size); if (StOutBound && !StInBound) { - // These checks are either enabled by the CString out-of-bounds checker - // explicitly or implicitly by the Malloc checker. - // In the latter case we only do modeling but do not emit warning. - if (!Filter.CheckCStringOutOfBounds) + if (!OutOfBounds.isEnabled()) return nullptr; - // Emit a bug report. ErrorMessage Message = createOutOfBoundErrorMsg(CurrentFunctionDescription, Access); emitOutOfBoundsBug(C, StOutBound, Buffer.Expression, Message); @@ -620,7 +611,7 @@ CStringChecker::CheckBufferAccess(CheckerContext &C, ProgramStateRef State, return nullptr; // If out-of-bounds checking is turned off, skip the rest. - if (!Filter.CheckCStringOutOfBounds) + if (!OutOfBounds.isEnabled()) return State; SVal BufStart = @@ -670,7 +661,7 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, SizeArgExpr Size, AnyArgExpr First, AnyArgExpr Second, CharKind CK) const { - if (!Filter.CheckCStringBufferOverlap) + if (!BufferOverlap.isEnabled()) return state; // Do a simple check for overlap: if the two arguments are from the same @@ -789,13 +780,9 @@ void CStringChecker::emitOverlapBug(CheckerContext &C, ProgramStateRef state, if (!N) return; - if (!BT_Overlap) - BT_Overlap.reset(new BugType(Filter.CheckNameCStringBufferOverlap, - categories::UnixAPI, "Improper arguments")); - // Generate a report for this bug. auto report = std::make_unique<PathSensitiveBugReport>( - *BT_Overlap, "Arguments must not be overlapping buffers", N); + BufferOverlap, "Arguments must not be overlapping buffers", N); report->addRange(First->getSourceRange()); report->addRange(Second->getSourceRange()); @@ -805,15 +792,8 @@ void CStringChecker::emitOverlapBug(CheckerContext &C, ProgramStateRef state, void CStringChecker::emitNullArgBug(CheckerContext &C, ProgramStateRef State, const Stmt *S, StringRef WarningMsg) const { if (ExplodedNode *N = C.generateErrorNode(State)) { - if (!BT_Null) { - // FIXME: This call uses the string constant 'categories::UnixAPI' as the - // description of the bug; it should be replaced by a real description. - BT_Null.reset( - new BugType(Filter.CheckNameCStringNullArg, categories::UnixAPI)); - } - auto Report = - std::make_unique<PathSensitiveBugReport>(*BT_Null, WarningMsg, N); + std::make_unique<PathSensitiveBugReport>(NullArg, WarningMsg, N); Report->addRange(S->getSourceRange()); if (const auto *Ex = dyn_cast<Expr>(S)) bugreporter::trackExpressionValue(N, Ex, *Report); @@ -826,12 +806,8 @@ void CStringChecker::emitUninitializedReadBug(CheckerContext &C, const Expr *E, const MemRegion *R, StringRef Msg) const { if (ExplodedNode *N = C.generateErrorNode(State)) { - if (!BT_UninitRead) - BT_UninitRead.reset(new BugType(Filter.CheckNameCStringUninitializedRead, - "Accessing unitialized/garbage values")); - auto Report = - std::make_unique<PathSensitiveBugReport>(*BT_UninitRead, Msg, N); + std::make_unique<PathSensitiveBugReport>(UninitializedRead, Msg, N); Report->addNote("Other elements might also be undefined", Report->getLocation()); Report->addRange(E->getSourceRange()); @@ -845,17 +821,11 @@ void CStringChecker::emitOutOfBoundsBug(CheckerContext &C, ProgramStateRef State, const Stmt *S, StringRef WarningMsg) const { if (ExplodedNode *N = C.generateErrorNode(State)) { - if (!BT_Bounds) - BT_Bounds.reset(new BugType(Filter.CheckCStringOutOfBounds - ? Filter.CheckNameCStringOutOfBounds - : Filter.CheckNameCStringNullArg, - "Out-of-bound array access")); - // FIXME: It would be nice to eventually make this diagnostic more clear, // e.g., by referencing the original declaration or by saying *why* this // reference is outside the range. auto Report = - std::make_unique<PathSensitiveBugReport>(*BT_Bounds, WarningMsg, N); + std::make_unique<PathSensitiveBugReport>(OutOfBounds, WarningMsg, N); Report->addRange(S->getSourceRange()); C.emitReport(std::move(Report)); } @@ -865,15 +835,8 @@ void CStringChecker::emitNotCStringBug(CheckerContext &C, ProgramStateRef State, const Stmt *S, StringRef WarningMsg) const { if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) { - if (!BT_NotCString) { - // FIXME: This call uses the string constant 'categories::UnixAPI' as the - // description of the bug; it should be replaced by a real description. - BT_NotCString.reset( - new BugType(Filter.CheckNameCStringNotNullTerm, categories::UnixAPI)); - } - auto Report = - std::make_unique<PathSensitiveBugReport>(*BT_NotCString, WarningMsg, N); + std::make_unique<PathSensitiveBugReport>(NotNullTerm, WarningMsg, N); Report->addRange(S->getSourceRange()); C.emitReport(std::move(Report)); @@ -883,14 +846,6 @@ void CStringChecker::emitNotCStringBug(CheckerContext &C, ProgramStateRef State, void CStringChecker::emitAdditionOverflowBug(CheckerContext &C, ProgramStateRef State) const { if (ExplodedNode *N = C.generateErrorNode(State)) { - if (!BT_AdditionOverflow) { - // FIXME: This call uses the word "API" as the description of the bug; - // it should be replaced by a better error message (if this unlikely - // situation continues to exist as a separate bug type). - BT_AdditionOverflow.reset( - new BugType(Filter.CheckNameCStringOutOfBounds, "API")); - } - // This isn't a great error message, but this should never occur in real // code anyway -- you'd have to create a buffer longer than a size_t can // represent, which is sort of a contradiction. @@ -898,7 +853,7 @@ void CStringChecker::emitAdditionOverflowBug(CheckerContext &C, "This expression will create a string whose length is too big to " "be represented as a size_t"; - auto Report = std::make_unique<PathSensitiveBugReport>(*BT_AdditionOverflow, + auto Report = std::make_unique<PathSensitiveBugReport>(AdditionOverflow, WarningMsg, N); C.emitReport(std::move(Report)); } @@ -909,7 +864,7 @@ ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, NonLoc left, NonLoc right) const { // If out-of-bounds checking is turned off, skip the rest. - if (!Filter.CheckCStringOutOfBounds) + if (!OutOfBounds.isEnabled()) return state; // If a previous check has failed, propagate the failure. @@ -1048,7 +1003,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, // C string. In the context of locations, the only time we can issue such // a warning is for labels. if (std::optional<loc::GotoLabel> Label = Buf.getAs<loc::GotoLabel>()) { - if (Filter.CheckCStringNotNullTerm) { + if (NotNullTerm.isEnabled()) { SmallString<120> buf; llvm::raw_svector_ostream os(buf); assert(CurrentFunctionDescription); @@ -1110,7 +1065,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, // Other regions (mostly non-data) can't have a reliable C string length. // In this case, an error is emitted and UndefinedVal is returned. // The caller should always be prepared to handle this case. - if (Filter.CheckCStringNotNullTerm) { + if (NotNullTerm.isEnabled()) { SmallString<120> buf; llvm::raw_svector_ostream os(buf); @@ -2873,24 +2828,27 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR, } void ento::registerCStringModeling(CheckerManager &Mgr) { - Mgr.registerChecker<CStringChecker>(); + // Other checker relies on the modeling implemented in this checker family, + // so this "modeling checker" can register the 'CStringChecker' backend for + // its callbacks without enabling any of its frontends. + Mgr.getChecker<CStringChecker>(); } -bool ento::shouldRegisterCStringModeling(const CheckerManager &mgr) { +bool ento::shouldRegisterCStringModeling(const CheckerManager &) { return true; } -#define REGISTER_CHECKER(name) \ - void ento::register##name(CheckerManager &mgr) { \ - CStringChecker *checker = mgr.getChecker<CStringChecker>(); \ - checker->Filter.Check##name = true; \ - checker->Filter.CheckName##name = mgr.getCurrentCheckerName(); \ +#define REGISTER_CHECKER(NAME) \ + void ento::registerCString##NAME(CheckerManager &Mgr) { \ + Mgr.getChecker<CStringChecker>()->NAME.enable(Mgr); \ } \ \ - bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; } + bool ento::shouldRegisterCString##NAME(const CheckerManager &) { \ + return true; \ + } -REGISTER_CHECKER(CStringNullArg) -REGISTER_CHECKER(CStringOutOfBounds) -REGISTER_CHECKER(CStringBufferOverlap) -REGISTER_CHECKER(CStringNotNullTerm) -REGISTER_CHECKER(CStringUninitializedRead) +REGISTER_CHECKER(NullArg) +REGISTER_CHECKER(OutOfBounds) +REGISTER_CHECKER(BufferOverlap) +REGISTER_CHECKER(NotNullTerm) +REGISTER_CHECKER(UninitializedRead) diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp index 839c8bc..a227ca0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp @@ -111,32 +111,12 @@ bool PlacementNewChecker::checkPlaceCapacityIsSufficient( if (!SizeOfPlaceCI) return true; - if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue()) || - (IsArrayTypeAllocated && - SizeOfPlaceCI->getValue() >= SizeOfTargetCI->getValue())) { + if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue())) { if (ExplodedNode *N = C.generateErrorNode(C.getState())) { - std::string Msg; - // TODO: use clang constant - if (IsArrayTypeAllocated && - SizeOfPlaceCI->getValue() > SizeOfTargetCI->getValue()) - Msg = std::string(llvm::formatv( - "{0} bytes is possibly not enough for array allocation which " - "requires {1} bytes. Current overhead requires the size of {2} " - "bytes", - SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue(), - *SizeOfPlaceCI->getValue() - SizeOfTargetCI->getValue())); - else if (IsArrayTypeAllocated && - SizeOfPlaceCI->getValue() == SizeOfTargetCI->getValue()) - Msg = std::string(llvm::formatv( - "Storage provided to placement new is only {0} bytes, " - "whereas the allocated array type requires more space for " - "internal needs", - SizeOfPlaceCI->getValue())); - else - Msg = std::string(llvm::formatv( - "Storage provided to placement new is only {0} bytes, " - "whereas the allocated type requires {1} bytes", - SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue())); + std::string Msg = + llvm::formatv("Storage provided to placement new is only {0} bytes, " + "whereas the allocated type requires {1} bytes", + SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()); auto R = std::make_unique<PathSensitiveBugReport>(SBT, Msg, N); bugreporter::trackExpressionValue(N, NE->getPlacementArg(0), *R); diff --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp index 15fd9a0..d2760ca 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -142,34 +142,19 @@ void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D, //===----------------------------------------------------------------------===// namespace { +class NSOrCFErrorDerefChecker + : public CheckerFamily<check::Location, + check::Event<ImplicitNullDerefEvent>> { + mutable IdentifierInfo *NSErrorII = nullptr; + mutable IdentifierInfo *CFErrorII = nullptr; -class NSErrorDerefBug : public BugType { -public: - NSErrorDerefBug(const CheckerNameRef Checker) - : BugType(Checker, "NSError** null dereference", - "Coding conventions (Apple)") {} -}; - -class CFErrorDerefBug : public BugType { public: - CFErrorDerefBug(const CheckerNameRef Checker) - : BugType(Checker, "CFErrorRef* null dereference", - "Coding conventions (Apple)") {} -}; - -} + CheckerFrontendWithBugType NSError{"NSError** null dereference", + "Coding conventions (Apple)"}; + CheckerFrontendWithBugType CFError{"CFErrorRef* null dereference", + "Coding conventions (Apple)"}; -namespace { -class NSOrCFErrorDerefChecker - : public Checker< check::Location, - check::Event<ImplicitNullDerefEvent> > { - mutable IdentifierInfo *NSErrorII, *CFErrorII; - mutable std::unique_ptr<NSErrorDerefBug> NSBT; - mutable std::unique_ptr<CFErrorDerefBug> CFBT; -public: - bool ShouldCheckNSError = false, ShouldCheckCFError = false; - CheckerNameRef NSErrorName, CFErrorName; - NSOrCFErrorDerefChecker() : NSErrorII(nullptr), CFErrorII(nullptr) {} + StringRef getDebugTag() const override { return "NSOrCFErrorDerefChecker"; } void checkLocation(SVal loc, bool isLoad, const Stmt *S, CheckerContext &C) const; @@ -236,12 +221,12 @@ void NSOrCFErrorDerefChecker::checkLocation(SVal loc, bool isLoad, if (!CFErrorII) CFErrorII = &Ctx.Idents.get("CFErrorRef"); - if (ShouldCheckNSError && IsNSError(parmT, NSErrorII)) { + if (NSError.isEnabled() && IsNSError(parmT, NSErrorII)) { setFlag<NSErrorOut>(state, state->getSVal(loc.castAs<Loc>()), C); return; } - if (ShouldCheckCFError && IsCFError(parmT, CFErrorII)) { + if (CFError.isEnabled() && IsCFError(parmT, CFErrorII)) { setFlag<CFErrorOut>(state, state->getSVal(loc.castAs<Loc>()), C); return; } @@ -274,19 +259,9 @@ void NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const { os << " may be null"; - BugType *bug = nullptr; - if (isNSError) { - if (!NSBT) - NSBT.reset(new NSErrorDerefBug(NSErrorName)); - bug = NSBT.get(); - } - else { - if (!CFBT) - CFBT.reset(new CFErrorDerefBug(CFErrorName)); - bug = CFBT.get(); - } + const BugType &BT = isNSError ? NSError : CFError; BR.emitReport( - std::make_unique<PathSensitiveBugReport>(*bug, os.str(), event.SinkNode)); + std::make_unique<PathSensitiveBugReport>(BT, os.str(), event.SinkNode)); } static bool IsNSError(QualType T, IdentifierInfo *II) { @@ -320,32 +295,21 @@ static bool IsCFError(QualType T, IdentifierInfo *II) { return TT->getDecl()->getIdentifier() == II; } -void ento::registerNSOrCFErrorDerefChecker(CheckerManager &mgr) { - mgr.registerChecker<NSOrCFErrorDerefChecker>(); -} - -bool ento::shouldRegisterNSOrCFErrorDerefChecker(const CheckerManager &mgr) { - return true; -} - -void ento::registerNSErrorChecker(CheckerManager &mgr) { - mgr.registerChecker<NSErrorMethodChecker>(); - NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>(); - checker->ShouldCheckNSError = true; - checker->NSErrorName = mgr.getCurrentCheckerName(); -} - -bool ento::shouldRegisterNSErrorChecker(const CheckerManager &mgr) { - return true; -} - -void ento::registerCFErrorChecker(CheckerManager &mgr) { - mgr.registerChecker<CFErrorFunctionChecker>(); - NSOrCFErrorDerefChecker *checker = mgr.getChecker<NSOrCFErrorDerefChecker>(); - checker->ShouldCheckCFError = true; - checker->CFErrorName = mgr.getCurrentCheckerName(); -} +// This source file implements two user-facing checkers ("osx.cocoa.NSError" +// and "osx.coreFoundation.CFError") which are both implemented as the +// combination of two `CheckerFrontend`s that are registered under the same +// name (but otherwise act independently). Among these 2+2 `CheckerFrontend`s +// two are coming from the checker family `NSOrCFErrorDerefChecker` while the +// other two (the `ADDITIONAL_PART`s) are small standalone checkers. +#define REGISTER_CHECKER(NAME, ADDITIONAL_PART) \ + void ento::register##NAME##Checker(CheckerManager &Mgr) { \ + Mgr.getChecker<NSOrCFErrorDerefChecker>()->NAME.enable(Mgr); \ + Mgr.registerChecker<ADDITIONAL_PART>(); \ + } \ + \ + bool ento::shouldRegister##NAME##Checker(const CheckerManager &) { \ + return true; \ + } -bool ento::shouldRegisterCFErrorChecker(const CheckerManager &mgr) { - return true; -} +REGISTER_CHECKER(NSError, NSErrorMethodChecker) +REGISTER_CHECKER(CFError, CFErrorFunctionChecker) diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index a63497c..019e81f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -28,23 +28,22 @@ using namespace ento; namespace { class StackAddrEscapeChecker - : public Checker<check::PreCall, check::PreStmt<ReturnStmt>, - check::EndFunction> { + : public CheckerFamily<check::PreCall, check::PreStmt<ReturnStmt>, + check::EndFunction> { mutable IdentifierInfo *dispatch_semaphore_tII = nullptr; - mutable std::unique_ptr<BugType> BT_stackleak; - mutable std::unique_ptr<BugType> BT_returnstack; - mutable std::unique_ptr<BugType> BT_capturedstackasync; - mutable std::unique_ptr<BugType> BT_capturedstackret; public: - enum CheckKind { - CK_StackAddrEscapeChecker, - CK_StackAddrAsyncEscapeChecker, - CK_NumCheckKinds - }; + StringRef getDebugTag() const override { return "StackAddrEscapeChecker"; } + + CheckerFrontend StackAddrEscape; + CheckerFrontend StackAddrAsyncEscape; - bool ChecksEnabled[CK_NumCheckKinds] = {false}; - CheckerNameRef CheckNames[CK_NumCheckKinds]; + const BugType StackLeak{&StackAddrEscape, + "Stack address leaks outside of stack frame"}; + const BugType ReturnStack{&StackAddrEscape, + "Return of address to stack-allocated memory"}; + const BugType CapturedStackAsync{ + &StackAddrAsyncEscape, "Address of stack-allocated memory is captured"}; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; @@ -170,10 +169,6 @@ void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C, ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) return; - if (!BT_returnstack) - BT_returnstack = std::make_unique<BugType>( - CheckNames[CK_StackAddrEscapeChecker], - "Return of address to stack-allocated memory"); // Generate a report for this bug. SmallString<128> buf; @@ -184,7 +179,7 @@ void StackAddrEscapeChecker::EmitReturnLeakError(CheckerContext &C, EmitReturnedAsPartOfError(os, C.getSVal(RetE), R); auto report = - std::make_unique<PathSensitiveBugReport>(*BT_returnstack, os.str(), N); + std::make_unique<PathSensitiveBugReport>(ReturnStack, os.str(), N); report->addRange(RetE->getSourceRange()); if (range.isValid()) report->addRange(range); @@ -215,16 +210,12 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures( ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) continue; - if (!BT_capturedstackasync) - BT_capturedstackasync = std::make_unique<BugType>( - CheckNames[CK_StackAddrAsyncEscapeChecker], - "Address of stack-allocated memory is captured"); SmallString<128> Buf; llvm::raw_svector_ostream Out(Buf); SourceRange Range = genName(Out, Region, C.getASTContext()); Out << " is captured by an asynchronously-executed block"; - auto Report = std::make_unique<PathSensitiveBugReport>( - *BT_capturedstackasync, Out.str(), N); + auto Report = std::make_unique<PathSensitiveBugReport>(CapturedStackAsync, + Out.str(), N); if (Range.isValid()) Report->addRange(Range); C.emitReport(std::move(Report)); @@ -233,7 +224,7 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures( void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker]) + if (!StackAddrAsyncEscape.isEnabled()) return; if (!Call.isGlobalCFunction("dispatch_after") && !Call.isGlobalCFunction("dispatch_async")) @@ -357,7 +348,7 @@ FindEscapingStackRegions(CheckerContext &C, const Expr *RetE, SVal RetVal) { void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { - if (!ChecksEnabled[CK_StackAddrEscapeChecker]) + if (!StackAddrEscape.isEnabled()) return; const Expr *RetE = RS->getRetValue(); @@ -456,7 +447,7 @@ static bool isInvalidatedSymbolRegion(const MemRegion *Region) { void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, CheckerContext &Ctx) const { - if (!ChecksEnabled[CK_StackAddrEscapeChecker]) + if (!StackAddrEscape.isEnabled()) return; ExplodedNode *Node = Ctx.getPredecessor(); @@ -581,11 +572,6 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, if (!N) return; - if (!BT_stackleak) - BT_stackleak = - std::make_unique<BugType>(CheckNames[CK_StackAddrEscapeChecker], - "Stack address leaks outside of stack frame"); - for (const auto &P : Cb.V) { const MemRegion *Referrer = P.first->getBaseRegion(); const MemRegion *Referred = P.second; @@ -604,7 +590,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, Out << " is still referred to by a temporary object on the stack" << CommonSuffix; auto Report = - std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N); + std::make_unique<PathSensitiveBugReport>(StackLeak, Out.str(), N); if (Range.isValid()) Report->addRange(Range); Ctx.emitReport(std::move(Report)); @@ -618,7 +604,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, Out << " is still referred to by the " << *ReferrerVariable << CommonSuffix; auto Report = - std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N); + std::make_unique<PathSensitiveBugReport>(StackLeak, Out.str(), N); if (Range.isValid()) Report->addRange(Range); @@ -626,23 +612,14 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, } } -void ento::registerStackAddrEscapeBase(CheckerManager &mgr) { - mgr.registerChecker<StackAddrEscapeChecker>(); -} - -bool ento::shouldRegisterStackAddrEscapeBase(const CheckerManager &mgr) { - return true; -} - -#define REGISTER_CHECKER(name) \ - void ento::register##name(CheckerManager &Mgr) { \ - StackAddrEscapeChecker *Chk = Mgr.getChecker<StackAddrEscapeChecker>(); \ - Chk->ChecksEnabled[StackAddrEscapeChecker::CK_##name] = true; \ - Chk->CheckNames[StackAddrEscapeChecker::CK_##name] = \ - Mgr.getCurrentCheckerName(); \ +#define REGISTER_CHECKER(NAME) \ + void ento::register##NAME##Checker(CheckerManager &Mgr) { \ + Mgr.getChecker<StackAddrEscapeChecker>()->NAME.enable(Mgr); \ } \ \ - bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; } + bool ento::shouldRegister##NAME##Checker(const CheckerManager &) { \ + return true; \ + } -REGISTER_CHECKER(StackAddrEscapeChecker) -REGISTER_CHECKER(StackAddrAsyncEscapeChecker) +REGISTER_CHECKER(StackAddrEscape) +REGISTER_CHECKER(StackAddrAsyncEscape) |