From 33533baf631a4c6ea9b04eb1dda0090f80d143c5 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Tue, 17 Sep 2024 18:39:22 -0700 Subject: [PATCH] [alpha.webkit.UncountedCallArgsChecker] Add support for Objective-C++ property access (#108669) Treat a function call or property access via a Objective-C++ selector which returns a Ref/RefPtr as safe. --- .../StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 15 ++++++++++++++- .../Checkers/WebKit/PtrTypesSemantics.cpp | 5 ++--- .../Checkers/WebKit/PtrTypesSemantics.h | 4 ++-- .../Checkers/WebKit/uncounted-obj-arg.mm | 17 +++++++++++++++++ 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index be07cf51eefb3d..394cb26f03cf99 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 f48b2fd9dca71b..9da3e54e454317 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 2932e62ad06e4b..e2d0342bebd52c 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 db0c5b19eec5bb..9ad1880e9d1188 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm @@ -24,3 +24,20 @@ - (void)execute { } @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(); +}