[analyzer] Improve FuchsiaHandleChecker's diagnostic messages

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

Author: Xazax-hun

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

@@ -314,6 +314,17 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
   // Function returns an open handle.
   if (hasFuchsiaAttr<AcquireHandleAttr>(FuncDecl)) {
     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->getNameAsString()
+           << "' returns an open handle";
+        return OS.str();
+      } else
+        return "";
+    });
     State =
         State->set<HStateMap>(RetSym, HandleState::getMaybeAllocated(nullptr));
   }
@@ -322,6 +333,7 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
     if (Arg >= FuncDecl->getNumParams())
       break;
     const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
+    unsigned ParamDiagIdx = PVD->getFunctionScopeIndex() + 1;
     SymbolRef Handle =
         getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
     if (!Handle)
@@ -335,20 +347,28 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
         reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C);
         return;
       } else {
-        Notes.push_back([Handle](BugReport &BR) {
+        Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
           auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
           if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
-            return "Handle released here.";
+            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](BugReport &BR) {
+      Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
         auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
         if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
-          return "Handle allocated here.";
+          std::string SBuf;
+          llvm::raw_string_ostream OS(SBuf);
+          OS << "Handle allocated through " << ParamDiagIdx
+             << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
+          return OS.str();
         } else
           return "";
       });

clang/test/Analysis/fuchsia_handle.cpp

@@ -26,6 +26,9 @@ zx_status_t zx_channel_create(
 zx_status_t zx_handle_close(
     zx_handle_t handle ZX_HANDLE_RELEASE);
 
+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; 
@@ -117,7 +120,7 @@ void checkNoLeak06() {
 
 void checkLeak01(int tag) {
   zx_handle_t sa, sb;
-  if (zx_channel_create(0, &sa, &sb)) // expected-note    {{Handle allocated here}}
+  if (zx_channel_create(0, &sa, &sb)) // expected-note    {{Handle allocated through 2nd parameter}}
     return;                           // expected-note@-1 {{Assuming the condition is false}}
                                       // expected-note@-2 {{Taking false branch}}
   use1(&sa);
@@ -129,9 +132,15 @@ void checkLeak01(int tag) {
   zx_handle_close(sb);
 }
 
+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}}
+
 void checkReportLeakOnOnePath(int tag) {
   zx_handle_t sa, sb;
-  if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated here}}
+  if (zx_channel_create(0, &sa, &sb)) // expected-note {{Handle allocated through 2nd parameter}}
     return;                           // expected-note@-1 {{Assuming the condition is false}}
                                       // expected-note@-2 {{Taking false branch}}
   zx_handle_close(sb);
@@ -161,9 +170,9 @@ void checkReportLeakOnOnePath(int tag) {
 void checkDoubleRelease01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, &sa, &sb);
-  // expected-note@-1 {{Handle allocated here}}
+  // expected-note@-1 {{Handle allocated through 2nd parameter}}
   if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
-    zx_handle_close(sa); // expected-note {{Handle released here}}
+    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}}
   // expected-note@-1 {{Releasing a previously released handle}}
@@ -173,18 +182,18 @@ void checkDoubleRelease01(int tag) {
 void checkUseAfterFree01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, &sa, &sb);
-  // expected-note@-1 {{Handle allocated here}}
-  // expected-note@-2 {{Handle allocated here}}
+  // expected-note@-1 {{Handle allocated through 2nd parameter}}
+  // expected-note@-2 {{Handle allocated through 3rd parameter}}
   // expected-note@+2 {{Taking true branch}}
   // expected-note@+1 {{Taking false branch}}
   if (tag) {
     // expected-note@-1 {{Assuming 'tag' is not equal to 0}}
-    zx_handle_close(sa); // expected-note {{Handle released here}}
+    zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}}
     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 here}}
+  zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
   use2(sb); // expected-warning {{Using a previously released handle}}
   // expected-note@-1 {{Using a previously released handle}}
 }