https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/76655
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. Please, actually have a look at the diff. I did this manually, and we rarely check the bug type descriptions and stuff in tests, so the testing might be shallow on this one. >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] [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)); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits