[StaticAnalyzer] Support struct annotations in FuchsiaHandleChecker

Support adding handle annotations to sturucture that contains
handles. All the handles referenced by the structure (direct
value or ptr) would be treated as containing the
release/use/acquire annotations directly.

Patch by Yu Shan

Differential Revision: https://reviews.llvm.org/D91223

diff --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
index b2822e5..d4901eb 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -53,7 +53,7 @@
 //
 // Note that, the analyzer does not always know for sure if a function failed
 // or succeeded. In those cases we use the state MaybeAllocated.
-// Thus, the diagramm above captures the intent, not implementation details.
+// Thus, the diagram above captures the intent, not implementation details.
 //
 // Due to the fact that the number of handle related syscalls in Fuchsia
 // is large, we adopt the annotation attributes to descript syscalls'
@@ -226,32 +226,70 @@ static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym,
   return nullptr;
 }
 
-/// Returns the symbols extracted from the argument or null if it cannot be
-/// found.
-static SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg,
-                                        ProgramStateRef State) {
+namespace {
+class FuchsiaHandleSymbolVisitor final : public SymbolVisitor {
+public:
+  FuchsiaHandleSymbolVisitor(ProgramStateRef State) : State(std::move(State)) {}
+  ProgramStateRef getState() const { return State; }
+
+  bool VisitSymbol(SymbolRef S) override {
+    if (const auto *HandleType = S->getType()->getAs<TypedefType>())
+      if (HandleType->getDecl()->getName() == HandleTypeName)
+        Symbols.push_back(S);
+    return true;
+  }
+
+  SmallVector<SymbolRef, 1024> GetSymbols() { return Symbols; }
+
+private:
+  SmallVector<SymbolRef, 1024> Symbols;
+  ProgramStateRef State;
+};
+} // end anonymous namespace
+
+/// Returns the symbols extracted from the argument or empty vector if it cannot
+/// be found. It is unlikely to have over 1024 symbols in one argument.
+static SmallVector<SymbolRef, 1024>
+getFuchsiaHandleSymbols(QualType QT, SVal Arg, ProgramStateRef State) {
   int PtrToHandleLevel = 0;
   while (QT->isAnyPointerType() || QT->isReferenceType()) {
     ++PtrToHandleLevel;
     QT = QT->getPointeeType();
   }
+  if (QT->isStructureType()) {
+    // If we see a structure, see if there is any handle referenced by the
+    // structure.
+    FuchsiaHandleSymbolVisitor Visitor(State);
+    State->scanReachableSymbols(Arg, Visitor);
+    return Visitor.GetSymbols();
+  }
   if (const auto *HandleType = QT->getAs<TypedefType>()) {
     if (HandleType->getDecl()->getName() != HandleTypeName)
-      return nullptr;
-    if (PtrToHandleLevel > 1) {
+      return {};
+    if (PtrToHandleLevel > 1)
       // Not supported yet.
-      return nullptr;
-    }
+      return {};
 
     if (PtrToHandleLevel == 0) {
-      return Arg.getAsSymbol();
+      SymbolRef Sym = Arg.getAsSymbol();
+      if (Sym) {
+        return {Sym};
+      } else {
+        return {};
+      }
     } else {
       assert(PtrToHandleLevel == 1);
-      if (Optional<Loc> ArgLoc = Arg.getAs<Loc>())
-        return State->getSVal(*ArgLoc).getAsSymbol();
+      if (Optional<Loc> ArgLoc = Arg.getAs<Loc>()) {
+        SymbolRef Sym = State->getSVal(*ArgLoc).getAsSymbol();
+        if (Sym) {
+          return {Sym};
+        } else {
+          return {};
+        }
+      }
     }
   }
-  return nullptr;
+  return {};
 }
 
 void FuchsiaHandleChecker::checkPreCall(const CallEvent &Call,
@@ -273,30 +311,31 @@ void FuchsiaHandleChecker::checkPreCall(const CallEvent &Call,
     if (Arg >= FuncDecl->getNumParams())
       break;
     const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
-    SymbolRef Handle =
-        getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
-    if (!Handle)
-      continue;
+    SmallVector<SymbolRef, 1024> Handles =
+        getFuchsiaHandleSymbols(PVD->getType(), Call.getArgSVal(Arg), State);
 
     // Handled in checkPostCall.
     if (hasFuchsiaAttr<ReleaseHandleAttr>(PVD) ||
         hasFuchsiaAttr<AcquireHandleAttr>(PVD))
       continue;
 
-    const HandleState *HState = State->get<HStateMap>(Handle);
-    if (!HState || HState->isEscaped())
-      continue;
+    for (SymbolRef Handle : Handles) {
+      const HandleState *HState = State->get<HStateMap>(Handle);
+      if (!HState || HState->isEscaped())
+        continue;
 
-    if (hasFuchsiaAttr<UseHandleAttr>(PVD) || PVD->getType()->isIntegerType()) {
-      if (HState->isReleased()) {
-        reportUseAfterFree(Handle, Call.getArgSourceRange(Arg), C);
-        return;
+      if (hasFuchsiaAttr<UseHandleAttr>(PVD) ||
+          PVD->getType()->isIntegerType()) {
+        if (HState->isReleased()) {
+          reportUseAfterFree(Handle, Call.getArgSourceRange(Arg), C);
+          return;
+        }
+      }
+      if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
+          PVD->getType()->isIntegerType()) {
+        // Working around integer by-value escapes.
+        State = State->set<HStateMap>(Handle, HandleState::getEscaped());
       }
-    }
-    if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
-        PVD->getType()->isIntegerType()) {
-      // Working around integer by-value escapes.
-      State = State->set<HStateMap>(Handle, HandleState::getEscaped());
     }
   }
   C.addTransition(State);
@@ -339,63 +378,63 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
       break;
     const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
     unsigned ParamDiagIdx = PVD->getFunctionScopeIndex() + 1;
-    SymbolRef Handle =
-        getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
-    if (!Handle)
-      continue;
+    SmallVector<SymbolRef, 1024> Handles =
+        getFuchsiaHandleSymbols(PVD->getType(), Call.getArgSVal(Arg), State);
 
-    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 {
+    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 {
+          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";
+              return OS.str();
+            } else
+              return "";
+          });
+          State = State->set<HStateMap>(Handle, HandleState::getReleased());
+        }
+      } else if (hasFuchsiaAttr<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 << "Handle released through " << ParamDiagIdx
+            OS << "Handle allocated through " << ParamDiagIdx
                << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
             return OS.str();
           } else
             return "";
         });
-        State = State->set<HStateMap>(Handle, HandleState::getReleased());
+        State = State->set<HStateMap>(
+            Handle, HandleState::getMaybeAllocated(ResultSymbol));
       }
-    } else if (hasFuchsiaAttr<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 << "Handle allocated through " << ParamDiagIdx
-             << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
-          return OS.str();
-        } else
-          return "";
-      });
-      State = State->set<HStateMap>(
-          Handle, HandleState::getMaybeAllocated(ResultSymbol));
     }
   }
   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)
-            return "";
-          for (auto &Note : Notes) {
-            std::string Text = Note(BR);
-            if (!Text.empty())
-              return Text;
-          }
-          return "";
-        });
+      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);
 }
@@ -481,13 +520,14 @@ ProgramStateRef FuchsiaHandleChecker::checkPointerEscape(
       if (Arg >= FuncDecl->getNumParams())
         break;
       const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
-      SymbolRef Handle =
-          getFuchsiaHandleSymbol(PVD->getType(), Call->getArgSVal(Arg), State);
-      if (!Handle)
-        continue;
-      if (hasFuchsiaAttr<UseHandleAttr>(PVD) ||
-          hasFuchsiaAttr<ReleaseHandleAttr>(PVD))
-        UnEscaped.insert(Handle);
+      SmallVector<SymbolRef, 1024> Handles =
+          getFuchsiaHandleSymbols(PVD->getType(), Call->getArgSVal(Arg), State);
+      for (SymbolRef Handle : Handles) {
+        if (hasFuchsiaAttr<UseHandleAttr>(PVD) ||
+            hasFuchsiaAttr<ReleaseHandleAttr>(PVD)) {
+          UnEscaped.insert(Handle);
+        }
+      }
     }
   }
 

diff --git a/clang/test/Analysis/fuchsia_handle.cpp b/clang/test/Analysis/fuchsia_handle.cpp
index d104b13..911ab7a 100644 (file)
--- a/clang/test/Analysis/fuchsia_handle.cpp
+++ b/clang/test/Analysis/fuchsia_handle.cpp
@@ -9,9 +9,9 @@ typedef unsigned int uint32_t;
 #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 __attribute__((acquire_handle("Fuchsia")))
+#define ZX_HANDLE_RELEASE __attribute__((release_handle("Fuchsia")))
+#define ZX_HANDLE_USE __attribute__((use_handle("Fuchsia")))
 #else
 #define ZX_HANDLE_ACQUIRE
 #define ZX_HANDLE_RELEASE
@@ -31,7 +31,7 @@ zx_handle_t return_handle();
 
 void escape1(zx_handle_t *in);
 void escape2(zx_handle_t in);
-void (*escape3)(zx_handle_t) = escape2; 
+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);
@@ -82,8 +82,8 @@ void handleDieBeforeErrorSymbol02() {
   // expected-note-re@-3 {{Handle allocated through {{(2nd|3rd)}} parameter}}
   if (status == 0) { // expected-note {{Assuming 'status' is equal to 0}}
                      // expected-note@-1 {{Taking true branch}}
-    return; // expected-warning {{Potential leak of handle}}
-            // expected-note@-1 {{Potential leak of handle}}
+    return;          // expected-warning {{Potential leak of handle}}
+                     // expected-note@-1 {{Potential leak of handle}}
   }
   __builtin_trap();
 }
@@ -138,7 +138,7 @@ void checkNoLeak06() {
     return;
   zx_handle_close(sa);
   zx_handle_close(sb);
-} 
+}
 
 void checkLeak01(int tag) {
   zx_handle_t sa, sb;
@@ -158,7 +158,7 @@ void checkLeakFromReturn01(int tag) {
   zx_handle_t sa = return_handle(); // expected-note {{Function 'return_handle' returns an open handle}}
   (void)sa;
 } // expected-note {{Potential leak of handle}}
-  // expected-warning@-1 {{Potential leak of handle}}
+// expected-warning@-1 {{Potential leak of handle}}
 
 void checkReportLeakOnOnePath(int tag) {
   zx_handle_t sa, sb;
@@ -166,26 +166,26 @@ void checkReportLeakOnOnePath(int tag) {
     return;                           // expected-note@-1 {{Assuming the condition is false}}
                                       // expected-note@-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@-1 {{Potential leak of handle}}
+  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@-1 {{Potential leak of handle}}
   }
 }
 
@@ -193,7 +193,7 @@ void checkDoubleRelease01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, &sa, &sb);
   // expected-note@-1 {{Handle allocated through 2nd parameter}}
-  if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
+  if (tag)               // expected-note {{Assuming 'tag' is not equal to 0}}
     zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}}
   // expected-note@-2 {{Taking true branch}}
   zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
@@ -211,12 +211,12 @@ void checkUseAfterFree01(int tag) {
   if (tag) {
     // expected-note@-1 {{Assuming 'tag' is not equal to 0}}
     zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}}
-    use1(&sa); // expected-warning {{Using a previously released handle}}
+    use1(&sa);           // expected-warning {{Using a previously released handle}}
     // expected-note@-1 {{Using a previously released handle}}
   }
   // expected-note@-6 {{Assuming 'tag' is 0}}
   zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
-  use2(sb); // expected-warning {{Using a previously released handle}}
+  use2(sb);            // expected-warning {{Using a previously released handle}}
   // expected-note@-1 {{Using a previously released handle}}
 }
 
@@ -229,6 +229,92 @@ void checkMemberOperatorIndices() {
   zx_handle_close(sc);
 }
 
+struct HandleStruct {
+  zx_handle_t h;
+};
+
+void close_handle_struct(HandleStruct hs ZX_HANDLE_RELEASE);
+
+void use_handle_struct(HandleStruct hs ZX_HANDLE_USE);
+
+void checkHandleInStructureUseAfterFree() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+  HandleStruct hs;
+  hs.h = sb;
+  use_handle_struct(hs);
+  close_handle_struct(hs); // expected-note {{Handle released through 1st parameter}}
+  zx_handle_close(sa);
+
+  use2(sb); // expected-warning {{Using a previously released handle}}
+  // expected-note@-1 {{Using a previously released handle}}
+}
+
+void checkHandleInStructureUseAfterFree2() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+  HandleStruct hs;
+  hs.h = sb;
+  use_handle_struct(hs);
+  zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
+  zx_handle_close(sa);
+
+  use_handle_struct(hs); // expected-warning {{Using a previously released handle}}
+  // expected-note@-1 {{Using a previously released handle}}
+}
+
+void checkHandleInStructureLeak() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+  HandleStruct hs;
+  hs.h = sb;
+  zx_handle_close(sa); // expected-warning {{Potential leak of handle}}
+  // expected-note@-1 {{Potential leak of handle}}
+}
+
+struct HandlePtrStruct {
+  zx_handle_t *h;
+};
+
+void close_handle_struct(HandlePtrStruct hs ZX_HANDLE_RELEASE);
+
+void use_handle_struct(HandlePtrStruct hs ZX_HANDLE_USE);
+
+void checkHandlePtrInStructureUseAfterFree() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  HandlePtrStruct hs;
+  hs.h = &sb;
+  use_handle_struct(hs);
+  close_handle_struct(hs); // expected-note {{Handle released through 1st parameter}}
+  zx_handle_close(sa);
+
+  use2(sb); // expected-warning {{Using a previously released handle}}
+  // expected-note@-1 {{Using a previously released handle}}
+}
+
+void checkHandlePtrInStructureUseAfterFree2() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb);
+  HandlePtrStruct hs;
+  hs.h = &sb;
+  use_handle_struct(hs);
+  zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
+  zx_handle_close(sa);
+
+  use_handle_struct(hs); // expected-warning {{Using a previously released handle}}
+  // expected-note@-1 {{Using a previously released handle}}
+}
+
+void checkHandlePtrInStructureLeak() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, &sa, &sb); // expected-note {{Handle allocated through 3rd parameter}}
+  HandlePtrStruct hs;
+  hs.h = &sb;
+  zx_handle_close(sa); // expected-warning {{Potential leak of handle}}
+  // expected-note@-1 {{Potential leak of handle}}
+}
+
 // RAII
 
 template <typename T>
@@ -239,6 +325,7 @@ struct HandleWrapper {
       zx_handle_close(handle);
   }
   T *get_handle_address() { return &handle; }
+
 private:
   T handle;
 };
@@ -259,6 +346,7 @@ struct HandleWrapperUnkonwDtor {
       zx_handle_close(handle);
   }
   T *get_handle_address() { return &handle; }
+
 private:
   T handle;
 };
@@ -270,7 +358,7 @@ void doNotWarnOnUnknownDtor() {
     return;
   zx_handle_close(sb);
 }
- 
+
 // Various escaping scenarios
 
 zx_handle_t *get_handle_address();