Detect a return value of Ref<T> & RefPtr<T> #81580
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesThis PR makes the checker not emit warning when a function is called with a return value of another function when the return value is of type Ref<T> or RefPtr<T>. Full diff: https://github.com/llvm/llvm-project/pull/81580.diff 7 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 4526fac64735bf..b76c0551c77bb0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -19,6 +19,10 @@ namespace clang {
std::pair<const Expr *, bool>
tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) {
while (E) {
+ if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
+ E = tempExpr->getSubExpr();
+ continue;
+ }
if (auto *cast = dyn_cast<CastExpr>(E)) {
if (StopAtFirstRefCountedObj) {
if (auto *ConversionFunc =
@@ -62,6 +66,8 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) {
E = call->getArg(0);
continue;
}
+ if (isReturnValueRefCounted(callee))
+ return {E, true};
if (isPtrConversion(callee)) {
E = call->getArg(0);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index d2b66341058000..9e1035ac13ce31 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -118,6 +118,26 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
|| FunctionName == "Identifier";
}
+bool isReturnValueRefCounted(const clang::FunctionDecl *F) {
+ assert(F);
+ auto *type = F->getReturnType().getTypePtrOrNull();
+ while (type) {
+ if (auto *elaboratedT = dyn_cast<ElaboratedType>(type)) {
+ type = elaboratedT->desugar().getTypePtrOrNull();
+ continue;
+ }
+ if (auto *specialT = dyn_cast<TemplateSpecializationType>(type)) {
+ if (auto* decl = specialT->getTemplateName().getAsTemplateDecl()) {
+ auto name = decl->getNameAsString();
+ return name == "Ref" || name == "RefPtr";
+ }
+ return false;
+ }
+ return false;
+ }
+ return false;
+}
+
std::optional<bool> isUncounted(const CXXRecordDecl* Class)
{
// Keep isRefCounted first as it's cheaper.
@@ -192,8 +212,9 @@ bool isPtrConversion(const FunctionDecl *F) {
// FIXME: check # of params == 1
const auto FunctionName = safeGetName(F);
if (FunctionName == "getPtr" || FunctionName == "WeakPtr" ||
- FunctionName == "dynamicDowncast"
- || FunctionName == "downcast" || FunctionName == "bitwise_cast")
+ FunctionName == "dynamicDowncast" || FunctionName == "downcast" ||
+ FunctionName == "checkedDowncast" ||
+ FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast")
return true;
return false;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 45b21cc0918443..c2c5b74442ba43 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -50,6 +50,9 @@ std::optional<bool> isUncountedPtr(const clang::Type* T);
/// false if not.
bool isCtorOfRefCounted(const clang::FunctionDecl *F);
+/// \returns true if \p F returns a ref-counted object, false if not.
+bool isReturnValueRefCounted(const clang::FunctionDecl *F);
+
/// \returns true if \p M is getter of a ref-counted class, false if not.
std::optional<bool> isGetterOfRefCounted(const clang::CXXMethodDecl* Method);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 31ccae8b097b89..cc4585a0b0eeff 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -148,13 +148,14 @@ class UncountedCallArgsChecker
auto name = safeGetName(Callee);
if (name == "adoptRef" || name == "getPtr" || name == "WeakPtr" ||
- name == "dynamicDowncast" || name == "downcast" || name == "bitwise_cast" ||
- name == "is" || name == "equal" || name == "hash" ||
- name == "isType"
+ name == "dynamicDowncast" || name == "downcast" ||
+ name == "checkedDowncast" || name == "uncheckedDowncast" ||
+ name == "bitwise_cast" || name == "is" || name == "equal" ||
+ name == "hash" || name == "isType" ||
// FIXME: Most/all of these should be implemented via attributes.
- || name == "equalIgnoringASCIICase" ||
+ name == "equalIgnoringASCIICase" ||
name == "equalIgnoringASCIICaseCommon" ||
- name == "equalIgnoringNullity")
+ name == "equalIgnoringNullity" || name == "toString")
return true;
return false;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp
new file mode 100644
index 00000000000000..1c4b3df211b1e3
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-protected-return-value.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+// expected-no-diagnostics
+
+#include "mock-types.h"
+
+class RefCounted {
+public:
+ void ref();
+ void deref();
+};
+
+class Object {
+public:
+ void someFunction(RefCounted&);
+};
+
+RefPtr<Object> object();
+RefPtr<RefCounted> protectedTargetObject();
+
+void testFunction() {
+ if (RefPtr obj = object())
+ obj->someFunction(*protectedTargetObject());
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-dynamic-downcast.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
similarity index 55%
rename from clang/test/Analysis/Checkers/WebKit/call-args-dynamic-downcast.cpp
rename to clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
index 28156623d9a0fd..a87446564870cd 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args-dynamic-downcast.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-safe-functions.cpp
@@ -23,13 +23,34 @@ class OtherObject {
Derived* obj();
};
+class String {
+};
+
template<typename Target, typename Source>
inline Target* dynamicDowncast(Source* source)
{
return static_cast<Target*>(source);
}
+template<typename Target, typename Source>
+inline Target* checkedDowncast(Source* source)
+{
+ return static_cast<Target*>(source);
+}
+
+template<typename Target, typename Source>
+inline Target* uncheckedDowncast(Source* source)
+{
+ return static_cast<Target*>(source);
+}
+
+template<typename... Types>
+String toString(const Types&... values);
+
void foo(OtherObject* other)
{
dynamicDowncast<SubDerived>(other->obj());
+ checkedDowncast<SubDerived>(other->obj());
+ uncheckedDowncast<SubDerived>(other->obj());
+ toString(other->obj());
}
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 5f570b8bee8cb8..814e0944145992 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -20,6 +20,7 @@ template <typename T> struct RefPtr {
T *operator->() { return t; }
T &operator*() { return *t; }
RefPtr &operator=(T *) { return *this; }
+ operator bool() { return t; }
};
template <typename T> bool operator==(const RefPtr<T> &, const RefPtr<T> &) {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
rniwa
force-pushed
the
detect-ref-ptr-return-value
branch
2 times, most recently
from
51fb3aa
to
478c7d2
Compare
…<T>` & `RefPtr<T>` This PR makes the checker not emit warning when a function is called with a return value of another function when the return value is of type `Ref<T>` or `RefPtr<T>`.
rniwa
force-pushed
the
detect-ref-ptr-return-value
branch
from
478c7d2
to
24fc757
Compare
haoNoQ
reviewed
Feb 13, 2024
|
Nvm everything's fine. |
haoNoQ
pushed a commit
to haoNoQ/llvm-project
that referenced
this pull request
Feb 14, 2024
This PR makes the checker not emit warning when a function is called with a return value of another function when the return value is of type Ref<T> or RefPtr<T>. (cherry picked from commit 7249692)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes the checker not emit warning when a function is called with a return value of another function when the return value is of type Ref or RefPtr.