[clang] 8deaec1 - [analyzer] Update Fuchsia checker to catch releasing unowned handles.
Haowei Wu via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 6 16:24:04 PST 2021
Author: Daniel Hwang
Date: 2021-01-06T16:23:49-08:00
New Revision: 8deaec122ec68746c53ec2afb893873124053d8d
URL: https://github.com/llvm/llvm-project/commit/8deaec122ec68746c53ec2afb893873124053d8d
DIFF: https://github.com/llvm/llvm-project/commit/8deaec122ec68746c53ec2afb893873124053d8d.diff
LOG: [analyzer] Update Fuchsia checker to catch releasing unowned handles.
Certain Fuchsia functions may return handles that are not owned by the
current closure. This adds a check in order to determine when these
handles are released.
Differential Revision: https://reviews.llvm.org/D93868
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/test/Analysis/fuchsia_handle.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
index c246a8db3067..e3f4be0726c8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -20,29 +20,39 @@
// Art:
//
//
-// +-+---------v-+ +------------+
-// acquire_func succeeded | | Escape | |
-// +-----------------> Allocated +---------> Escaped <--+
-// | | | | | |
-// | +-----+------++ +------------+ |
-// | | | |
-// | release_func | +--+ |
-// | | | handle +--------+ |
-// | | | dies | | |
-// | +----v-----+ +---------> Leaked | |
-// | | | |(REPORT)| |
-// +----------+--+ | Released | Escape +--------+ |
-// | | | +---------------------------+
-// | Not tracked <--+ +----+---+-+
-// | | | | | As argument by value
-// +------+------+ | release_func | +------+ in function call
-// | | | | or by reference in
-// | | | | use_func call
-// +---------+ +----v-----+ | +-----------+
-// acquire_func failed | Double | +-----> Use after |
-// | released | | released |
-// | (REPORT) | | (REPORT) |
-// +----------+ +-----------+
+// +-------------+ +------------+
+// acquire_func succeeded | | Escape | |
+// +-----------------> Allocated +---------> Escaped <--+
+// | | | | | |
+// | +-----+------++ +------------+ |
+// | | | |
+// acquire_func | release_func | +--+ |
+// failed | | | handle +--------+ |
+// +---------+ | | | dies | | |
+// | | | +----v-----+ +---------> Leaked | |
+// | | | | | |(REPORT)| |
+// | +----------+--+ | Released | Escape +--------+ |
+// | | | | +---------------------------+
+// +--> Not tracked | +----+---+-+
+// | | | | As argument by value
+// +----------+--+ release_func | +------+ in function call
+// | | | or by reference in
+// | | | use_func call
+// unowned | +----v-----+ | +-----------+
+// acquire_func | | Double | +-----> Use after |
+// succeeded | | released | | released |
+// | | (REPORT) | | (REPORT) |
+// +---------------+ +----------+ +-----------+
+// | Allocated |
+// | Unowned | release_func
+// | +---------+
+// +---------------+ |
+// |
+// +-----v----------+
+// | Release of |
+// | unowned handle |
+// | (REPORT) |
+// +----------------+
//
// acquire_func represents the functions or syscalls that may acquire a handle.
// release_func represents the functions or syscalls that may release a handle.
@@ -102,7 +112,7 @@ static const StringRef ErrorTypeName = "zx_status_t";
class HandleState {
private:
- enum class Kind { MaybeAllocated, Allocated, Released, Escaped } K;
+ enum class Kind { MaybeAllocated, Allocated, Released, Escaped, Unowned } K;
SymbolRef ErrorSym;
HandleState(Kind K, SymbolRef ErrorSym) : K(K), ErrorSym(ErrorSym) {}
@@ -114,6 +124,7 @@ class HandleState {
bool maybeAllocated() const { return K == Kind::MaybeAllocated; }
bool isReleased() const { return K == Kind::Released; }
bool isEscaped() const { return K == Kind::Escaped; }
+ bool isUnowned() const { return K == Kind::Unowned; }
static HandleState getMaybeAllocated(SymbolRef ErrorSym) {
return HandleState(Kind::MaybeAllocated, ErrorSym);
@@ -131,6 +142,9 @@ class HandleState {
static HandleState getEscaped() {
return HandleState(Kind::Escaped, nullptr);
}
+ static HandleState getUnowned() {
+ return HandleState(Kind::Unowned, nullptr);
+ }
SymbolRef getErrorSym() const { return ErrorSym; }
@@ -149,6 +163,7 @@ class HandleState {
CASE(Kind::Allocated)
CASE(Kind::Released)
CASE(Kind::Escaped)
+ CASE(Kind::Unowned)
}
if (ErrorSym) {
OS << " ErrorSym: ";
@@ -163,6 +178,11 @@ template <typename Attr> static bool hasFuchsiaAttr(const Decl *D) {
return D->hasAttr<Attr>() && D->getAttr<Attr>()->getHandleType() == "Fuchsia";
}
+template <typename Attr> static bool hasFuchsiaUnownedAttr(const Decl *D) {
+ return D->hasAttr<Attr>() &&
+ D->getAttr<Attr>()->getHandleType() == "FuchsiaUnowned";
+}
+
class FuchsiaHandleChecker
: public Checker<check::PostCall, check::PreCall, check::DeadSymbols,
check::PointerEscape, eval::Assume> {
@@ -172,6 +192,8 @@ class FuchsiaHandleChecker
"Fuchsia Handle Error"};
BugType UseAfterReleaseBugType{this, "Fuchsia handle use after release",
"Fuchsia Handle Error"};
+ BugType ReleaseUnownedBugType{
+ this, "Fuchsia handle release of unowned handle", "Fuchsia Handle Error"};
public:
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -190,6 +212,9 @@ class FuchsiaHandleChecker
void reportDoubleRelease(SymbolRef HandleSym, const SourceRange &Range,
CheckerContext &C) const;
+ void reportUnownedRelease(SymbolRef HandleSym, const SourceRange &Range,
+ CheckerContext &C) const;
+
void reportUseAfterFree(SymbolRef HandleSym, const SourceRange &Range,
CheckerContext &C) const;
@@ -370,6 +395,21 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
});
State =
State->set<HStateMap>(RetSym, HandleState::getMaybeAllocated(nullptr));
+ } else if (hasFuchsiaUnownedAttr<AcquireHandleAttr>(FuncDecl)) {
+ // Function returns an unowned handle
+ SymbolRef RetSym = Call.getReturnValue().getAsSymbol();
+ Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string {
+ auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
+ if (auto IsInteresting = PathBR->getInterestingnessKind(RetSym)) {
+ std::string SBuf;
+ llvm::raw_string_ostream OS(SBuf);
+ OS << "Function '" << FuncDecl->getDeclName()
+ << "' returns an unowned handle";
+ return OS.str();
+ } else
+ return "";
+ });
+ State = State->set<HStateMap>(RetSym, HandleState::getUnowned());
}
for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
@@ -388,6 +428,9 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
if (HState && HState->isReleased()) {
reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C);
return;
+ } else if (HState && HState->isUnowned()) {
+ reportUnownedRelease(Handle, Call.getArgSourceRange(Arg), C);
+ return;
} else {
Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
@@ -416,6 +459,19 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
});
State = State->set<HStateMap>(
Handle, HandleState::getMaybeAllocated(ResultSymbol));
+ } else if (hasFuchsiaUnownedAttr<AcquireHandleAttr>(PVD)) {
+ Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
+ auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
+ if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
+ std::string SBuf;
+ llvm::raw_string_ostream OS(SBuf);
+ OS << "Unowned handle allocated through " << ParamDiagIdx
+ << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
+ return OS.str();
+ } else
+ return "";
+ });
+ State = State->set<HStateMap>(Handle, HandleState::getUnowned());
} else if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
PVD->getType()->isIntegerType()) {
// Working around integer by-value escapes.
@@ -433,7 +489,8 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
PathSensitiveBugReport &BR) -> std::string {
if (&BR.getBugType() != &UseAfterReleaseBugType &&
&BR.getBugType() != &LeakBugType &&
- &BR.getBugType() != &DoubleReleaseBugType)
+ &BR.getBugType() != &DoubleReleaseBugType &&
+ &BR.getBugType() != &ReleaseUnownedBugType)
return "";
for (auto &Note : Notes) {
std::string Text = Note(BR);
@@ -572,6 +629,14 @@ void FuchsiaHandleChecker::reportDoubleRelease(SymbolRef HandleSym,
"Releasing a previously released handle");
}
+void FuchsiaHandleChecker::reportUnownedRelease(SymbolRef HandleSym,
+ const SourceRange &Range,
+ CheckerContext &C) const {
+ ExplodedNode *ErrNode = C.generateErrorNode(C.getState());
+ reportBug(HandleSym, ErrNode, C, &Range, ReleaseUnownedBugType,
+ "Releasing an unowned handle");
+}
+
void FuchsiaHandleChecker::reportUseAfterFree(SymbolRef HandleSym,
const SourceRange &Range,
CheckerContext &C) const {
diff --git a/clang/test/Analysis/fuchsia_handle.cpp b/clang/test/Analysis/fuchsia_handle.cpp
index 99f0449a4902..f86cc50df045 100644
--- a/clang/test/Analysis/fuchsia_handle.cpp
+++ b/clang/test/Analysis/fuchsia_handle.cpp
@@ -12,10 +12,12 @@ typedef unsigned int uint32_t;
#define ZX_HANDLE_ACQUIRE __attribute__((acquire_handle("Fuchsia")))
#define ZX_HANDLE_RELEASE __attribute__((release_handle("Fuchsia")))
#define ZX_HANDLE_USE __attribute__((use_handle("Fuchsia")))
+#define ZX_HANDLE_ACQUIRE_UNOWNED __attribute__((acquire_handle("FuchsiaUnowned")))
#else
#define ZX_HANDLE_ACQUIRE
#define ZX_HANDLE_RELEASE
#define ZX_HANDLE_USE
+#define ZX_HANDLE_ACQUIRE_UNOWNED
#endif
zx_status_t zx_channel_create(
@@ -26,6 +28,11 @@ zx_status_t zx_channel_create(
zx_status_t zx_handle_close(
zx_handle_t handle ZX_HANDLE_RELEASE);
+ZX_HANDLE_ACQUIRE_UNOWNED
+zx_handle_t zx_process_self();
+
+void zx_process_self_param(zx_handle_t *out ZX_HANDLE_ACQUIRE_UNOWNED);
+
ZX_HANDLE_ACQUIRE
zx_handle_t return_handle();
@@ -45,6 +52,20 @@ struct MyType {
zx_handle_t operator+(zx_handle_t ZX_HANDLE_RELEASE replace);
};
+void checkUnownedHandle01() {
+ zx_handle_t h0;
+ h0 = zx_process_self(); // expected-note {{Function 'zx_process_self' returns an unowned handle}}
+ zx_handle_close(h0); // expected-warning {{Releasing an unowned handle}}
+ // expected-note at -1 {{Releasing an unowned handle}}
+}
+
+void checkUnownedHandle02() {
+ zx_handle_t h0;
+ zx_process_self_param(&h0); // expected-note {{Unowned handle allocated through 1st parameter}}
+ zx_handle_close(h0); // expected-warning {{Releasing an unowned handle}}
+ // expected-note at -1 {{Releasing an unowned handle}}
+}
+
void checkInvalidHandle01() {
zx_handle_t sa, sb;
zx_channel_create(0, &sa, &sb);
More information about the cfe-commits
mailing list