[analyzer] Improved bug reporting in MagentaHandleChecker

[haowei]

Summary

This commit improves the bug reporting of MagentaHandleChecker introduced in D35968 , D36022 and D36023.

After this commit, the allocation and release sites of the handle that causes a bug will be marked in the generated bug report. If the handle is declared as a local variable, the naming of the handle will also be marked in the generated bug report as well.

[haowei]

An updated version was landed in f4c26d993bdc . This diff is no longer needed.

lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp

void Profile(llvm::FoldingSetNodeID &ID) const {
     ID.AddPointer(Region);
   }
 };
 
 static const char *ANNOTATION_PREFIX = "mx_";
 static const char *HANDLE_TYPE_NAME = "mx_handle_t";
 static const char *SYSCALL_RETURN_TYPE_NAME = "mx_status_t";
 static const int MAX_HANDLE_IN_ARRAY = 64;
+typedef std::pair<const ExplodedNode *, const MemRegion *> AllocInfo;
 
 class MagentaHandleChecker
     : public Checker<check::DeadSymbols, check::PointerEscape, eval::Call,
                      check::PostCall> {
   std::unique_ptr<BugType> LeakBugType;
   std::unique_ptr<BugType> DoubleReleaseBugType;
   std::unique_ptr<BugType> UseAfterFreeBugType;
 
private:
 
   void reportUseAfterFree(SymbolRef HandleSym, const SourceRange &Range,
                           CheckerContext &Ctx) const;
 
   void reportBug(SymbolRef Sym, ExplodedNode *ErrorNode, CheckerContext &C,
                  const SourceRange *Range, const std::unique_ptr<BugType> &Type,
                  StringRef Msg) const;
   // Utility functions
+  AllocInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
+                              CheckerContext &Ctx) const;
   // Generate function spec info based on inline annotations in declaration
   bool generateSpecForFuncionDecl(const FunctionDecl *FuncDecl) const;
   // Extract magenta related annotation string from declarations
   StringRef extractAnnotationWithoutPrefix(const Decl *D) const;
 
   bool CheckExprIsZero(const Expr *ArgExpr, CheckerContext &Ctx) const;
 
   int64_t getArgValueFromExpr(const Expr *ArgExpr, ProgramStateRef State,
private:
   bool isArgumentEscaped(SVal Loc) const {
     if (Optional<loc::MemRegionVal> regionLoc =
             Loc.getAs<loc::MemRegionVal>()) {
       if (!regionLoc->getRegion()->hasStackStorage())
         return true;
     }
     return false;
   }
+
+  bool shouldReport(CheckerContext &Ctx) const;
+};
+
+class MagentaBugVisitor : public BugReporterVisitorImpl<MagentaBugVisitor> {
+private:
+  SymbolRef Sym;
+
+public:
+  MagentaBugVisitor(SymbolRef sym) : Sym(sym) {}
+  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(Sym); }
+  std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+                                                 const ExplodedNode *PrevN,
+                                                 BugReporterContext &BRC,
+                                                 BugReport &BR);
 };
 } // end anonymous namespace
 
 // Handle -> HandleState map
 REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState)
 
+std::shared_ptr<PathDiagnosticPiece>
+MagentaBugVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN,
+                             BugReporterContext &BRC, BugReport &BR) {
+  ProgramStateRef State = N->getState();
+  ProgramStateRef StatePref = PrevN->getState();
+  const HandleState *HSCurrent = State->get<HStateMap>(Sym);
+  const HandleState *HSPrev = StatePref->get<HStateMap>(Sym);
+  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+  if (!S)
+    return nullptr;
+
+  SmallString<200> buf;
+  llvm::raw_svector_ostream os(buf);
+  if (!HSPrev && HSCurrent && HSCurrent->isAllocated()) {
+    PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+                               N->getLocationContext());
+    const MemRegion *Region = HSCurrent->getRegion();
+    if (Region && Region->canPrintPretty()) {
+      os << "Handle ";
+      Region->printPretty(os);
+      os << " allocated here.";
+    } else {
+      os << "Handle allocated here.";
+    }
+    return std::make_shared<PathDiagnosticEventPiece>(Pos, os.str());
+  }
+
+  if (HSPrev && HSCurrent && HSPrev->isAllocated() && HSCurrent->isReleased()) {
+    PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+                               N->getLocationContext());
+    const MemRegion *Region = HSCurrent->getRegion();
+    if (Region && Region->canPrintPretty()) {
+      os << "Handle ";
+      Region->printPretty(os);
+      os << " released here.";
+    } else {
+      os << "Handle released here.";
+    }
+    return std::make_shared<PathDiagnosticEventPiece>(Pos, os.str());
+  }
+  return nullptr;
+}
+
 MagentaHandleChecker::MagentaHandleChecker() {
   // Initialize the bug types.
   LeakBugType.reset(
       new BugType(this, "MX handle leak", "Magenta Handle Error"));
   DoubleReleaseBugType.reset(
       new BugType(this, "MX handle double release", "Magenta Handle Error"));
   UseAfterFreeBugType.reset(
       new BugType(this, "MX handle use after free", "Magenta Handle Error"));

 
 void MagentaHandleChecker::reportBug(SymbolRef Sym, ExplodedNode *ErrorNode,
                                      CheckerContext &C,
                                      const SourceRange *Range,
                                      const std::unique_ptr<BugType> &Type,
                                      StringRef Msg) const {
   if (!ErrorNode || !Sym)
     return;
-  auto R = llvm::make_unique<BugReport>(*Type, Msg, ErrorNode);
+  PathDiagnosticLocation LocUsedForUniqueing;
+  const ExplodedNode *AllocNode = nullptr;
+  const MemRegion *Region = nullptr;
+  std::tie(AllocNode, Region) = getAllocationSite(ErrorNode, Sym, C);
+  const Stmt *AllocStmt = PathDiagnosticLocation::getStmt(AllocNode);
+  if (AllocStmt) {
+    LocUsedForUniqueing = PathDiagnosticLocation::createBegin(
+        AllocStmt, C.getSourceManager(), AllocNode->getLocationContext());
+  }
+  SmallString<200> buf;
+  llvm::raw_svector_ostream os(buf);
+  if (Region && Region->canPrintPretty()) {
+    os << Msg << " pointed to by ";
+    Region->printPretty(os);
+  } else {
+    os << Msg;
+  }
+  auto R = llvm::make_unique<BugReport>(
+      *Type, os.str(), ErrorNode, LocUsedForUniqueing,
+      AllocNode->getLocationContext()->getDecl());
   if (Range)
     R->addRange(*Range);
   R->markInteresting(Sym);
+  R->addVisitor(llvm::make_unique<MagentaBugVisitor>(Sym));
   C.emitReport(std::move(R));
 }
 
 void MagentaHandleChecker::reportLeaks(ArrayRef<SymbolRef> LeakedHandles,
                                        CheckerContext &C,
                                        ExplodedNode *ErrorNode) const {
+  if (!shouldReport(C))
+    return;
   for (SymbolRef LeakedHandle : LeakedHandles) {
     reportBug(LeakedHandle, ErrorNode, C, nullptr, LeakBugType,
               "Potential leak of handle");
   }
 }
 
 void MagentaHandleChecker::reportDoubleRelease(SymbolRef HandleSym,
                                                const SourceRange &Range,
                                                CheckerContext &C) const {
+  if (!shouldReport(C))
+    return;
   ExplodedNode *ErrNode = C.generateErrorNode();
   if (!ErrNode)
     return;
   reportBug(HandleSym, ErrNode, C, &Range, DoubleReleaseBugType,
             "Releasing a previously released handle");
 }
 
 void MagentaHandleChecker::reportUseAfterFree(SymbolRef HandleSym,
                                               const SourceRange &Range,
                                               CheckerContext &C) const {
+  if (!shouldReport(C))
+    return;
   ExplodedNode *ErrNode = C.generateErrorNode();
   if (!ErrNode)
     return;
   reportBug(HandleSym, ErrNode, C, &Range, UseAfterFreeBugType,
             "Using a previously released handle");
 }
 
+// Looks for functions that are marked as "DONOTREPORT" in the call stack. If
+// there exists one, return false to stop reporting a bug.
+bool MagentaHandleChecker::shouldReport(CheckerContext &Ctx) const {
+  const StackFrameContext *SFCtx = Ctx.getStackFrame();
+  // Reverse lookup call stack. Similar to StackFrameContext.dump()
+  for (const LocationContext *CurLCtx = SFCtx; CurLCtx;
+       CurLCtx = CurLCtx->getParent()) {
+    const StackFrameContext *CurSFCtx = dyn_cast<StackFrameContext>(CurLCtx);
+    if (!CurSFCtx)
+      continue;
+
+    const FunctionDecl *FD =
+        dyn_cast_or_null<FunctionDecl>(CurSFCtx->getDecl());
+    if (!FD)
+      continue;
+    // Top-level functions are not evaluated by evalCall. So their annotations
+    // are not processed. Process the annotation here.
+    if (!FuncKindMap.count(FD)) {
+      if (!generateSpecForFuncionDecl(FD)) {
+        FuncKindMap[FD] = UNANNOTATED_FUNC;
+      } else {
+        FuncSpec FS = FuncDeclMap[FD];
+        if (FS.RetAction == DONOTREPORT) {
+          FuncKindMap[FD] = DONOTREPORT_FUNC;
+        }
+      }
+    }
+    if (FuncKindMap.count(FD) && FuncKindMap[FD] == DONOTREPORT_FUNC)
+      return false;
+  }
+  return true;
+}
+
+// Reverse traverse the exploded graph to look for allocation side information
+// for handle "Sym".
+AllocInfo MagentaHandleChecker::getAllocationSite(const ExplodedNode *N,
+                                                  SymbolRef Sym,
+                                                  CheckerContext &C) const {
+  const LocationContext *LeakContext = N->getLocationContext();
+  // Walk the ExplodedGraph backwards and find the first node that referred to
+  // the tracked symbol.
+  const ExplodedNode *AllocNode = N;
+  const MemRegion *ReferenceRegion = nullptr;
+
+  ProgramStateRef State = N->getState();
+  // When bug type is handle leak, exploded node N does not have state info for
+  // leaking handle. Get the predecessor of N instead.
+  if (!State->get<HStateMap>(Sym)) {
+    N = N->pred_empty() ? nullptr : *(N->pred_begin());
+  }
+
+  while (N) {
+    State = N->getState();
+    if (!State->get<HStateMap>(Sym))
+      break;
+    // Find the most recent expression bound to the symbol in the current
+    // context.
+    if (!ReferenceRegion) {
+      const HandleState *HS = State->get<HStateMap>(Sym);
+      if (HS)
+        ReferenceRegion = HS->getRegion();
+    }
+    // Allocation node, is the last node in the current or parent context in
+    // which the symbol was tracked.
+    const LocationContext *NContext = N->getLocationContext();
+    if (NContext == LeakContext || NContext->isParentOf(LeakContext))
+      AllocNode = N;
+    N = N->pred_empty() ? nullptr : *(N->pred_begin());
+  }
+  return AllocInfo(AllocNode, ReferenceRegion);
+}
+
 void ento::registerMagentaHandleChecker(CheckerManager &mgr) {
   mgr.registerChecker<MagentaHandleChecker>();
 }

test/Analysis/mxhandle.c

void checkNoLeak15(int tag) {
   }
   mx_handle_close(sa);
   mx_handle_close(sb);
 }
 
 void checkLeak01() {
   mx_handle_t sa, sb;
   mx_channel_create(0, &sa, &sb);
-} // expected-warning {{Potential leak of handle}}
+} // expected-warning 2 {{Potential leak of handle pointed to by}}
 
 
 void checkLeak02(int tag) {
   mx_handle_t sa, sb;
   mx_channel_create(0, &sa, &sb);
   if (tag) {
     mx_handle_close(sa);
   }
-  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle pointed to by 'sa'}}
 }
 
 void checkLeak03(mx_handle_t handle) {
   mx_handle_t handlebuf[4];
   uint32_t hcount;
   mx_status_t r = mx_channel_read(handle, 0, NULL, handlebuf, 0, 4, 0, &hcount);
   if (r < 0) {
     return;
void checkLeak06(mx_handle_t handle, uint32_t hcount) {
   mx_channel_write(handle, 0, NULL, 0, handlebuf, hcount); // It may fail and handles are not released.
 } // expected-warning {{Potential leak of handle}}
 
 void checkLeak07() {
   mx_handle_t sa, sb;
   mx_channel_create(0, &sa, &sb);
 
   useHandle01(sa);
-  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle pointed to by 'sa'}}
 }
 
 void checkLeak08() {
   mx_handle_t sa, sb;
   mx_channel_create(0, &sa, &sb);
 
   useHandle02(&sa);
-  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle pointed to by 'sa'}}
 }
 
 void checkDoubleRelease01(int tag) {
   mx_handle_t sa, sb;
   mx_channel_create(0, &sa, &sb);
   if (tag) {
     mx_handle_close(sa);
   }
-  mx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  mx_handle_close(sa); // expected-warning {{Releasing a previously released handle pointed to by 'sa'}}
   mx_handle_close(sb);
 }
 
 void checkUseAfterFree01(int tag) {
   mx_handle_t sa, sb;
   mx_channel_create(0, &sa, &sb);
   if (tag) {
     mx_handle_close(sa);
   }
-  useHandle01(sa); // expected-warning {{Using a previously released handle}}
+  useHandle01(sa); // expected-warning {{Using a previously released handle pointed to by 'sa'}}
   mx_handle_close(sa);
   mx_handle_close(sb);
 }
+
+void checkSuppressWarning01() MX_SYSCALL_PARAM_ATTR(suppress_warning) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, &sa, &sb) < 0) {
+    return;
+  }
+  mx_handle_close(sa); // Should not report any bugs here
+}
+
+void checkSuppressWarning02() {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, &sa, &sb) < 0) {
+    return;
+  }
+  mx_handle_close(sa); // Should not report any bugs here
+}
+
+void checkSuppressWarning03() MX_SYSCALL_PARAM_ATTR(suppress_warning) {
+  checkSuppressWarning02();
+}