[clang] [analyzer] Modernize FuchsiaHandleChecker (PR #111588)

[Donát Nagy via cfe-commits]

================
@@ -267,12 +286,128 @@ class FuchsiaHandleSymbolVisitor final : public SymbolVisitor {
 private:
   SmallVector<SymbolRef, 1024> Symbols;
 };
+
+class FuchsiaBugVisitor final : public BugReporterVisitor {
+  // Handle that caused a problem.
+  SymbolRef Sym;
+
+  bool IsLeak;
+
+public:
+  FuchsiaBugVisitor(SymbolRef S, bool Leak = false) : Sym(S), IsLeak(Leak) {}
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(Sym);
+  }
+
+  static inline bool isAllocated(const HandleState *RSCurr,
+                                 const HandleState *RSPrev, const Stmt *Stmt) {
+    return isa_and_nonnull<CallExpr>(Stmt) &&
+           ((RSCurr && (RSCurr->isAllocated() || RSCurr->maybeAllocated()) &&
+             (!RSPrev ||
+              !(RSPrev->isAllocated() || RSPrev->maybeAllocated()))));
+  }
+
+  static inline bool isAllocatedUnowned(const HandleState *RSCurr,
+                                        const HandleState *RSPrev,
+                                        const Stmt *Stmt) {
+    return isa_and_nonnull<CallExpr>(Stmt) &&
+           ((RSCurr && RSCurr->isUnowned()) &&
+            (!RSPrev || !RSPrev->isUnowned()));
+  }
+
+  static inline bool isReleased(const HandleState *RSCurr,
+                                const HandleState *RSPrev, const Stmt *Stmt) {
+    return isa_and_nonnull<CallExpr>(Stmt) &&
+           ((RSCurr && RSCurr->isReleased()) &&
+            (!RSPrev || !RSPrev->isReleased()));
+  }
+
+  PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
+                                   BugReporterContext &BRC,
+                                   PathSensitiveBugReport &BR) override;
+
+  PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC,
+                                    const ExplodedNode *EndPathNode,
+                                    PathSensitiveBugReport &BR) override {
+    if (!IsLeak)
+      return nullptr;
+
+    PathDiagnosticLocation L = BR.getLocation();
+    // Do not add the statement itself as a range in case of leak.
+    return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(),
+                                                      false);
+  }
+};
 } // end anonymous namespace
 
+PathDiagnosticPieceRef
+FuchsiaBugVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+                             PathSensitiveBugReport &BR) {
+  ProgramStateRef State = N->getState();
+  ProgramStateRef StatePrev = N->getFirstPred()->getState();
+
+  const HandleState *RSCurr = State->get<HStateMap>(Sym);
+  const HandleState *RSPrev = StatePrev->get<HStateMap>(Sym);
+
+  const Stmt *S = N->getStmtForDiagnostics();
+
+  StringRef Msg;
+  SmallString<256> Buf;
+  llvm::raw_svector_ostream OS(Buf);
+
+  if (isAllocated(RSCurr, RSPrev, S)) {
+    auto Index = RSCurr->getIndex();
+
+    if (Index > 0)
+      OS << "Handle allocated through " << Index
+         << llvm::getOrdinalSuffix(Index) << " parameter";
+    else
+      OS << "Function returns an open handle";
+
+    Msg = OS.str();
+  } else if (isAllocatedUnowned(RSCurr, RSPrev, S)) {
+    auto Index = RSCurr->getIndex();
+
+    if (Index > 0)
+      OS << "Unowned handle allocated through " << Index
+         << llvm::getOrdinalSuffix(Index) << " parameter";
+    else
+      OS << "Function returns an unowned handle";
+
+    Msg = OS.str();
+  } else if (isReleased(RSCurr, RSPrev, S)) {
+    auto Index = RSCurr->getIndex();
+
+    assert(Index > 0);
+
+    OS << "Handle released through " << Index << llvm::getOrdinalSuffix(Index)
+       << " parameter";
+    Msg = OS.str();
+  }
+
+  if (Msg.empty())
+    return nullptr;
+
+  PathDiagnosticLocation Pos;
+  if (!S) {
+    auto PostImplCall = N->getLocation().getAs<PostImplicitCall>();
+    if (!PostImplCall)
+      return nullptr;
+    Pos = PathDiagnosticLocation(PostImplCall->getLocation(),
+                                 BRC.getSourceManager());
+  } else {
+    Pos = PathDiagnosticLocation(S, BRC.getSourceManager(),
+                                 N->getLocationContext());
+  }
+
+  return std::make_shared<PathDiagnosticEventPiece>(Pos, Msg, true);
+}
+
 /// 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) {
+SmallVector<SymbolRef, 1024> getFuchsiaHandleSymbols(QualType QT, SVal Arg,
----------------
NagyDonat wrote:

```suggestion
SmallVector<SymbolRef> getFuchsiaHandleSymbols(QualType QT, SVal Arg,
```
This `1024` is extremely suspicious here, and although I'm completely unfamiliar with Fuchsia, I'd be very surprised to see an argument that contains more than a handful of handles (considering that loop modeling is limited and we won't fill a long array with handles).

The [LLVM Programmer's Manual](https://www.llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h) says that
> In the absence of a well-motivated choice for the number of inlined elements `N`, it is recommended to use `SmallVector<T>` (that is, omitting the `N`). This will choose a default number of inlined elements reasonable for allocation on the stack (for example, trying to keep `sizeof(SmallVector<T>)` around 64 bytes).

but I could also accept something like `SmallVector<SymbolRef, 1>` to save space in the usual case.

Also note that the body of this function is a bit ugly and suspicious, if you have some free time, you could have a look at it.

https://github.com/llvm/llvm-project/pull/111588