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.
In case the handle symbol dies too early, even before we check the status, we
might generate spurious leak warnings.
This pattern is not very common in production code but it is very common in
unittests where we do know that certain syscall will fail.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73151
Files:
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
@@ -63,6 +63,14 @@
zx_handle_close(sa);
}
+void handleDieBeforeErrorSymbol() {
+ zx_handle_t sa, sb;
+ zx_status_t status = zx_channel_create(0, &sa, &sb);
+ if (status < 0)
+ return;
+ __builtin_trap();
+}
+
void checkNoCrash01() {
zx_handle_t sa, sb;
zx_channel_create(0, &sa, &sb);
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -130,6 +130,9 @@
static HandleState getEscaped() {
return HandleState(Kind::Escaped, nullptr);
}
+ static HandleState getWithoutError(HandleState S) {
+ return HandleState(S.K, nullptr);
+ }
SymbolRef getErrorSym() const { return ErrorSym; }
@@ -149,6 +152,10 @@
CASE(Kind::Released)
CASE(Kind::Escaped)
}
+ if (ErrorSym) {
+ OS << " ErrorSym: ";
+ ErrorSym->dumpToStream(OS);
+ }
}
LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
@@ -160,7 +167,7 @@
class FuchsiaHandleChecker
: public Checker<check::PostCall, check::PreCall, check::DeadSymbols,
- check::PointerEscape, eval::Assume> {
+ check::LiveSymbols, check::PointerEscape, eval::Assume> {
BugType LeakBugType{this, "Fuchsia handle leak", "Fuchsia Handle Error",
/*SuppressOnSink=*/true};
BugType DoubleReleaseBugType{this, "Fuchsia handle double release",
@@ -172,6 +179,7 @@
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+ void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
bool Assumption) const;
ProgramStateRef checkPointerEscape(ProgramStateRef State,
@@ -381,6 +389,10 @@
SmallVector<SymbolRef, 2> LeakedSyms;
HStateMapTy TrackedHandles = State->get<HStateMap>();
for (auto &CurItem : TrackedHandles) {
+ SymbolRef ErrorSym = CurItem.second.getErrorSym();
+ if (ErrorSym && SymReaper.isDead(ErrorSym))
+ State = State->set<HStateMap>(
+ CurItem.first, HandleState::getWithoutError(CurItem.second));
if (!SymReaper.isDead(CurItem.first))
continue;
if (CurItem.second.isAllocated() || CurItem.second.maybeAllocated())
@@ -395,6 +407,16 @@
C.addTransition(State, N);
}
+void FuchsiaHandleChecker::checkLiveSymbols(ProgramStateRef State,
+ SymbolReaper &SymReaper) const {
+ HStateMapTy TrackedHandles = State->get<HStateMap>();
+ for (auto &CurItem : TrackedHandles) {
+ SymbolRef ErrorSym = CurItem.second.getErrorSym();
+ if (ErrorSym)
+ SymReaper.markLive(CurItem.first);
+ }
+}
+
// Acquiring a handle is not always successful. In Fuchsia most functions
// return a status code that determines the status of the handle.
// When we split the path based on this status code we know that on one
Index: clang/test/Analysis/fuchsia_handle.cpp
===================================================================
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -63,6 +63,14 @@
zx_handle_close(sa);
}
+void handleDieBeforeErrorSymbol() {
+ zx_handle_t sa, sb;
+ zx_status_t status = zx_channel_create(0, &sa, &sb);
+ if (status < 0)
+ return;
+ __builtin_trap();
+}
+
void checkNoCrash01() {
zx_handle_t sa, sb;
zx_channel_create(0, &sa, &sb);
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -130,6 +130,9 @@
static HandleState getEscaped() {
return HandleState(Kind::Escaped, nullptr);
}
+ static HandleState getWithoutError(HandleState S) {
+ return HandleState(S.K, nullptr);
+ }
SymbolRef getErrorSym() const { return ErrorSym; }
@@ -149,6 +152,10 @@
CASE(Kind::Released)
CASE(Kind::Escaped)
}
+ if (ErrorSym) {
+ OS << " ErrorSym: ";
+ ErrorSym->dumpToStream(OS);
+ }
}
LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
@@ -160,7 +167,7 @@
class FuchsiaHandleChecker
: public Checker<check::PostCall, check::PreCall, check::DeadSymbols,
- check::PointerEscape, eval::Assume> {
+ check::LiveSymbols, check::PointerEscape, eval::Assume> {
BugType LeakBugType{this, "Fuchsia handle leak", "Fuchsia Handle Error",
/*SuppressOnSink=*/true};
BugType DoubleReleaseBugType{this, "Fuchsia handle double release",
@@ -172,6 +179,7 @@
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+ void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
bool Assumption) const;
ProgramStateRef checkPointerEscape(ProgramStateRef State,
@@ -381,6 +389,10 @@
SmallVector<SymbolRef, 2> LeakedSyms;
HStateMapTy TrackedHandles = State->get<HStateMap>();
for (auto &CurItem : TrackedHandles) {
+ SymbolRef ErrorSym = CurItem.second.getErrorSym();
+ if (ErrorSym && SymReaper.isDead(ErrorSym))
+ State = State->set<HStateMap>(
+ CurItem.first, HandleState::getWithoutError(CurItem.second));
if (!SymReaper.isDead(CurItem.first))
continue;
if (CurItem.second.isAllocated() || CurItem.second.maybeAllocated())
@@ -395,6 +407,16 @@
C.addTransition(State, N);
}
+void FuchsiaHandleChecker::checkLiveSymbols(ProgramStateRef State,
+ SymbolReaper &SymReaper) const {
+ HStateMapTy TrackedHandles = State->get<HStateMap>();
+ for (auto &CurItem : TrackedHandles) {
+ SymbolRef ErrorSym = CurItem.second.getErrorSym();
+ if (ErrorSym)
+ SymReaper.markLive(CurItem.first);
+ }
+}
+
// Acquiring a handle is not always successful. In Fuchsia most functions
// return a status code that determines the status of the handle.
// When we split the path based on this status code we know that on one
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
