diff --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp --- 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 @@ 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 @@ 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 @@ 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 @@ CASE(Kind::Allocated) CASE(Kind::Released) CASE(Kind::Escaped) + CASE(Kind::Unowned) } if (ErrorSym) { OS << " ErrorSym: "; @@ -163,6 +178,11 @@ return D->hasAttr() && D->getAttr()->getHandleType() == "Fuchsia"; } +template static bool hasFuchsiaUnownedAttr(const Decl *D) { + return D->hasAttr() && + D->getAttr()->getHandleType() == "FuchsiaUnowned"; +} + class FuchsiaHandleChecker : public Checker { @@ -172,6 +192,8 @@ "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 @@ 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 @@ }); State = State->set(RetSym, HandleState::getMaybeAllocated(nullptr)); + } else if (hasFuchsiaUnownedAttr(FuncDecl)) { + // Function returns an unowned handle + SymbolRef RetSym = Call.getReturnValue().getAsSymbol(); + Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string { + auto *PathBR = static_cast(&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(RetSym, HandleState::getUnowned()); } for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) { @@ -388,6 +428,9 @@ 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(&BR); @@ -416,6 +459,19 @@ }); State = State->set( Handle, HandleState::getMaybeAllocated(ResultSymbol)); + } else if (hasFuchsiaUnownedAttr(PVD)) { + Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string { + auto *PathBR = static_cast(&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(Handle, HandleState::getUnowned()); } else if (!hasFuchsiaAttr(PVD) && PVD->getType()->isIntegerType()) { // Working around integer by-value escapes. @@ -433,7 +489,8 @@ 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 @@ "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 --- a/clang/test/Analysis/fuchsia_handle.cpp +++ b/clang/test/Analysis/fuchsia_handle.cpp @@ -12,10 +12,12 @@ #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_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 @@ 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@-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@-1 {{Releasing an unowned handle}} +} + void checkInvalidHandle01() { zx_handle_t sa, sb; zx_channel_create(0, &sa, &sb);