[clang] 59878ec - [analyzer] Add path notes to FuchsiaHandleCheck.
Gabor Horvath via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 20 12:41:07 PST 2019
Author: Gabor Horvath
Date: 2019-12-20T12:40:41-08:00
New Revision: 59878ec8092bef656a71d22261fd3b70651e8318
URL: https://github.com/llvm/llvm-project/commit/59878ec8092bef656a71d22261fd3b70651e8318
DIFF: https://github.com/llvm/llvm-project/commit/59878ec8092bef656a71d22261fd3b70651e8318.diff
LOG: [analyzer] Add path notes to FuchsiaHandleCheck.
Differential Revision: https://reviews.llvm.org/D70725
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/test/Analysis/fuchsia_handle.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index e94b544172a0..69593e2b6c93 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -386,7 +386,7 @@ class PathSensitiveBugReport : public BugReport {
/// 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)
diff --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
index 25eb4f5ba82d..861dfef02393 100644
--- a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -199,6 +199,28 @@ class FuchsiaHandleChecker
REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState)
+static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym,
+ CheckerContext &Ctx) {
+ 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->getFirstPred();
+
+ 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()))
+ return N;
+ }
+ Pred = N;
+ N = N->getFirstPred();
+ }
+ return nullptr;
+}
+
/// Returns the symbols extracted from the argument or null if it cannot be
/// found.
SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg, ProgramStateRef State) {
@@ -282,6 +304,7 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
ProgramStateRef State = C.getState();
+ std::vector<std::function<std::string(BugReport & BR)>> Notes;
SymbolRef ResultSymbol = nullptr;
if (const auto *TypeDefTy = FuncDecl->getReturnType()->getAs<TypedefType>())
if (TypeDefTy->getDecl()->getName() == ErrorTypeName)
@@ -310,14 +333,45 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
if (HState && HState->isReleased()) {
reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C);
return;
- } else
+ } else {
+ Notes.push_back([Handle](BugReport &BR) {
+ auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
+ if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
+ return "Handle released here.";
+ } else
+ return "";
+ });
State = State->set<HStateMap>(Handle, HandleState::getReleased());
+ }
} else if (hasFuchsiaAttr<AcquireHandleAttr>(PVD)) {
+ Notes.push_back([Handle](BugReport &BR) {
+ auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
+ if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
+ return "Handle allocated here.";
+ } else
+ return "";
+ });
State = State->set<HStateMap>(
Handle, HandleState::getMaybeAllocated(ResultSymbol));
}
}
- C.addTransition(State);
+ const NoteTag *T = nullptr;
+ if (!Notes.empty()) {
+ T = C.getNoteTag(
+ [this, Notes{std::move(Notes)}](BugReport &BR) -> std::string {
+ if (&BR.getBugType() != &UseAfterReleaseBugType &&
+ &BR.getBugType() != &LeakBugType &&
+ &BR.getBugType() != &DoubleReleaseBugType)
+ return "";
+ for (auto &Note : Notes) {
+ std::string Text = Note(BR);
+ if (!Text.empty())
+ return Text;
+ }
+ return "";
+ });
+ }
+ C.addTransition(State, T);
}
void FuchsiaHandleChecker::checkDeadSymbols(SymbolReaper &SymReaper,
@@ -353,6 +407,7 @@ void FuchsiaHandleChecker::checkDeadSymbols(SymbolReaper &SymReaper,
ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State,
SVal Cond,
bool Assumption) const {
+ // TODO: add notes about successes/fails for APIs.
ConstraintManager &Cmr = State->getConstraintManager();
HStateMapTy TrackedHandles = State->get<HStateMap>();
for (auto &CurItem : TrackedHandles) {
@@ -453,7 +508,22 @@ void FuchsiaHandleChecker::reportBug(SymbolRef Sym, ExplodedNode *ErrorNode,
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);
diff --git a/clang/test/Analysis/fuchsia_handle.cpp b/clang/test/Analysis/fuchsia_handle.cpp
index 62a5b7dd7ad7..0543eb92d01f 100644
--- a/clang/test/Analysis/fuchsia_handle.cpp
+++ b/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;
@@ -116,33 +117,76 @@ void checkNoLeak06() {
void checkLeak01(int tag) {
zx_handle_t sa, sb;
- if (zx_channel_create(0, &sa, &sb))
- return;
+ if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}}
+ return; // expected-note at -1 {{Assuming the condition is false}}
+ // expected-note at -2 {{Taking false branch}}
use1(&sa);
- if (tag)
+ if (tag) // expected-note {{Assuming 'tag' is 0}}
zx_handle_close(sa);
+ // expected-note at -2 {{Taking false branch}}
use2(sb); // expected-warning {{Potential leak of handle}}
+ // expected-note at -1 {{Potential leak of handle}}
+ zx_handle_close(sb);
+}
+
+void checkReportLeakOnOnePath(int tag) {
+ zx_handle_t sa, sb;
+ if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}}
+ return; // expected-note at -1 {{Assuming the condition is false}}
+ // expected-note at -2 {{Taking false branch}}
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 at -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 at -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 at -2 {{Taking true branch}}
zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+ // expected-note at -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 at -1 {{Handle allocated here}}
+ // expected-note at -2 {{Handle allocated here}}
+ // expected-note at +2 {{Taking true branch}}
+ // expected-note at +1 {{Taking false branch}}
if (tag) {
- zx_handle_close(sa);
+ // expected-note at -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 at -1 {{Using a previously released handle}}
}
- zx_handle_close(sb);
+ // expected-note at -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 at -1 {{Using a previously released handle}}
}
void checkMemberOperatorIndices() {
More information about the cfe-commits
mailing list