Skip to content

Commit 06d100a

Browse files
vrnithinkumarvrnithinkumar
committed
[Analyzer] Support note tags for smart ptr checker
Summary: Added support for note tags for null smart_ptr reporting Reviewers: NoQ, Szelethus, vsavchenko, xazax.hun Reviewed By: NoQ, vsavchenko, xazax.hun Subscribers: martong, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D84600
1 parent cfdc967 commit 06d100a

File tree

6 files changed

+298
-67
lines changed

6 files changed

+298
-67
lines changed

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,26 @@ class CheckerContext {
301301
IsPrunable);
302302
}
303303

304+
/// A shorthand version of getNoteTag that accepts a lambda with stream for
305+
/// note.
306+
///
307+
/// @param Cb Callback with 'BugReport &' and 'llvm::raw_ostream &'.
308+
/// @param IsPrunable Whether the note is prunable. It allows BugReporter
309+
/// to omit the note from the report if it would make the displayed
310+
/// bug path significantly shorter.
311+
const NoteTag *getNoteTag(
312+
std::function<void(PathSensitiveBugReport &BR, llvm::raw_ostream &OS)> &&Cb,
313+
bool IsPrunable = false) {
314+
return getNoteTag(
315+
[Cb](PathSensitiveBugReport &BR) -> std::string {
316+
llvm::SmallString<128> Str;
317+
llvm::raw_svector_ostream OS(Str);
318+
Cb(BR, OS);
319+
return std::string(OS.str());
320+
},
321+
IsPrunable);
322+
}
323+
304324
/// Returns the word that should be used to refer to the declaration
305325
/// in the report.
306326
StringRef getDeclDescription(const Decl *D);

‎clang/lib/StaticAnalyzer/Checkers/SmartPtr.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ bool isStdSmartPtrCall(const CallEvent &Call);
2626
/// Returns whether the smart pointer is null or not.
2727
bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion);
2828

29+
const BugType *getNullDereferenceBugType();
30+
2931
} // namespace smartptr
3032
} // namespace ento
3133
} // namespace clang

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

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,40 @@
2323
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
2424
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
2525
#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
26+
#include "llvm/ADT/StringRef.h"
2627

2728
using namespace clang;
2829
using namespace ento;
2930

3031
namespace {
31-
class SmartPtrChecker : public Checker<check::PreCall> {
32-
BugType NullDereferenceBugType{this, "Null SmartPtr dereference",
33-
"C++ Smart Pointer"};
3432

33+
static const BugType *NullDereferenceBugTypePtr;
34+
35+
class SmartPtrChecker : public Checker<check::PreCall> {
3536
public:
3637
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
38+
BugType NullDereferenceBugType{this, "Null SmartPtr dereference",
39+
"C++ Smart Pointer"};
3740

3841
private:
39-
void reportBug(CheckerContext &C, const CallEvent &Call) const;
42+
void reportBug(CheckerContext &C, const MemRegion *DerefRegion,
43+
const CallEvent &Call) const;
44+
void explainDereference(llvm::raw_ostream &OS, const MemRegion *DerefRegion,
45+
const CallEvent &Call) const;
4046
};
4147
} // end of anonymous namespace
4248

49+
// Define the inter-checker API.
50+
namespace clang {
51+
namespace ento {
52+
namespace smartptr {
53+
54+
const BugType *getNullDereferenceBugType() { return NullDereferenceBugTypePtr; }
55+
56+
} // namespace smartptr
57+
} // namespace ento
58+
} // namespace clang
59+
4360
void SmartPtrChecker::checkPreCall(const CallEvent &Call,
4461
CheckerContext &C) const {
4562
if (!smartptr::isStdSmartPtrCall(Call))
@@ -55,23 +72,34 @@ void SmartPtrChecker::checkPreCall(const CallEvent &Call,
5572
OverloadedOperatorKind OOK = OC->getOverloadedOperator();
5673
if (OOK == OO_Star || OOK == OO_Arrow) {
5774
if (smartptr::isNullSmartPtr(State, ThisRegion))
58-
reportBug(C, Call);
75+
reportBug(C, ThisRegion, Call);
5976
}
6077
}
6178

62-
void SmartPtrChecker::reportBug(CheckerContext &C,
79+
void SmartPtrChecker::reportBug(CheckerContext &C, const MemRegion *DerefRegion,
6380
const CallEvent &Call) const {
6481
ExplodedNode *ErrNode = C.generateErrorNode();
6582
if (!ErrNode)
6683
return;
67-
68-
auto R = std::make_unique<PathSensitiveBugReport>(
69-
NullDereferenceBugType, "Dereference of null smart pointer", ErrNode);
84+
llvm::SmallString<128> Str;
85+
llvm::raw_svector_ostream OS(Str);
86+
explainDereference(OS, DerefRegion, Call);
87+
auto R = std::make_unique<PathSensitiveBugReport>(NullDereferenceBugType,
88+
OS.str(), ErrNode);
89+
R->markInteresting(DerefRegion);
7090
C.emitReport(std::move(R));
7191
}
7292

93+
void SmartPtrChecker::explainDereference(llvm::raw_ostream &OS,
94+
const MemRegion *DerefRegion,
95+
const CallEvent &Call) const {
96+
OS << "Dereference of null smart pointer ";
97+
DerefRegion->printPretty(OS);
98+
}
99+
73100
void ento::registerSmartPtrChecker(CheckerManager &Mgr) {
74-
Mgr.registerChecker<SmartPtrChecker>();
101+
SmartPtrChecker *Checker = Mgr.registerChecker<SmartPtrChecker>();
102+
NullDereferenceBugTypePtr = &Checker->NullDereferenceBugType;
75103
}
76104

77105
bool ento::shouldRegisterSmartPtrChecker(const CheckerManager &mgr) {

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

Lines changed: 92 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,18 @@
1717
#include "clang/AST/DeclCXX.h"
1818
#include "clang/AST/ExprCXX.h"
1919
#include "clang/AST/Type.h"
20+
#include "clang/Basic/LLVM.h"
2021
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
2122
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
2223
#include "clang/StaticAnalyzer/Core/Checker.h"
2324
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
2425
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
2526
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
27+
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
2628
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
2729
#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
30+
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
31+
#include <string>
2832

2933
using namespace clang;
3034
using namespace ento;
@@ -49,8 +53,6 @@ class SmartPtrModeling
4953
const LocationContext *LCtx, const CallEvent *Call) const;
5054

5155
private:
52-
ProgramStateRef updateTrackedRegion(const CallEvent &Call, CheckerContext &C,
53-
const MemRegion *ThisValRegion) const;
5456
void handleReset(const CallEvent &Call, CheckerContext &C) const;
5557
void handleRelease(const CallEvent &Call, CheckerContext &C) const;
5658
void handleSwap(const CallEvent &Call, CheckerContext &C) const;
@@ -131,11 +133,11 @@ bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
131133
bool SmartPtrModeling::evalCall(const CallEvent &Call,
132134
CheckerContext &C) const {
133135

136+
ProgramStateRef State = C.getState();
134137
if (!smartptr::isStdSmartPtrCall(Call))
135138
return false;
136139

137140
if (isNullAfterMoveMethod(Call)) {
138-
ProgramStateRef State = C.getState();
139141
const MemRegion *ThisR =
140142
cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
141143

@@ -159,12 +161,46 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
159161
if (CC->getDecl()->isCopyOrMoveConstructor())
160162
return false;
161163

162-
const MemRegion *ThisValRegion = CC->getCXXThisVal().getAsRegion();
163-
if (!ThisValRegion)
164+
const MemRegion *ThisRegion = CC->getCXXThisVal().getAsRegion();
165+
if (!ThisRegion)
164166
return false;
165167

166-
auto State = updateTrackedRegion(Call, C, ThisValRegion);
167-
C.addTransition(State);
168+
if (Call.getNumArgs() == 0) {
169+
auto NullVal = C.getSValBuilder().makeNull();
170+
State = State->set<TrackedRegionMap>(ThisRegion, NullVal);
171+
172+
C.addTransition(
173+
State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
174+
llvm::raw_ostream &OS) {
175+
if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
176+
!BR.isInteresting(ThisRegion))
177+
return;
178+
OS << "Default constructed smart pointer ";
179+
ThisRegion->printPretty(OS);
180+
OS << " is null";
181+
}));
182+
} else {
183+
const auto *TrackingExpr = Call.getArgExpr(0);
184+
assert(TrackingExpr->getType()->isPointerType() &&
185+
"Adding a non pointer value to TrackedRegionMap");
186+
auto ArgVal = Call.getArgSVal(0);
187+
State = State->set<TrackedRegionMap>(ThisRegion, ArgVal);
188+
189+
C.addTransition(State, C.getNoteTag([ThisRegion, TrackingExpr,
190+
ArgVal](PathSensitiveBugReport &BR,
191+
llvm::raw_ostream &OS) {
192+
if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
193+
!BR.isInteresting(ThisRegion))
194+
return;
195+
bugreporter::trackExpressionValue(BR.getErrorNode(), TrackingExpr, BR);
196+
OS << "Smart pointer ";
197+
ThisRegion->printPretty(OS);
198+
if (ArgVal.isZeroConstant())
199+
OS << " is constructed using a null value";
200+
else
201+
OS << " is constructed";
202+
}));
203+
}
168204
return true;
169205
}
170206

@@ -207,37 +243,65 @@ ProgramStateRef SmartPtrModeling::checkRegionChanges(
207243

208244
void SmartPtrModeling::handleReset(const CallEvent &Call,
209245
CheckerContext &C) const {
246+
ProgramStateRef State = C.getState();
210247
const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
211248
if (!IC)
212249
return;
213250

214-
const MemRegion *ThisValRegion = IC->getCXXThisVal().getAsRegion();
215-
if (!ThisValRegion)
251+
const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
252+
if (!ThisRegion)
216253
return;
217-
auto State = updateTrackedRegion(Call, C, ThisValRegion);
218-
C.addTransition(State);
254+
255+
assert(Call.getArgExpr(0)->getType()->isPointerType() &&
256+
"Adding a non pointer value to TrackedRegionMap");
257+
State = State->set<TrackedRegionMap>(ThisRegion, Call.getArgSVal(0));
258+
const auto *TrackingExpr = Call.getArgExpr(0);
259+
C.addTransition(
260+
State, C.getNoteTag([ThisRegion, TrackingExpr](PathSensitiveBugReport &BR,
261+
llvm::raw_ostream &OS) {
262+
if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
263+
!BR.isInteresting(ThisRegion))
264+
return;
265+
bugreporter::trackExpressionValue(BR.getErrorNode(), TrackingExpr, BR);
266+
OS << "Smart pointer ";
267+
ThisRegion->printPretty(OS);
268+
OS << " reset using a null value";
269+
}));
219270
// TODO: Make sure to ivalidate the region in the Store if we don't have
220271
// time to model all methods.
221272
}
222273

223274
void SmartPtrModeling::handleRelease(const CallEvent &Call,
224275
CheckerContext &C) const {
276+
ProgramStateRef State = C.getState();
225277
const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
226278
if (!IC)
227279
return;
228280

229-
const MemRegion *ThisValRegion = IC->getCXXThisVal().getAsRegion();
230-
if (!ThisValRegion)
281+
const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
282+
if (!ThisRegion)
231283
return;
232284

233-
auto State = updateTrackedRegion(Call, C, ThisValRegion);
285+
const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
234286

235-
const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisValRegion);
236287
if (InnerPointVal) {
237288
State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
238289
*InnerPointVal);
239290
}
240-
C.addTransition(State);
291+
292+
auto ValueToUpdate = C.getSValBuilder().makeNull();
293+
State = State->set<TrackedRegionMap>(ThisRegion, ValueToUpdate);
294+
295+
C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
296+
llvm::raw_ostream &OS) {
297+
if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
298+
!BR.isInteresting(ThisRegion))
299+
return;
300+
301+
OS << "Smart pointer ";
302+
ThisRegion->printPretty(OS);
303+
OS << " is released and set to null";
304+
}));
241305
// TODO: Add support to enable MallocChecker to start tracking the raw
242306
// pointer.
243307
}
@@ -267,27 +331,18 @@ void SmartPtrModeling::handleSwap(const CallEvent &Call,
267331
State = updateSwappedRegion(State, ThisRegion, ArgRegionInnerPointerVal);
268332
State = updateSwappedRegion(State, ArgRegion, ThisRegionInnerPointerVal);
269333

270-
C.addTransition(State);
271-
}
272-
273-
ProgramStateRef
274-
SmartPtrModeling::updateTrackedRegion(const CallEvent &Call, CheckerContext &C,
275-
const MemRegion *ThisValRegion) const {
276-
// TODO: Refactor and clean up handling too many things.
277-
ProgramStateRef State = C.getState();
278-
auto NumArgs = Call.getNumArgs();
279-
280-
if (NumArgs == 0) {
281-
auto NullSVal = C.getSValBuilder().makeNull();
282-
State = State->set<TrackedRegionMap>(ThisValRegion, NullSVal);
283-
} else if (NumArgs == 1) {
284-
auto ArgVal = Call.getArgSVal(0);
285-
assert(Call.getArgExpr(0)->getType()->isPointerType() &&
286-
"Adding a non pointer value to TrackedRegionMap");
287-
State = State->set<TrackedRegionMap>(ThisValRegion, ArgVal);
288-
}
289-
290-
return State;
334+
C.addTransition(
335+
State, C.getNoteTag([ThisRegion, ArgRegion](PathSensitiveBugReport &BR,
336+
llvm::raw_ostream &OS) {
337+
if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
338+
!BR.isInteresting(ThisRegion))
339+
return;
340+
BR.markInteresting(ArgRegion);
341+
OS << "Swapped null smart pointer ";
342+
ArgRegion->printPretty(OS);
343+
OS << " with smart pointer ";
344+
ThisRegion->printPretty(OS);
345+
}));
291346
}
292347

293348
void ento::registerSmartPtrModeling(CheckerManager &Mgr) {

0 commit comments

Comments
 (0)