From 241fde13679b85e63126318520cd3f19029f49ad Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Fri, 13 Sep 2024 19:01:57 -0700 Subject: [PATCH 1/2] [alpha.webkit.UncountedCallArgsChecker] Add support for Objective-C++ property access Treat a function call or property access via a Objective-C++ selector which returns a Ref/RefPtr as safe. --- .../Checkers/WebKit/ASTUtils.cpp | 15 ++++++++++++- .../Checkers/WebKit/PtrTypesSemantics.cpp | 5 ++--- .../Checkers/WebKit/PtrTypesSemantics.h | 4 ++-- .../Checkers/WebKit/uncounted-obj-arg.mm | 22 +++++++++++++++++++ 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index be07cf51eefb3..394cb26f03cf9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -12,6 +12,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/ExprObjC.h" #include namespace clang { @@ -35,6 +36,12 @@ bool tryToFindPtrOrigin( break; } } + if (auto *POE = dyn_cast(E)) { + if (auto *RF = POE->getResultExpr()) { + E = RF; + continue; + } + } if (auto *tempExpr = dyn_cast(E)) { E = tempExpr->getSubExpr(); continue; @@ -88,7 +95,7 @@ bool tryToFindPtrOrigin( continue; } - if (isReturnValueRefCounted(callee)) + if (isRefType(callee->getReturnType())) return callback(E, true); if (isSingleton(callee)) @@ -100,6 +107,12 @@ bool tryToFindPtrOrigin( } } } + if (auto *ObjCMsgExpr = dyn_cast(E)) { + if (auto *Method = ObjCMsgExpr->getMethodDecl()) { + if (isRefType(Method->getReturnType())) + return callback(E, true); + } + } if (auto *unaryOp = dyn_cast(E)) { // FIXME: Currently accepts ANY unary operator. Is it OK? E = unaryOp->getSubExpr(); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index f48b2fd9dca71..9da3e54e45431 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -123,9 +123,8 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) { || FunctionName == "Identifier"; } -bool isReturnValueRefCounted(const clang::FunctionDecl *F) { - assert(F); - QualType type = F->getReturnType(); +bool isRefType(const clang::QualType T) { + QualType type = T; while (!type.isNull()) { if (auto *elaboratedT = type->getAs()) { type = elaboratedT->desugar(); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 2932e62ad06e4..e2d0342bebd52 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -62,8 +62,8 @@ bool isRefType(const std::string &Name); /// 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 T is RefPtr, Ref, or its variant, false if not. +bool isRefType(const clang::QualType T); /// \returns true if \p M is getter of a ref-counted class, false if not. std::optional isGetterOfRefCounted(const clang::CXXMethodDecl* Method); diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm index db0c5b19eec5b..1d81a8851ece0 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm @@ -11,6 +11,7 @@ @interface Foo : NSObject - (void)execute; - (RefPtr)_protectedRefCountable; +- (Ref)_protectedRefCountable2; @end @implementation Foo @@ -23,4 +24,25 @@ - (void)execute { return _countable; } +- (Ref)_protectedRefCountable2 { + return *_countable; +} + @end + +class RefCountedObject { +public: + void ref() const; + void deref() const; + Ref copy() const; +}; + +@interface WrapperObj : NSObject + +- (Ref)_protectedWebExtensionControllerConfiguration; + +@end + +static void foo(WrapperObj *configuration) { + configuration._protectedWebExtensionControllerConfiguration->copy(); +} From 89565cebeda804a34359f75bb4fd64154aa429ac Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Fri, 13 Sep 2024 19:04:48 -0700 Subject: [PATCH 2/2] Remove the superflous method. --- clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm | 5 ----- 1 file changed, 5 deletions(-) diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm index 1d81a8851ece0..9ad1880e9d118 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm @@ -11,7 +11,6 @@ @interface Foo : NSObject - (void)execute; - (RefPtr)_protectedRefCountable; -- (Ref)_protectedRefCountable2; @end @implementation Foo @@ -24,10 +23,6 @@ - (void)execute { return _countable; } -- (Ref)_protectedRefCountable2 { - return *_countable; -} - @end class RefCountedObject {