https://github.com/Szelethus updated
https://github.com/llvm/llvm-project/pull/100719
From d176b03b211144dadaa1efb4b7da959110d7725c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= <dkszelet...@gmail.com>
Date: Fri, 26 Jul 2024 11:01:41 +0200
Subject: [PATCH 1/2] [analyzer][NFC] Eliminate a dyn_cast
---
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 5 ++---
clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h | 2 +-
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 5 ++---
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fe202c79ed620..39e5c7c014a2a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -796,14 +796,13 @@ class NoMemOwnershipChangeVisitor final : public
NoOwnershipChangeVisitor {
/// done syntactically, because we are trying to argue about alternative
/// paths of execution, and as a consequence we don't have path-sensitive
/// information.
- bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
ASTContext &ACtx) final {
using namespace clang::ast_matchers;
- const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
auto Matches = match(findAll(stmt(anyOf(cxxDeleteExpr().bind("delete"),
callExpr().bind("call")))),
- *FD->getBody(), ACtx);
+ Callee->getBody(), ACtx);
for (BoundNodes Match : Matches) {
if (Match.getNodeAs<CXXDeleteExpr>("delete"))
return true;
diff --git a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
index 027f1a156a7c0..7be74860d863b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
+++ b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
@@ -26,7 +26,7 @@ class NoOwnershipChangeVisitor : public
NoStateChangeFuncVisitor {
/// is done syntactically, because we are trying to argue about alternative
/// paths of execution, and as a consequence we don't have path-sensitive
/// information.
- virtual bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ virtual bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
ASTContext &ACtx) = 0;
virtual bool hasResourceStateChanged(ProgramStateRef CallEnterState,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 9aee7f952ad2d..41187ee9b5cdf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -746,13 +746,12 @@ class NoStreamStateChangeVisitor final : public
NoOwnershipChangeVisitor {
return StreamChk->FCloseDesc.matchesAsWritten(Call);
}
- bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
ASTContext &ACtx) final {
using namespace clang::ast_matchers;
- const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
auto Matches =
- match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+ match(findAll(callExpr().bind("call")), Callee->getBody(), ACtx);
for (BoundNodes Match : Matches) {
if (const auto *Call = Match.getNodeAs<CallExpr>("call"))
if (isClosingCallAsWritten(*Call))
From f96c21c018029bd314fefdbca7ea868490cb6992 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= <dkszelet...@gmail.com>
Date: Mon, 5 Aug 2024 12:08:11 +0200
Subject: [PATCH 2/2] Make users responsible for parsing Callee instead
---
.../lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 14 ++++++++++++--
.../Checkers/NoOwnershipChangeVisitor.cpp | 10 ----------
.../Checkers/NoOwnershipChangeVisitor.h | 2 +-
.../lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 14 ++++++++++++--
4 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 39e5c7c014a2a..3a13b45d9c440 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -796,13 +796,23 @@ class NoMemOwnershipChangeVisitor final : public
NoOwnershipChangeVisitor {
/// done syntactically, because we are trying to argue about alternative
/// paths of execution, and as a consequence we don't have path-sensitive
/// information.
- bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
+ bool doesFnIntendToHandleOwnership(const Decl *Callee,
ASTContext &ACtx) final {
+ const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
+
+ // Given that the stack frame was entered, the body should always be
+ // theoretically obtainable. In case of body farms, the synthesized body
+ // is not attached to declaration, thus triggering the '!FD->hasBody()'
+ // branch. That said, would a synthesized body ever intend to handle
+ // ownership? As of today they don't. And if they did, how would we
+ // put notes inside it, given that it doesn't match any source locations?
+ if (!FD || !FD->hasBody())
+ return false;
using namespace clang::ast_matchers;
auto Matches = match(findAll(stmt(anyOf(cxxDeleteExpr().bind("delete"),
callExpr().bind("call")))),
- Callee->getBody(), ACtx);
+ *FD->getBody(), ACtx);
for (BoundNodes Match : Matches) {
if (Match.getNodeAs<CXXDeleteExpr>("delete"))
return true;
diff --git a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.cpp
b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.cpp
index 22b5ebfd6fab0..91f4ca371aa98 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.cpp
@@ -72,16 +72,6 @@ bool NoOwnershipChangeVisitor::wasModifiedInFunction(
const ExplodedNode *CallEnterN, const ExplodedNode *CallExitEndN) {
const Decl *Callee =
CallExitEndN->getFirstPred()->getLocationContext()->getDecl();
- const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
-
- // Given that the stack frame was entered, the body should always be
- // theoretically obtainable. In case of body farms, the synthesized body
- // is not attached to declaration, thus triggering the '!FD->hasBody()'
- // branch. That said, would a synthesized body ever intend to handle
- // ownership? As of today they don't. And if they did, how would we
- // put notes inside it, given that it doesn't match any source locations?
- if (!FD || !FD->hasBody())
- return false;
if (!doesFnIntendToHandleOwnership(
Callee,
CallExitEndN->getState()->getAnalysisManager().getASTContext()))
diff --git a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
index 7be74860d863b..027f1a156a7c0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
+++ b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
@@ -26,7 +26,7 @@ class NoOwnershipChangeVisitor : public
NoStateChangeFuncVisitor {
/// is done syntactically, because we are trying to argue about alternative
/// paths of execution, and as a consequence we don't have path-sensitive
/// information.
- virtual bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
+ virtual bool doesFnIntendToHandleOwnership(const Decl *Callee,
ASTContext &ACtx) = 0;
virtual bool hasResourceStateChanged(ProgramStateRef CallEnterState,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 41187ee9b5cdf..32ec4cbc236f1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -746,12 +746,22 @@ class NoStreamStateChangeVisitor final : public
NoOwnershipChangeVisitor {
return StreamChk->FCloseDesc.matchesAsWritten(Call);
}
- bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
+ bool doesFnIntendToHandleOwnership(const Decl *Callee,
ASTContext &ACtx) final {
+ const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
+
+ // Given that the stack frame was entered, the body should always be
+ // theoretically obtainable. In case of body farms, the synthesized body
+ // is not attached to declaration, thus triggering the '!FD->hasBody()'
+ // branch. That said, would a synthesized body ever intend to handle
+ // ownership? As of today they don't. And if they did, how would we
+ // put notes inside it, given that it doesn't match any source locations?
+ if (!FD || !FD->hasBody())
+ return false;
using namespace clang::ast_matchers;
auto Matches =
- match(findAll(callExpr().bind("call")), Callee->getBody(), ACtx);
+ match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
for (BoundNodes Match : Matches) {
if (const auto *Call = Match.getNodeAs<CallExpr>("call"))
if (isClosingCallAsWritten(*Call))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
