[analyzer] Update Fuchsia checker to catch when releasing unowned handles.

[isthismyaccount]

Summary

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.

[aabbaabb]

LGTM

[haowei]

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

22–23

Would you mind updating this ascii art since you added another state in the state machine? It would help other maintainer to understand how the checker works.

[isthismyaccount]

Updated flow diagram to include release of unowned handles.

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

 //   - If a handle is released, it should not be used for other purposes
 //        such as I/O.
 //
 // In this checker, each tracked handle is associated with a state. When the
 // handle variable is passed to different function calls or syscalls, its state
 // changes. The state changes can be generally represented by following ASCII
 // 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.
 // use_func represents the functions or syscall that requires an open handle.
 //
 // If a tracked handle dies in "Released" or "Not Tracked" state, we assume it
 // is properly used. Otherwise a bug and will be reported.
 //

 
 namespace {
 
 static const StringRef HandleTypeName = "zx_handle_t";
 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) {}
 
 public:
   bool operator==(const HandleState &Other) const {
     return K == Other.K && ErrorSym == Other.ErrorSym;
   }
   bool isAllocated() const { return K == Kind::Allocated; }
   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);
   }
   static HandleState getAllocated(ProgramStateRef State, HandleState S) {
     assert(S.maybeAllocated());
     assert(State->getConstraintManager()
                .isNull(State, S.getErrorSym())
                .isConstrained());
     return HandleState(Kind::Allocated, nullptr);
   }
   static HandleState getReleased() {
     return HandleState(Kind::Released, nullptr);
   }
   static HandleState getEscaped() {
     return HandleState(Kind::Escaped, nullptr);
   }
+  static HandleState getUnowned() {
+    return HandleState(Kind::Unowned, nullptr);
+  }
 
   SymbolRef getErrorSym() const { return ErrorSym; }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.AddInteger(static_cast<int>(K));
     ID.AddPointer(ErrorSym);
   }
 
   LLVM_DUMP_METHOD void dump(raw_ostream &OS) const {
     switch (K) {
 #define CASE(ID)                                                               \
   case ID:                                                                     \
     OS << #ID;                                                                 \
     break;
       CASE(Kind::MaybeAllocated)
       CASE(Kind::Allocated)
       CASE(Kind::Released)
       CASE(Kind::Escaped)
+      CASE(Kind::Unowned)
     }
     if (ErrorSym) {
       OS << " ErrorSym: ";
       ErrorSym->dumpToStream(OS);
     }
   }
 
   LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); }
 };
 
 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> {
   BugType LeakBugType{this, "Fuchsia handle leak", "Fuchsia Handle Error",
                       /*SuppressOnSink=*/true};
   BugType DoubleReleaseBugType{this, "Fuchsia handle double release",
                                "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;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
                              bool Assumption) const;
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
                                      const InvalidatedSymbols &Escaped,
                                      const CallEvent *Call,
                                      PointerEscapeKind Kind) const;
 
   ExplodedNode *reportLeaks(ArrayRef<SymbolRef> LeakedHandles,
                             CheckerContext &C, ExplodedNode *Pred) const;
 
   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;
 
   void reportBug(SymbolRef Sym, ExplodedNode *ErrorNode, CheckerContext &C,
                  const SourceRange *Range, const BugType &Type,
                  StringRef Msg) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string {
         OS << "Function '" << FuncDecl->getDeclName()
            << "' returns an open handle";
         return OS.str();
       } else
         return "";
     });
     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) {
     if (Arg >= FuncDecl->getNumParams())
       break;
     const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
     unsigned ParamDiagIdx = PVD->getFunctionScopeIndex() + 1;
     SmallVector<SymbolRef, 1024> Handles =
         getFuchsiaHandleSymbols(PVD->getType(), Call.getArgSVal(Arg), State);
 
     for (SymbolRef Handle : Handles) {
       const HandleState *HState = State->get<HStateMap>(Handle);
       if (HState && HState->isEscaped())
         continue;
       if (hasFuchsiaAttr<ReleaseHandleAttr>(PVD)) {
         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);
             if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
               std::string SBuf;
               llvm::raw_string_ostream OS(SBuf);
               OS << "Handle released through " << ParamDiagIdx
                  << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
for (SymbolRef Handle : Handles) {
             OS << "Handle allocated through " << ParamDiagIdx
                << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
             return OS.str();
           } else
             return "";
         });
         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.
         // The by-value escape would not be captured in checkPointerEscape.
         // If the function was not analyzed (otherwise wasInlined should be
         // true) and there is no annotation on the handle, we assume the handle
         // is escaped.
         State = State->set<HStateMap>(Handle, HandleState::getEscaped());
       }
     }
   }
   const NoteTag *T = nullptr;
   if (!Notes.empty()) {
     T = C.getNoteTag([this, Notes{std::move(Notes)}](
                          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);
         if (!Text.empty())
           return Text;
       }
       return "";
     });

 void FuchsiaHandleChecker::reportDoubleRelease(SymbolRef HandleSym,
                                                const SourceRange &Range,
                                                CheckerContext &C) const {
   ExplodedNode *ErrNode = C.generateErrorNode(C.getState());
   reportBug(HandleSym, ErrNode, C, &Range, DoubleReleaseBugType,
             "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 {
   ExplodedNode *ErrNode = C.generateErrorNode(C.getState());
   reportBug(HandleSym, ErrNode, C, &Range, UseAfterReleaseBugType,
             "Using a previously released handle");
 }
 

clang/test/Analysis/fuchsia_handle.cpp

 // 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;
 typedef __typeof__(sizeof(int)) zx_handle_t;
 typedef unsigned int uint32_t;
 #define NULL ((void *)0)
 #define ZX_HANDLE_INVALID 0
 
 #if defined(__clang__)
 #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(
     uint32_t options,
     zx_handle_t *out0 ZX_HANDLE_ACQUIRE,
     zx_handle_t *out1 ZX_HANDLE_ACQUIRE);
 
 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();
 
 void escape1(zx_handle_t *in);
 void escape2(zx_handle_t in);
 void (*escape3)(zx_handle_t) = escape2;
 
 void use1(const zx_handle_t *in ZX_HANDLE_USE);
 void use2(zx_handle_t in ZX_HANDLE_USE);
 
 void moreArgs(zx_handle_t, int, ...);
 void lessArgs(zx_handle_t, int a = 5);
 
 // To test if argument indexes are OK for operator calls.
 struct MyType {
   ZX_HANDLE_ACQUIRE
   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);
   if (sa == ZX_HANDLE_INVALID)
     ;
   // Will we ever see a warning like below?
   // We eagerly replace the symbol with a constant and lose info...
   use2(sa); // TODOexpected-warning {{Use of an invalid handle}}