From 569781528eb317a9d186d2b7d19bb0468d177d27 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Sat, 27 Sep 2025 13:04:11 -0700 Subject: [PATCH] [alpha.webkit.UnretainedCallArgsChecker] Recognize [allocObj() init] pattern Generalize the check for recognizing [[Obj alloc] init] to also recognize [allocObj() init]. We do this by utilizing isAllocInit function in RetainPtrCtorAdoptChecker. --- .../Checkers/WebKit/ASTUtils.cpp | 45 +++++++++++++++++++ .../StaticAnalyzer/Checkers/WebKit/ASTUtils.h | 4 ++ .../WebKit/RawPtrRefCallArgsChecker.cpp | 9 +--- .../WebKit/RetainPtrCtorAdoptChecker.cpp | 44 ------------------ .../Checkers/WebKit/unretained-call-args.mm | 3 ++ 5 files changed, 54 insertions(+), 51 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 84adbf318e9f8..e46bba7be4aea 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -321,6 +321,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) { return result && *result; } +bool isAllocInit(const Expr *E, const Expr **InnerExpr) { + auto *ObjCMsgExpr = dyn_cast(E); + if (auto *POE = dyn_cast(E)) { + if (unsigned ExprCount = POE->getNumSemanticExprs()) { + auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); + ObjCMsgExpr = dyn_cast(Expr); + if (InnerExpr) + *InnerExpr = ObjCMsgExpr; + } + } + if (!ObjCMsgExpr) + return false; + auto Selector = ObjCMsgExpr->getSelector(); + auto NameForFirstSlot = Selector.getNameForSlot(0); + if (NameForFirstSlot.starts_with("alloc") || + NameForFirstSlot.starts_with("copy") || + NameForFirstSlot.starts_with("mutableCopy")) + return true; + if (!NameForFirstSlot.starts_with("init") && + !NameForFirstSlot.starts_with("_init")) + return false; + if (!ObjCMsgExpr->isInstanceMessage()) + return false; + auto *Receiver = ObjCMsgExpr->getInstanceReceiver(); + if (!Receiver) + return false; + Receiver = Receiver->IgnoreParenCasts(); + if (auto *Inner = dyn_cast(Receiver)) { + if (InnerExpr) + *InnerExpr = Inner; + auto InnerSelector = Inner->getSelector(); + return InnerSelector.getNameForSlot(0).starts_with("alloc"); + } else if (auto *CE = dyn_cast(Receiver)) { + if (InnerExpr) + *InnerExpr = CE; + if (auto *Callee = CE->getDirectCallee()) { + if (Callee->getDeclName().isIdentifier()) { + auto CalleeName = Callee->getName(); + return CalleeName.starts_with("alloc"); + } + } + } + return false; +} + class EnsureFunctionVisitor : public ConstStmtVisitor { public: diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index 9fff456b7e8b8..d0a3e471365e2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -77,6 +77,10 @@ bool isConstOwnerPtrMemberExpr(const clang::Expr *E); /// supports CheckedPtr. bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E); +/// \returns true if \p E is a [[alloc] init] pattern expression. +/// Sets \p InnerExpr to the inner function call or selector invocation. +bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr); + /// \returns true if E is a CXXMemberCallExpr which returns a const smart /// pointer type. class EnsureFunctionAnalysis { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index 791e70998477f..dcc14a0aecdf7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -177,16 +177,11 @@ class RawPtrRefCallArgsChecker if (BR->getSourceManager().isInSystemHeader(E->getExprLoc())) return; - auto Selector = E->getSelector(); if (auto *Receiver = E->getInstanceReceiver()) { std::optional IsUnsafe = isUnsafePtr(E->getReceiverType()); if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) { - if (auto *InnerMsg = dyn_cast(Receiver)) { - auto InnerSelector = InnerMsg->getSelector(); - if (InnerSelector.getNameForSlot(0) == "alloc" && - Selector.getNameForSlot(0).starts_with("init")) - return; - } + if (isAllocInit(E)) + return; reportBugOnReceiver(Receiver, D); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp index 955b8d19a820c..e10ad3242fe79 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp @@ -423,50 +423,6 @@ class RetainPtrCtorAdoptChecker return std::nullopt; } - bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr) const { - auto *ObjCMsgExpr = dyn_cast(E); - if (auto *POE = dyn_cast(E)) { - if (unsigned ExprCount = POE->getNumSemanticExprs()) { - auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); - ObjCMsgExpr = dyn_cast(Expr); - if (InnerExpr) - *InnerExpr = ObjCMsgExpr; - } - } - if (!ObjCMsgExpr) - return false; - auto Selector = ObjCMsgExpr->getSelector(); - auto NameForFirstSlot = Selector.getNameForSlot(0); - if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") || - NameForFirstSlot.starts_with("mutableCopy")) - return true; - if (!NameForFirstSlot.starts_with("init") && - !NameForFirstSlot.starts_with("_init")) - return false; - if (!ObjCMsgExpr->isInstanceMessage()) - return false; - auto *Receiver = ObjCMsgExpr->getInstanceReceiver(); - if (!Receiver) - return false; - Receiver = Receiver->IgnoreParenCasts(); - if (auto *Inner = dyn_cast(Receiver)) { - if (InnerExpr) - *InnerExpr = Inner; - auto InnerSelector = Inner->getSelector(); - return InnerSelector.getNameForSlot(0) == "alloc"; - } else if (auto *CE = dyn_cast(Receiver)) { - if (InnerExpr) - *InnerExpr = CE; - if (auto *Callee = CE->getDirectCallee()) { - if (Callee->getDeclName().isIdentifier()) { - auto CalleeName = Callee->getName(); - return CalleeName.starts_with("alloc"); - } - } - } - return false; - } - bool isCreateOrCopy(const Expr *E) const { auto *CE = dyn_cast(E); if (!CE) diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 8bef24f93ceed..cfc214fae33e5 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -625,6 +625,8 @@ void foo() { } // namespace template_function +SomeObj *allocObj(); + @interface TestObject : NSObject - (void)doWork:(NSString *)msg, ...; - (void)doWorkOnSelf; @@ -647,6 +649,7 @@ - (void)doWorkOnSelf { [self doWork:__null]; [self doWork:nil]; [NSApp run]; + adoptNS([allocObj() init]); } - (SomeObj *)getSomeObj {