Skip to content

Commit 18f219c

Browse files
steakhalsteakhal
authored
[analyzer][NFC] Cleanup BugType lazy-init patterns (#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.
1 parent c92d3ce commit 18f219c

File tree

55 files changed

+276
-519
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+276
-519
lines changed

‎clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
namespace clang {
1414
namespace ento {
1515
namespace categories {
16+
extern const char *const AppleAPIMisuse;
1617
extern const char *const CoreFoundationObjectiveC;
1718
extern const char *const LogicError;
1819
extern const char *const MemoryRefCount;

‎clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ using namespace ento;
2525
namespace {
2626
class ArrayBoundChecker :
2727
public Checker<check::Location> {
28-
mutable std::unique_ptr<BugType> BT;
28+
const BugType BT{this, "Out-of-bound array access"};
2929

3030
public:
3131
void checkLocation(SVal l, bool isLoad, const Stmt* S,
@@ -65,16 +65,13 @@ void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS,
6565
if (!N)
6666
return;
6767

68-
if (!BT)
69-
BT.reset(new BugType(this, "Out-of-bound array access"));
70-
7168
// FIXME: It would be nice to eventually make this diagnostic more clear,
7269
// e.g., by referencing the original declaration or by saying *why* this
7370
// reference is outside the range.
7471

7572
// Generate a report for this bug.
7673
auto report = std::make_unique<PathSensitiveBugReport>(
77-
*BT, "Access out-of-bound array element (buffer overflow)", N);
74+
BT, "Access out-of-bound array element (buffer overflow)", N);
7875

7976
report->addRange(LoadS->getSourceRange());
8077
C.emitReport(std::move(report));

‎clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ namespace {
4444
class APIMisuse : public BugType {
4545
public:
4646
APIMisuse(const CheckerBase *checker, const char *name)
47-
: BugType(checker, name, "API Misuse (Apple)") {}
47+
: BugType(checker, name, categories::AppleAPIMisuse) {}
4848
};
4949
} // end anonymous namespace
5050

‎clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,31 @@ using namespace clang;
2525
using namespace ento;
2626

2727
namespace {
28-
2928
class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
30-
31-
mutable IdentifierInfo *IILockGuard, *IIUniqueLock;
32-
33-
CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn,
34-
PthreadLockFn, PthreadTryLockFn, PthreadUnlockFn,
35-
MtxLock, MtxTimedLock, MtxTryLock, MtxUnlock;
36-
37-
StringRef ClassLockGuard, ClassUniqueLock;
38-
39-
mutable bool IdentifierInfoInitialized;
40-
41-
std::unique_ptr<BugType> BlockInCritSectionBugType;
29+
mutable IdentifierInfo *IILockGuard = nullptr;
30+
mutable IdentifierInfo *IIUniqueLock = nullptr;
31+
mutable bool IdentifierInfoInitialized = false;
32+
33+
const CallDescription LockFn{{"lock"}};
34+
const CallDescription UnlockFn{{"unlock"}};
35+
const CallDescription SleepFn{{"sleep"}};
36+
const CallDescription GetcFn{{"getc"}};
37+
const CallDescription FgetsFn{{"fgets"}};
38+
const CallDescription ReadFn{{"read"}};
39+
const CallDescription RecvFn{{"recv"}};
40+
const CallDescription PthreadLockFn{{"pthread_mutex_lock"}};
41+
const CallDescription PthreadTryLockFn{{"pthread_mutex_trylock"}};
42+
const CallDescription PthreadUnlockFn{{"pthread_mutex_unlock"}};
43+
const CallDescription MtxLock{{"mtx_lock"}};
44+
const CallDescription MtxTimedLock{{"mtx_timedlock"}};
45+
const CallDescription MtxTryLock{{"mtx_trylock"}};
46+
const CallDescription MtxUnlock{{"mtx_unlock"}};
47+
48+
const llvm::StringLiteral ClassLockGuard{"lock_guard"};
49+
const llvm::StringLiteral ClassUniqueLock{"unique_lock"};
50+
51+
const BugType BlockInCritSectionBugType{
52+
this, "Call to blocking function in critical section", "Blocking Error"};
4253

4354
void initIdentifierInfo(ASTContext &Ctx) const;
4455

@@ -47,8 +58,6 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
4758
CheckerContext &C) const;
4859

4960
public:
50-
BlockInCriticalSectionChecker();
51-
5261
bool isBlockingFunction(const CallEvent &Call) const;
5362
bool isLockFunction(const CallEvent &Call) const;
5463
bool isUnlockFunction(const CallEvent &Call) const;
@@ -63,22 +72,6 @@ class BlockInCriticalSectionChecker : public Checker<check::PostCall> {
6372

6473
REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
6574

66-
BlockInCriticalSectionChecker::BlockInCriticalSectionChecker()
67-
: IILockGuard(nullptr), IIUniqueLock(nullptr), LockFn({"lock"}),
68-
UnlockFn({"unlock"}), SleepFn({"sleep"}), GetcFn({"getc"}),
69-
FgetsFn({"fgets"}), ReadFn({"read"}), RecvFn({"recv"}),
70-
PthreadLockFn({"pthread_mutex_lock"}),
71-
PthreadTryLockFn({"pthread_mutex_trylock"}),
72-
PthreadUnlockFn({"pthread_mutex_unlock"}), MtxLock({"mtx_lock"}),
73-
MtxTimedLock({"mtx_timedlock"}), MtxTryLock({"mtx_trylock"}),
74-
MtxUnlock({"mtx_unlock"}), ClassLockGuard("lock_guard"),
75-
ClassUniqueLock("unique_lock"), IdentifierInfoInitialized(false) {
76-
// Initialize the bug type.
77-
BlockInCritSectionBugType.reset(
78-
new BugType(this, "Call to blocking function in critical section",
79-
"Blocking Error"));
80-
}
81-
8275
void BlockInCriticalSectionChecker::initIdentifierInfo(ASTContext &Ctx) const {
8376
if (!IdentifierInfoInitialized) {
8477
/* In case of checking C code, or when the corresponding headers are not
@@ -151,7 +144,7 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection(
151144
llvm::raw_string_ostream os(msg);
152145
os << "Call to blocking function '" << Call.getCalleeIdentifier()->getName()
153146
<< "' inside of critical section";
154-
auto R = std::make_unique<PathSensitiveBugReport>(*BlockInCritSectionBugType,
147+
auto R = std::make_unique<PathSensitiveBugReport>(BlockInCritSectionBugType,
155148
os.str(), ErrNode);
156149
R->addRange(Call.getSourceRange());
157150
R->markInteresting(BlockDescSym);

‎clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ using namespace ento;
2424

2525
namespace {
2626
class BoolAssignmentChecker : public Checker< check::Bind > {
27-
mutable std::unique_ptr<BugType> BT;
27+
const BugType BT{this, "Assignment of a non-Boolean value"};
2828
void emitReport(ProgramStateRef state, CheckerContext &C,
2929
bool IsTainted = false) const;
3030

@@ -36,12 +36,9 @@ namespace {
3636
void BoolAssignmentChecker::emitReport(ProgramStateRef state, CheckerContext &C,
3737
bool IsTainted) const {
3838
if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
39-
if (!BT)
40-
BT.reset(new BugType(this, "Assignment of a non-Boolean value"));
41-
4239
StringRef Msg = IsTainted ? "Might assign a tainted non-Boolean value"
4340
: "Assignment of a non-Boolean value";
44-
C.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, Msg, N));
41+
C.emitReport(std::make_unique<PathSensitiveBugReport>(BT, Msg, N));
4542
}
4643
}
4744

‎clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
3232
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
3333
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
34+
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
3435
#include "clang/StaticAnalyzer/Core/Checker.h"
3536
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
3637
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -65,7 +66,8 @@ class CXXDeleteChecker : public Checker<check::PreStmt<CXXDeleteExpr>> {
6566
};
6667

6768
class DeleteWithNonVirtualDtorChecker : public CXXDeleteChecker {
68-
mutable std::unique_ptr<BugType> BT;
69+
const BugType BT{
70+
this, "Destruction of a polymorphic object with no virtual destructor"};
6971

7072
void
7173
checkTypedDeleteExpr(const CXXDeleteExpr *DE, CheckerContext &C,
@@ -74,7 +76,8 @@ class DeleteWithNonVirtualDtorChecker : public CXXDeleteChecker {
7476
};
7577

7678
class CXXArrayDeleteChecker : public CXXDeleteChecker {
77-
mutable std::unique_ptr<BugType> BT;
79+
const BugType BT{this,
80+
"Deleting an array of polymorphic objects is undefined"};
7881

7982
void
8083
checkTypedDeleteExpr(const CXXDeleteExpr *DE, CheckerContext &C,
@@ -123,17 +126,10 @@ void DeleteWithNonVirtualDtorChecker::checkTypedDeleteExpr(
123126
if (!DerivedClass->isDerivedFrom(BaseClass))
124127
return;
125128

126-
if (!BT)
127-
BT.reset(new BugType(this,
128-
"Destruction of a polymorphic object with no "
129-
"virtual destructor",
130-
"Logic error"));
131-
132129
ExplodedNode *N = C.generateNonFatalErrorNode();
133130
if (!N)
134131
return;
135-
auto R =
136-
std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N);
132+
auto R = std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N);
137133

138134
// Mark region of problematic base class for later use in the BugVisitor.
139135
R->markInteresting(BaseClassRegion);
@@ -160,12 +156,6 @@ void CXXArrayDeleteChecker::checkTypedDeleteExpr(
160156
if (!DerivedClass->isDerivedFrom(BaseClass))
161157
return;
162158

163-
if (!BT)
164-
BT.reset(new BugType(this,
165-
"Deleting an array of polymorphic objects "
166-
"is undefined",
167-
"Logic error"));
168-
169159
ExplodedNode *N = C.generateNonFatalErrorNode();
170160
if (!N)
171161
return;
@@ -182,7 +172,7 @@ void CXXArrayDeleteChecker::checkTypedDeleteExpr(
182172
<< SourceType.getAsString(C.getASTContext().getPrintingPolicy())
183173
<< "' is undefined";
184174

185-
auto R = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N);
175+
auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N);
186176

187177
// Mark region of problematic base class for later use in the BugVisitor.
188178
R->markInteresting(BaseClassRegion);

‎clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ using namespace ento;
2424

2525
namespace {
2626
class CastSizeChecker : public Checker< check::PreStmt<CastExpr> > {
27-
mutable std::unique_ptr<BugType> BT;
27+
const BugType BT{this, "Cast region with wrong size."};
2828

2929
public:
3030
void checkPreStmt(const CastExpr *CE, CheckerContext &C) const;
@@ -131,12 +131,10 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const {
131131
return;
132132

133133
if (ExplodedNode *errorNode = C.generateErrorNode()) {
134-
if (!BT)
135-
BT.reset(new BugType(this, "Cast region with wrong size."));
136134
constexpr llvm::StringLiteral Msg =
137135
"Cast a region whose size is not a multiple of the destination type "
138136
"size.";
139-
auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, errorNode);
137+
auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, errorNode);
140138
R->addRange(CE->getSourceRange());
141139
C.emitReport(std::move(R));
142140
}

‎clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -99,18 +99,23 @@ class ObjCDeallocChecker
9999
check::PointerEscape,
100100
check::PreStmt<ReturnStmt>> {
101101

102-
mutable IdentifierInfo *NSObjectII, *SenTestCaseII, *XCTestCaseII,
103-
*Block_releaseII, *CIFilterII;
104-
105-
mutable Selector DeallocSel, ReleaseSel;
106-
107-
std::unique_ptr<BugType> MissingReleaseBugType;
108-
std::unique_ptr<BugType> ExtraReleaseBugType;
109-
std::unique_ptr<BugType> MistakenDeallocBugType;
102+
mutable const IdentifierInfo *NSObjectII = nullptr;
103+
mutable const IdentifierInfo *SenTestCaseII = nullptr;
104+
mutable const IdentifierInfo *XCTestCaseII = nullptr;
105+
mutable const IdentifierInfo *Block_releaseII = nullptr;
106+
mutable const IdentifierInfo *CIFilterII = nullptr;
107+
108+
mutable Selector DeallocSel;
109+
mutable Selector ReleaseSel;
110+
111+
const BugType MissingReleaseBugType{this, "Missing ivar release (leak)",
112+
categories::MemoryRefCount};
113+
const BugType ExtraReleaseBugType{this, "Extra ivar release",
114+
categories::MemoryRefCount};
115+
const BugType MistakenDeallocBugType{this, "Mistaken dealloc",
116+
categories::MemoryRefCount};
110117

111118
public:
112-
ObjCDeallocChecker();
113-
114119
void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr,
115120
BugReporter &BR) const;
116121
void checkBeginFunction(CheckerContext &Ctx) const;
@@ -579,7 +584,7 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const {
579584
OS << " by a synthesized property but not released"
580585
" before '[super dealloc]'";
581586

582-
auto BR = std::make_unique<PathSensitiveBugReport>(*MissingReleaseBugType,
587+
auto BR = std::make_unique<PathSensitiveBugReport>(MissingReleaseBugType,
583588
OS.str(), ErrNode);
584589
C.emitReport(std::move(BR));
585590
}
@@ -701,7 +706,7 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue,
701706
OS << " property but was released in 'dealloc'";
702707
}
703708

704-
auto BR = std::make_unique<PathSensitiveBugReport>(*ExtraReleaseBugType,
709+
auto BR = std::make_unique<PathSensitiveBugReport>(ExtraReleaseBugType,
705710
OS.str(), ErrNode);
706711
BR->addRange(M.getOriginExpr()->getSourceRange());
707712

@@ -743,7 +748,7 @@ bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
743748
OS << "'" << *PropImpl->getPropertyIvarDecl()
744749
<< "' should be released rather than deallocated";
745750

746-
auto BR = std::make_unique<PathSensitiveBugReport>(*MistakenDeallocBugType,
751+
auto BR = std::make_unique<PathSensitiveBugReport>(MistakenDeallocBugType,
747752
OS.str(), ErrNode);
748753
BR->addRange(M.getOriginExpr()->getSourceRange());
749754

@@ -752,23 +757,6 @@ bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
752757
return true;
753758
}
754759

755-
ObjCDeallocChecker::ObjCDeallocChecker()
756-
: NSObjectII(nullptr), SenTestCaseII(nullptr), XCTestCaseII(nullptr),
757-
Block_releaseII(nullptr), CIFilterII(nullptr) {
758-
759-
MissingReleaseBugType.reset(
760-
new BugType(this, "Missing ivar release (leak)",
761-
categories::MemoryRefCount));
762-
763-
ExtraReleaseBugType.reset(
764-
new BugType(this, "Extra ivar release",
765-
categories::MemoryRefCount));
766-
767-
MistakenDeallocBugType.reset(
768-
new BugType(this, "Mistaken dealloc",
769-
categories::MemoryRefCount));
770-
}
771-
772760
void ObjCDeallocChecker::initIdentifierInfoAndSelectors(
773761
ASTContext &Ctx) const {
774762
if (NSObjectII)

‎clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ bool isRootChanged(intptr_t k) { return k == ROOT_CHANGED; }
4141
// bug<--foo()-- JAIL_ENTERED<--foo()--
4242
class ChrootChecker : public Checker<eval::Call, check::PreCall> {
4343
// This bug refers to possibly break out of a chroot() jail.
44-
mutable std::unique_ptr<BugType> BT_BreakJail;
44+
const BugType BT_BreakJail{this, "Break out of jail"};
4545

4646
const CallDescription Chroot{{"chroot"}, 1}, Chdir{{"chdir"}, 1};
4747

@@ -124,12 +124,10 @@ void ChrootChecker::checkPreCall(const CallEvent &Call,
124124
if (k)
125125
if (isRootChanged((intptr_t) *k))
126126
if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
127-
if (!BT_BreakJail)
128-
BT_BreakJail.reset(new BugType(this, "Break out of jail"));
129127
constexpr llvm::StringLiteral Msg =
130128
"No call of chdir(\"/\") immediately after chroot";
131129
C.emitReport(
132-
std::make_unique<PathSensitiveBugReport>(*BT_BreakJail, Msg, N));
130+
std::make_unique<PathSensitiveBugReport>(BT_BreakJail, Msg, N));
133131
}
134132
}
135133

‎clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ class CloneChecker
3535

3636
private:
3737
mutable CloneDetector Detector;
38-
mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
38+
const BugType BT_Exact{this, "Exact code clone", "Code clone"};
39+
const BugType BT_Suspicious{this, "Suspicious code clone", "Code clone"};
3940

4041
public:
4142
void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
@@ -107,15 +108,11 @@ static PathDiagnosticLocation makeLocation(const StmtSequence &S,
107108
void CloneChecker::reportClones(
108109
BugReporter &BR, AnalysisManager &Mgr,
109110
std::vector<CloneDetector::CloneGroup> &CloneGroups) const {
110-
111-
if (!BT_Exact)
112-
BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone"));
113-
114111
for (const CloneDetector::CloneGroup &Group : CloneGroups) {
115112
// We group the clones by printing the first as a warning and all others
116113
// as a note.
117114
auto R = std::make_unique<BasicBugReport>(
118-
*BT_Exact, "Duplicate code detected", makeLocation(Group.front(), Mgr));
115+
BT_Exact, "Duplicate code detected", makeLocation(Group.front(), Mgr));
119116
R->addRange(Group.front().getSourceRange());
120117

121118
for (unsigned i = 1; i < Group.size(); ++i)
@@ -154,10 +151,6 @@ void CloneChecker::reportSuspiciousClones(
154151
}
155152
}
156153

157-
if (!BT_Suspicious)
158-
BT_Suspicious.reset(
159-
new BugType(this, "Suspicious code clone", "Code clone"));
160-
161154
ASTContext &ACtx = BR.getContext();
162155
SourceManager &SM = ACtx.getSourceManager();
163156
AnalysisDeclContext *ADC =
@@ -170,7 +163,7 @@ void CloneChecker::reportSuspiciousClones(
170163
// Think how to perform more accurate suggestions?
171164

172165
auto R = std::make_unique<BasicBugReport>(
173-
*BT_Suspicious,
166+
BT_Suspicious,
174167
"Potential copy-paste error; did you really mean to use '" +
175168
Pair.FirstCloneInfo.Variable->getNameAsString() + "' here?",
176169
PathDiagnosticLocation::createBegin(Pair.FirstCloneInfo.Mention, SM,

0 commit comments

Comments
 (0)