https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/76655
>From 0362ae2c888edc749711209174f592c5c311db76 Mon Sep 17 00:00:00 2001 From: Balazs Benics <benicsbal...@gmail.com> Date: Sun, 31 Dec 2023 11:33:15 +0100 Subject: [PATCH 1/2] [analyzer][NFC] Cleanup BugType lazy-init patterns Cleanup most of the lazy-init `BugType` legacy. Some will be preserved, as those are slightly more complicated to refactor. Notice, that the default category for `BugType` is `LogicError`. I omitted setting this explicitly where I could. --- .../Core/BugReporter/CommonBugCategories.h | 1 + .../Checkers/ArrayBoundChecker.cpp | 7 +- .../Checkers/BasicObjCFoundationChecks.cpp | 2 +- .../Checkers/BoolAssignmentChecker.cpp | 7 +- .../Checkers/CXXDeleteChecker.cpp | 24 ++--- .../Checkers/CastSizeChecker.cpp | 6 +- .../StaticAnalyzer/Checkers/ChrootChecker.cpp | 6 +- .../Checkers/ConversionChecker.cpp | 7 +- .../Checkers/DivZeroChecker.cpp | 16 ++-- .../Checkers/DynamicTypeChecker.cpp | 10 +-- .../Checkers/EnumCastOutOfRangeChecker.cpp | 8 +- .../Checkers/ExprInspectionChecker.cpp | 8 +- .../Checkers/FixedAddressChecker.cpp | 6 +- .../Checkers/LocalizationChecker.cpp | 12 +-- .../Checkers/MacOSKeychainAPIChecker.cpp | 22 ++--- .../Checkers/MacOSXAPIChecker.cpp | 10 +-- .../Checkers/MmapWriteExecChecker.cpp | 16 ++-- .../Checkers/NSAutoreleasePoolChecker.cpp | 9 +- .../Checkers/NonNullParamChecker.cpp | 21 ++--- .../Checkers/ObjCAtSyncChecker.cpp | 17 ++-- .../Checkers/ObjCContainersChecker.cpp | 11 +-- .../Checkers/ObjCSelfInitChecker.cpp | 9 +- .../Checkers/PaddingChecker.cpp | 9 +- .../Checkers/PointerArithChecker.cpp | 12 +-- .../Checkers/PointerSubChecker.cpp | 6 +- .../Checkers/ReturnPointerRangeChecker.cpp | 10 +-- .../Checkers/ReturnUndefChecker.cpp | 15 ++-- .../Checkers/StdLibraryFunctionsChecker.cpp | 8 +- .../Checkers/TestAfterDivZeroChecker.cpp | 10 +-- .../Checkers/UndefBranchChecker.cpp | 89 +++++++++---------- .../Checkers/UndefCapturedBlockVarChecker.cpp | 8 +- .../Checkers/UndefResultChecker.cpp | 8 +- .../UndefinedArraySubscriptChecker.cpp | 7 +- .../Checkers/UndefinedAssignmentChecker.cpp | 11 +-- .../Checkers/UnixAPIChecker.cpp | 32 +++---- .../Checkers/VLASizeChecker.cpp | 20 ++--- .../StaticAnalyzer/Checkers/VforkChecker.cpp | 7 +- .../Core/CommonBugCategories.cpp | 1 + .../NoStateChangeFuncVisitorTest.cpp | 7 +- 39 files changed, 171 insertions(+), 324 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h index 5d2c96e5bc9de3..45187433c069fd 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h @@ -13,6 +13,7 @@ namespace clang { namespace ento { namespace categories { +extern const char *const AppleAPIMisuse; extern const char *const CoreFoundationObjectiveC; extern const char *const LogicError; extern const char *const MemoryRefCount; diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index ce126541265551..c990ad138f8905 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -25,7 +25,7 @@ using namespace ento; namespace { class ArrayBoundChecker : public Checker<check::Location> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Out-of-bound array access"}; public: void checkLocation(SVal l, bool isLoad, const Stmt* S, @@ -65,16 +65,13 @@ void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS, if (!N) return; - if (!BT) - BT.reset(new BugType(this, "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. // Generate a report for this bug. auto report = std::make_unique<PathSensitiveBugReport>( - *BT, "Access out-of-bound array element (buffer overflow)", N); + BT, "Access out-of-bound array element (buffer overflow)", N); report->addRange(LoadS->getSourceRange()); C.emitReport(std::move(report)); diff --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 5e25153a148fea..c72a97cc01e914 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -44,7 +44,7 @@ namespace { class APIMisuse : public BugType { public: APIMisuse(const CheckerBase *checker, const char *name) - : BugType(checker, name, "API Misuse (Apple)") {} + : BugType(checker, name, categories::AppleAPIMisuse) {} }; } // end anonymous namespace diff --git a/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp index 361a4eed922108..a09db6d2d0ec5b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp @@ -24,7 +24,7 @@ using namespace ento; namespace { class BoolAssignmentChecker : public Checker< check::Bind > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Assignment of a non-Boolean value"}; void emitReport(ProgramStateRef state, CheckerContext &C, bool IsTainted = false) const; @@ -36,12 +36,9 @@ namespace { void BoolAssignmentChecker::emitReport(ProgramStateRef state, CheckerContext &C, bool IsTainted) const { if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) { - if (!BT) - BT.reset(new BugType(this, "Assignment of a non-Boolean value")); - StringRef Msg = IsTainted ? "Might assign a tainted non-Boolean value" : "Assignment of a non-Boolean value"; - C.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, Msg, N)); + C.emitReport(std::make_unique<PathSensitiveBugReport>(BT, Msg, N)); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp index eb265f4dde68bc..b4dee1e300e886 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp @@ -31,6 +31,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" @@ -65,7 +66,8 @@ class CXXDeleteChecker : public Checker<check::PreStmt<CXXDeleteExpr>> { }; class DeleteWithNonVirtualDtorChecker : public CXXDeleteChecker { - mutable std::unique_ptr<BugType> BT; + const BugType BT{ + this, "Destruction of a polymorphic object with no virtual destructor"}; void checkTypedDeleteExpr(const CXXDeleteExpr *DE, CheckerContext &C, @@ -74,7 +76,8 @@ class DeleteWithNonVirtualDtorChecker : public CXXDeleteChecker { }; class CXXArrayDeleteChecker : public CXXDeleteChecker { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, + "Deleting an array of polymorphic objects is undefined"}; void checkTypedDeleteExpr(const CXXDeleteExpr *DE, CheckerContext &C, @@ -123,17 +126,10 @@ void DeleteWithNonVirtualDtorChecker::checkTypedDeleteExpr( if (!DerivedClass->isDerivedFrom(BaseClass)) return; - if (!BT) - BT.reset(new BugType(this, - "Destruction of a polymorphic object with no " - "virtual destructor", - "Logic error")); - ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) return; - auto R = - std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N); // Mark region of problematic base class for later use in the BugVisitor. R->markInteresting(BaseClassRegion); @@ -160,12 +156,6 @@ void CXXArrayDeleteChecker::checkTypedDeleteExpr( if (!DerivedClass->isDerivedFrom(BaseClass)) return; - if (!BT) - BT.reset(new BugType(this, - "Deleting an array of polymorphic objects " - "is undefined", - "Logic error")); - ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) return; @@ -182,7 +172,7 @@ void CXXArrayDeleteChecker::checkTypedDeleteExpr( << SourceType.getAsString(C.getASTContext().getPrintingPolicy()) << "' is undefined"; - auto R = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N); // Mark region of problematic base class for later use in the BugVisitor. R->markInteresting(BaseClassRegion); diff --git a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp index d1d4f3baf6a852..a50772f881f7d0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp @@ -24,7 +24,7 @@ using namespace ento; namespace { class CastSizeChecker : public Checker< check::PreStmt<CastExpr> > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Cast region with wrong size."}; public: void checkPreStmt(const CastExpr *CE, CheckerContext &C) const; @@ -131,12 +131,10 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const { return; if (ExplodedNode *errorNode = C.generateErrorNode()) { - if (!BT) - BT.reset(new BugType(this, "Cast region with wrong size.")); constexpr llvm::StringLiteral Msg = "Cast a region whose size is not a multiple of the destination type " "size."; - auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, errorNode); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, errorNode); R->addRange(CE->getSourceRange()); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp index 9e11d8d9ecbc3c..be7be15022d360 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp @@ -41,7 +41,7 @@ bool isRootChanged(intptr_t k) { return k == ROOT_CHANGED; } // bug<--foo()-- JAIL_ENTERED<--foo()-- class ChrootChecker : public Checker<eval::Call, check::PreCall> { // This bug refers to possibly break out of a chroot() jail. - mutable std::unique_ptr<BugType> BT_BreakJail; + const BugType BT_BreakJail{this, "Break out of jail"}; const CallDescription Chroot{{"chroot"}, 1}, Chdir{{"chdir"}, 1}; @@ -124,12 +124,10 @@ void ChrootChecker::checkPreCall(const CallEvent &Call, if (k) if (isRootChanged((intptr_t) *k)) if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - if (!BT_BreakJail) - BT_BreakJail.reset(new BugType(this, "Break out of jail")); constexpr llvm::StringLiteral Msg = "No call of chdir(\"/\") immediately after chroot"; C.emitReport( - std::make_unique<PathSensitiveBugReport>(*BT_BreakJail, Msg, N)); + std::make_unique<PathSensitiveBugReport>(BT_BreakJail, Msg, N)); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp index 8b34b41bab21c3..eca8d3cc072292 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -42,7 +42,7 @@ class ConversionChecker : public Checker<check::PreStmt<ImplicitCastExpr>> { void checkPreStmt(const ImplicitCastExpr *Cast, CheckerContext &C) const; private: - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Conversion"}; bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType, CheckerContext &C) const; @@ -126,11 +126,8 @@ void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast, void ConversionChecker::reportBug(ExplodedNode *N, const Expr *E, CheckerContext &C, const char Msg[]) const { - if (!BT) - BT.reset(new BugType(this, "Conversion")); - // Generate a report for this bug. - auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); bugreporter::trackExpressionValue(N, E, *R); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 5331d9574743fb..5496f087447fbe 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Checkers/Taint.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -25,8 +26,8 @@ using namespace taint; namespace { class DivZeroChecker : public Checker< check::PreStmt<BinaryOperator> > { - mutable std::unique_ptr<BugType> BT; - mutable std::unique_ptr<BugType> TaintBT; + const BugType BT{this, "Division by zero"}; + const BugType TaintBT{this, "Division by zero", categories::TaintedData}; void reportBug(StringRef Msg, ProgramStateRef StateZero, CheckerContext &C) const; void reportTaintBug(StringRef Msg, ProgramStateRef StateZero, @@ -48,10 +49,7 @@ static const Expr *getDenomExpr(const ExplodedNode *N) { void DivZeroChecker::reportBug(StringRef Msg, ProgramStateRef StateZero, CheckerContext &C) const { if (ExplodedNode *N = C.generateErrorNode(StateZero)) { - if (!BT) - BT.reset(new BugType(this, "Division by zero", categories::LogicError)); - - auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); bugreporter::trackExpressionValue(N, getDenomExpr(N), *R); C.emitReport(std::move(R)); } @@ -61,11 +59,7 @@ void DivZeroChecker::reportTaintBug( StringRef Msg, ProgramStateRef StateZero, CheckerContext &C, llvm::ArrayRef<SymbolRef> TaintedSyms) const { if (ExplodedNode *N = C.generateErrorNode(StateZero)) { - if (!TaintBT) - TaintBT.reset( - new BugType(this, "Division by zero", categories::TaintedData)); - - auto R = std::make_unique<PathSensitiveBugReport>(*TaintBT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(TaintBT, Msg, N); bugreporter::trackExpressionValue(N, getDenomExpr(N), *R); for (auto Sym : TaintedSyms) R->markInteresting(Sym); diff --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp index dbc930d7d37b7c..0ad307d3ebd504 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp @@ -30,12 +30,7 @@ using namespace ento; namespace { class DynamicTypeChecker : public Checker<check::PostStmt<ImplicitCastExpr>> { - mutable std::unique_ptr<BugType> BT; - void initBugType() const { - if (!BT) - BT.reset( - new BugType(this, "Dynamic and static type mismatch", "Type Error")); - } + const BugType BT{this, "Dynamic and static type mismatch", "Type Error"}; class DynamicTypeBugVisitor : public BugReporterVisitor { public: @@ -70,7 +65,6 @@ void DynamicTypeChecker::reportTypeError(QualType DynamicType, const MemRegion *Reg, const Stmt *ReportedNode, CheckerContext &C) const { - initBugType(); SmallString<192> Buf; llvm::raw_svector_ostream OS(Buf); OS << "Object has a dynamic type '"; @@ -81,7 +75,7 @@ void DynamicTypeChecker::reportTypeError(QualType DynamicType, llvm::Twine()); OS << "'"; auto R = std::make_unique<PathSensitiveBugReport>( - *BT, OS.str(), C.generateNonFatalErrorNode()); + BT, OS.str(), C.generateNonFatalErrorNode()); R->markInteresting(Reg); R->addVisitor(std::make_unique<DynamicTypeBugVisitor>(Reg)); R->addRange(ReportedNode->getSourceRange()); diff --git a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp index 7c51673422a0a2..0fa20428c1b560 100644 --- a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp @@ -60,7 +60,7 @@ class ConstraintBasedEQEvaluator { // Being conservative, it does not warn if there is slight possibility the // value can be matching. class EnumCastOutOfRangeChecker : public Checker<check::PreStmt<CastExpr>> { - mutable std::unique_ptr<BugType> EnumValueCastOutOfRange; + const BugType EnumValueCastOutOfRange{this, "Enum cast out of range"}; void reportWarning(CheckerContext &C, const CastExpr *CE, const EnumDecl *E) const; @@ -85,10 +85,6 @@ void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C, const EnumDecl *E) const { assert(E && "valid EnumDecl* is expected"); if (const ExplodedNode *N = C.generateNonFatalErrorNode()) { - if (!EnumValueCastOutOfRange) - EnumValueCastOutOfRange.reset( - new BugType(this, "Enum cast out of range")); - std::string ValueStr = "", NameStr = "the enum"; // Try to add details to the message: @@ -105,7 +101,7 @@ void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C, "not in the valid range of values for {1}", ValueStr, NameStr); - auto BR = std::make_unique<PathSensitiveBugReport>(*EnumValueCastOutOfRange, + auto BR = std::make_unique<PathSensitiveBugReport>(EnumValueCastOutOfRange, Msg, N); bugreporter::trackExpressionValue(N, CE->getSubExpr(), *BR); BR->addNote("enum declared here", diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index 2c7bacac33a687..3096999e9fd163 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -25,7 +25,7 @@ using namespace ento; namespace { class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols, check::EndAnalysis> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Checking analyzer assumptions", "debug"}; // These stats are per-analysis, not per-branch, hence they shouldn't // stay inside the program state. @@ -176,11 +176,7 @@ ExprInspectionChecker::reportBug(llvm::StringRef Msg, BugReporter &BR, std::optional<SVal> ExprVal) const { if (!N) return nullptr; - - if (!BT) - BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); - - auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); if (ExprVal) { R->markInteresting(*ExprVal); } diff --git a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp index 2ee201b640089d..7aefcdc6d358aa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp @@ -24,7 +24,7 @@ using namespace ento; namespace { class FixedAddressChecker : public Checker< check::PreStmt<BinaryOperator> > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Use fixed address"}; public: void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const; @@ -50,12 +50,10 @@ void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, if (ExplodedNode *N = C.generateNonFatalErrorNode()) { // FIXME: improve grammar in the following strings: - if (!BT) - BT.reset(new BugType(this, "Use fixed address")); constexpr llvm::StringLiteral Msg = "Using a fixed address is not portable because that address will " "probably not be valid in all environments or platforms."; - auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); R->addRange(B->getRHS()->getSourceRange()); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp index 70f911fc66abca..de9efe17d220b1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -62,7 +62,8 @@ class NonLocalizedStringChecker check::PostObjCMessage, check::PostStmt<ObjCStringLiteral>> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Unlocalizable string", + "Localizability Issue (Apple)"}; // Methods that require a localized string mutable llvm::DenseMap<const IdentifierInfo *, @@ -89,8 +90,6 @@ class NonLocalizedStringChecker Selector S) const; public: - NonLocalizedStringChecker(); - // When this parameter is set to true, the checker assumes all // methods that return NSStrings are unlocalized. Thus, more false // positives will be reported. @@ -108,11 +107,6 @@ class NonLocalizedStringChecker REGISTER_MAP_WITH_PROGRAMSTATE(LocalizedMemMap, const MemRegion *, LocalizedState) -NonLocalizedStringChecker::NonLocalizedStringChecker() { - BT.reset(new BugType(this, "Unlocalizable string", - "Localizability Issue (Apple)")); -} - namespace { class NonLocalizedStringBRVisitor final : public BugReporterVisitor { @@ -764,7 +758,7 @@ void NonLocalizedStringChecker::reportLocalizationError( // Generate the bug report. auto R = std::make_unique<PathSensitiveBugReport>( - *BT, "User-facing text should use localized string macro", ErrNode); + BT, "User-facing text should use localized string macro", ErrNode); if (argumentNumber) { R->addRange(M.getArgExpr(argumentNumber - 1)->getSourceRange()); } else { diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 771c0a5fbb8d80..12bf12a0b23227 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -33,7 +33,8 @@ class MacOSKeychainAPIChecker : public Checker<check::PreStmt<CallExpr>, check::DeadSymbols, check::PointerEscape, eval::Assume> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Improper use of SecKeychain API", + categories::AppleAPIMisuse}; public: /// AllocationState is a part of the checker specific state together with the @@ -101,12 +102,6 @@ class MacOSKeychainAPIChecker : public Checker<check::PreStmt<CallExpr>, /// function. static unsigned getTrackedFunctionIndex(StringRef Name, bool IsAllocator); - inline void initBugType() const { - if (!BT) - BT.reset(new BugType(this, "Improper use of SecKeychain API", - "API Misuse (Apple)")); - } - void generateDeallocatorMismatchReport(const AllocationPair &AP, const Expr *ArgExpr, CheckerContext &C) const; @@ -232,7 +227,6 @@ void MacOSKeychainAPIChecker:: if (!N) return; - initBugType(); SmallString<80> sbuf; llvm::raw_svector_ostream os(sbuf); unsigned int PDeallocIdx = @@ -240,7 +234,7 @@ void MacOSKeychainAPIChecker:: os << "Deallocator doesn't match the allocator: '" << FunctionsToTrack[PDeallocIdx].Name << "' should be used."; - auto Report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + auto Report = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); Report->addVisitor(std::make_unique<SecKeychainBugVisitor>(AP.first)); Report->addRange(ArgExpr->getSourceRange()); markInteresting(Report.get(), AP); @@ -276,7 +270,6 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, ExplodedNode *N = C.generateNonFatalErrorNode(State); if (!N) return; - initBugType(); SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); unsigned int DIdx = FunctionsToTrack[AS->AllocatorIdx].DeallocatorIdx; @@ -284,8 +277,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, << "the allocator: missing a call to '" << FunctionsToTrack[DIdx].Name << "'."; - auto Report = - std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + auto Report = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); Report->addVisitor(std::make_unique<SecKeychainBugVisitor>(V)); Report->addRange(ArgExpr->getSourceRange()); Report->markInteresting(AS->Region); @@ -338,9 +330,8 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, ExplodedNode *N = C.generateNonFatalErrorNode(State); if (!N) return; - initBugType(); auto Report = std::make_unique<PathSensitiveBugReport>( - *BT, "Trying to free data which has not been allocated.", N); + BT, "Trying to free data which has not been allocated.", N); Report->addRange(ArgExpr->getSourceRange()); if (AS) Report->markInteresting(AS->Region); @@ -474,7 +465,6 @@ std::unique_ptr<PathSensitiveBugReport> MacOSKeychainAPIChecker::generateAllocatedDataNotReleasedReport( const AllocationPair &AP, ExplodedNode *N, CheckerContext &C) const { const ADFunctionInfo &FI = FunctionsToTrack[AP.second->AllocatorIdx]; - initBugType(); SmallString<70> sbuf; llvm::raw_svector_ostream os(sbuf); os << "Allocated data is not released: missing a call to '" @@ -493,7 +483,7 @@ MacOSKeychainAPIChecker::generateAllocatedDataNotReleasedReport( AllocNode->getLocationContext()); auto Report = std::make_unique<PathSensitiveBugReport>( - *BT, os.str(), N, LocUsedForUniqueing, + BT, os.str(), N, LocUsedForUniqueing, AllocNode->getLocationContext()->getDecl()); Report->addVisitor(std::make_unique<SecKeychainBugVisitor>(AP.first)); diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp index 04e7f8dec8d77e..754b167642961c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp @@ -18,6 +18,7 @@ #include "clang/Basic/TargetInfo.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -31,7 +32,8 @@ using namespace ento; namespace { class MacOSXAPIChecker : public Checker< check::PreStmt<CallExpr> > { - mutable std::unique_ptr<BugType> BT_dispatchOnce; + const BugType BT_dispatchOnce{this, "Improper use of 'dispatch_once'", + categories::AppleAPIMisuse}; static const ObjCIvarRegion *getParentIvarRegion(const MemRegion *R); @@ -136,12 +138,8 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE, if (!N) return; - if (!BT_dispatchOnce) - BT_dispatchOnce.reset(new BugType(this, "Improper use of 'dispatch_once'", - "API Misuse (Apple)")); - auto report = - std::make_unique<PathSensitiveBugReport>(*BT_dispatchOnce, os.str(), N); + std::make_unique<PathSensitiveBugReport>(BT_dispatchOnce, os.str(), N); report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(report)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp index 8fc44e78be6f07..2e31c16e457c2e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp @@ -15,6 +15,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" @@ -31,7 +32,9 @@ class MmapWriteExecChecker : public Checker<check::PreCall> { static int ProtWrite; static int ProtExec; static int ProtRead; - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "W^X check fails, Write Exec prot flags set", + "Security"}; + public: MmapWriteExecChecker() : MmapFn({"mmap"}, 6), MprotectFn({"mprotect"}, 3) {} void checkPreCall(const CallEvent &Call, CheckerContext &C) const; @@ -62,17 +65,16 @@ void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, return; if ((Prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) { - if (!BT) - BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security")); - ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) return; auto Report = std::make_unique<PathSensitiveBugReport>( - *BT, "Both PROT_WRITE and PROT_EXEC flags are set. This can " - "lead to exploitable memory regions, which could be overwritten " - "with malicious code", N); + BT, + "Both PROT_WRITE and PROT_EXEC flags are set. This can " + "lead to exploitable memory regions, which could be overwritten " + "with malicious code", + N); Report->addRange(Call.getArgSourceRange(2)); C.emitReport(std::move(Report)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp index bb01a3b7761726..0648084a7d3999 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp @@ -31,7 +31,8 @@ using namespace ento; namespace { class NSAutoreleasePoolChecker : public Checker<check::PreObjCMessage> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Use -drain instead of -release", + "API Upgrade (Apple)"}; mutable Selector releaseS; public: @@ -57,10 +58,6 @@ void NSAutoreleasePoolChecker::checkPreObjCMessage(const ObjCMethodCall &msg, if (msg.getSelector() != releaseS) return; - if (!BT) - BT.reset(new BugType(this, "Use -drain instead of -release", - "API Upgrade (Apple)")); - ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) { assert(0); @@ -68,7 +65,7 @@ void NSAutoreleasePoolChecker::checkPreObjCMessage(const ObjCMethodCall &msg, } auto Report = std::make_unique<PathSensitiveBugReport>( - *BT, + BT, "Use -drain instead of -release when using NSAutoreleasePool and " "garbage collection", N); diff --git a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp index 44b69ef31911c3..a9002ee7c9661a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp @@ -18,6 +18,7 @@ #include "clang/Analysis/AnyCall.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" @@ -31,8 +32,9 @@ namespace { class NonNullParamChecker : public Checker<check::PreCall, check::BeginFunction, EventDispatcher<ImplicitNullDerefEvent>> { - mutable std::unique_ptr<BugType> BTAttrNonNull; - mutable std::unique_ptr<BugType> BTNullRefArg; + const BugType BTAttrNonNull{ + this, "Argument with 'nonnull' attribute passed null", "API"}; + const BugType BTNullRefArg{this, "Dereference of null pointer"}; public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; @@ -278,13 +280,6 @@ std::unique_ptr<PathSensitiveBugReport> NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode, const Expr *ArgE, unsigned IdxOfArg) const { - // Lazily allocate the BugType object if it hasn't already been - // created. Ownership is transferred to the BugReporter object once - // the BugReport is passed to 'EmitWarning'. - if (!BTAttrNonNull) - BTAttrNonNull.reset(new BugType( - this, "Argument with 'nonnull' attribute passed null", "API")); - llvm::SmallString<256> SBuf; llvm::raw_svector_ostream OS(SBuf); OS << "Null pointer passed to " @@ -292,7 +287,7 @@ NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode, << " parameter expecting 'nonnull'"; auto R = - std::make_unique<PathSensitiveBugReport>(*BTAttrNonNull, SBuf, ErrorNode); + std::make_unique<PathSensitiveBugReport>(BTAttrNonNull, SBuf, ErrorNode); if (ArgE) bugreporter::trackExpressionValue(ErrorNode, ArgE, *R); @@ -302,11 +297,8 @@ NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode, std::unique_ptr<PathSensitiveBugReport> NonNullParamChecker::genReportReferenceToNullPointer( const ExplodedNode *ErrorNode, const Expr *ArgE) const { - if (!BTNullRefArg) - BTNullRefArg.reset(new BugType(this, "Dereference of null pointer")); - auto R = std::make_unique<PathSensitiveBugReport>( - *BTNullRefArg, "Forming reference to null pointer", ErrorNode); + BTNullRefArg, "Forming reference to null pointer", ErrorNode); if (ArgE) { const Expr *ArgEDeref = bugreporter::getDerefExpr(ArgE); if (!ArgEDeref) @@ -314,7 +306,6 @@ NonNullParamChecker::genReportReferenceToNullPointer( bugreporter::trackExpressionValue(ErrorNode, ArgEDeref, *R); } return R; - } void ento::registerNonNullParamChecker(CheckerManager &mgr) { diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp index 7906b787cd5346..552c222a251a63 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp @@ -25,8 +25,10 @@ using namespace ento; namespace { class ObjCAtSyncChecker : public Checker< check::PreStmt<ObjCAtSynchronizedStmt> > { - mutable std::unique_ptr<BugType> BT_null; - mutable std::unique_ptr<BugType> BT_undef; + const BugType BT_null{this, "Nil value used as mutex for @synchronized() " + "(no synchronization will occur)"}; + const BugType BT_undef{this, "Uninitialized value used as mutex " + "for @synchronized"}; public: void checkPreStmt(const ObjCAtSynchronizedStmt *S, CheckerContext &C) const; @@ -43,11 +45,8 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, // Uninitialized value used for the mutex? if (isa<UndefinedVal>(V)) { if (ExplodedNode *N = C.generateErrorNode()) { - if (!BT_undef) - BT_undef.reset(new BugType(this, "Uninitialized value used as mutex " - "for @synchronized")); auto report = std::make_unique<PathSensitiveBugReport>( - *BT_undef, BT_undef->getDescription(), N); + BT_undef, BT_undef.getDescription(), N); bugreporter::trackExpressionValue(N, Ex, *report); C.emitReport(std::move(report)); } @@ -66,12 +65,8 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, // Generate an error node. This isn't a sink since // a null mutex just means no synchronization occurs. if (ExplodedNode *N = C.generateNonFatalErrorNode(nullState)) { - if (!BT_null) - BT_null.reset( - new BugType(this, "Nil value used as mutex for @synchronized() " - "(no synchronization will occur)")); auto report = std::make_unique<PathSensitiveBugReport>( - *BT_null, BT_null->getDescription(), N); + BT_null, BT_null.getDescription(), N); bugreporter::trackExpressionValue(N, Ex, *report); C.emitReport(std::move(report)); diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index 0244a7a3ebffb9..28e88245ca95a0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -30,12 +30,7 @@ namespace { class ObjCContainersChecker : public Checker< check::PreStmt<CallExpr>, check::PostStmt<CallExpr>, check::PointerEscape> { - mutable std::unique_ptr<BugType> BT; - inline void initBugType() const { - if (!BT) - BT.reset(new BugType(this, "CFArray API", - categories::CoreFoundationObjectiveC)); - } + const BugType BT{this, "CFArray API", categories::CoreFoundationObjectiveC}; inline SymbolRef getArraySym(const Expr *E, CheckerContext &C) const { SVal ArrayRef = C.getSVal(E); @@ -140,9 +135,9 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, ExplodedNode *N = C.generateErrorNode(StOutBound); if (!N) return; - initBugType(); + auto R = std::make_unique<PathSensitiveBugReport>( - *BT, "Index is out of bounds", N); + BT, "Index is out of bounds", N); R->addRange(IdxExpr->getSourceRange()); bugreporter::trackExpressionValue(N, IdxExpr, *R, {bugreporter::TrackingKind::Thorough, diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index d88d6a94a30f28..217c46451f80f7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -61,13 +61,13 @@ class ObjCSelfInitChecker : public Checker< check::PostObjCMessage, check::PostCall, check::Location, check::Bind > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Missing \"self = [(super or self) init...]\"", + categories::CoreFoundationObjectiveC}; void checkForInvalidSelf(const Expr *E, CheckerContext &C, const char *errorStr) const; public: - ObjCSelfInitChecker() {} void checkPostObjCMessage(const ObjCMethodCall &Msg, CheckerContext &C) const; void checkPostStmt(const ObjCIvarRefExpr *E, CheckerContext &C) const; void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const; @@ -157,10 +157,7 @@ void ObjCSelfInitChecker::checkForInvalidSelf(const Expr *E, CheckerContext &C, if (!N) return; - if (!BT) - BT.reset(new BugType(this, "Missing \"self = [(super or self) init...]\"", - categories::CoreFoundationObjectiveC)); - C.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, errorStr, N)); + C.emitReport(std::make_unique<PathSensitiveBugReport>(BT, errorStr, N)); } void ObjCSelfInitChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, diff --git a/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp index bd6e1ec3a8fc7d..eee9449f31805c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp @@ -32,7 +32,7 @@ using namespace ento; namespace { class PaddingChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { private: - mutable std::unique_ptr<BugType> PaddingBug; + const BugType PaddingBug{this, "Excessive Padding", "Performance"}; mutable BugReporter *BR; public: @@ -310,10 +310,6 @@ class PaddingChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { void reportRecord( const RecordDecl *RD, CharUnits BaselinePad, CharUnits OptimalPad, const SmallVector<const FieldDecl *, 20> &OptimalFieldsOrder) const { - if (!PaddingBug) - PaddingBug = - std::make_unique<BugType>(this, "Excessive Padding", "Performance"); - SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); Os << "Excessive padding in '"; @@ -341,8 +337,7 @@ class PaddingChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { PathDiagnosticLocation CELoc = PathDiagnosticLocation::create(RD, BR->getSourceManager()); - auto Report = - std::make_unique<BasicBugReport>(*PaddingBug, Os.str(), CELoc); + auto Report = std::make_unique<BasicBugReport>(PaddingBug, Os.str(), CELoc); Report->setDeclWithIssue(RD); Report->addRange(RD->getSourceRange()); BR->emitReport(std::move(Report)); diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp index 1d63c0dd01f3d6..1141f07428b4cb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp @@ -56,8 +56,8 @@ class PointerArithChecker bool PointedNeeded = false) const; void initAllocIdentifiers(ASTContext &C) const; - mutable std::unique_ptr<BugType> BT_pointerArith; - mutable std::unique_ptr<BugType> BT_polyArray; + const BugType BT_pointerArith{this, "Dangerous pointer arithmetic"}; + const BugType BT_polyArray{this, "Dangerous pointer arithmetic"}; mutable llvm::SmallSet<IdentifierInfo *, 8> AllocFunctions; public: @@ -168,12 +168,10 @@ void PointerArithChecker::reportPointerArithMisuse(const Expr *E, if (!IsPolymorphic) return; if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - if (!BT_polyArray) - BT_polyArray.reset(new BugType(this, "Dangerous pointer arithmetic")); constexpr llvm::StringLiteral Msg = "Pointer arithmetic on a pointer to base class is dangerous " "because derived and base class may have different size."; - auto R = std::make_unique<PathSensitiveBugReport>(*BT_polyArray, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT_polyArray, Msg, N); R->addRange(E->getSourceRange()); R->markInteresting(ArrayRegion); C.emitReport(std::move(R)); @@ -190,12 +188,10 @@ void PointerArithChecker::reportPointerArithMisuse(const Expr *E, return; if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - if (!BT_pointerArith) - BT_pointerArith.reset(new BugType(this, "Dangerous pointer arithmetic")); constexpr llvm::StringLiteral Msg = "Pointer arithmetic on non-array variables relies on memory layout, " "which is dangerous."; - auto R = std::make_unique<PathSensitiveBugReport>(*BT_pointerArith, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT_pointerArith, Msg, N); R->addRange(SR); R->markInteresting(Region); C.emitReport(std::move(R)); diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index 96d38eef3c03a0..2438cf30b39b59 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -25,7 +25,7 @@ using namespace ento; namespace { class PointerSubChecker : public Checker< check::PreStmt<BinaryOperator> > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Pointer subtraction"}; public: void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const; @@ -59,12 +59,10 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, return; if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - if (!BT) - BT.reset(new BugType(this, "Pointer subtraction")); constexpr llvm::StringLiteral Msg = "Subtraction of two pointers that do not point to the same memory " "chunk may cause incorrect result."; - auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); R->addRange(B->getSourceRange()); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp index 11dca1ff8831d0..09d82ebabd4c97 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp @@ -26,7 +26,9 @@ using namespace ento; namespace { class ReturnPointerRangeChecker : public Checker< check::PreStmt<ReturnStmt> > { - mutable std::unique_ptr<BugType> BT; + // FIXME: This bug correspond to CWE-466. Eventually we should have bug + // types explicitly reference such exploit categories (when applicable). + const BugType BT{this, "Buffer overflow"}; public: void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; @@ -76,16 +78,12 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS, if (!N) return; - // FIXME: This bug correspond to CWE-466. Eventually we should have bug - // types explicitly reference such exploit categories (when applicable). - if (!BT) - BT.reset(new BugType(this, "Buffer overflow")); constexpr llvm::StringLiteral Msg = "Returned pointer value points outside the original object " "(potential buffer overflow)"; // Generate a report for this bug. - auto Report = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); + auto Report = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); Report->addRange(RetE->getSourceRange()); const auto ConcreteElementCount = ElementCount.getAs<nonloc::ConcreteInt>(); diff --git a/clang/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp index 78cd0100bea42b..efffbf2ee7559b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp @@ -24,8 +24,8 @@ using namespace ento; namespace { class ReturnUndefChecker : public Checker< check::PreStmt<ReturnStmt> > { - mutable std::unique_ptr<BugType> BT_Undef; - mutable std::unique_ptr<BugType> BT_NullReference; + const BugType BT_Undef{this, "Garbage return value"}; + const BugType BT_NullReference{this, "Returning null reference"}; void emitUndef(CheckerContext &C, const Expr *RetE) const; void checkReference(CheckerContext &C, const Expr *RetE, @@ -77,7 +77,7 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS, } } -static void emitBug(CheckerContext &C, BugType &BT, StringRef Msg, +static void emitBug(CheckerContext &C, const BugType &BT, StringRef Msg, const Expr *RetE, const Expr *TrackingE = nullptr) { ExplodedNode *N = C.generateErrorNode(); if (!N) @@ -92,9 +92,7 @@ static void emitBug(CheckerContext &C, BugType &BT, StringRef Msg, } void ReturnUndefChecker::emitUndef(CheckerContext &C, const Expr *RetE) const { - if (!BT_Undef) - BT_Undef.reset(new BugType(this, "Garbage return value")); - emitBug(C, *BT_Undef, "Undefined or garbage value returned to caller", RetE); + emitBug(C, BT_Undef, "Undefined or garbage value returned to caller", RetE); } void ReturnUndefChecker::checkReference(CheckerContext &C, const Expr *RetE, @@ -109,10 +107,7 @@ void ReturnUndefChecker::checkReference(CheckerContext &C, const Expr *RetE, } // The return value is known to be null. Emit a bug report. - if (!BT_NullReference) - BT_NullReference.reset(new BugType(this, "Returning null reference")); - - emitBug(C, *BT_NullReference, BT_NullReference->getDescription(), RetE, + emitBug(C, BT_NullReference, BT_NullReference.getDescription(), RetE, bugreporter::getDerefExpr(RetE)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 4ca49b9c0546d9..6560fd239ce668 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -823,7 +823,7 @@ class StdLibraryFunctionsChecker using FunctionSummaryMapType = llvm::DenseMap<const FunctionDecl *, Summary>; mutable FunctionSummaryMapType FunctionSummaryMap; - mutable std::unique_ptr<BugType> BT_InvalidArg; + const BugType BT_InvalidArg{this, "Function call with invalid argument"}; mutable bool SummariesInitialized = false; static SVal getArgSVal(const CallEvent &Call, ArgNo ArgN) { @@ -875,11 +875,7 @@ class StdLibraryFunctionsChecker VC->describe(ValueConstraint::Violation, Call, C.getState(), Summary, MsgOs); Msg[0] = toupper(Msg[0]); - if (!BT_InvalidArg) - BT_InvalidArg = std::make_unique<BugType>( - CheckName, "Function call with invalid argument", - categories::LogicError); - auto R = std::make_unique<PathSensitiveBugReport>(*BT_InvalidArg, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(BT_InvalidArg, Msg, N); for (ArgNo ArgN : VC->getArgsToTrack()) { bugreporter::trackExpressionValue(N, Call.getArgExpr(ArgN), *R); diff --git a/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp index 5cdcc1075f444c..667b19f8120eaa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp @@ -78,7 +78,7 @@ class DivisionBRVisitor : public BugReporterVisitor { class TestAfterDivZeroChecker : public Checker<check::PreStmt<BinaryOperator>, check::BranchCondition, check::EndFunction> { - mutable std::unique_ptr<BugType> DivZeroBug; + const BugType DivZeroBug{this, "Division by zero"}; void reportBug(SVal Val, CheckerContext &C) const; public: @@ -165,12 +165,10 @@ bool TestAfterDivZeroChecker::hasDivZeroMap(SVal Var, void TestAfterDivZeroChecker::reportBug(SVal Val, CheckerContext &C) const { if (ExplodedNode *N = C.generateErrorNode(C.getState())) { - if (!DivZeroBug) - DivZeroBug.reset(new BugType(this, "Division by zero")); - auto R = std::make_unique<PathSensitiveBugReport>( - *DivZeroBug, "Value being compared against zero has already been used " - "for division", + DivZeroBug, + "Value being compared against zero has already been used " + "for division", N); R->addVisitor(std::make_unique<DivisionBRVisitor>(Val.getAsSymbol(), diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index db886501a162d0..aa478b69aade1a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -27,7 +27,7 @@ using namespace ento; namespace { class UndefBranchChecker : public Checker<check::BranchCondition> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Branch condition evaluates to a garbage value"}; struct FindUndefExpr { ProgramStateRef St; @@ -64,52 +64,47 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition, // ObjCForCollection is a loop, but has no actual condition. if (isa<ObjCForCollectionStmt>(Condition)) return; - SVal X = Ctx.getSVal(Condition); - if (X.isUndef()) { - // Generate a sink node, which implicitly marks both outgoing branches as - // infeasible. - ExplodedNode *N = Ctx.generateErrorNode(); - if (N) { - if (!BT) - BT.reset( - new BugType(this, "Branch condition evaluates to a garbage value")); - - // What's going on here: we want to highlight the subexpression of the - // condition that is the most likely source of the "uninitialized - // branch condition." We do a recursive walk of the condition's - // subexpressions and roughly look for the most nested subexpression - // that binds to Undefined. We then highlight that expression's range. - - // Get the predecessor node and check if is a PostStmt with the Stmt - // being the terminator condition. We want to inspect the state - // of that node instead because it will contain main information about - // the subexpressions. - - // Note: any predecessor will do. They should have identical state, - // since all the BlockEdge did was act as an error sink since the value - // had to already be undefined. - assert (!N->pred_empty()); - const Expr *Ex = cast<Expr>(Condition); - ExplodedNode *PrevN = *N->pred_begin(); - ProgramPoint P = PrevN->getLocation(); - ProgramStateRef St = N->getState(); - - if (std::optional<PostStmt> PS = P.getAs<PostStmt>()) - if (PS->getStmt() == Ex) - St = PrevN->getState(); - - FindUndefExpr FindIt(St, Ctx.getLocationContext()); - Ex = FindIt.FindExpr(Ex); - - // Emit the bug report. - auto R = std::make_unique<PathSensitiveBugReport>( - *BT, BT->getDescription(), N); - bugreporter::trackExpressionValue(N, Ex, *R); - R->addRange(Ex->getSourceRange()); - - Ctx.emitReport(std::move(R)); - } - } + if (!Ctx.getSVal(Condition).isUndef()) + return; + + // Generate a sink node, which implicitly marks both outgoing branches as + // infeasible. + ExplodedNode *N = Ctx.generateErrorNode(); + if (!N) + return; + // What's going on here: we want to highlight the subexpression of the + // condition that is the most likely source of the "uninitialized + // branch condition." We do a recursive walk of the condition's + // subexpressions and roughly look for the most nested subexpression + // that binds to Undefined. We then highlight that expression's range. + + // Get the predecessor node and check if is a PostStmt with the Stmt + // being the terminator condition. We want to inspect the state + // of that node instead because it will contain main information about + // the subexpressions. + + // Note: any predecessor will do. They should have identical state, + // since all the BlockEdge did was act as an error sink since the value + // had to already be undefined. + assert(!N->pred_empty()); + const Expr *Ex = cast<Expr>(Condition); + ExplodedNode *PrevN = *N->pred_begin(); + ProgramPoint P = PrevN->getLocation(); + ProgramStateRef St = N->getState(); + + if (std::optional<PostStmt> PS = P.getAs<PostStmt>()) + if (PS->getStmt() == Ex) + St = PrevN->getState(); + + FindUndefExpr FindIt(St, Ctx.getLocationContext()); + Ex = FindIt.FindExpr(Ex); + + // Emit the bug report. + auto R = std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N); + bugreporter::trackExpressionValue(N, Ex, *R); + R->addRange(Ex->getSourceRange()); + + Ctx.emitReport(std::move(R)); } void ento::registerUndefBranchChecker(CheckerManager &mgr) { diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp index ecb6ed36ee40cb..2839ef0b6d2e61 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -27,7 +27,7 @@ using namespace ento; namespace { class UndefCapturedBlockVarChecker : public Checker< check::PostStmt<BlockExpr> > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "uninitialized variable captured by block"}; public: void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const; @@ -70,10 +70,6 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, if (std::optional<UndefinedVal> V = state->getSVal(Var.getOriginalRegion()).getAs<UndefinedVal>()) { if (ExplodedNode *N = C.generateErrorNode()) { - if (!BT) - BT.reset( - new BugType(this, "uninitialized variable captured by block")); - // Generate a bug report. SmallString<128> buf; llvm::raw_svector_ostream os(buf); @@ -81,7 +77,7 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, os << "Variable '" << VD->getName() << "' is uninitialized when captured by block"; - auto R = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) R->addRange(Ex->getSourceRange()); bugreporter::trackStoredValue(*V, VR, *R, diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index d593a6bd74cfea..4b845bb3ded23e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -28,7 +28,7 @@ namespace { class UndefResultChecker : public Checker< check::PostStmt<BinaryOperator> > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Result of operation is garbage or undefined"}; public: void checkPostStmt(const BinaryOperator *B, CheckerContext &C) const; @@ -74,10 +74,6 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, if (!N) return; - if (!BT) - BT.reset( - new BugType(this, "Result of operation is garbage or undefined")); - SmallString<256> sbuf; llvm::raw_svector_ostream OS(sbuf); const Expr *Ex = nullptr; @@ -104,7 +100,7 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, << BinaryOperator::getOpcodeStr(B->getOpcode()) << "' expression is undefined"; } - auto report = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N); if (Ex) { report->addRange(Ex->getSourceRange()); bugreporter::trackExpressionValue(N, Ex, *report); diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp index a6cc8cac8c9925..baa07fa66764e3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp @@ -24,7 +24,7 @@ using namespace ento; namespace { class UndefinedArraySubscriptChecker : public Checker< check::PreStmt<ArraySubscriptExpr> > { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Array subscript is undefined"}; public: void checkPreStmt(const ArraySubscriptExpr *A, CheckerContext &C) const; @@ -48,11 +48,8 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A, ExplodedNode *N = C.generateErrorNode(); if (!N) return; - if (!BT) - BT.reset(new BugType(this, "Array subscript is undefined")); - // Generate a report for this bug. - auto R = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N); R->addRange(A->getIdx()->getSourceRange()); bugreporter::trackExpressionValue(N, A->getIdx(), *R); C.emitReport(std::move(R)); diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index 49ac94f65dd0c0..ddc6cc9e8202c7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -23,7 +23,7 @@ using namespace ento; namespace { class UndefinedAssignmentChecker : public Checker<check::Bind> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Assigned value is garbage or undefined"}; public: void checkBind(SVal location, SVal val, const Stmt *S, @@ -49,11 +49,6 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, if (!N) return; - static const char *const DefaultMsg = - "Assigned value is garbage or undefined"; - if (!BT) - BT.reset(new BugType(this, DefaultMsg)); - // Generate a report for this bug. llvm::SmallString<128> Str; llvm::raw_svector_ostream OS(Str); @@ -105,9 +100,9 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, } if (OS.str().empty()) - OS << DefaultMsg; + OS << BT.getDescription(); - auto R = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N); if (ex) { R->addRange(ex->getSourceRange()); bugreporter::trackExpressionValue(N, ex, *R); diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index f503b3e88bb35d..b05ce610067cfa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -11,9 +11,10 @@ // //===----------------------------------------------------------------------===// -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/Basic/TargetInfo.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -39,7 +40,9 @@ enum class OpenVariant { namespace { class UnixAPIMisuseChecker : public Checker< check::PreStmt<CallExpr> > { - mutable std::unique_ptr<BugType> BT_open, BT_pthreadOnce; + const BugType BT_open{this, "Improper use of 'open'", categories::UnixAPI}; + const BugType BT_pthreadOnce{this, "Improper use of 'pthread_once'", + categories::UnixAPI}; mutable std::optional<uint64_t> Val_O_CREAT; public: @@ -64,7 +67,9 @@ class UnixAPIPortabilityChecker : public Checker< check::PreStmt<CallExpr> > { void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; private: - mutable std::unique_ptr<BugType> BT_mallocZero; + const BugType BT_mallocZero{ + this, "Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)", + categories::UnixAPI}; void CheckCallocZero(CheckerContext &C, const CallExpr *CE) const; void CheckMallocZero(CheckerContext &C, const CallExpr *CE) const; @@ -87,14 +92,6 @@ class UnixAPIPortabilityChecker : public Checker< check::PreStmt<CallExpr> > { } //end anonymous namespace -static void LazyInitialize(const CheckerBase *Checker, - std::unique_ptr<BugType> &BT, - const char *name) { - if (BT) - return; - BT.reset(new BugType(Checker, name, categories::UnixAPI)); -} - //===----------------------------------------------------------------------===// // "open" (man 2 open) //===----------------------------------------------------------------------===/ @@ -132,9 +129,7 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C, if (!N) return; - LazyInitialize(this, BT_open, "Improper use of 'open'"); - - auto Report = std::make_unique<PathSensitiveBugReport>(*BT_open, Msg, N); + auto Report = std::make_unique<PathSensitiveBugReport>(BT_open, Msg, N); Report->addRange(SR); C.emitReport(std::move(Report)); } @@ -301,10 +296,8 @@ void UnixAPIMisuseChecker::CheckPthreadOnce(CheckerContext &C, if (isa<VarRegion>(R) && isa<StackLocalsSpaceRegion>(R->getMemorySpace())) os << " Perhaps you intended to declare the variable as 'static'?"; - LazyInitialize(this, BT_pthreadOnce, "Improper use of 'pthread_once'"); - auto report = - std::make_unique<PathSensitiveBugReport>(*BT_pthreadOnce, os.str(), N); + std::make_unique<PathSensitiveBugReport>(BT_pthreadOnce, os.str(), N); report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(report)); } @@ -341,14 +334,11 @@ bool UnixAPIPortabilityChecker::ReportZeroByteAllocation( if (!N) return false; - LazyInitialize(this, BT_mallocZero, - "Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)"); - SmallString<256> S; llvm::raw_svector_ostream os(S); os << "Call to '" << fn_name << "' has an allocation size of 0 bytes"; auto report = - std::make_unique<PathSensitiveBugReport>(*BT_mallocZero, os.str(), N); + std::make_unique<PathSensitiveBugReport>(BT_mallocZero, os.str(), N); report->addRange(arg->getSourceRange()); bugreporter::trackExpressionValue(N, arg, *report); diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index 1d03d1656b3cb1..d76fe49918690c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -34,8 +34,10 @@ namespace { class VLASizeChecker : public Checker<check::PreStmt<DeclStmt>, check::PreStmt<UnaryExprOrTypeTraitExpr>> { - mutable std::unique_ptr<BugType> BT; - mutable std::unique_ptr<BugType> TaintBT; + const BugType BT{this, "Dangerous variable-length array (VLA) declaration"}; + const BugType TaintBT{this, + "Dangerous variable-length array (VLA) declaration", + categories::TaintedData}; enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Negative, VLA_Overflow }; /// Check a VLA for validity. @@ -213,17 +215,12 @@ void VLASizeChecker::reportTaintBug(const Expr *SizeE, ProgramStateRef State, if (!N) return; - if (!TaintBT) - TaintBT.reset( - new BugType(this, "Dangerous variable-length array (VLA) declaration", - categories::TaintedData)); - SmallString<256> buf; llvm::raw_svector_ostream os(buf); os << "Declared variable-length array (VLA) "; os << "has tainted size"; - auto report = std::make_unique<PathSensitiveBugReport>(*TaintBT, os.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(TaintBT, os.str(), N); report->addRange(SizeE->getSourceRange()); bugreporter::trackExpressionValue(N, SizeE, *report); // The vla size may be a complex expression where multiple memory locations @@ -240,11 +237,6 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind, const Expr *SizeE, if (!N) return; - if (!BT) - BT.reset(new BugType(this, - "Dangerous variable-length array (VLA) declaration", - categories::LogicError)); - SmallString<256> buf; llvm::raw_svector_ostream os(buf); os << "Declared variable-length array (VLA) "; @@ -263,7 +255,7 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind, const Expr *SizeE, break; } - auto report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); report->addRange(SizeE->getSourceRange()); bugreporter::trackExpressionValue(N, SizeE, *report); C.emitReport(std::move(report)); diff --git a/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp index 8a1e02748c9b2b..cb73ac68edd1ea 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp @@ -44,7 +44,7 @@ namespace { class VforkChecker : public Checker<check::PreCall, check::PostCall, check::Bind, check::PreStmt<ReturnStmt>> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "Dangerous construct in a vforked process"}; mutable llvm::SmallSet<const IdentifierInfo *, 10> VforkAllowlist; mutable const IdentifierInfo *II_vfork = nullptr; @@ -123,9 +123,6 @@ bool VforkChecker::isCallExplicitelyAllowed(const IdentifierInfo *II, void VforkChecker::reportBug(const char *What, CheckerContext &C, const char *Details) const { if (ExplodedNode *N = C.generateErrorNode(C.getState())) { - if (!BT) - BT.reset(new BugType(this, "Dangerous construct in a vforked process")); - SmallString<256> buf; llvm::raw_svector_ostream os(buf); @@ -134,7 +131,7 @@ void VforkChecker::reportBug(const char *What, CheckerContext &C, if (Details) os << "; " << Details; - auto Report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + auto Report = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N); // TODO: mark vfork call in BugReportVisitor C.emitReport(std::move(Report)); } diff --git a/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp b/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp index e9cc080caf5f85..66fab523c86472 100644 --- a/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp +++ b/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp @@ -13,6 +13,7 @@ namespace clang { namespace ento { namespace categories { +const char *const AppleAPIMisuse = "API Misuse (Apple)"; const char *const CoreFoundationObjectiveC = "Core Foundation/Objective-C"; const char *const LogicError = "Logic error"; const char *const MemoryRefCount = diff --git a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp b/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp index 69e29b47f92572..51aca42a26b059 100644 --- a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp +++ b/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp @@ -82,7 +82,7 @@ class ErrorNotPreventedFuncVisitor : public NoStateChangeFuncVisitor { template <class Visitor> class StatefulChecker : public Checker<check::PreCall> { - mutable std::unique_ptr<BugType> BT; + const BugType BT{this, "error()", categories::SecurityError}; public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const { @@ -102,11 +102,8 @@ class StatefulChecker : public Checker<check::PreCall> { const ExplodedNode *N = C.generateErrorNode(); if (!N) return; - if (!BT) - BT.reset(new BugType(this->getCheckerName(), "error()", - categories::SecurityError)); auto R = - std::make_unique<PathSensitiveBugReport>(*BT, "error() called", N); + std::make_unique<PathSensitiveBugReport>(BT, "error() called", N); R->template addVisitor<Visitor>(); C.emitReport(std::move(R)); } >From bca9208e086b63d4a4483364d560c3e2b0b4347e Mon Sep 17 00:00:00 2001 From: Balazs Benics <benicsbal...@gmail.com> Date: Sun, 31 Dec 2023 13:30:26 +0100 Subject: [PATCH 2/2] Part two --- .../BlockInCriticalSectionChecker.cpp | 57 ++++++++----------- .../Checkers/CheckObjCDealloc.cpp | 48 ++++++---------- .../StaticAnalyzer/Checkers/CloneChecker.cpp | 15 ++--- .../Checkers/DebugContainerModeling.cpp | 15 ++--- .../Checkers/DebugIteratorModeling.cpp | 15 ++--- .../Checkers/InvalidatedIteratorChecker.cpp | 19 +++---- .../Checkers/IteratorRangeChecker.cpp | 12 +--- .../Checkers/MPI-Checker/MPIBugReporter.cpp | 6 +- .../Checkers/MPI-Checker/MPIBugReporter.h | 21 +++---- .../Checkers/MismatchedIteratorChecker.cpp | 16 ++---- .../Checkers/ObjCSuperDeallocChecker.cpp | 19 ++----- .../Checkers/SimpleStreamChecker.cpp | 27 +++------ .../Checkers/TaintTesterChecker.cpp | 5 +- .../UninitializedObjectChecker.cpp | 9 +-- .../StaticAnalyzer/CallEventTest.cpp | 5 -- .../RegisterCustomCheckersTest.cpp | 7 +-- 16 files changed, 103 insertions(+), 193 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp index 76f091562cd57f..66e080adb1382b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp @@ -25,20 +25,31 @@ using namespace clang; using namespace ento; namespace { - class BlockInCriticalSectionChecker : public Checker<check::PostCall> { - - mutable IdentifierInfo *IILockGuard, *IIUniqueLock; - - CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn, - PthreadLockFn, PthreadTryLockFn, PthreadUnlockFn, - MtxLock, MtxTimedLock, MtxTryLock, MtxUnlock; - - StringRef ClassLockGuard, ClassUniqueLock; - - mutable bool IdentifierInfoInitialized; - - std::unique_ptr<BugType> BlockInCritSectionBugType; + mutable IdentifierInfo *IILockGuard = nullptr; + mutable IdentifierInfo *IIUniqueLock = nullptr; + mutable bool IdentifierInfoInitialized = false; + + const CallDescription LockFn{{"lock"}}; + const CallDescription UnlockFn{{"unlock"}}; + const CallDescription SleepFn{{"sleep"}}; + const CallDescription GetcFn{{"getc"}}; + const CallDescription FgetsFn{{"fgets"}}; + const CallDescription ReadFn{{"read"}}; + const CallDescription RecvFn{{"recv"}}; + const CallDescription PthreadLockFn{{"pthread_mutex_lock"}}; + const CallDescription PthreadTryLockFn{{"pthread_mutex_trylock"}}; + const CallDescription PthreadUnlockFn{{"pthread_mutex_unlock"}}; + const CallDescription MtxLock{{"mtx_lock"}}; + const CallDescription MtxTimedLock{{"mtx_timedlock"}}; + const CallDescription MtxTryLock{{"mtx_trylock"}}; + const CallDescription MtxUnlock{{"mtx_unlock"}}; + + const llvm::StringLiteral ClassLockGuard{"lock_guard"}; + const llvm::StringLiteral ClassUniqueLock{"unique_lock"}; + + const BugType BlockInCritSectionBugType{ + this, "Call to blocking function in critical section", "Blocking Error"}; void initIdentifierInfo(ASTContext &Ctx) const; @@ -47,8 +58,6 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> { CheckerContext &C) const; public: - BlockInCriticalSectionChecker(); - bool isBlockingFunction(const CallEvent &Call) const; bool isLockFunction(const CallEvent &Call) const; bool isUnlockFunction(const CallEvent &Call) const; @@ -63,22 +72,6 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> { REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned) -BlockInCriticalSectionChecker::BlockInCriticalSectionChecker() - : IILockGuard(nullptr), IIUniqueLock(nullptr), LockFn({"lock"}), - UnlockFn({"unlock"}), SleepFn({"sleep"}), GetcFn({"getc"}), - FgetsFn({"fgets"}), ReadFn({"read"}), RecvFn({"recv"}), - PthreadLockFn({"pthread_mutex_lock"}), - PthreadTryLockFn({"pthread_mutex_trylock"}), - PthreadUnlockFn({"pthread_mutex_unlock"}), MtxLock({"mtx_lock"}), - MtxTimedLock({"mtx_timedlock"}), MtxTryLock({"mtx_trylock"}), - MtxUnlock({"mtx_unlock"}), ClassLockGuard("lock_guard"), - ClassUniqueLock("unique_lock"), IdentifierInfoInitialized(false) { - // Initialize the bug type. - BlockInCritSectionBugType.reset( - new BugType(this, "Call to blocking function in critical section", - "Blocking Error")); -} - void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const { if (!IdentifierInfoInitialized) { /* In case of checking C code, or when the corresponding headers are not @@ -151,7 +144,7 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection( llvm::raw_string_ostream os(msg); os << "Call to blocking function '" << Call.getCalleeIdentifier()->getName() << "' inside of critical section"; - auto R = std::make_unique<PathSensitiveBugReport>(*BlockInCritSectionBugType, + auto R = std::make_unique<PathSensitiveBugReport>(BlockInCritSectionBugType, os.str(), ErrNode); R->addRange(Call.getSourceRange()); R->markInteresting(BlockDescSym); diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index fedc6db3723aac..978bc0bb082f80 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -99,18 +99,23 @@ class ObjCDeallocChecker check::PointerEscape, check::PreStmt<ReturnStmt>> { - mutable IdentifierInfo *NSObjectII, *SenTestCaseII, *XCTestCaseII, - *Block_releaseII, *CIFilterII; - - mutable Selector DeallocSel, ReleaseSel; - - std::unique_ptr<BugType> MissingReleaseBugType; - std::unique_ptr<BugType> ExtraReleaseBugType; - std::unique_ptr<BugType> MistakenDeallocBugType; + mutable const IdentifierInfo *NSObjectII = nullptr; + mutable const IdentifierInfo *SenTestCaseII = nullptr; + mutable const IdentifierInfo *XCTestCaseII = nullptr; + mutable const IdentifierInfo *Block_releaseII = nullptr; + mutable const IdentifierInfo *CIFilterII = nullptr; + + mutable Selector DeallocSel; + mutable Selector ReleaseSel; + + const BugType MissingReleaseBugType{this, "Missing ivar release (leak)", + categories::MemoryRefCount}; + const BugType ExtraReleaseBugType{this, "Extra ivar release", + categories::MemoryRefCount}; + const BugType MistakenDeallocBugType{this, "Mistaken dealloc", + categories::MemoryRefCount}; public: - ObjCDeallocChecker(); - void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr, BugReporter &BR) const; void checkBeginFunction(CheckerContext &Ctx) const; @@ -579,7 +584,7 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const { OS << " by a synthesized property but not released" " before '[super dealloc]'"; - auto BR = std::make_unique<PathSensitiveBugReport>(*MissingReleaseBugType, + auto BR = std::make_unique<PathSensitiveBugReport>(MissingReleaseBugType, OS.str(), ErrNode); C.emitReport(std::move(BR)); } @@ -701,7 +706,7 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue, OS << " property but was released in 'dealloc'"; } - auto BR = std::make_unique<PathSensitiveBugReport>(*ExtraReleaseBugType, + auto BR = std::make_unique<PathSensitiveBugReport>(ExtraReleaseBugType, OS.str(), ErrNode); BR->addRange(M.getOriginExpr()->getSourceRange()); @@ -743,7 +748,7 @@ bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue, OS << "'" << *PropImpl->getPropertyIvarDecl() << "' should be released rather than deallocated"; - auto BR = std::make_unique<PathSensitiveBugReport>(*MistakenDeallocBugType, + auto BR = std::make_unique<PathSensitiveBugReport>(MistakenDeallocBugType, OS.str(), ErrNode); BR->addRange(M.getOriginExpr()->getSourceRange()); @@ -752,23 +757,6 @@ bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue, return true; } -ObjCDeallocChecker::ObjCDeallocChecker() - : NSObjectII(nullptr), SenTestCaseII(nullptr), XCTestCaseII(nullptr), - Block_releaseII(nullptr), CIFilterII(nullptr) { - - MissingReleaseBugType.reset( - new BugType(this, "Missing ivar release (leak)", - categories::MemoryRefCount)); - - ExtraReleaseBugType.reset( - new BugType(this, "Extra ivar release", - categories::MemoryRefCount)); - - MistakenDeallocBugType.reset( - new BugType(this, "Mistaken dealloc", - categories::MemoryRefCount)); -} - void ObjCDeallocChecker::initIdentifierInfoAndSelectors( ASTContext &Ctx) const { if (NSObjectII) diff --git a/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp index 0e21ea7e90c9c2..6692a45a09f776 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp @@ -35,7 +35,8 @@ class CloneChecker private: mutable CloneDetector Detector; - mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious; + const BugType BT_Exact{this, "Exact code clone", "Code clone"}; + const BugType BT_Suspicious{this, "Suspicious code clone", "Code clone"}; public: void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr, @@ -107,15 +108,11 @@ static PathDiagnosticLocation makeLocation(const StmtSequence &S, void CloneChecker::reportClones( BugReporter &BR, AnalysisManager &Mgr, std::vector<CloneDetector::CloneGroup> &CloneGroups) const { - - if (!BT_Exact) - BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone")); - for (const CloneDetector::CloneGroup &Group : CloneGroups) { // We group the clones by printing the first as a warning and all others // as a note. auto R = std::make_unique<BasicBugReport>( - *BT_Exact, "Duplicate code detected", makeLocation(Group.front(), Mgr)); + BT_Exact, "Duplicate code detected", makeLocation(Group.front(), Mgr)); R->addRange(Group.front().getSourceRange()); for (unsigned i = 1; i < Group.size(); ++i) @@ -154,10 +151,6 @@ void CloneChecker::reportSuspiciousClones( } } - if (!BT_Suspicious) - BT_Suspicious.reset( - new BugType(this, "Suspicious code clone", "Code clone")); - ASTContext &ACtx = BR.getContext(); SourceManager &SM = ACtx.getSourceManager(); AnalysisDeclContext *ADC = @@ -170,7 +163,7 @@ void CloneChecker::reportSuspiciousClones( // Think how to perform more accurate suggestions? auto R = std::make_unique<BasicBugReport>( - *BT_Suspicious, + BT_Suspicious, "Potential copy-paste error; did you really mean to use '" + Pair.FirstCloneInfo.Variable->getNameAsString() + "' here?", PathDiagnosticLocation::createBegin(Pair.FirstCloneInfo.Mention, SM, diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp index 832bb78c48035c..97f769b1c451f6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp @@ -28,7 +28,8 @@ namespace { class DebugContainerModeling : public Checker<eval::Call> { - std::unique_ptr<BugType> DebugMsgBugType; + const BugType DebugMsgBugType{this, "Checking analyzer assumptions", "debug", + /*SuppressOnSink=*/true}; template <typename Getter> void analyzerContainerDataField(const CallExpr *CE, CheckerContext &C, @@ -48,19 +49,11 @@ class DebugContainerModeling }; public: - DebugContainerModeling(); - bool evalCall(const CallEvent &Call, CheckerContext &C) const; }; } //namespace -DebugContainerModeling::DebugContainerModeling() { - DebugMsgBugType.reset( - new BugType(this, "Checking analyzer assumptions", "debug", - /*SuppressOnSink=*/true)); -} - bool DebugContainerModeling::evalCall(const CallEvent &Call, CheckerContext &C) const { const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); @@ -137,8 +130,8 @@ ExplodedNode *DebugContainerModeling::reportDebugMsg(llvm::StringRef Msg, return nullptr; auto &BR = C.getBugReporter(); - BR.emitReport(std::make_unique<PathSensitiveBugReport>(*DebugMsgBugType, - Msg, N)); + BR.emitReport( + std::make_unique<PathSensitiveBugReport>(DebugMsgBugType, Msg, N)); return N; } diff --git a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp index d05298b42c55cd..ff479c7b0ac89d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp @@ -28,7 +28,8 @@ namespace { class DebugIteratorModeling : public Checker<eval::Call> { - std::unique_ptr<BugType> DebugMsgBugType; + const BugType DebugMsgBugType{this, "Checking analyzer assumptions", "debug", + /*SuppressOnSink=*/true}; template <typename Getter> void analyzerIteratorDataField(const CallExpr *CE, CheckerContext &C, @@ -51,19 +52,11 @@ class DebugIteratorModeling }; public: - DebugIteratorModeling(); - bool evalCall(const CallEvent &Call, CheckerContext &C) const; }; } //namespace -DebugIteratorModeling::DebugIteratorModeling() { - DebugMsgBugType.reset( - new BugType(this, "Checking analyzer assumptions", "debug", - /*SuppressOnSink=*/true)); -} - bool DebugIteratorModeling::evalCall(const CallEvent &Call, CheckerContext &C) const { const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); @@ -131,8 +124,8 @@ ExplodedNode *DebugIteratorModeling::reportDebugMsg(llvm::StringRef Msg, return nullptr; auto &BR = C.getBugReporter(); - BR.emitReport(std::make_unique<PathSensitiveBugReport>(*DebugMsgBugType, - Msg, N)); + BR.emitReport( + std::make_unique<PathSensitiveBugReport>(DebugMsgBugType, Msg, N)); return N; } diff --git a/clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp index 6955ba11a28ff7..b979f3c1c0366c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp @@ -31,14 +31,14 @@ class InvalidatedIteratorChecker check::PreStmt<ArraySubscriptExpr>, check::PreStmt<MemberExpr>> { - std::unique_ptr<BugType> InvalidatedBugType; + const BugType InvalidatedBugType{this, "Iterator invalidated", + "Misuse of STL APIs"}; void verifyAccess(CheckerContext &C, const SVal &Val) const; - void reportBug(const StringRef &Message, const SVal &Val, - CheckerContext &C, ExplodedNode *ErrNode) const; -public: - InvalidatedIteratorChecker(); + void reportBug(const StringRef &Message, const SVal &Val, CheckerContext &C, + ExplodedNode *ErrNode) const; +public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPreStmt(const UnaryOperator *UO, CheckerContext &C) const; void checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const; @@ -49,11 +49,6 @@ class InvalidatedIteratorChecker } //namespace -InvalidatedIteratorChecker::InvalidatedIteratorChecker() { - InvalidatedBugType.reset( - new BugType(this, "Iterator invalidated", "Misuse of STL APIs")); -} - void InvalidatedIteratorChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { // Check for access of invalidated position @@ -129,8 +124,8 @@ void InvalidatedIteratorChecker::verifyAccess(CheckerContext &C, const SVal &Val void InvalidatedIteratorChecker::reportBug(const StringRef &Message, const SVal &Val, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = std::make_unique<PathSensitiveBugReport>(*InvalidatedBugType, - Message, ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>(InvalidatedBugType, Message, + ErrNode); R->markInteresting(Val); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp index 7740c3d4da1ec2..f966ea63da2181 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp @@ -32,7 +32,8 @@ class IteratorRangeChecker check::PreStmt<ArraySubscriptExpr>, check::PreStmt<MemberExpr>> { - std::unique_ptr<BugType> OutOfRangeBugType; + const BugType OutOfRangeBugType{this, "Iterator out of range", + "Misuse of STL APIs"}; void verifyDereference(CheckerContext &C, SVal Val) const; void verifyIncrement(CheckerContext &C, SVal Iter) const; @@ -46,8 +47,6 @@ class IteratorRangeChecker ExplodedNode *ErrNode) const; public: - IteratorRangeChecker(); - void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPreStmt(const UnaryOperator *UO, CheckerContext &C) const; void checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const; @@ -71,11 +70,6 @@ bool isZero(ProgramStateRef State, const NonLoc &Val); } //namespace -IteratorRangeChecker::IteratorRangeChecker() { - OutOfRangeBugType.reset( - new BugType(this, "Iterator out of range", "Misuse of STL APIs")); -} - void IteratorRangeChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { // Check for out of range access @@ -278,7 +272,7 @@ void IteratorRangeChecker::verifyNext(CheckerContext &C, SVal LHS, void IteratorRangeChecker::reportBug(const StringRef &Message, SVal Val, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = std::make_unique<PathSensitiveBugReport>(*OutOfRangeBugType, Message, + auto R = std::make_unique<PathSensitiveBugReport>(OutOfRangeBugType, Message, ErrNode); const auto *Pos = getIteratorPosition(C.getState(), Val); diff --git a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp index bbf2ddec576207..3e374e6c240e76 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp @@ -31,7 +31,7 @@ void MPIBugReporter::reportDoubleNonblocking( RequestRegion->getDescriptiveName() + ". "; auto Report = std::make_unique<PathSensitiveBugReport>( - *DoubleNonblockingBugType, ErrorText, ExplNode); + DoubleNonblockingBugType, ErrorText, ExplNode); Report->addRange(MPICallEvent.getSourceRange()); SourceRange Range = RequestRegion->sourceRange(); @@ -53,7 +53,7 @@ void MPIBugReporter::reportMissingWait( std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() + " has no matching wait. "}; - auto Report = std::make_unique<PathSensitiveBugReport>(*MissingWaitBugType, + auto Report = std::make_unique<PathSensitiveBugReport>(MissingWaitBugType, ErrorText, ExplNode); SourceRange Range = RequestRegion->sourceRange(); @@ -73,7 +73,7 @@ void MPIBugReporter::reportUnmatchedWait( std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() + " has no matching nonblocking call. "}; - auto Report = std::make_unique<PathSensitiveBugReport>(*UnmatchedWaitBugType, + auto Report = std::make_unique<PathSensitiveBugReport>(UnmatchedWaitBugType, ErrorText, ExplNode); Report->addRange(CE.getSourceRange()); diff --git a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h index 9871da026b0428..0222a2120b34f3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h +++ b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h @@ -17,6 +17,7 @@ #include "MPITypes.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "llvm/ADT/StringRef.h" namespace clang { namespace ento { @@ -24,12 +25,10 @@ namespace mpi { class MPIBugReporter { public: - MPIBugReporter(const CheckerBase &CB) { - UnmatchedWaitBugType.reset(new BugType(&CB, "Unmatched wait", MPIError)); - DoubleNonblockingBugType.reset( - new BugType(&CB, "Double nonblocking", MPIError)); - MissingWaitBugType.reset(new BugType(&CB, "Missing wait", MPIError)); - } + MPIBugReporter(const CheckerBase &CB) + : UnmatchedWaitBugType(&CB, "Unmatched wait", MPIError), + MissingWaitBugType(&CB, "Missing wait", MPIError), + DoubleNonblockingBugType(&CB, "Double nonblocking", MPIError) {} /// Report duplicate request use by nonblocking calls without intermediate /// wait. @@ -68,12 +67,10 @@ class MPIBugReporter { BugReporter &BReporter) const; private: - const std::string MPIError = "MPI Error"; - - // path-sensitive bug types - std::unique_ptr<BugType> UnmatchedWaitBugType; - std::unique_ptr<BugType> MissingWaitBugType; - std::unique_ptr<BugType> DoubleNonblockingBugType; + const llvm::StringLiteral MPIError = "MPI Error"; + const BugType UnmatchedWaitBugType; + const BugType MissingWaitBugType; + const BugType DoubleNonblockingBugType; /// Bug visitor class to find the node where the request region was previously /// used in order to include it into the BugReport path. diff --git a/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp index 2020dc7cc7910e..38f80ba5e1ff39 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp @@ -30,7 +30,9 @@ namespace { class MismatchedIteratorChecker : public Checker<check::PreCall, check::PreStmt<BinaryOperator>> { - std::unique_ptr<BugType> MismatchedBugType; + const BugType MismatchedBugType{this, "Iterator(s) mismatched", + "Misuse of STL APIs", + /*SuppressOnSink=*/true}; void verifyMatch(CheckerContext &C, const SVal &Iter, const MemRegion *Cont) const; @@ -44,8 +46,6 @@ class MismatchedIteratorChecker ExplodedNode *ErrNode) const; public: - MismatchedIteratorChecker(); - void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPreStmt(const BinaryOperator *BO, CheckerContext &C) const; @@ -53,12 +53,6 @@ class MismatchedIteratorChecker } // namespace -MismatchedIteratorChecker::MismatchedIteratorChecker() { - MismatchedBugType.reset( - new BugType(this, "Iterator(s) mismatched", "Misuse of STL APIs", - /*SuppressOnSink=*/true)); -} - void MismatchedIteratorChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { // Check for iterator mismatches @@ -282,7 +276,7 @@ void MismatchedIteratorChecker::reportBug(const StringRef &Message, const SVal &Val2, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = std::make_unique<PathSensitiveBugReport>(*MismatchedBugType, Message, + auto R = std::make_unique<PathSensitiveBugReport>(MismatchedBugType, Message, ErrNode); R->markInteresting(Val1); R->markInteresting(Val2); @@ -293,7 +287,7 @@ void MismatchedIteratorChecker::reportBug(const StringRef &Message, const SVal &Val, const MemRegion *Reg, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = std::make_unique<PathSensitiveBugReport>(*MismatchedBugType, Message, + auto R = std::make_unique<PathSensitiveBugReport>(MismatchedBugType, Message, ErrNode); R->markInteresting(Val); R->markInteresting(Reg); diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp index 3547b7bb61a248..eb40711812e16b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp @@ -26,18 +26,19 @@ namespace { class ObjCSuperDeallocChecker : public Checker<check::PostObjCMessage, check::PreObjCMessage, check::PreCall, check::Location> { - - mutable IdentifierInfo *IIdealloc, *IINSObject; + mutable IdentifierInfo *IIdealloc = nullptr; + mutable IdentifierInfo *IINSObject = nullptr; mutable Selector SELdealloc; - std::unique_ptr<BugType> DoubleSuperDeallocBugType; + const BugType DoubleSuperDeallocBugType{ + this, "[super dealloc] should not be called more than once", + categories::CoreFoundationObjectiveC}; void initIdentifierInfoAndSelectors(ASTContext &Ctx) const; bool isSuperDeallocMessage(const ObjCMethodCall &M) const; public: - ObjCSuperDeallocChecker(); void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; @@ -188,7 +189,7 @@ void ObjCSuperDeallocChecker::reportUseAfterDealloc(SymbolRef Sym, Desc = "Use of 'self' after it has been deallocated"; // Generate the report. - auto BR = std::make_unique<PathSensitiveBugReport>(*DoubleSuperDeallocBugType, + auto BR = std::make_unique<PathSensitiveBugReport>(DoubleSuperDeallocBugType, Desc, ErrNode); BR->addRange(S->getSourceRange()); BR->addVisitor(std::make_unique<SuperDeallocBRVisitor>(Sym)); @@ -213,14 +214,6 @@ void ObjCSuperDeallocChecker::diagnoseCallArguments(const CallEvent &CE, } } -ObjCSuperDeallocChecker::ObjCSuperDeallocChecker() - : IIdealloc(nullptr), IINSObject(nullptr) { - - DoubleSuperDeallocBugType.reset( - new BugType(this, "[super dealloc] should not be called more than once", - categories::CoreFoundationObjectiveC)); -} - void ObjCSuperDeallocChecker::initIdentifierInfoAndSelectors(ASTContext &Ctx) const { if (IIdealloc) diff --git a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp index 2ac9f65c9793f4..7cbe271dfbf93a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -52,10 +52,13 @@ class SimpleStreamChecker : public Checker<check::PostCall, check::PreCall, check::DeadSymbols, check::PointerEscape> { - CallDescription OpenFn, CloseFn; + const CallDescription OpenFn{{"fopen"}, 2}; + const CallDescription CloseFn{{"fclose"}, 1}; - std::unique_ptr<BugType> DoubleCloseBugType; - std::unique_ptr<BugType> LeakBugType; + const BugType DoubleCloseBugType{this, "Double fclose", + "Unix Stream API Error"}; + const BugType LeakBugType{this, "Resource Leak", "Unix Stream API Error", + /*SuppressOnSink=*/true}; void reportDoubleClose(SymbolRef FileDescSym, const CallEvent &Call, @@ -67,8 +70,6 @@ class SimpleStreamChecker : public Checker<check::PostCall, bool guaranteedNotToCloseFile(const CallEvent &Call) const; public: - SimpleStreamChecker(); - /// Process fopen. void checkPostCall(const CallEvent &Call, CheckerContext &C) const; /// Process fclose. @@ -89,18 +90,6 @@ class SimpleStreamChecker : public Checker<check::PostCall, /// state. Let's store it in the ProgramState. REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) -SimpleStreamChecker::SimpleStreamChecker() - : OpenFn({"fopen"}, 2), CloseFn({"fclose"}, 1) { - // Initialize the bug types. - DoubleCloseBugType.reset( - new BugType(this, "Double fclose", "Unix Stream API Error")); - - // Sinks are higher importance bugs as well as calls to assert() or exit(0). - LeakBugType.reset( - new BugType(this, "Resource Leak", "Unix Stream API Error", - /*SuppressOnSink=*/true)); -} - void SimpleStreamChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { if (!Call.isGlobalCFunction()) @@ -192,7 +181,7 @@ void SimpleStreamChecker::reportDoubleClose(SymbolRef FileDescSym, // Generate the report. auto R = std::make_unique<PathSensitiveBugReport>( - *DoubleCloseBugType, "Closing a previously closed file stream", ErrNode); + DoubleCloseBugType, "Closing a previously closed file stream", ErrNode); R->addRange(Call.getSourceRange()); R->markInteresting(FileDescSym); C.emitReport(std::move(R)); @@ -205,7 +194,7 @@ void SimpleStreamChecker::reportLeaks(ArrayRef<SymbolRef> LeakedStreams, // TODO: Identify the leaked file descriptor. for (SymbolRef LeakedStream : LeakedStreams) { auto R = std::make_unique<PathSensitiveBugReport>( - *LeakBugType, "Opened file is never closed; potential resource leak", + LeakBugType, "Opened file is never closed; potential resource leak", ErrNode); R->markInteresting(LeakedStream); C.emitReport(std::move(R)); diff --git a/clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp index 614a2b2e4ec79f..acf4e833095b7f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp @@ -23,8 +23,7 @@ using namespace taint; namespace { class TaintTesterChecker : public Checker<check::PostStmt<Expr>> { - std::unique_ptr<BugType> BT = - std::make_unique<BugType>(this, "Tainted data", "General"); + const BugType BT{this, "Tainted data", "General"}; public: void checkPostStmt(const Expr *E, CheckerContext &C) const; @@ -39,7 +38,7 @@ void TaintTesterChecker::checkPostStmt(const Expr *E, if (isTainted(State, E, C.getLocationContext())) { if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - auto report = std::make_unique<PathSensitiveBugReport>(*BT, "tainted", N); + auto report = std::make_unique<PathSensitiveBugReport>(BT, "tainted", N); report->addRange(E->getSourceRange()); C.emitReport(std::move(report)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp index 3647c49cf3f97d..91e9426c98750b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -38,15 +38,12 @@ namespace { class UninitializedObjectChecker : public Checker<check::EndFunction, check::DeadSymbols> { - std::unique_ptr<BugType> BT_uninitField; + const BugType BT_uninitField{this, "Uninitialized fields"}; public: // The fields of this struct will be initialized when registering the checker. UninitObjCheckerOptions Opts; - UninitializedObjectChecker() - : BT_uninitField(new BugType(this, "Uninitialized fields")) {} - void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const; }; @@ -186,7 +183,7 @@ void UninitializedObjectChecker::checkEndFunction( for (const auto &Pair : UninitFields) { auto Report = std::make_unique<PathSensitiveBugReport>( - *BT_uninitField, Pair.second, Node, LocUsedForUniqueing, + BT_uninitField, Pair.second, Node, LocUsedForUniqueing, Node->getLocationContext()->getDecl()); Context.emitReport(std::move(Report)); } @@ -200,7 +197,7 @@ void UninitializedObjectChecker::checkEndFunction( << " at the end of the constructor call"; auto Report = std::make_unique<PathSensitiveBugReport>( - *BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, + BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, Node->getLocationContext()->getDecl()); for (const auto &Pair : UninitFields) { diff --git a/clang/unittests/StaticAnalyzer/CallEventTest.cpp b/clang/unittests/StaticAnalyzer/CallEventTest.cpp index 5be25d2ada6738..adbfe02a284dc5 100644 --- a/clang/unittests/StaticAnalyzer/CallEventTest.cpp +++ b/clang/unittests/StaticAnalyzer/CallEventTest.cpp @@ -36,12 +36,7 @@ void reportBug(const CheckerBase *Checker, const CallEvent &Call, } class CXXDeallocatorChecker : public Checker<check::PreCall> { - std::unique_ptr<BugType> BT_uninitField; - public: - CXXDeallocatorChecker() - : BT_uninitField(new BugType(this, "CXXDeallocator")) {} - void checkPreCall(const CallEvent &Call, CheckerContext &C) const { const auto *DC = dyn_cast<CXXDeallocatorCall>(&Call); if (!DC) { diff --git a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp index d5c7a5c9bb8233..cd46efc8ad762c 100644 --- a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp +++ b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp @@ -89,8 +89,7 @@ TEST(RegisterCustomCheckers, CheckLocationIncDec) { class CheckerRegistrationOrderPrinter : public Checker<check::PreStmt<DeclStmt>> { - std::unique_ptr<BugType> BT = - std::make_unique<BugType>(this, "Registration order"); + const BugType BT{this, "Registration order"}; public: void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { @@ -104,7 +103,7 @@ class CheckerRegistrationOrderPrinter .printEnabledCheckerList(OS); // Strip a newline off. auto R = - std::make_unique<PathSensitiveBugReport>(*BT, OS.str().drop_back(1), N); + std::make_unique<PathSensitiveBugReport>(BT, OS.str().drop_back(1), N); C.emitReport(std::move(R)); } }; @@ -125,8 +124,6 @@ void addCheckerRegistrationOrderPrinter(CheckerRegistry &Registry) { #define UNITTEST_CHECKER(CHECKER_NAME, DIAG_MSG) \ class CHECKER_NAME : public Checker<check::PreStmt<DeclStmt>> { \ - std::unique_ptr<BugType> BT = std::make_unique<BugType>(this, DIAG_MSG); \ - \ public: \ void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {} \ }; \ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits