[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)
DonĂ¡t Nagy via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 10 04:01:12 PDT 2024
================
@@ -696,6 +732,69 @@ struct StreamOperationEvaluator {
} // end anonymous namespace
+//===----------------------------------------------------------------------===//
+// Definition of NoStreamStateChangeVisitor.
+//===----------------------------------------------------------------------===//
+
+namespace {
+class NoStreamStateChangeVisitor final : public NoOwnershipChangeVisitor {
+protected:
+ /// Syntactically checks whether the callee is a freeing function. Since
+ /// we have no path-sensitive information on this call (we would need a
+ /// CallEvent instead of a CallExpr for that), its possible that a
+ /// freeing function was called indirectly through a function pointer,
+ /// but we are not able to tell, so this is a best effort analysis.
+ bool isFreeingCallAsWritten(const CallExpr &Call) const {
+ const auto *StreamChk = static_cast<const StreamChecker *>(&Checker);
+ if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+ return true;
+
+ return false;
+ }
+
+ bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext &ACtx) override {
+ using namespace clang::ast_matchers;
+ const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
+
+ auto Matches =
+ match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+ for (BoundNodes Match : Matches) {
+ if (const auto *Call = Match.getNodeAs<CallExpr>("call"))
+ if (isFreeingCallAsWritten(*Call))
+ return true;
+ }
+ // TODO: Ownership might change with an attempt to store stream object, not
+ // only through freeing it. Check for attempted stores as well.
+ return false;
+ }
+
+ virtual bool hasResourceStateChanged(ProgramStateRef CallEnterState,
+ ProgramStateRef CallExitEndState) final {
+ return CallEnterState->get<StreamMap>(Sym) !=
+ CallExitEndState->get<StreamMap>(Sym);
+ }
+
+ PathDiagnosticPieceRef emitNote(const ExplodedNode *N) override {
+ PathDiagnosticLocation L = PathDiagnosticLocation::create(
+ N->getLocation(),
+ N->getState()->getStateManager().getContext().getSourceManager());
+ return std::make_shared<PathDiagnosticEventPiece>(
+ L, "Returning without freeing stream object or storing the pointer for "
----------------
NagyDonat wrote:
```suggestion
L, "Returning without freeing stream object or storing it for "
```
The current phrasing is technically correct, because the stream object is a `FILE *` _pointer_, but IMO "it" would be both shorter and easier to understand
https://github.com/llvm/llvm-project/pull/94957
More information about the cfe-commits
mailing list