Skip to content

Commit 707c3f5

Browse files
haoNoQhaoNoQ
committed
[analyzer] NFC: Re-implement stack hints as a side map in BugReport.
That's one of the few random entities in the PathDiagnostic interface that are specific to the Static Analyzer. By moving them out we could let everybody use path diagnostics without linking against Static Analyzer. Differential Revision: https://reviews.llvm.org/D67381 llvm-svn: 371658 (cherry picked from commit 8535b8e)
1 parent decdf63 commit 707c3f5

File tree

10 files changed

+178
-174
lines changed

10 files changed

+178
-174
lines changed

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,50 @@ class SValBuilder;
7070
using DiagnosticForConsumerMapTy =
7171
llvm::DenseMap<PathDiagnosticConsumer *, std::unique_ptr<PathDiagnostic>>;
7272

73+
/// Interface for classes constructing Stack hints.
74+
///
75+
/// If a PathDiagnosticEvent occurs in a different frame than the final
76+
/// diagnostic the hints can be used to summarize the effect of the call.
77+
class StackHintGenerator {
78+
public:
79+
virtual ~StackHintGenerator() = 0;
80+
81+
/// Construct the Diagnostic message for the given ExplodedNode.
82+
virtual std::string getMessage(const ExplodedNode *N) = 0;
83+
};
84+
85+
/// Constructs a Stack hint for the given symbol.
86+
///
87+
/// The class knows how to construct the stack hint message based on
88+
/// traversing the CallExpr associated with the call and checking if the given
89+
/// symbol is returned or is one of the arguments.
90+
/// The hint can be customized by redefining 'getMessageForX()' methods.
91+
class StackHintGeneratorForSymbol : public StackHintGenerator {
92+
private:
93+
SymbolRef Sym;
94+
std::string Msg;
95+
96+
public:
97+
StackHintGeneratorForSymbol(SymbolRef S, StringRef M) : Sym(S), Msg(M) {}
98+
~StackHintGeneratorForSymbol() override = default;
99+
100+
/// Search the call expression for the symbol Sym and dispatch the
101+
/// 'getMessageForX()' methods to construct a specific message.
102+
std::string getMessage(const ExplodedNode *N) override;
103+
104+
/// Produces the message of the following form:
105+
/// 'Msg via Nth parameter'
106+
virtual std::string getMessageForArg(const Expr *ArgE, unsigned ArgIndex);
107+
108+
virtual std::string getMessageForReturn(const CallExpr *CallExpr) {
109+
return Msg;
110+
}
111+
112+
virtual std::string getMessageForSymbolNotFound() {
113+
return Msg;
114+
}
115+
};
116+
73117
/// This class provides an interface through which checkers can create
74118
/// individual bug reports.
75119
class BugReport {
@@ -313,6 +357,14 @@ class PathSensitiveBugReport : public BugReport {
313357

314358
const Stmt *getStmt() const;
315359

360+
/// If an event occurs in a different frame than the final diagnostic,
361+
/// supply a message that will be used to construct an extra hint on the
362+
/// returns from all the calls on the stack from this event to the final
363+
/// diagnostic.
364+
// FIXME: Allow shared_ptr keys in DenseMap?
365+
std::map<PathDiagnosticPieceRef, std::unique_ptr<StackHintGenerator>>
366+
StackHints;
367+
316368
public:
317369
PathSensitiveBugReport(const BugType &bt, StringRef desc,
318370
const ExplodedNode *errorNode)
@@ -455,6 +507,26 @@ class PathSensitiveBugReport : public BugReport {
455507
bool addTrackedCondition(const ExplodedNode *Cond) {
456508
return TrackedConditions.insert(Cond).second;
457509
}
510+
511+
void addCallStackHint(PathDiagnosticPieceRef Piece,
512+
std::unique_ptr<StackHintGenerator> StackHint) {
513+
StackHints[Piece] = std::move(StackHint);
514+
}
515+
516+
bool hasCallStackHint(PathDiagnosticPieceRef Piece) const {
517+
return StackHints.count(Piece) > 0;
518+
}
519+
520+
/// Produce the hint for the given node. The node contains
521+
/// information about the call for which the diagnostic can be generated.
522+
std::string
523+
getCallStackMessage(PathDiagnosticPieceRef Piece,
524+
const ExplodedNode *N) const {
525+
auto I = StackHints.find(Piece);
526+
if (I != StackHints.end())
527+
return I->second->getMessage(N);
528+
return "";
529+
}
458530
};
459531

460532
//===----------------------------------------------------------------------===//

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

Lines changed: 2 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -502,65 +502,13 @@ class PathDiagnosticSpotPiece : public PathDiagnosticPiece {
502502
}
503503
};
504504

505-
/// Interface for classes constructing Stack hints.
506-
///
507-
/// If a PathDiagnosticEvent occurs in a different frame than the final
508-
/// diagnostic the hints can be used to summarize the effect of the call.
509-
class StackHintGenerator {
510-
public:
511-
virtual ~StackHintGenerator() = 0;
512-
513-
/// Construct the Diagnostic message for the given ExplodedNode.
514-
virtual std::string getMessage(const ExplodedNode *N) = 0;
515-
};
516-
517-
/// Constructs a Stack hint for the given symbol.
518-
///
519-
/// The class knows how to construct the stack hint message based on
520-
/// traversing the CallExpr associated with the call and checking if the given
521-
/// symbol is returned or is one of the arguments.
522-
/// The hint can be customized by redefining 'getMessageForX()' methods.
523-
class StackHintGeneratorForSymbol : public StackHintGenerator {
524-
private:
525-
SymbolRef Sym;
526-
std::string Msg;
527-
528-
public:
529-
StackHintGeneratorForSymbol(SymbolRef S, StringRef M) : Sym(S), Msg(M) {}
530-
~StackHintGeneratorForSymbol() override = default;
531-
532-
/// Search the call expression for the symbol Sym and dispatch the
533-
/// 'getMessageForX()' methods to construct a specific message.
534-
std::string getMessage(const ExplodedNode *N) override;
535-
536-
/// Produces the message of the following form:
537-
/// 'Msg via Nth parameter'
538-
virtual std::string getMessageForArg(const Expr *ArgE, unsigned ArgIndex);
539-
540-
virtual std::string getMessageForReturn(const CallExpr *CallExpr) {
541-
return Msg;
542-
}
543-
544-
virtual std::string getMessageForSymbolNotFound() {
545-
return Msg;
546-
}
547-
};
548-
549505
class PathDiagnosticEventPiece : public PathDiagnosticSpotPiece {
550506
Optional<bool> IsPrunable;
551507

552-
/// If the event occurs in a different frame than the final diagnostic,
553-
/// supply a message that will be used to construct an extra hint on the
554-
/// returns from all the calls on the stack from this event to the final
555-
/// diagnostic.
556-
std::unique_ptr<StackHintGenerator> CallStackHint;
557-
558508
public:
559509
PathDiagnosticEventPiece(const PathDiagnosticLocation &pos,
560-
StringRef s, bool addPosRange = true,
561-
StackHintGenerator *stackHint = nullptr)
562-
: PathDiagnosticSpotPiece(pos, s, Event, addPosRange),
563-
CallStackHint(stackHint) {}
510+
StringRef s, bool addPosRange = true)
511+
: PathDiagnosticSpotPiece(pos, s, Event, addPosRange) {}
564512
~PathDiagnosticEventPiece() override;
565513

566514
/// Mark the diagnostic piece as being potentially prunable. This
@@ -577,16 +525,6 @@ class PathDiagnosticEventPiece : public PathDiagnosticSpotPiece {
577525
return IsPrunable.hasValue() ? IsPrunable.getValue() : false;
578526
}
579527

580-
bool hasCallStackHint() { return (bool)CallStackHint; }
581-
582-
/// Produce the hint for the given node. The node contains
583-
/// information about the call for which the diagnostic can be generated.
584-
std::string getCallStackMessage(const ExplodedNode *N) {
585-
if (CallStackHint)
586-
return CallStackHint->getMessage(N);
587-
return {};
588-
}
589-
590528
void dump() const override;
591529

592530
static bool classof(const PathDiagnosticPiece *P) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,7 @@ DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode(
140140
OS << "Conversion from derived to base happened here";
141141
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
142142
N->getLocationContext());
143-
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true,
144-
nullptr);
143+
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true);
145144
}
146145

147146
void ento::registerDeleteWithNonVirtualDtorChecker(CheckerManager &mgr) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,7 @@ PathDiagnosticPieceRef DynamicTypeChecker::DynamicTypeBugVisitor::VisitNode(
139139
// Generate the extra diagnostic.
140140
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
141141
N->getLocationContext());
142-
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true,
143-
nullptr);
142+
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true);
144143
}
145144

146145
static bool hasDefinition(const ObjCObjectPointerType *ObjPtr) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -972,8 +972,7 @@ PathDiagnosticPieceRef DynamicTypePropagation::GenericsBugVisitor::VisitNode(
972972
// Generate the extra diagnostic.
973973
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
974974
N->getLocationContext());
975-
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true,
976-
nullptr);
975+
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true);
977976
}
978977

979978
/// Register checkers.

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,7 @@ PathDiagnosticPieceRef InnerPointerChecker::InnerPointerBRVisitor::VisitNode(
299299
<< "' obtained here";
300300
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
301301
N->getLocationContext());
302-
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true,
303-
nullptr);
302+
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true);
304303
}
305304

306305
void ento::registerInnerPointerChecker(CheckerManager &Mgr) {

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2953,15 +2953,15 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
29532953

29542954
// Find out if this is an interesting point and what is the kind.
29552955
StringRef Msg;
2956-
StackHintGeneratorForSymbol *StackHint = nullptr;
2956+
std::unique_ptr<StackHintGeneratorForSymbol> StackHint = nullptr;
29572957
SmallString<256> Buf;
29582958
llvm::raw_svector_ostream OS(Buf);
29592959

29602960
if (Mode == Normal) {
29612961
if (isAllocated(RS, RSPrev, S)) {
29622962
Msg = "Memory is allocated";
2963-
StackHint = new StackHintGeneratorForSymbol(Sym,
2964-
"Returned allocated memory");
2963+
StackHint = std::make_unique<StackHintGeneratorForSymbol>(
2964+
Sym, "Returned allocated memory");
29652965
} else if (isReleased(RS, RSPrev, S)) {
29662966
const auto Family = RS->getAllocationFamily();
29672967
switch (Family) {
@@ -2971,8 +2971,8 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
29712971
case AF_CXXNewArray:
29722972
case AF_IfNameIndex:
29732973
Msg = "Memory is released";
2974-
StackHint = new StackHintGeneratorForSymbol(Sym,
2975-
"Returning; memory was released");
2974+
StackHint = std::make_unique<StackHintGeneratorForSymbol>(
2975+
Sym, "Returning; memory was released");
29762976
break;
29772977
case AF_InnerBuffer: {
29782978
const MemRegion *ObjRegion =
@@ -2983,8 +2983,8 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
29832983

29842984
if (N->getLocation().getKind() == ProgramPoint::PostImplicitCallKind) {
29852985
OS << "deallocated by call to destructor";
2986-
StackHint = new StackHintGeneratorForSymbol(Sym,
2987-
"Returning; inner buffer was deallocated");
2986+
StackHint = std::make_unique<StackHintGeneratorForSymbol>(
2987+
Sym, "Returning; inner buffer was deallocated");
29882988
} else {
29892989
OS << "reallocated by call to '";
29902990
const Stmt *S = RS->getStmt();
@@ -2999,8 +2999,8 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
29992999
OS << (D ? D->getNameAsString() : "unknown");
30003000
}
30013001
OS << "'";
3002-
StackHint = new StackHintGeneratorForSymbol(Sym,
3003-
"Returning; inner buffer was reallocated");
3002+
StackHint = std::make_unique<StackHintGeneratorForSymbol>(
3003+
Sym, "Returning; inner buffer was reallocated");
30043004
}
30053005
Msg = OS.str();
30063006
break;
@@ -3040,12 +3040,12 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
30403040
}
30413041
} else if (isRelinquished(RS, RSPrev, S)) {
30423042
Msg = "Memory ownership is transferred";
3043-
StackHint = new StackHintGeneratorForSymbol(Sym, "");
3043+
StackHint = std::make_unique<StackHintGeneratorForSymbol>(Sym, "");
30443044
} else if (isReallocFailedCheck(RS, RSPrev, S)) {
30453045
Mode = ReallocationFailed;
30463046
Msg = "Reallocation failed";
3047-
StackHint = new StackHintGeneratorForReallocationFailed(Sym,
3048-
"Reallocation failed");
3047+
StackHint = std::make_unique<StackHintGeneratorForReallocationFailed>(
3048+
Sym, "Reallocation failed");
30493049

30503050
if (SymbolRef sym = findFailedReallocSymbol(state, statePrev)) {
30513051
// Is it possible to fail two reallocs WITHOUT testing in between?
@@ -3064,16 +3064,15 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
30643064
if (!statePrev->get<RegionState>(FailedReallocSymbol)) {
30653065
// We're at the reallocation point.
30663066
Msg = "Attempt to reallocate memory";
3067-
StackHint = new StackHintGeneratorForSymbol(Sym,
3068-
"Returned reallocated memory");
3067+
StackHint = std::make_unique<StackHintGeneratorForSymbol>(
3068+
Sym, "Returned reallocated memory");
30693069
FailedReallocSymbol = nullptr;
30703070
Mode = Normal;
30713071
}
30723072
}
30733073

30743074
if (Msg.empty()) {
3075-
// Silence a memory leak warning by MallocChecker in MallocChecker.cpp :)
3076-
assert(!StackHint && "Memory leak!");
3075+
assert(!StackHint);
30773076
return nullptr;
30783077
}
30793078

@@ -3093,7 +3092,9 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N,
30933092
N->getLocationContext());
30943093
}
30953094

3096-
return std::make_shared<PathDiagnosticEventPiece>(Pos, Msg, true, StackHint);
3095+
auto P = std::make_shared<PathDiagnosticEventPiece>(Pos, Msg, true);
3096+
BR.addCallStackHint(P, std::move(StackHint));
3097+
return P;
30973098
}
30983099

30993100
void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State,

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,7 @@ PathDiagnosticPieceRef NullabilityChecker::NullabilityBugVisitor::VisitNode(
323323
// Generate the extra diagnostic.
324324
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
325325
N->getLocationContext());
326-
return std::make_shared<PathDiagnosticEventPiece>(Pos, InfoText, true,
327-
nullptr);
326+
return std::make_shared<PathDiagnosticEventPiece>(Pos, InfoText, true);
328327
}
329328

330329
/// Returns true when the value stored at the given location has been

0 commit comments

Comments
 (0)