xazax.hun created this revision.
xazax.hun added reviewers: NoQ, haowei.
xazax.hun added a project: clang.
Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
xazax.hun added a reviewer: Szelethus.
xazax.hun added a parent revision: D70470: [analyzer] Add FuchsiaHandleCheck to
catch handle leaks, use after frees and double frees.
I also added a missing const to a BugReporter API.
There are some TODOs for potential future updates if we want richer notes
during reporting.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D70725
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/test/Analysis/fuchsia_handle.cpp
Index: clang/test/Analysis/fuchsia_handle.cpp
===================================================================
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -analyzer-output=text \
+// RUN: -verify %s
typedef __typeof__(sizeof(int)) size_t;
typedef int zx_status_t;
@@ -102,31 +103,73 @@
void checkLeak01(int tag) {
zx_handle_t sa, sb;
zx_channel_create(0, &sa, &sb);
+ // expected-note@-1 {{Handle allocated here}}
use1(&sa);
- if (tag)
+ if (tag) // expected-note {{Assuming 'tag' is 0}}
zx_handle_close(sa);
+ // expected-note@-2 {{Taking false branch}}
use2(sb); // expected-warning {{Potential leak of handle}}
+ // expected-note@-1 {{Potential leak of handle}}
zx_handle_close(sb);
}
+void checkReportLeakOnOnePath(int tag) {
+ zx_handle_t sa, sb;
+ zx_channel_create(0, &sa, &sb);
+ // expected-note@-1 {{Handle allocated here}}
+ zx_handle_close(sb);
+ switch(tag) { // expected-note {{Control jumps to the 'default' case at line}}
+ case 0:
+ use2(sa);
+ return;
+ case 1:
+ use2(sa);
+ return;
+ case 2:
+ use2(sa);
+ return;
+ case 3:
+ use2(sa);
+ return;
+ case 4:
+ use2(sa);
+ return;
+ default:
+ use2(sa);
+ return; // expected-warning {{Potential leak of handle}}
+ // expected-note@-1 {{Potential leak of handle}}
+ }
+}
+
void checkDoubleRelease01(int tag) {
zx_handle_t sa, sb;
zx_channel_create(0, &sa, &sb);
- if (tag)
- zx_handle_close(sa);
+ // expected-note@-1 {{Handle allocated here}}
+ if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
+ zx_handle_close(sa); // expected-note {{Handle released here}}
+ // expected-note@-2 {{Taking true branch}}
zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+ // expected-note@-1 {{Releasing a previously released handle}}
zx_handle_close(sb);
}
void checkUseAfterFree01(int tag) {
zx_handle_t sa, sb;
zx_channel_create(0, &sa, &sb);
+ // expected-note@-1 {{Handle allocated here}}
+ // expected-note@-2 {{Handle allocated here}}
+ // expected-note@+2 {{Taking true branch}}
+ // expected-note@+1 {{Taking false branch}}
if (tag) {
- zx_handle_close(sa);
+ // expected-note@-1 {{Assuming 'tag' is not equal to 0}}
+ zx_handle_close(sa); // expected-note {{Handle released here}}
use1(&sa); // expected-warning {{Using a previously released handle}}
+ // expected-note@-1 {{Using a previously released handle}}
}
- zx_handle_close(sb);
+ // expected-note@-6 {{Assuming 'tag' is 0}}
+ zx_handle_close(sb); // expected-note {{Handle released here}}
use2(sb); // expected-warning {{Using a previously released handle}}
+ // expected-note@-1 {{Using a previously released handle}}
}
void checkMemberOperatorIndices() {
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -163,6 +163,28 @@
LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
};
+class HandleBugVisitor : public BugReporterVisitor {
+private:
+ SymbolRef HandleSym;
+
+ static void *getTag() {
+ static int Tag = 0;
+ return &Tag;
+ }
+
+public:
+ HandleBugVisitor(SymbolRef HandleSym) : HandleSym(HandleSym) {}
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.AddPointer(HandleSym);
+ ID.AddPointer(getTag());
+ }
+
+ PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC,
+ PathSensitiveBugReport &BR) override;
+};
+
class FuchsiaHandleChecker
: public Checker<check::PostCall, check::PreCall, check::DeadSymbols,
check::PointerEscape, eval::Assume> {
@@ -204,6 +226,71 @@
REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState)
+PathDiagnosticPieceRef HandleBugVisitor::VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC,
+ PathSensitiveBugReport &BR) {
+ ProgramStateRef State = N->getState();
+ ProgramStateRef PrevState = N->getFirstPred()->getState();
+
+ const Stmt *S = N->getStmtForDiagnostics();
+ if (!S)
+ return nullptr;
+
+ const HandleState *HState = State->get<HStateMap>(HandleSym);
+ const HandleState *PrevHState = PrevState->get<HStateMap>(HandleSym);
+ if (HState == PrevHState)
+ return nullptr;
+
+ SmallString<200> Buf;
+ llvm::raw_svector_ostream OS(Buf);
+ if (!PrevHState && (HState->isAllocated() || HState->maybeAllocated())) {
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+ N->getLocationContext());
+ OS << "Handle allocated here.";
+ return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str());
+ }
+ if ((!PrevHState || PrevHState->isAllocated() ||
+ PrevHState->maybeAllocated()) &&
+ HState && (HState->isReleased() || HState->maybeReleased())) {
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+ N->getLocationContext());
+ OS << "Handle released here.";
+ return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str());
+ }
+
+ // TODO: do we want to emit notes for escapes?
+ // do we want to emit notes for for error checks? I.e. on which branch
+ // do we consider the acquire/release success or fail.
+
+ return nullptr;
+}
+
+static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym,
+ CheckerContext &Ctx) {
+ const ExplodedNode *AcquireNode = N;
+
+ ProgramStateRef State = N->getState();
+ // When bug type is handle leak, exploded node N does not have state info for
+ // leaking handle. Get the predecessor of N instead.
+ if (!State->get<HStateMap>(Sym)) {
+ N = N->pred_empty() ? nullptr : *(N->pred_begin());
+ }
+
+ const ExplodedNode *Pred = N;
+ while (N) {
+ State = N->getState();
+ if (!State->get<HStateMap>(Sym)) {
+ const HandleState *HState = Pred->getState()->get<HStateMap>(Sym);
+ if (HState && (HState->isAllocated() || HState->maybeAllocated()))
+ AcquireNode = N;
+ break;
+ }
+ Pred = N;
+ N = N->pred_empty() ? nullptr : *(N->pred_begin());
+ }
+ return AcquireNode;
+}
+
/// Returns the symbols extracted from the argument or null if it cannot be
/// found.
SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg, ProgramStateRef State) {
@@ -429,10 +516,26 @@
if (!ErrorNode)
return;
- auto R = std::make_unique<PathSensitiveBugReport>(Type, Msg, ErrorNode);
+ std::unique_ptr<PathSensitiveBugReport> R;
+ if (Type.isSuppressOnSink()) {
+ const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
+ if (AcquireNode) {
+ PathDiagnosticLocation LocUsedForUniqueing =
+ PathDiagnosticLocation::createBegin(
+ AcquireNode->getStmtForDiagnostics(), C.getSourceManager(),
+ AcquireNode->getLocationContext());
+
+ R = std::make_unique<PathSensitiveBugReport>(
+ Type, Msg, ErrorNode, LocUsedForUniqueing,
+ AcquireNode->getLocationContext()->getDecl());
+ }
+ }
+ if (!R)
+ R = std::make_unique<PathSensitiveBugReport>(Type, Msg, ErrorNode);
if (Range)
R->addRange(*Range);
R->markInteresting(Sym);
+ R->addVisitor(std::make_unique<HandleBugVisitor>(Sym));
C.emitReport(std::move(R));
}
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -386,7 +386,7 @@
/// to the user. This method allows to rest the location which should be used
/// for uniquing reports. For example, memory leaks checker, could set this to
/// the allocation site, rather then the location where the bug is reported.
- PathSensitiveBugReport(BugType &bt, StringRef desc,
+ PathSensitiveBugReport(const BugType &bt, StringRef desc,
const ExplodedNode *errorNode,
PathDiagnosticLocation LocationToUnique,
const Decl *DeclToUnique)
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
