llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> Generalize the check for recognizing [[Obj alloc] init] to also recognize [allocObj() init]. We do this by utilizing isAllocInit function in RetainPtrCtorAdoptChecker. --- Full diff: https://github.com/llvm/llvm-project/pull/161019.diff 5 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+45) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h (+4) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp (+2-7) - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp (-44) - (modified) clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm (+3) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 00a1b8b6e7e89..455871690ceb2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -301,6 +301,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) { return result && *result; } +bool isAllocInit(const Expr *E, const Expr **InnerExpr) { + auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E); + if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) { + if (unsigned ExprCount = POE->getNumSemanticExprs()) { + auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); + ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(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<ObjCMessageExpr>(Receiver)) { + if (InnerExpr) + *InnerExpr = Inner; + auto InnerSelector = Inner->getSelector(); + return InnerSelector.getNameForSlot(0).starts_with("alloc"); + } else if (auto *CE = dyn_cast<CallExpr>(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<EnsureFunctionVisitor, bool> { public: diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index 3a009d65efea6..209b59de6c26e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -76,6 +76,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 9585ceb40f95e..6419ed843ac20 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -176,16 +176,11 @@ class RawPtrRefCallArgsChecker if (BR->getSourceManager().isInSystemHeader(E->getExprLoc())) return; - auto Selector = E->getSelector(); if (auto *Receiver = E->getInstanceReceiver()) { std::optional<bool> IsUnsafe = isUnsafePtr(E->getReceiverType()); if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) { - if (auto *InnerMsg = dyn_cast<ObjCMessageExpr>(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 e1f9a77f5a5ca..02aebfc61f4c0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp @@ -419,50 +419,6 @@ class RetainPtrCtorAdoptChecker return std::nullopt; } - bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr) const { - auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E); - if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) { - if (unsigned ExprCount = POE->getNumSemanticExprs()) { - auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); - ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(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<ObjCMessageExpr>(Receiver)) { - if (InnerExpr) - *InnerExpr = Inner; - auto InnerSelector = Inner->getSelector(); - return InnerSelector.getNameForSlot(0) == "alloc"; - } else if (auto *CE = dyn_cast<CallExpr>(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<CallExpr>(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 c9d2fe861bb49..2986580dbcef4 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -561,6 +561,8 @@ void foo() { } // namespace ns_retained_return_value +SomeObj *allocObj(); + @interface TestObject : NSObject - (void)doWork:(NSString *)msg, ...; - (void)doWorkOnSelf; @@ -582,6 +584,7 @@ - (void)doWorkOnSelf { [self doWork:@"hello", RetainPtr<SomeObj> { provide() }.get(), RetainPtr<CFMutableArrayRef> { provide_cf() }.get(), OSObjectPtr { provide_dispatch() }.get()]; [self doWork:__null]; [self doWork:nil]; + adoptNS([allocObj() init]); } - (SomeObj *)getSomeObj { `````````` </details> https://github.com/llvm/llvm-project/pull/161019 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
