From 3023c391b092d718ea13b3467f2a0a1b38e80410 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 16 Oct 2025 00:51:05 -0700 Subject: [PATCH 01/13] [webkit.UncountedLambdaCapturesChecker] Treat arguments of std::ranges::all_of as [[clang::noescape]] (#158419) The checker already had std::ranges hard-coded to treat its arguments as [[clang::oescape]] but the fact std::ranges::all_of is implemented as a struct instead of a function confused the checker and resuled in a superflous warning being emitted for std::ranges::all_of. This PR adds the support for recognizing DeclRefExpr which appears as a callee in VisitCallExpr and generalizes the check in shouldTreatAllArgAsNoEscape to walk up the decl contexts to find the target namespaces such as std::ranges:: or a namespace and a function like WTF::switchOn. (cherry picked from commit 725d9e80770e4a8e5888e38a380ff0f7033aa9e0) --- .../WebKit/RawPtrRefLambdaCapturesChecker.cpp | 64 +++++++++++-------- .../WebKit/uncounted-lambda-captures.cpp | 35 +++++++++- 2 files changed, 69 insertions(+), 30 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 03eeb9999c4dd..63f0ed4a54fb5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -126,20 +126,21 @@ class RawPtrRefLambdaCapturesChecker return true; } - bool shouldTreatAllArgAsNoEscape(FunctionDecl *Decl) { - auto *NsDecl = Decl->getParent(); - if (!NsDecl || !isa(NsDecl)) - return false; - // WTF::switchOn(T, F... f) is a variadic template function and couldn't - // be annotated with NOESCAPE. We hard code it here to workaround that. - if (safeGetName(NsDecl) == "WTF" && safeGetName(Decl) == "switchOn") - return true; - // Treat every argument of functions in std::ranges as noescape. - if (safeGetName(NsDecl) == "ranges") { - if (auto *OuterDecl = NsDecl->getParent(); - OuterDecl && isa(OuterDecl) && - safeGetName(OuterDecl) == "std") + bool shouldTreatAllArgAsNoEscape(FunctionDecl *FDecl) { + std::string PreviousName = safeGetName(FDecl); + for (auto *Decl = FDecl->getParent(); Decl; Decl = Decl->getParent()) { + if (!isa(Decl) && !isa(Decl)) + return false; + auto Name = safeGetName(Decl); + // WTF::switchOn(T, F... f) is a variadic template function and + // couldn't be annotated with NOESCAPE. We hard code it here to + // workaround that. + if (Name == "WTF" && PreviousName == "switchOn") return true; + // Treat every argument of functions in std::ranges as noescape. + if (Name == "std" && PreviousName == "ranges") + return true; + PreviousName = Name; } return false; } @@ -167,25 +168,34 @@ class RawPtrRefLambdaCapturesChecker bool VisitCallExpr(CallExpr *CE) override { checkCalleeLambda(CE); - if (auto *Callee = CE->getDirectCallee()) { - unsigned ArgIndex = isa(CE); - bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee); - for (auto *Param : Callee->parameters()) { - if (ArgIndex >= CE->getNumArgs()) - return true; - auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts(); - if (auto *L = findLambdaInArg(Arg)) { - LambdasToIgnore.insert(L); - if (!Param->hasAttr() && !TreatAllArgsAsNoEscape) - Checker->visitLambdaExpr( - L, shouldCheckThis() && !hasProtectedThis(L), ClsType); - } - ++ArgIndex; + if (auto *Callee = CE->getDirectCallee()) + checkParameters(CE, Callee); + else if (auto *CalleeE = CE->getCallee()) { + if (auto *DRE = dyn_cast(CalleeE->IgnoreParenCasts())) { + if (auto *Callee = dyn_cast_or_null(DRE->getDecl())) + checkParameters(CE, Callee); } } return true; } + void checkParameters(CallExpr *CE, FunctionDecl *Callee) { + unsigned ArgIndex = isa(CE); + bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee); + for (auto *Param : Callee->parameters()) { + if (ArgIndex >= CE->getNumArgs()) + return; + auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts(); + if (auto *L = findLambdaInArg(Arg)) { + LambdasToIgnore.insert(L); + if (!Param->hasAttr() && !TreatAllArgsAsNoEscape) + Checker->visitLambdaExpr( + L, shouldCheckThis() && !hasProtectedThis(L), ClsType); + } + ++ArgIndex; + } + } + LambdaExpr *findLambdaInArg(Expr *E) { if (auto *Lambda = dyn_cast_or_null(E)) return Lambda; diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp index 0b8af0d1e8dc1..a4ad741182f56 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp @@ -17,6 +17,18 @@ void for_each(IteratorType first, IteratorType last, CallbackType callback) { callback(*it); } +struct all_of_impl { + template + constexpr bool operator()(const Collection& collection, Predicate predicate) const { + for (auto it = collection.begin(); it != collection.end(); ++it) { + if (!predicate(*it)) + return false; + } + return true; + } +}; +inline constexpr auto all_of = all_of_impl {}; + } } @@ -435,7 +447,7 @@ class Iterator { bool operator==(const Iterator&); Iterator& operator++(); - void* operator*(); + int& operator*(); private: void* current { nullptr }; @@ -444,22 +456,39 @@ class Iterator { void ranges_for_each(RefCountable* obj) { int array[] = { 1, 2, 3, 4, 5 }; - std::ranges::for_each(Iterator(array, sizeof(*array), 0), Iterator(array, sizeof(*array), 5), [&](void* item) { + std::ranges::for_each(Iterator(array, sizeof(*array), 0), Iterator(array, sizeof(*array), 5), [&](int& item) { obj->method(); - ++(*static_cast(item)); + ++item; }); } +class IntCollection { +public: + int* begin(); + int* end(); + const int* begin() const; + const int* end() const; +}; + class RefCountedObj { public: void ref(); void deref(); + bool allOf(const IntCollection&); + bool isMatch(int); + void call() const; void callLambda([[clang::noescape]] const WTF::Function& callback) const; void doSomeWork() const; }; +bool RefCountedObj::allOf(const IntCollection& collection) { + return std::ranges::all_of(collection, [&](auto& number) { + return isMatch(number); + }); +} + void RefCountedObj::callLambda([[clang::noescape]] const WTF::Function& callback) const { callback(); From 6fbd9c4c1d78d472a95405d38bfc4a1cb550bf1b Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 18 Sep 2025 21:46:22 -0700 Subject: [PATCH 02/13] [WebKit checkers] Add the support for OSObjectPtr (#159484) Add the support for OSObjectPtr, which behaves like RetainPtr. (cherry picked from commit 9d933c794af32362dbbbefe53825054b533d7b2c) --- clang/docs/analyzer/checkers.rst | 20 ++- .../clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- .../Checkers/WebKit/ASTUtils.cpp | 2 +- .../Checkers/WebKit/ForwardDeclChecker.cpp | 2 +- .../Checkers/WebKit/PtrTypesSemantics.cpp | 29 +-- .../Checkers/WebKit/PtrTypesSemantics.h | 6 +- .../WebKit/RawPtrRefCallArgsChecker.cpp | 4 +- .../WebKit/RawPtrRefLambdaCapturesChecker.cpp | 2 +- .../WebKit/RawPtrRefLocalVarsChecker.cpp | 4 +- .../WebKit/RawPtrRefMemberChecker.cpp | 11 +- .../WebKit/RetainPtrCtorAdoptChecker.cpp | 16 +- .../Checkers/WebKit/call-args-checked.cpp | 14 -- .../Checkers/WebKit/forward-decl-checker.mm | 14 -- .../Analysis/Checkers/WebKit/mock-types.h | 19 ++ .../Checkers/WebKit/objc-mock-types.h | 165 ++++++++++++++++++ .../Checkers/WebKit/uncounted-obj-arg.cpp | 3 - .../WebKit/unretained-call-args-arc.mm | 15 +- .../Checkers/WebKit/unretained-call-args.mm | 95 ++++++++-- .../WebKit/unretained-lambda-captures-arc.mm | 49 +++++- .../WebKit/unretained-lambda-captures.mm | 67 ++++++- .../WebKit/unretained-local-vars-arc.mm | 19 +- .../Checkers/WebKit/unretained-local-vars.mm | 128 +++++++++++++- .../Checkers/WebKit/unretained-members-arc.mm | 24 ++- .../Checkers/WebKit/unretained-members.mm | 48 +++-- 24 files changed, 630 insertions(+), 128 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 26c5028e04955..8155c9c83cd0b 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3598,12 +3598,13 @@ See `WebKit Guidelines for Safer C++ Programming , Documentation; def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">, - HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">, + HelpText<"Check for correct use of RetainPtr/OSObjectPtr constructor, adoptNS, adoptCF, and adoptOSObject">, Documentation; } // end alpha.webkit diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 1b17226f02e30..7b277ef04b255 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -115,7 +115,7 @@ bool tryToFindPtrOrigin( if (auto *Callee = operatorCall->getDirectCallee()) { auto ClsName = safeGetName(Callee->getParent()); if (isRefType(ClsName) || isCheckedPtr(ClsName) || - isRetainPtr(ClsName) || ClsName == "unique_ptr" || + isRetainPtrOrOSPtr(ClsName) || ClsName == "unique_ptr" || ClsName == "UniqueRef" || ClsName == "WeakPtr" || ClsName == "WeakRef") { if (operatorCall->getNumArgs() == 1) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp index 9deb1845a2f1a..d8539eaaac49f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp @@ -162,7 +162,7 @@ class ForwardDeclChecker : public Checker> { // Ref-counted smartpointers actually have raw-pointer to uncounted type as // a member but we trust them to handle it correctly. auto R = llvm::dyn_cast_or_null(RD); - if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtr(R)) + if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtrOrOSPtr(R)) return; for (auto *Member : RD->fields()) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 511f4204d99cd..8bb701751e79e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -129,8 +129,9 @@ bool isRefType(const std::string &Name) { Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed"; } -bool isRetainPtr(const std::string &Name) { - return Name == "RetainPtr" || Name == "RetainPtrArc"; +bool isRetainPtrOrOSPtr(const std::string &Name) { + return Name == "RetainPtr" || Name == "RetainPtrArc" || + Name == "OSObjectPtr" || Name == "OSObjectPtrArc"; } bool isCheckedPtr(const std::string &Name) { @@ -138,7 +139,7 @@ bool isCheckedPtr(const std::string &Name) { } bool isSmartPtrClass(const std::string &Name) { - return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) || + return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) || Name == "WeakPtr" || Name == "WeakPtrFactory" || Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" || Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" || @@ -166,15 +167,17 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) { return isCheckedPtr(safeGetName(F)); } -bool isCtorOfRetainPtr(const clang::FunctionDecl *F) { +bool isCtorOfRetainPtrOrOSPtr(const clang::FunctionDecl *F) { const std::string &FunctionName = safeGetName(F); return FunctionName == "RetainPtr" || FunctionName == "adoptNS" || FunctionName == "adoptCF" || FunctionName == "retainPtr" || - FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc"; + FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc" || + FunctionName == "adoptOSObject" || FunctionName == "adoptOSObjectArc"; } bool isCtorOfSafePtr(const clang::FunctionDecl *F) { - return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) || isCtorOfRetainPtr(F); + return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) || + isCtorOfRetainPtrOrOSPtr(F); } template @@ -202,8 +205,8 @@ bool isRefOrCheckedPtrType(const clang::QualType T) { T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); }); } -bool isRetainPtrType(const clang::QualType T) { - return isPtrOfType(T, [](auto Name) { return isRetainPtr(Name); }); +bool isRetainPtrOrOSPtrType(const clang::QualType T) { + return isPtrOfType(T, [](auto Name) { return isRetainPtrOrOSPtr(Name); }); } bool isOwnerPtrType(const clang::QualType T) { @@ -286,7 +289,7 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) { std::optional isUnretained(const QualType T, bool IsARCEnabled) { if (auto *Subst = dyn_cast(T)) { if (auto *Decl = Subst->getAssociatedDecl()) { - if (isRetainPtr(safeGetName(Decl))) + if (isRetainPtrOrOSPtr(safeGetName(Decl))) return false; } } @@ -386,7 +389,7 @@ std::optional isGetterOfSafePtr(const CXXMethodDecl *M) { method == "impl")) return true; - if (isRetainPtr(className) && method == "get") + if (isRetainPtrOrOSPtr(className) && method == "get") return true; // Ref -> T conversion @@ -407,7 +410,7 @@ std::optional isGetterOfSafePtr(const CXXMethodDecl *M) { } } - if (isRetainPtr(className)) { + if (isRetainPtrOrOSPtr(className)) { if (auto *maybeRefToRawOperator = dyn_cast(M)) { auto QT = maybeRefToRawOperator->getConversionType(); auto *T = QT.getTypePtrOrNull(); @@ -438,10 +441,10 @@ bool isCheckedPtr(const CXXRecordDecl *R) { return false; } -bool isRetainPtr(const CXXRecordDecl *R) { +bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) { assert(R); if (auto *TmplR = R->getTemplateInstantiationPattern()) - return isRetainPtr(safeGetName(TmplR)); + return isRetainPtrOrOSPtr(safeGetName(TmplR)); return false; } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index d2095d07e1434..8300a6c051f3e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -57,7 +57,7 @@ bool isRefCounted(const clang::CXXRecordDecl *Class); bool isCheckedPtr(const clang::CXXRecordDecl *Class); /// \returns true if \p Class is a RetainPtr, false if not. -bool isRetainPtr(const clang::CXXRecordDecl *Class); +bool isRetainPtrOrOSPtr(const clang::CXXRecordDecl *Class); /// \returns true if \p Class is a smart pointer (RefPtr, WeakPtr, etc...), /// false if not. @@ -116,7 +116,7 @@ std::optional isUnsafePtr(const QualType T, bool IsArcEnabled); bool isRefOrCheckedPtrType(const clang::QualType T); /// \returns true if \p T is a RetainPtr, false if not. -bool isRetainPtrType(const clang::QualType T); +bool isRetainPtrOrOSPtrType(const clang::QualType T); /// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or /// unique_ptr, false if not. @@ -141,7 +141,7 @@ bool isRefType(const std::string &Name); bool isCheckedPtr(const std::string &Name); /// \returns true if \p Name is RetainPtr or its variant, false if not. -bool isRetainPtr(const std::string &Name); +bool isRetainPtrOrOSPtr(const std::string &Name); /// \returns true if \p Name is a smart pointer type name, false if not. bool isSmartPtrClass(const std::string &Name); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index e80f1749d595b..b841caf8c74b1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -465,11 +465,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker { } bool isSafePtr(const CXXRecordDecl *Record) const final { - return isRetainPtr(Record); + return isRetainPtrOrOSPtr(Record); } bool isSafePtrType(const QualType type) const final { - return isRetainPtrType(type); + return isRetainPtrOrOSPtrType(type); } bool isSafeExpr(const Expr *E) const final { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 63f0ed4a54fb5..033eb8cc299b0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -500,7 +500,7 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { } virtual bool isPtrType(const std::string &Name) const final { - return isRetainPtr(Name); + return isRetainPtrOrOSPtr(Name); } const char *ptrKind(QualType QT) const final { return "unretained"; } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index f4f6e28c97ea1..dd9701fbbb017 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -434,10 +434,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { return RTC->isUnretained(T); } bool isSafePtr(const CXXRecordDecl *Record) const final { - return isRetainPtr(Record); + return isRetainPtrOrOSPtr(Record); } bool isSafePtrType(const QualType type) const final { - return isRetainPtrType(type); + return isRetainPtrOrOSPtrType(type); } bool isSafeExpr(const Expr *E) const final { return ento::cocoa::isCocoaObjectRef(E->getType()) && diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index 8faf6a219450a..a97a37f85e96c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -119,10 +119,11 @@ class RawPtrRefMemberChecker auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); if (!Desugared) return nullptr; - auto *ObjCType = dyn_cast(Desugared); - if (!ObjCType) - return nullptr; - return ObjCType->getDecl(); + if (auto *ObjCType = dyn_cast(Desugared)) + return ObjCType->getDecl(); + if (auto *ObjCType = dyn_cast(Desugared)) + return ObjCType->getInterface(); + return nullptr; } void visitObjCDecl(const ObjCContainerDecl *CD) const { @@ -369,7 +370,7 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker { const char *typeName() const final { return "retainable type"; } const char *invariant() const final { - return "member variables must be a RetainPtr"; + return "member variables must be a RetainPtr or OSObjectPtr"; } PrintDeclKind printPointer(llvm::raw_svector_ostream &Os, diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp index 5c1b2d7cce45d..e1f9a77f5a5ca 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp @@ -67,7 +67,7 @@ class RetainPtrCtorAdoptChecker } bool TraverseClassTemplateDecl(ClassTemplateDecl *CTD) { - if (isRetainPtr(safeGetName(CTD))) + if (isRetainPtrOrOSPtr(safeGetName(CTD))) return true; // Skip the contents of RetainPtr. return Base::TraverseClassTemplateDecl(CTD); } @@ -121,7 +121,8 @@ class RetainPtrCtorAdoptChecker } bool isAdoptFnName(const std::string &Name) const { - return isAdoptNS(Name) || Name == "adoptCF" || Name == "adoptCFArc"; + return isAdoptNS(Name) || Name == "adoptCF" || Name == "adoptCFArc" || + Name == "adoptOSObject" || Name == "adoptOSObjectArc"; } bool isAdoptNS(const std::string &Name) const { @@ -304,7 +305,7 @@ class RetainPtrCtorAdoptChecker if (!Cls) return; - if (!isRetainPtr(safeGetName(Cls)) || !CE->getNumArgs()) + if (!isRetainPtrOrOSPtr(safeGetName(Cls)) || !CE->getNumArgs()) return; // Ignore RetainPtr construction inside adoptNS, adoptCF, and retainPtr. @@ -491,7 +492,7 @@ class RetainPtrCtorAdoptChecker return IsOwnedResult::NotOwned; if (auto *DRE = dyn_cast(E)) { auto QT = DRE->getType(); - if (isRetainPtrType(QT)) + if (isRetainPtrOrOSPtrType(QT)) return IsOwnedResult::NotOwned; QT = QT.getCanonicalType(); if (RTC.isUnretained(QT, true /* ignoreARC */)) @@ -530,12 +531,13 @@ class RetainPtrCtorAdoptChecker if (auto *CD = dyn_cast(MD)) { auto QT = CD->getConversionType().getCanonicalType(); auto *ResultType = QT.getTypePtrOrNull(); - if (isRetainPtr(safeGetName(Cls)) && ResultType && + if (isRetainPtrOrOSPtr(safeGetName(Cls)) && ResultType && (ResultType->isPointerType() || ResultType->isReferenceType() || ResultType->isObjCObjectPointerType())) return IsOwnedResult::NotOwned; } - if (safeGetName(MD) == "leakRef" && isRetainPtr(safeGetName(Cls))) + if (safeGetName(MD) == "leakRef" && + isRetainPtrOrOSPtr(safeGetName(Cls))) return IsOwnedResult::Owned; } } @@ -553,7 +555,7 @@ class RetainPtrCtorAdoptChecker continue; } auto RetType = Callee->getReturnType(); - if (isRetainPtrType(RetType)) + if (isRetainPtrOrOSPtrType(RetType)) return IsOwnedResult::NotOwned; if (isCreateOrCopyFunction(Callee)) { CreateOrCopyFnCall.insert(CE); diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp index b2fb042e7deff..e9f01c8ed7540 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp @@ -2,20 +2,6 @@ #include "mock-types.h" -namespace std { - -template struct remove_reference { - typedef T type; -}; - -template struct remove_reference { - typedef T type; -}; - -template typename remove_reference::type&& move(T&& t); - -} // namespace std - RefCountableAndCheckable* makeObj(); CheckedRef makeObjChecked(); void someFunction(RefCountableAndCheckable*); diff --git a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm index 084b47322d7f9..104b555c1c41d 100644 --- a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm +++ b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm @@ -4,20 +4,6 @@ #include "objc-mock-types.h" #include "mock-system-header.h" -namespace std { - -template struct remove_reference { - typedef T type; -}; - -template struct remove_reference { - typedef T type; -}; - -template typename remove_reference::type&& move(T&& t); - -} // namespace std - typedef struct OpaqueJSString * JSStringRef; class Obj; diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index a03d31870ee0d..a49faa1d25336 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -1,3 +1,22 @@ +#ifndef std_move +#define std_move + +namespace std { + +template struct remove_reference { +typedef T type; +}; + +template struct remove_reference { +typedef T type; +}; + +template typename remove_reference::type&& move(T&& t); + +} + +#endif + #ifndef mock_types_1103988513531 #define mock_types_1103988513531 diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h index 09b303961fd6a..854742b82a2d4 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -1,3 +1,22 @@ +#ifndef std_move +#define std_move + +namespace std { + +template struct remove_reference { +typedef T type; +}; + +template struct remove_reference { +typedef T type; +}; + +template typename remove_reference::type&& move(T&& t); + +} + +#endif + @class NSString; @class NSArray; @class NSMutableArray; @@ -157,6 +176,39 @@ __attribute__((objc_root_class)) - (void)doMoreWork:(OtherObj *)other; @end +@protocol OS_dispatch_queue +@end + +typedef NSObject *dispatch_queue_t; + +@protocol OS_dispatch_queue_attr +@end + +typedef NSObject *dispatch_queue_attr_t; + +NS_RETURNS_RETAINED dispatch_queue_t dispatch_queue_create(const char *label, dispatch_queue_attr_t attr); +const char *dispatch_queue_get_label(dispatch_queue_t queue); + +namespace std { + +template +void swap(StorageType& a, StorageType& b) +{ + StorageType temp = static_cast(a); + a = static_cast(b); + b = static_cast(temp); +} + +template +StorageType exchange(StorageType& obj, ValueType& value) +{ + StorageType returnValue = static_cast(obj); + obj = static_cast(value); + return returnValue; +} + +} + namespace WTF { void WTFCrash(void); @@ -293,6 +345,117 @@ template inline RetainPtr retainPtr(T* ptr) return ptr; } +template class OSObjectPtr; +template OSObjectPtr adoptOSObject(T); + +template static inline void retainOSObject(T ptr) +{ +#if !__has_feature(objc_arc) + [ptr retain]; +#endif +} + +template static inline void releaseOSObject(T ptr) +{ +#if !__has_feature(objc_arc) + [ptr release]; +#endif +} + +template class OSObjectPtr { +public: + OSObjectPtr() + : m_ptr(nullptr) + { + } + + ~OSObjectPtr() + { + if (m_ptr) + releaseOSObject(m_ptr); + } + + T get() const { return m_ptr; } + + explicit operator bool() const { return m_ptr; } + bool operator!() const { return !m_ptr; } + + OSObjectPtr(const OSObjectPtr& other) + : m_ptr(other.m_ptr) + { + if (m_ptr) + retainOSObject(m_ptr); + } + + OSObjectPtr(OSObjectPtr&& other) + : m_ptr(std::move(other.m_ptr)) + { + other.m_ptr = nullptr; + } + + OSObjectPtr(T ptr) + : m_ptr(std::move(ptr)) + { + if (m_ptr) + retainOSObject(m_ptr); + } + + OSObjectPtr& operator=(const OSObjectPtr& other) + { + OSObjectPtr ptr = other; + swap(ptr); + return *this; + } + + OSObjectPtr& operator=(OSObjectPtr&& other) + { + OSObjectPtr ptr = std::move(other); + swap(ptr); + return *this; + } + + OSObjectPtr& operator=(decltype(nullptr)) + { + if (m_ptr) + releaseOSObject(m_ptr); + m_ptr = nullptr; + return *this; + } + + OSObjectPtr& operator=(T other) + { + OSObjectPtr ptr = std::move(other); + swap(ptr); + return *this; + } + + void swap(OSObjectPtr& other) + { + std::swap(m_ptr, other.m_ptr); + } + + T leakRef() + { + return std::exchange(m_ptr, nullptr); + } + + friend OSObjectPtr adoptOSObject(T); + +private: + struct AdoptOSObject { }; + OSObjectPtr(AdoptOSObject, T ptr) + : m_ptr(std::move(ptr)) + { + } + + T m_ptr; +}; + +template inline OSObjectPtr adoptOSObject(T ptr) +{ + return OSObjectPtr { typename OSObjectPtr::AdoptOSObject { }, std::move(ptr) }; +} + inline NSObject *bridge_cast(CFTypeRef object) { return (__bridge NSObject *)object; @@ -455,6 +618,8 @@ using WTF::RetainPtr; using WTF::adoptNS; using WTF::adoptCF; using WTF::retainPtr; +using WTF::OSObjectPtr; +using WTF::adoptOSObject; using WTF::downcast; using WTF::bridge_cast; using WTF::bridge_id_cast; diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp index 2c6ccb55e2ce8..a9cd77c066f6f 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp @@ -74,9 +74,6 @@ T* addressof(T& arg); template T&& forward(T& arg); -template -T&& move( T&& t ); - template ToType bit_cast(FromType from); diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm index fa866258a2f6d..2763d8a188d8a 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm @@ -4,6 +4,7 @@ SomeObj *provide(); CFMutableArrayRef provide_cf(); +dispatch_queue_t provide_os(); void someFunction(); CGImageRef provideImage(); NSString *stringForImage(CGImageRef); @@ -14,6 +15,7 @@ void foo() { [provide() doWork]; CFArrayAppendValue(provide_cf(), nullptr); // expected-warning@-1{{Call argument for parameter 'theArray' is unretained and unsafe [alpha.webkit.UnretainedCallArgsChecker]}} + dispatch_queue_get_label(provide_os()); } } // namespace raw_ptr @@ -22,15 +24,17 @@ void foo() { extern NSString * const SomeConstant; extern CFDictionaryRef const SomeDictionary; -void doWork(NSString *str, CFDictionaryRef dict); +extern dispatch_queue_t const SomeDispatch; +void doWork(NSString *str, CFDictionaryRef dict, dispatch_queue_t dispatch); void use_const_global() { - doWork(SomeConstant, SomeDictionary); + doWork(SomeConstant, SomeDictionary, SomeDispatch); } NSString *provide_str(); CFDictionaryRef provide_dict(); +dispatch_queue_t provide_dispatch(); void use_const_local() { - doWork(provide_str(), provide_dict()); + doWork(provide_str(), provide_dict(), provide_dispatch()); // expected-warning@-1{{Call argument for parameter 'dict' is unretained and unsafe}} } @@ -65,4 +69,9 @@ - (NSString *)convertImage { RetainPtr image = [self createImage]; return stringForImage(image.get()); } + +- (const char *)dispatchLabel { + OSObjectPtr obj = provide_os(); + return dispatch_queue_get_label(obj.get()); +} @end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 75eead070fdf9..343e37d30e054 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -9,6 +9,9 @@ CFMutableArrayRef provide_cf(); void consume_cf(CFMutableArrayRef); +dispatch_queue_t provide_dispatch(); +void consume_dispatch(dispatch_queue_t); + CGImageRef provideImage(); NSString *stringForImage(CGImageRef); @@ -20,6 +23,8 @@ void foo() { // expected-warning@-1{{Call argument is unretained and unsafe}} consume_cf(provide_cf()); // expected-warning@-1{{Call argument is unretained and unsafe}} + consume_dispatch(provide_dispatch()); + // expected-warning@-1{{Call argument is unretained and unsafe}} } // Test that the checker works with [[clang::suppress]]. @@ -31,32 +36,32 @@ void foo_suppressed() { } namespace multi_arg { - void consume_retainable(int, SomeObj* foo, CFMutableArrayRef bar, bool); + void consume_retainable(int, SomeObj* foo, CFMutableArrayRef bar, dispatch_queue_t baz, bool); void foo() { - consume_retainable(42, provide(), provide_cf(), true); + consume_retainable(42, provide(), provide_cf(), provide_dispatch(), true); // expected-warning@-1{{Call argument for parameter 'foo' is unretained and unsafe}} // expected-warning@-2{{Call argument for parameter 'bar' is unretained and unsafe}} + // expected-warning@-3{{Call argument for parameter 'baz' is unretained and unsafe}} } void consume_retainable(SomeObj* foo, ...); void bar() { - consume_retainable(provide(), 1, provide_cf(), RetainPtr { provide_cf() }.get()); + consume_retainable(provide(), 1, provide_cf(), RetainPtr { provide_cf() }.get(), provide_dispatch()); // expected-warning@-1{{Call argument for parameter 'foo' is unretained and unsafe}} // expected-warning@-2{{Call argument is unretained and unsafe}} + // expected-warning@-3{{Call argument is unretained and unsafe}} consume_retainable(RetainPtr { provide() }.get(), 1, RetainPtr { provide_cf() }.get()); } } namespace retained { - RetainPtr provide_obj() { return RetainPtr{}; } - void consume_obj(RetainPtr) {} - - RetainPtr provide_cf() { return CFMutableArrayRef{}; } - void consume_cf(RetainPtr) {} - + RetainPtr provide_obj(); + RetainPtr provide_cf(); + OSObjectPtr provide_dispatch(); void foo() { consume_obj(provide_obj().get()); // no warning consume_cf(provide_cf().get()); // no warning + consume_dispatch(provide_dispatch().get()); // no warning } } @@ -64,6 +69,7 @@ void foo() { struct Consumer { void consume_obj(SomeObj* ptr); void consume_cf(CFMutableArrayRef ref); + void consume_dispatch(dispatch_queue_t ptr); }; void foo() { @@ -73,6 +79,8 @@ void foo() { // expected-warning@-1{{Call argument for parameter 'ptr' is unretained and unsafe}} c.consume_cf(provide_cf()); // expected-warning@-1{{Call argument for parameter 'ref' is unretained and unsafe}} + c.consume_dispatch(provide_dispatch()); + // expected-warning@-1{{Call argument for parameter 'ptr' is unretained and unsafe}} } void foo2() { @@ -88,6 +96,12 @@ void something() { consume_cf(provide_cf()); // expected-warning@-1{{Call argument is unretained and unsafe}} } + + void consume_dispatch(dispatch_queue_t) { some_function(); } + void anything() { + consume_dispatch(provide_dispatch()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } }; } @@ -104,6 +118,12 @@ void something() { this->consume_cf(provide_cf()); // expected-warning@-1{{Call argument is unretained and unsafe}} } + + void consume_dispatch(dispatch_queue_t) { some_function(); } + void anything() { + this->consume_dispatch(provide_dispatch()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } }; } @@ -131,6 +151,8 @@ void foo_ref() { consume_obj(0); consume_cf(nullptr); consume_cf(0); + consume_dispatch(nullptr); + consume_dispatch(0); } } @@ -161,6 +183,7 @@ void bar() { namespace param_formarding_function { void consume_more_obj(OtherObj*); void consume_more_cf(CFMutableArrayRef); + void consume_more_dispatch(dispatch_queue_t); namespace objc { void foo(SomeObj* param) { @@ -178,6 +201,7 @@ void foo(CFMutableArrayRef param) { namespace param_formarding_lambda { auto consume_more_obj = [](OtherObj*) { some_function(); }; auto consume_more_cf = [](CFMutableArrayRef) { some_function(); }; + auto consume_more_dispatch = [](dispatch_queue_t) { some_function(); }; namespace objc { void foo(SomeObj* param) { @@ -190,6 +214,12 @@ void foo(CFMutableArrayRef param) { consume_more_cf(param); } } + + namespace os_obj { + void foo(dispatch_queue_t param) { + consume_more_dispatch(param); + } + } } namespace param_forwarding_method { @@ -198,6 +228,8 @@ void foo(CFMutableArrayRef param) { static void consume_obj_s(SomeObj*); void consume_cf(CFMutableArrayRef); static void consume_cf_s(CFMutableArrayRef); + void consume_dispatch(dispatch_queue_t); + static void consume_dispatch_s(dispatch_queue_t); }; void bar(Consumer* consumer, SomeObj* param) { @@ -212,12 +244,18 @@ void baz(Consumer* consumer, CFMutableArrayRef param) { consumer->consume_cf(param); Consumer::consume_cf_s(param); } + + void baz(Consumer* consumer, dispatch_queue_t param) { + consumer->consume_dispatch(param); + Consumer::consume_dispatch_s(param); + } } namespace default_arg { SomeObj* global; CFMutableArrayRef global_cf; + dispatch_queue_t global_dispatch; void function_with_default_arg1(SomeObj* param = global); // expected-warning@-1{{Call argument for parameter 'param' is unretained and unsafe}} @@ -225,9 +263,13 @@ void baz(Consumer* consumer, CFMutableArrayRef param) { void function_with_default_arg2(CFMutableArrayRef param = global_cf); // expected-warning@-1{{Call argument for parameter 'param' is unretained and unsafe}} + void function_with_default_arg3(dispatch_queue_t param = global_dispatch); + // expected-warning@-1{{Call argument for parameter 'param' is unretained and unsafe}} + void foo() { function_with_default_arg1(); function_with_default_arg2(); + function_with_default_arg3(); } } @@ -259,9 +301,11 @@ void bar() { Foo& operator+(SomeObj* bad); friend Foo& operator-(Foo& lhs, SomeObj* bad); void operator()(SomeObj* bad); + Foo& operator<<(dispatch_queue_t bad); }; SomeObj* global; + dispatch_queue_t global_dispatch; void foo() { Foo f; @@ -271,6 +315,8 @@ void foo() { // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} f(global); // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + f << global_dispatch; + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} } } @@ -280,6 +326,8 @@ void foo() { void foo() { RetainPtr ptr; ptr = provide(); + OSObjectPtr objPtr; + objPtr = provide_dispatch(); } } @@ -287,8 +335,10 @@ void foo() { namespace call_with_ptr_on_ref { RetainPtr provideProtected(); RetainPtr provideProtectedCF(); + OSObjectPtr provideProtectedDispatch(); void bar(SomeObj* bad); void bar_cf(CFMutableArrayRef bad); + void bar_dispatch(dispatch_queue_t); bool baz(); void foo(bool v) { bar(v ? nullptr : provideProtected().get()); @@ -304,6 +354,13 @@ void foo(bool v) { // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} bar_cf(v ? provideProtectedCF().get() : provide_cf()); // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + + bar_dispatch(v ? nullptr : provideProtectedDispatch().get()); + bar_dispatch(baz() ? provideProtectedDispatch().get() : nullptr); + bar_dispatch(v ? provide_dispatch() : provideProtectedDispatch().get()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + bar_dispatch(v ? provideProtectedDispatch().get() : provide_dispatch()); + // expected-warning@-1{{Call argument is unretained and unsafe}} } } @@ -320,12 +377,16 @@ void bar() { void baz() { bar(); } + void os_ptr() { + consume_dispatch(OSObjectPtr { provide_dispatch() }.get()); + } } namespace call_with_adopt_ref { void foo() { [adoptNS(provide()).get() doWork]; CFArrayAppendValue(adoptCF(provide_cf()).get(), nullptr); + consume_dispatch(adoptOSObject(provide_dispatch()).get()); } } @@ -423,17 +484,19 @@ void idcf(CFTypeRef obj) { extern NSString * const SomeConstant; extern CFDictionaryRef const SomeDictionary; -void doWork(NSString *str, CFDictionaryRef dict); +extern dispatch_queue_t const SomeDispatch; +void doWork(NSString *str, CFDictionaryRef dict, dispatch_queue_t dispatch); void use_const_global() { - doWork(SomeConstant, SomeDictionary); + doWork(SomeConstant, SomeDictionary, SomeDispatch); } NSString *provide_str(); CFDictionaryRef provide_dict(); void use_const_local() { - doWork(provide_str(), provide_dict()); + doWork(provide_str(), provide_dict(), provide_dispatch()); // expected-warning@-1{{Call argument for parameter 'str' is unretained and unsafe}} // expected-warning@-2{{Call argument for parameter 'dict' is unretained and unsafe}} + // expected-warning@-3{{Call argument for parameter 'dispatch' is unretained and unsafe}} } } // namespace const_global @@ -470,12 +533,15 @@ bool baz(NSObject *obj) { NSString *provideNS() NS_RETURNS_RETAINED; CFDictionaryRef provideCF() CF_RETURNS_RETAINED; +dispatch_queue_t provideDispatch() NS_RETURNS_RETAINED; void consumeNS(NSString *); void consumeCF(CFDictionaryRef); +void consumeDispatch(dispatch_queue_t); void foo() { consumeNS(provideNS()); consumeCF(provideCF()); + consumeDispatch(provideDispatch()); } struct Base { @@ -506,10 +572,11 @@ - (void)doWork:(NSString *)msg, ... { - (void)doWorkOnSelf { [self doWork:nil]; - [self doWork:@"hello", provide(), provide_cf()]; + [self doWork:@"hello", provide(), provide_cf(), provide_dispatch()]; // expected-warning@-1{{Call argument is unretained and unsafe}} // expected-warning@-2{{Call argument is unretained and unsafe}} - [self doWork:@"hello", RetainPtr { provide() }.get(), RetainPtr { provide_cf() }.get()]; + // expected-warning@-3{{Call argument is unretained and unsafe}} + [self doWork:@"hello", RetainPtr { provide() }.get(), RetainPtr { provide_cf() }.get(), OSObjectPtr { provide_dispatch() }.get()]; [self doWork:__null]; [self doWork:nil]; } diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm index 63526a5047157..4e024dea7037a 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm @@ -112,6 +112,7 @@ explicit CallableWrapper(CallableType& callable) SomeObj* make_obj(); CFMutableArrayRef make_cf(); +dispatch_queue_t make_os(); void someFunction(); template void call(Callback callback) { @@ -125,8 +126,6 @@ void raw_ptr() { auto foo1 = [obj](){ [obj doWork]; }; - call(foo1); - auto foo2 = [&obj](){ [obj doWork]; }; @@ -157,6 +156,21 @@ void raw_ptr() { // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} }; + auto os = make_os(); + auto baz1 = [os](){ + dispatch_queue_get_label(os); + }; + auto baz2 = [&os](){ + dispatch_queue_get_label(os); + }; + auto baz3 = [&](){ + dispatch_queue_get_label(os); + os = nullptr; + }; + auto baz4 = [=](){ + dispatch_queue_get_label(os); + }; + call(foo1); call(foo2); call(foo3); @@ -166,6 +180,11 @@ void raw_ptr() { call(bar2); call(bar3); call(bar4); + + call(baz1); + call(baz2); + call(baz3); + call(baz4); } void quiet() { @@ -208,6 +227,14 @@ void get_count_cf(CFArrayRef array, [[clang::noescape]] Callback1&& callback1, C callback2(count); } +template +void get_count_os(dispatch_queue_t queue, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + auto* label = dispatch_queue_get_label(queue); + callback1(label); + callback2(label); +} + void noescape_lambda() { SomeObj* someObj = make_obj(); SomeObj* otherObj = make_obj(); @@ -230,6 +257,13 @@ void noescape_lambda() { CFArrayAppendValue(someCF, nullptr); // expected-warning@-1{{Implicitly captured reference 'someCF' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} }); + + dispatch_queue_t someOS = make_os(); + get_count_os(make_os(), [&](const char* label) { + dispatch_queue_get_label(someOS); + }, [&](const char* label) { + dispatch_queue_get_label(someOS); + }); } void callFunctionOpaque(WTF::Function&&); @@ -238,22 +272,25 @@ void callFunction(WTF::Function&& function) { function(); } -void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf) +void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf, dispatch_queue_t os) { callFunction([&]() { [obj doWork]; CFArrayAppendValue(cf, nullptr); // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + dispatch_queue_get_label(os); }); callFunctionOpaque([&]() { [obj doWork]; CFArrayAppendValue(cf, nullptr); // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + dispatch_queue_get_label(os); }); } @interface ObjWithSelf : NSObject { RetainPtr delegate; + OSObjectPtr queue; } -(void)doWork; -(void)doMoreWork; @@ -274,9 +311,15 @@ -(void)doWork { someFunction(); [delegate doWork]; }; + auto* queuePtr = queue.get(); + auto doAdditionalWork = [&] { + someFunction(); + dispatch_queue_get_label(queuePtr); + }; callFunctionOpaque(doWork); callFunctionOpaque(doMoreWork); callFunctionOpaque(doExtraWork); + callFunctionOpaque(doAdditionalWork); } -(void)doMoreWork { diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm index dcc6672261d8c..76a1da7707a2a 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm @@ -112,6 +112,7 @@ explicit CallableWrapper(CallableType& callable) SomeObj* make_obj(); CFMutableArrayRef make_cf(); +dispatch_queue_t make_os(); void someFunction(); template void call(Callback callback) { @@ -161,6 +162,25 @@ void raw_ptr() { // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} }; + auto os = make_os(); + auto baz1 = [os](){ + // expected-warning@-1{{Captured reference 'os' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + dispatch_queue_get_label(os); + }; + auto baz2 = [&os](){ + // expected-warning@-1{{Captured reference 'os' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + dispatch_queue_get_label(os); + }; + auto baz3 = [&](){ + dispatch_queue_get_label(os); + // expected-warning@-1{{Implicitly captured reference 'os' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + os = nullptr; + }; + auto baz4 = [=](){ + dispatch_queue_get_label(os); + // expected-warning@-1{{Implicitly captured reference 'os' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; + call(foo1); call(foo2); call(foo3); @@ -171,8 +191,13 @@ void raw_ptr() { call(bar3); call(bar4); + call(baz1); + call(baz2); + call(baz3); + call(baz4); + // Confirm that the checker respects [[clang::suppress]]. - SomeObj* suppressed_obj = nullptr; + SomeObj* suppressed_obj = make_obj(); [[clang::suppress]] auto foo5 = [suppressed_obj](){ [suppressed_obj doWork]; }; @@ -180,12 +205,20 @@ void raw_ptr() { call(foo5); // Confirm that the checker respects [[clang::suppress]]. - CFMutableArrayRef suppressed_cf = nullptr; + CFMutableArrayRef suppressed_cf = make_cf(); [[clang::suppress]] auto bar5 = [suppressed_cf](){ CFArrayAppendValue(suppressed_cf, nullptr); }; // no warning. call(bar5); + + // Confirm that the checker respects [[clang::suppress]]. + dispatch_queue_t suppressed_os = make_os(); + [[clang::suppress]] auto baz5 = [suppressed_os](){ + dispatch_queue_get_label(suppressed_os); + }; + // no warning. + call(baz5); } void quiet() { @@ -228,6 +261,14 @@ void get_count_cf(CFArrayRef array, [[clang::noescape]] Callback1&& callback1, C callback2(count); } +template +void get_count_os(dispatch_queue_t queue, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + auto* label = dispatch_queue_get_label(queue); + callback1(label); + callback2(label); +} + void noescape_lambda() { SomeObj* someObj = make_obj(); SomeObj* otherObj = make_obj(); @@ -251,6 +292,14 @@ void noescape_lambda() { CFArrayAppendValue(someCF, nullptr); // expected-warning@-1{{Implicitly captured reference 'someCF' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} }); + + dispatch_queue_t someOS = make_os(); + get_count_os(make_os(), [&](const char* label) { + dispatch_queue_get_label(someOS); + }, [&](const char* label) { + dispatch_queue_get_label(someOS); + // expected-warning@-1{{Implicitly captured reference 'someOS' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); } void callFunctionOpaque(WTF::Function&&); @@ -259,24 +308,29 @@ void callFunction(WTF::Function&& function) { function(); } -void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf) +void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf, dispatch_queue_t os) { callFunction([&]() { [obj doWork]; // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} CFArrayAppendValue(cf, nullptr); // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + dispatch_queue_get_label(os); + // expected-warning@-1{{Implicitly captured reference 'os' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} }); callFunctionOpaque([&]() { [obj doWork]; // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} CFArrayAppendValue(cf, nullptr); // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + dispatch_queue_get_label(os); + // expected-warning@-1{{Implicitly captured reference 'os' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} }); } @interface ObjWithSelf : NSObject { RetainPtr delegate; + OSObjectPtr queue; } -(void)doWork; -(void)doMoreWork; @@ -299,9 +353,16 @@ -(void)doWork { someFunction(); [delegate doWork]; }; + auto* queuePtr = queue.get(); + auto doAdditionalWork = [&] { + someFunction(); + dispatch_queue_get_label(queuePtr); + // expected-warning@-1{{Implicitly captured raw-pointer 'queuePtr' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; callFunctionOpaque(doWork); callFunctionOpaque(doMoreWork); callFunctionOpaque(doExtraWork); + callFunctionOpaque(doAdditionalWork); } -(void)doMoreWork { diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm index a84bee8529645..3d256f1599f04 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm @@ -23,24 +23,32 @@ void bar() { CFArrayAppendValue(array, nullptr); } +void baz() { + auto queue = dispatch_queue_create("some queue", nullptr); + dispatch_queue_get_label(queue); +} + } // namespace raw_ptr namespace const_global { extern NSString * const SomeConstant; extern CFDictionaryRef const SomeDictionary; -void doWork(NSString *, CFDictionaryRef); +extern dispatch_queue_t const SomeDispatch; +void doWork(NSString *, CFDictionaryRef, dispatch_queue_t); void use_const_global() { - doWork(SomeConstant, SomeDictionary); + doWork(SomeConstant, SomeDictionary, SomeDispatch); } NSString *provide_str(); CFDictionaryRef provide_dict(); +dispatch_queue_t provide_dispatch(); void use_const_local() { NSString * const str = provide_str(); CFDictionaryRef dict = provide_dict(); // expected-warning@-1{{Local variable 'dict' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} - doWork(str, dict); + auto dispatch = provide_dispatch(); + doWork(str, dict, dispatch); } } // namespace const_global @@ -63,4 +71,9 @@ - (void)bar { // expected-warning@-1{{Local variable 'array' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} CFArrayAppendValue(array, nullptr); } + +- (void)baz { + auto queue = dispatch_queue_create("some queue", nullptr); + dispatch_queue_get_label(queue); +} @end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm index 0ad8f707e254c..307a4d03fe101 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -30,18 +30,27 @@ void cf_ptr() { // expected-warning@-1{{Local variable 'array' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} CFArrayAppendValue(array, nullptr); } + +dispatch_queue_t provide_os(); +void os_ptr() { + auto queue = provide_os(); + // expected-warning@-1{{Local variable 'queue' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + dispatch_queue_get_label(queue); +} + } // namespace pointer namespace guardian_scopes { +SomeObj *provide(); void foo1() { - RetainPtr foo; + RetainPtr foo = provide(); { SomeObj *bar = foo.get(); } } void foo2() { - RetainPtr foo; + RetainPtr foo = provide(); // missing embedded scope here SomeObj *bar = foo.get(); // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} @@ -49,7 +58,7 @@ void foo2() { } void foo3() { - RetainPtr foo; + RetainPtr foo = provide(); { { SomeObj *bar = foo.get(); } } @@ -57,11 +66,35 @@ void foo3() { void foo4() { { - RetainPtr foo; + RetainPtr foo = provide(); { SomeObj *bar = foo.get(); } } } +CFMutableArrayRef provide_cf(); +void foo5() { + RetainPtr foo = provide_cf(); + { + CFMutableArrayRef bar = foo.get(); + CFArrayAppendValue(bar, nullptr); + } + CFMutableArrayRef baz = foo.get(); + // expected-warning@-1{{Local variable 'baz' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + CFArrayAppendValue(baz, nullptr); +} + +dispatch_queue_t provide_os(); +void foo6() { + OSObjectPtr queue = provide_os(); + { + dispatch_queue_t bar = queue.get(); + dispatch_queue_get_label(bar); + } + dispatch_queue_t baz = queue.get(); + // expected-warning@-1{{Local variable 'baz' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + dispatch_queue_get_label(baz); +} + struct SelfReferencingStruct { SelfReferencingStruct* ptr; SomeObj* obj { nullptr }; @@ -171,13 +204,35 @@ void foo10() { } } +bool trivialFunction(dispatch_queue_t queue) { return !!queue; } +void foo11() { + OSObjectPtr queue = adoptOSObject(dispatch_queue_create("some queue", nullptr)); + { + dispatch_queue_t queuePtr = queue.get(); + dispatch_queue_get_label(queuePtr); + } + { + dispatch_queue_t queuePtr = queue.get(); + // expected-warning@-1{{Local variable 'queuePtr' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + queue = nullptr; + dispatch_queue_get_label(queuePtr); + } + { + dispatch_queue_t queuePtr = queue.get(); + if (trivialFunction(queuePtr)) + queuePtr = nullptr; + } +} + } // namespace guardian_scopes namespace auto_keyword { class Foo { SomeObj *provide_obj(); CFMutableArrayRef provide_cf_array(); + dispatch_queue_t provide_queue(); void doWork(CFMutableArrayRef); + void doWork(dispatch_queue_t); void evil_func() { SomeObj *bar = provide_obj(); @@ -204,6 +259,14 @@ void bar() { doWork(baz); } + void baz() { + auto value1 = provide_queue(); + // expected-warning@-1{{Local variable 'value1' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + doWork(value1); + [[clang::suppress]] auto value2 = provide_queue(); // no-warning + doWork(value2); + } + }; } // namespace auto_keyword @@ -223,6 +286,14 @@ void foo2() { someFunction(); } } + +void foo3() { + OSObjectPtr foo; + { + dispatch_queue_t bar = foo.get(); + someFunction(); + } +} } // namespace guardian_casts namespace conditional_op { @@ -292,6 +363,15 @@ void bar(CFMutableArrayRef a) { doWork(a); } +dispatch_queue_t provide_queue(); +void doWork(dispatch_queue_t); + +void baz(dispatch_queue_t a) { + a = provide_queue(); + // expected-warning@-1{{Assignment to an unretained parameter 'a' is unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + doWork(a); +} + } // namespace local_assignment_to_parameter namespace local_assignment_to_static_local { @@ -317,6 +397,16 @@ void bar() { doWork(a); } +dispatch_queue_t provide_queue(); +void doWork(dispatch_queue_t); + +void baz() { + static dispatch_queue_t a = nullptr; + // expected-warning@-1{{Static local variable 'a' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + a = provide_queue(); + doWork(a); +} + } // namespace local_assignment_to_static_local namespace local_assignment_to_global { @@ -344,6 +434,16 @@ void bar() { doWork(g_b); } +dispatch_queue_t provide_queue(); +void doWork(dispatch_queue_t); +dispatch_queue_t g_c = nullptr; +// expected-warning@-1{{Global variable 'local_assignment_to_global::g_c' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + +void baz() { + g_c = provide_queue(); + doWork(g_c); +} + } // namespace local_assignment_to_global namespace local_var_for_singleton { @@ -358,6 +458,11 @@ void foo() { void bar() { CFMutableArrayRef cf = cfSingleton(); } + + dispatch_queue_t osSingleton(); + void baz() { + dispatch_queue_t os = osSingleton(); + } } namespace ptr_conversion { @@ -391,19 +496,23 @@ unsigned ccf(CFTypeRef obj) { extern NSString * const SomeConstant; extern CFDictionaryRef const SomeDictionary; -void doWork(NSString *, CFDictionaryRef); +extern dispatch_queue_t const SomeQueue; +void doWork(NSString *, CFDictionaryRef, dispatch_queue_t); void use_const_global() { - doWork(SomeConstant, SomeDictionary); + doWork(SomeConstant, SomeDictionary, SomeQueue); } NSString *provide_str(); CFDictionaryRef provide_dict(); +dispatch_queue_t provide_queue(); void use_const_local() { NSString * const str = provide_str(); // expected-warning@-1{{Local variable 'str' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} CFDictionaryRef dict = provide_dict(); // expected-warning@-1{{Local variable 'dict' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} - doWork(str, dict); + dispatch_queue_t queue = provide_queue(); + // expected-warning@-1{{Local variable 'queue' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + doWork(str, dict, queue); } } // namespace const_global @@ -412,13 +521,16 @@ void use_const_local() { NSString *provideNS() NS_RETURNS_RETAINED; CFDictionaryRef provideCF() CF_RETURNS_RETAINED; +dispatch_queue_t provideOS() CF_RETURNS_RETAINED; void consumeNS(NSString *); void consumeCF(CFDictionaryRef); +int consumeOS(dispatch_queue_t); unsigned foo() { auto *string = provideNS(); auto *dictionary = provideCF(); - return string.length + CFDictionaryGetCount(dictionary); + auto* queue = provideOS(); + return string.length + CFDictionaryGetCount(dictionary) + consumeOS(queue); } } // namespace ns_retained_return_value diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm index 00e6e6ec1dcfa..19c54c4dc07ba 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm @@ -20,22 +20,26 @@ CFMutableArrayRef e = nullptr; // expected-warning@-1{{Member variable 'e' in 'members::Foo' is a retainable type 'CFMutableArrayRef'}} + + dispatch_queue_t f = nullptr; }; union FooUnion { SomeObj* a; CFMutableArrayRef b; // expected-warning@-1{{Member variable 'b' in 'members::FooUnion' is a retainable type 'CFMutableArrayRef'}} + dispatch_queue_t c; }; - template + template struct FooTmpl { T* x; S y; -// expected-warning@-1{{Member variable 'y' in 'members::FooTmpl' is a raw pointer to retainable type}} +// expected-warning@-1{{Member variable 'y' in 'members::FooTmpl *>' is a raw pointer to retainable type}} + R z; }; - void forceTmplToInstantiate(FooTmpl) {} + void forceTmplToInstantiate(FooTmpl) {} struct [[clang::suppress]] FooSuppressed { private: @@ -51,16 +55,19 @@ void forceTmplToInstantiate(FooTmpl) {} // expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::List' contains a retainable type 'CFMutableArrayRef'}} }; - template + template struct TemplateList { + T* elements1; S* elements2; - // expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList' contains a raw pointer to retainable type '__CFArray'}} + // expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList *>' contains a raw pointer to retainable type '__CFArray'}} + R* elements3; }; - TemplateList list; + TemplateList list; struct SafeList { RetainPtr* elements1; RetainPtr* elements2; + OSObjectPtr elements3; }; } // namespace ptr_to_ptr_to_retained @@ -69,6 +76,7 @@ @interface AnotherObject : NSObject { NSString *ns_string; CFStringRef cf_string; // expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}} + dispatch_queue_t queue; } @property(nonatomic, strong) NSString *prop_string1; @property(nonatomic, assign) NSString *prop_string2; @@ -76,6 +84,7 @@ @interface AnotherObject : NSObject { @property(nonatomic, unsafe_unretained) NSString *prop_string3; // expected-warning@-1{{Property 'prop_string3' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} @property(nonatomic, readonly) NSString *prop_string4; +@property(nonatomic, readonly) dispatch_queue_t prop_string5; @end NS_REQUIRES_PROPERTY_DEFINITIONS @@ -90,6 +99,8 @@ @interface NoSynthObject : NSObject { // expected-warning@-1{{Property 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} @property(nonatomic, unsafe_unretained) NSString *prop_string4; // expected-warning@-1{{Property 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@property(nonatomic, unsafe_unretained) dispatch_queue_t prop_string5; +// expected-warning@-1{{Property 'prop_string5' in 'NoSynthObject' is a retainable type 'dispatch_queue_t'}} @end @implementation NoSynthObject @@ -99,4 +110,5 @@ - (NSString *)prop_string1 { @synthesize prop_string2; @synthesize prop_string3; @synthesize prop_string4; +@synthesize prop_string5; @end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm index 46f65dfa603ad..155848f9834af 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -11,6 +11,8 @@ private: SomeObj* a = nullptr; // expected-warning@-1{{Member variable 'a' in 'members::Foo' is a raw pointer to retainable type}} + dispatch_queue_t a2 = nullptr; +// expected-warning@-1{{Member variable 'a2' in 'members::Foo' is a retainable type 'dispatch_queue_t'}} [[clang::suppress]] SomeObj* a_suppressed = nullptr; @@ -19,25 +21,31 @@ protected: RetainPtr b; // No warning. + OSObjectPtr b2; +// No warning. public: SomeObj* c = nullptr; // expected-warning@-1{{Member variable 'c' in 'members::Foo' is a raw pointer to retainable type}} RetainPtr d; + OSObjectPtr d2; CFMutableArrayRef e = nullptr; // expected-warning@-1{{Member variable 'e' in 'members::Foo' is a retainable type 'CFMutableArrayRef'}} + }; - template + template struct FooTmpl { T* a; -// expected-warning@-1{{Member variable 'a' in 'members::FooTmpl' is a raw pointer to retainable type}} +// expected-warning@-1{{Member variable 'a' in 'members::FooTmpl *>' is a raw pointer to retainable type}} S b; -// expected-warning@-1{{Member variable 'b' in 'members::FooTmpl' is a raw pointer to retainable type}} +// expected-warning@-1{{Member variable 'b' in 'members::FooTmpl *>' is a raw pointer to retainable type}} + R c; +// expected-warning@-1{{Member variable 'c' in 'members::FooTmpl *>' is a raw pointer to retainable type}} }; - void forceTmplToInstantiate(FooTmpl) {} + void forceTmplToInstantiate(FooTmpl) {} struct [[clang::suppress]] FooSuppressed { private: @@ -54,6 +62,8 @@ void forceTmplToInstantiate(FooTmpl) {} RetainPtr b; CFMutableArrayRef c; // expected-warning@-1{{Member variable 'c' in 'unions::Foo' is a retainable type 'CFMutableArrayRef'}} + dispatch_queue_t d; + // expected-warning@-1{{Member variable 'd' in 'unions::Foo' is a retainable type 'dispatch_queue_t'}} }; template @@ -72,16 +82,20 @@ void forceTmplToInstantiate(FooTempl) {} // expected-warning@-1{{Member variable 'elements1' in 'ptr_to_ptr_to_retained::List' contains a raw pointer to retainable type 'SomeObj'}} CFMutableArrayRef* elements2; // expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::List' contains a retainable type 'CFMutableArrayRef'}} + dispatch_queue_t* elements3; + // expected-warning@-1{{Member variable 'elements3' in 'ptr_to_ptr_to_retained::List' contains a retainable type 'dispatch_queue_t'}} }; - template + template struct TemplateList { T** elements1; - // expected-warning@-1{{Member variable 'elements1' in 'ptr_to_ptr_to_retained::TemplateList' contains a raw pointer to retainable type 'SomeObj'}} + // expected-warning@-1{{Member variable 'elements1' in 'ptr_to_ptr_to_retained::TemplateList *>' contains a raw pointer to retainable type 'SomeObj'}} S* elements2; - // expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList' contains a raw pointer to retainable type '__CFArray'}} + // expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList *>' contains a raw pointer to retainable type '__CFArray'}} + R* elements3; + // expected-warning@-1{{Member variable 'elements3' in 'ptr_to_ptr_to_retained::TemplateList *>' contains a raw pointer to retainable type 'NSObject'}} }; - TemplateList list; + TemplateList list; struct SafeList { RetainPtr* elements1; @@ -92,20 +106,24 @@ void forceTmplToInstantiate(FooTempl) {} @interface AnotherObject : NSObject { NSString *ns_string; - // expected-warning@-1{{Instance variable 'ns_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} + // expected-warning@-1{{Instance variable 'ns_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'}} CFStringRef cf_string; - // expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}} + // expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'}} + dispatch_queue_t dispatch; + // expected-warning@-1{{Instance variable 'dispatch' in 'AnotherObject' is a retainable type 'dispatch_queue_t'}} } @property(nonatomic, strong) NSString *prop_string; -// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'}} @end NS_REQUIRES_PROPERTY_DEFINITIONS @interface NoSynthObject : NSObject { NSString *ns_string; - // expected-warning@-1{{Instance variable 'ns_string' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} + // expected-warning@-1{{Instance variable 'ns_string' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'}} CFStringRef cf_string; - // expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}} + // expected-warning@-1{{Instance variable 'cf_string' in 'NoSynthObject' is a retainable type 'CFStringRef'}} + dispatch_queue_t dispatch; + // expected-warning@-1{{Instance variable 'dispatch' in 'NoSynthObject' is a retainable type 'dispatch_queue_t'}} } @property(nonatomic, readonly, strong) NSString *prop_string1; @property(nonatomic, readonly, strong) NSString *prop_string2; @@ -114,6 +132,7 @@ @interface NoSynthObject : NSObject { // expected-warning@-1{{Property 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} @property(nonatomic, unsafe_unretained) NSString *prop_string4; // expected-warning@-1{{Property 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@property(nonatomic, readonly, strong) NSString *dispatch; @end @implementation NoSynthObject @@ -123,4 +142,7 @@ - (NSString *)prop_string1 { @synthesize prop_string2; @synthesize prop_string3; @synthesize prop_string4; +- (dispatch_queue_t)dispatch { + return nil; +} @end From 762257e849d509a761fa67041dca2bb65e071f37 Mon Sep 17 00:00:00 2001 From: Claudio Saavedra Date: Fri, 19 Sep 2025 14:31:17 +0300 Subject: [PATCH 03/13] [WebKit checkers] fix a typo in a message in one of the checkers (#159593) (cherry picked from commit a254f6524aa511eb0ead54f825262e75d467b82e) --- .../StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp | 2 +- clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index b841caf8c74b1..df13de158a646 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -384,7 +384,7 @@ class RawPtrRefCallArgsChecker SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); - Os << "Reciever is " << ptrKind() << " and unsafe."; + Os << "Receiver is " << ptrKind() << " and unsafe."; PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager()); auto Report = std::make_unique(Bug, Os.str(), BSLoc); diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 343e37d30e054..ddaa34d8ace45 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -279,7 +279,7 @@ void foo() { void foo() { [provide() doWork]; - // expected-warning@-1{{Reciever is unretained and unsafe}} + // expected-warning@-1{{Receiver is unretained and unsafe}} [protectedProvide().get() doWork]; CFArrayAppendValue(provide_cf(), nullptr); From 5a828b159923ba475c74fe0c37f7e6680873a804 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Tue, 23 Sep 2025 02:23:27 -0700 Subject: [PATCH 04/13] [alpha.webkit.UnretainedCallArgsChecker] Treat boolean literal as safe (#159705) (cherry picked from commit 86d767c3a0d422e4ee2a2d7ecc1a6880d9e33448) --- .../Checkers/WebKit/RawPtrRefCallArgsChecker.cpp | 2 ++ clang/test/Analysis/Checkers/WebKit/objc-mock-types.h | 1 + clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm | 5 ++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index df13de158a646..9585ceb40f95e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -224,6 +224,8 @@ class RawPtrRefCallArgsChecker // foo(123) return true; } + if (isa(ArgOrigin)) + return true; if (isa(ArgOrigin)) return true; if (isASafeCallArg(ArgOrigin)) diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h index 854742b82a2d4..39dee1746158b 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -160,6 +160,7 @@ __attribute__((objc_root_class)) - (int)intValue; - (id)initWithInt:(int)value; + (NSNumber *)numberWithInt:(int)value; ++ (NSNumber *)numberWithBool:(BOOL)value; @end @interface SomeObj : NSObject diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index ddaa34d8ace45..c9d2fe861bb49 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -390,17 +390,20 @@ void foo() { } } -#define YES 1 +#define YES __objc_yes +#define NO 0 namespace call_with_cf_constant { void bar(const NSArray *); void baz(const NSDictionary *); void boo(NSNumber *); + void boo(CFTypeRef); void foo() { CFArrayCreateMutable(kCFAllocatorDefault, 10); bar(@[@"hello"]); baz(@{@"hello": @3}); boo(@YES); + boo(@NO); } } From 3a5dc1b05c768555893f08f00918b919781fedd4 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 25 Sep 2025 11:12:23 -0700 Subject: [PATCH 05/13] [alpha.webkit.NoUnretainedMemberChecker] Only check @property when @implementation is seen (#159947) A @interface declaration with a raw pointer @property does not necessarily mean it synthesizes ivar of that type. To determine whether such a synthesis happens or not, we must wait for @implementation to appear. So this PR makes the checker only validate @property then. (cherry picked from commit 321a7c3caf7c5c6a208501e1406fcab14f8b54f9) --- .../WebKit/RawPtrRefMemberChecker.cpp | 19 +++---- .../Checkers/WebKit/unretained-members-arc.mm | 15 +++++ .../Checkers/WebKit/unretained-members.mm | 55 ++++++++++++++++++- 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index a97a37f85e96c..15a0c5a7fd9dc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -130,17 +130,16 @@ class RawPtrRefMemberChecker if (BR->getSourceManager().isInSystemHeader(CD->getLocation())) return; - ObjCContainerDecl::PropertyMap map; - CD->collectPropertiesToImplement(map); - for (auto it : map) - visitObjCPropertyDecl(CD, it.second); - - if (auto *ID = dyn_cast(CD)) { - for (auto *Ivar : ID->ivars()) - visitIvarDecl(CD, Ivar); - return; - } if (auto *ID = dyn_cast(CD)) { + ObjCContainerDecl::PropertyMap map; + CD->collectPropertiesToImplement(map); + for (auto it : map) + visitObjCPropertyDecl(CD, it.second); + + if (auto *Interface = ID->getClassInterface()) { + for (auto *Ivar : Interface->ivars()) + visitIvarDecl(CD, Ivar); + } for (auto *PropImpl : ID->property_impls()) visitPropImpl(CD, PropImpl); for (auto *Ivar : ID->ivars()) diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm index 19c54c4dc07ba..4eef372d26480 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm @@ -84,6 +84,21 @@ @interface AnotherObject : NSObject { @property(nonatomic, unsafe_unretained) NSString *prop_string3; // expected-warning@-1{{Property 'prop_string3' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} @property(nonatomic, readonly) NSString *prop_string4; +@property(nonatomic, readonly) NSString *prop_safe; +@end + +@implementation AnotherObject +- (NSString *)prop_safe { + return nil; +} +@end + +// No warnings for @interface declaration itself. +@interface InterfaceOnlyObject : NSObject +@property(nonatomic, strong) NSString *prop_string1; +@property(nonatomic, assign) NSString *prop_string2; +@property(nonatomic, unsafe_unretained) NSString *prop_string3; +@property(nonatomic, readonly) NSString *prop_string4; @property(nonatomic, readonly) dispatch_queue_t prop_string5; @end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm index 155848f9834af..adf1d8aef9d7d 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -112,8 +112,59 @@ @interface AnotherObject : NSObject { dispatch_queue_t dispatch; // expected-warning@-1{{Instance variable 'dispatch' in 'AnotherObject' is a retainable type 'dispatch_queue_t'}} } -@property(nonatomic, strong) NSString *prop_string; -// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'}} +@property(nonatomic, readonly, strong) NSString *prop_string; +// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@property(nonatomic, readonly) NSString *prop_safe; +@end + +@implementation AnotherObject +- (NSString *)prop_safe { + return nil; +} +@end + +@interface DerivedObject : AnotherObject { + NSNumber *ns_number; + // expected-warning@-1{{Instance variable 'ns_number' in 'DerivedObject' is a raw pointer to retainable type 'NSNumber'}} + CGImageRef cg_image; + // expected-warning@-1{{Instance variable 'cg_image' in 'DerivedObject' is a retainable type 'CGImageRef'}} + dispatch_queue_t os_dispatch; + // expected-warning@-1{{Instance variable 'os_dispatch' in 'DerivedObject' is a retainable type 'dispatch_queue_t'}} +} +@property(nonatomic, strong) NSNumber *prop_number; +// expected-warning@-1{{Property 'prop_number' in 'DerivedObject' is a raw pointer to retainable type 'NSNumber'; member variables must be a RetainPtr}} +@property(nonatomic, readonly) NSString *prop_string; +@end + +@implementation DerivedObject +- (NSString *)prop_string { + return nil; +} +@end + +// No warnings for @interface declaration itself. +@interface InterfaceOnlyObject : NSObject +@property(nonatomic, strong) NSString *prop_string1; +@property(nonatomic, assign) NSString *prop_string2; +@property(nonatomic, unsafe_unretained) NSString *prop_string3; +@property(nonatomic, readonly) NSString *prop_string4; +@end + +@interface InterfaceOnlyObject2 : NSObject +@property(nonatomic, strong) NSString *prop_string1; +@property(nonatomic, assign) NSString *prop_string2; +@property(nonatomic, unsafe_unretained) NSString *prop_string3; +// expected-warning@-1{{Property 'prop_string3' in 'DerivedObject2' is a raw pointer to retainable type 'NSString'}} +@property(nonatomic, readonly) NSString *prop_string4; +@end + +@interface DerivedObject2 : InterfaceOnlyObject2 +@property(nonatomic, readonly) NSString *prop_string5; +// expected-warning@-1{{Property 'prop_string5' in 'DerivedObject2' is a raw pointer to retainable type 'NSString'}} +@end + +@implementation DerivedObject2 +@synthesize prop_string3; @end NS_REQUIRES_PROPERTY_DEFINITIONS From 532c5b4e146b1d64c2d1603768276aed66a21044 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Wed, 1 Oct 2025 15:01:44 -0700 Subject: [PATCH 06/13] [alpha.webkit.RetainPtrCtorAdoptChecker] Allow leakRef in copy methods (#160986) Allow leakRef() in the return statement of an Objective-C copy method and other methods which return +1. (cherry picked from commit 103d2cae80160ebe79a91e4b4239140e2cd52283) --- .../WebKit/RetainPtrCtorAdoptChecker.cpp | 4 +++ .../Checkers/WebKit/objc-mock-types.h | 36 ++++++++++++++++++- .../WebKit/retain-ptr-ctor-adopt-use.mm | 12 +++++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp index e1f9a77f5a5ca..955b8d19a820c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp @@ -385,6 +385,10 @@ class RetainPtrCtorAdoptChecker if (RTC.isUnretained(RetValue->getType())) return; } + if (retainsRet && *retainsRet) { + CreateOrCopyFnCall.insert(RetValue); + return; + } if (auto *CE = dyn_cast(RetValue)) { auto *Callee = CE->getDirectCallee(); if (!Callee || !isCreateOrCopyFunction(Callee)) diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h index 39dee1746158b..dacb713130818 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -17,6 +17,20 @@ template typename remove_reference::type&& move(T&& t); #endif +namespace std { + +template struct enable_if { +}; + +template struct enable_if { + using type = T; +}; + +template +using enable_if_t = typename enable_if::type; + +} + @class NSString; @class NSArray; @class NSMutableArray; @@ -100,6 +114,7 @@ id CFBridgingRelease(CFTypeRef X) { __attribute__((objc_root_class)) @interface NSObject + (instancetype) alloc; ++ (instancetype) allocWithZone:(NSZone *)zone; + (Class) class; + (Class) superclass; - (instancetype) init; @@ -232,6 +247,14 @@ template struct RemovePointer { typedef T Type; }; +template struct IsPointer { + static constexpr bool value = false; +}; + +template struct IsPointer { + static constexpr bool value = true; +}; + template struct RetainPtr { using ValueType = typename RemovePointer::Type; using PtrType = ValueType*; @@ -285,12 +308,23 @@ template struct RetainPtr { PtrType operator->() const { return t; } T &operator*() const { return *t; } RetainPtr &operator=(PtrType t); - PtrType leakRef() + + template + std::enable_if_t::value, U> leakRef() CF_RETURNS_RETAINED + { + PtrType s = t; + t = nullptr; + return s; + } + + template + std::enable_if_t::value, U> leakRef() NS_RETURNS_RETAINED { PtrType s = t; t = nullptr; return s; } + operator PtrType() const { return t; } operator bool() const { return t; } diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm index 769901778cdf0..45705615f3196 100644 --- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm +++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm @@ -104,6 +104,14 @@ - (void)setValue:value { _number = value; } +- (id)copyWithZone:(NSZone *)zone { + auto copy = adoptNS([(SomeObj *)[SomeObj allocWithZone:zone] init]); + [copy setValue:_number]; + [copy setNext:_next]; + [copy setOther:_other]; + return copy.leakRef(); +} + @end; RetainPtr cf_out_argument() { @@ -151,7 +159,7 @@ CFTypeRef LeakWrapper() { extern Class (*getNSArrayClass)(); NSArray *allocArrayInstance() NS_RETURNS_RETAINED { - return [[getNSArrayClass() alloc] init]; + return adoptNS([[getNSArrayClass() alloc] init]).leakRef(); } extern int (*GetObj)(CF_RETURNS_RETAINED CFTypeRef* objOut); @@ -294,7 +302,7 @@ -(NSString *)leak_string { } -(NSString *)make_string { - return [[NSString alloc] initWithUTF8String:"hello"]; + return adoptNS([[NSString alloc] initWithUTF8String:"hello"]).leakRef(); } -(void)local_leak_string { From dc27599b8ccc75ab5bd536a0309aa4b05102e68d Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Tue, 7 Oct 2025 12:09:01 -0700 Subject: [PATCH 07/13] [WebKit Checkers] Treat a boxed value as a safe pointer origin (#161133) (cherry picked from commit e166816af0fc53723866608e1ff79f0a75ebcfdb) --- clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 2 ++ .../test/Analysis/Checkers/WebKit/unretained-call-args.mm | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 7b277ef04b255..4d27fbb2bf585 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -208,6 +208,8 @@ bool tryToFindPtrOrigin( continue; } if (auto *BoxedExpr = dyn_cast(E)) { + if (StopAtFirstRefCountedObj) + return callback(BoxedExpr, true); E = BoxedExpr->getSubExpr(); continue; } diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index c9d2fe861bb49..7046386e3e49b 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -398,12 +398,18 @@ void foo() { void baz(const NSDictionary *); void boo(NSNumber *); void boo(CFTypeRef); - void foo() { + + struct Details { + int value; + }; + + void foo(Details* details) { CFArrayCreateMutable(kCFAllocatorDefault, 10); bar(@[@"hello"]); baz(@{@"hello": @3}); boo(@YES); boo(@NO); + boo(@(details->value)); } } From 6098f30c90c5250b5a5c50c2f28b7898a2e91479 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Tue, 7 Oct 2025 12:09:29 -0700 Subject: [PATCH 08/13] [WebKit Checkers] Recognize NSApp as a safe global variable (#160990) Treat accessing NSApp without retaining it as safe (cherry picked from commit 872c4319dfc52886bbac03955ba1b7fe3ce83efc) --- .../StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 4 ++-- .../Analysis/Checkers/WebKit/objc-mock-types.h | 16 ++++++++++++++++ .../Checkers/WebKit/unretained-call-args.mm | 1 + 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 4d27fbb2bf585..3e03549622ca1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -31,9 +31,9 @@ bool tryToFindPtrOrigin( if (auto *DRE = dyn_cast(E)) { if (auto *VD = dyn_cast_or_null(DRE->getDecl())) { auto QT = VD->getType(); - if (VD->hasGlobalStorage() && QT.isConstQualified()) { + auto IsImmortal = safeGetName(VD) == "NSApp"; + if (VD->hasGlobalStorage() && (IsImmortal || QT.isConstQualified())) return callback(E, true); - } } } if (auto *tempExpr = dyn_cast(E)) { diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h index dacb713130818..a5fc3d7f9a932 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -178,6 +178,22 @@ __attribute__((objc_root_class)) + (NSNumber *)numberWithBool:(BOOL)value; @end +@interface NSResponder : NSObject +@end + +@interface NSApplication : NSResponder + +extern NSApplication * NSApp; + +@property (class, readonly, strong) NSApplication *sharedApplication; + +- (void)finishLaunching; +- (void)run; +- (void)stop:(id)sender; +- (void)terminate:(id)sender; + +@end + @interface SomeObj : NSObject - (instancetype)_init; - (SomeObj *)mutableCopy; diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 7046386e3e49b..a517dbc394dbb 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -588,6 +588,7 @@ - (void)doWorkOnSelf { [self doWork:@"hello", RetainPtr { provide() }.get(), RetainPtr { provide_cf() }.get(), OSObjectPtr { provide_dispatch() }.get()]; [self doWork:__null]; [self doWork:nil]; + [NSApp run]; } - (SomeObj *)getSomeObj { From e80a300bbd2ca074dc928422f6cf8ff8be1b3c68 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Thu, 9 Oct 2025 14:28:11 -0700 Subject: [PATCH 09/13] [alpha.webkit.NoUnretainedMemberChecker] Allow a retaining property synthesis (#162576) Don't emit a warning when an Objective-C property is defined using copy or strong semantics. (cherry picked from commit b7e256dce9b86ff35e38c4d0bf590969f689a8c5) --- .../Checkers/WebKit/RawPtrRefMemberChecker.cpp | 2 +- clang/test/Analysis/Checkers/WebKit/unretained-members.mm | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index 15a0c5a7fd9dc..ace639ce7ab18 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -232,7 +232,7 @@ class RawPtrRefMemberChecker bool ignoreARC = !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign; auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC); - return {IsUnsafePtr && *IsUnsafePtr, PropType}; + return {IsUnsafePtr && *IsUnsafePtr && !PD->isRetaining(), PropType}; } bool shouldSkipDecl(const RecordDecl *RD) const { diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm index adf1d8aef9d7d..2b120b9b1385c 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -113,7 +113,6 @@ @interface AnotherObject : NSObject { // expected-warning@-1{{Instance variable 'dispatch' in 'AnotherObject' is a retainable type 'dispatch_queue_t'}} } @property(nonatomic, readonly, strong) NSString *prop_string; -// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} @property(nonatomic, readonly) NSString *prop_safe; @end @@ -132,7 +131,6 @@ @interface DerivedObject : AnotherObject { // expected-warning@-1{{Instance variable 'os_dispatch' in 'DerivedObject' is a retainable type 'dispatch_queue_t'}} } @property(nonatomic, strong) NSNumber *prop_number; -// expected-warning@-1{{Property 'prop_number' in 'DerivedObject' is a raw pointer to retainable type 'NSNumber'; member variables must be a RetainPtr}} @property(nonatomic, readonly) NSString *prop_string; @end @@ -178,12 +176,12 @@ @interface NoSynthObject : NSObject { } @property(nonatomic, readonly, strong) NSString *prop_string1; @property(nonatomic, readonly, strong) NSString *prop_string2; -// expected-warning@-1{{Property 'prop_string2' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'}} @property(nonatomic, assign) NSString *prop_string3; // expected-warning@-1{{Property 'prop_string3' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} @property(nonatomic, unsafe_unretained) NSString *prop_string4; // expected-warning@-1{{Property 'prop_string4' in 'NoSynthObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} -@property(nonatomic, readonly, strong) NSString *dispatch; +@property(nonatomic, copy) NSString *prop_string5; +@property(nonatomic, readonly, strong) dispatch_queue_t dispatch; @end @implementation NoSynthObject @@ -193,6 +191,7 @@ - (NSString *)prop_string1 { @synthesize prop_string2; @synthesize prop_string3; @synthesize prop_string4; +@synthesize prop_string5; - (dispatch_queue_t)dispatch { return nil; } From 488563a9a36d8eac8cc71d6eafd54306b2dce555 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Tue, 14 Oct 2025 13:34:08 -0700 Subject: [PATCH 10/13] [WebKit checkers] Add support for ns_returns_autoreleased (#161236) Recognize ns_returns_autoreleased on a function and treat its return value as a safe pointer origin. (cherry picked from commit 0e4fb17971532302ca804e55f1ef5c2f8d31a42e) --- .../lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp | 3 ++- .../Analysis/Checkers/WebKit/unretained-call-args.mm | 11 +++++++++++ .../Checkers/WebKit/unretained-local-vars.mm | 12 ++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 3e03549622ca1..6777b9712b9d3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -93,7 +93,8 @@ bool tryToFindPtrOrigin( if (auto *call = dyn_cast(E)) { if (auto *Callee = call->getCalleeDecl()) { if (Callee->hasAttr() || - Callee->hasAttr()) { + Callee->hasAttr() || + Callee->hasAttr()) { return callback(E, true); } } diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index a517dbc394dbb..5dc3b38ccb61c 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -567,6 +567,17 @@ void foo() { } // namespace ns_retained_return_value +namespace autoreleased { + +NSString *provideAutoreleased() __attribute__((ns_returns_autoreleased)); +void consume(NSString *); + +void foo() { + consume(provideAutoreleased()); +} + +} // autoreleased + @interface TestObject : NSObject - (void)doWork:(NSString *)msg, ...; - (void)doWorkOnSelf; diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm index 307a4d03fe101..72ba05e9e3a71 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -535,6 +535,18 @@ unsigned foo() { } // namespace ns_retained_return_value +namespace autoreleased { + +NSString *provideAutoreleased() __attribute__((ns_returns_autoreleased)); +void consume(NSString *); + +void foo() { + auto *string = provideAutoreleased(); + consume(string); +} + +} // autoreleased + bool doMoreWorkOpaque(OtherObj*); SomeObj* provide(); From aabc74a40b25a3bdf0bac8dc985900886c6cb9cc Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Tue, 14 Oct 2025 16:52:19 -0700 Subject: [PATCH 11/13] [WebKit Checkers] Treat a NS/CF global defined in a system header as a safe pointer origin (#161146) (cherry picked from commit 1127dd775426238cdc57b2581fbb91a0252e06ae) --- .../Checkers/WebKit/ASTUtils.cpp | 9 +++++-- .../StaticAnalyzer/Checkers/WebKit/ASTUtils.h | 1 + .../WebKit/RawPtrRefCallArgsChecker.cpp | 9 ++++++- .../WebKit/RawPtrRefLocalVarsChecker.cpp | 8 +++++- .../Checkers/WebKit/mock-system-header.h | 6 +++++ .../Checkers/WebKit/unretained-local-vars.mm | 26 +++++++++++++++++++ .../Checkers/WebKit/unretained-obj-arg.mm | 18 +++++++++++++ 7 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 6777b9712b9d3..e45673d7722b7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -26,6 +26,7 @@ bool tryToFindPtrOrigin( const Expr *E, bool StopAtFirstRefCountedObj, std::function isSafePtr, std::function isSafePtrType, + std::function isSafeGlobalDecl, std::function callback) { while (E) { if (auto *DRE = dyn_cast(E)) { @@ -34,6 +35,8 @@ bool tryToFindPtrOrigin( auto IsImmortal = safeGetName(VD) == "NSApp"; if (VD->hasGlobalStorage() && (IsImmortal || QT.isConstQualified())) return callback(E, true); + if (VD->hasGlobalStorage() && isSafeGlobalDecl(VD)) + return callback(E, true); } } if (auto *tempExpr = dyn_cast(E)) { @@ -71,9 +74,11 @@ bool tryToFindPtrOrigin( } if (auto *Expr = dyn_cast(E)) { return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj, - isSafePtr, isSafePtrType, callback) && + isSafePtr, isSafePtrType, isSafeGlobalDecl, + callback) && tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj, - isSafePtr, isSafePtrType, callback); + isSafePtr, isSafePtrType, isSafeGlobalDecl, + callback); } if (auto *cast = dyn_cast(E)) { if (StopAtFirstRefCountedObj) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index 3a009d65efea6..9fff456b7e8b8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -56,6 +56,7 @@ bool tryToFindPtrOrigin( const clang::Expr *E, bool StopAtFirstRefCountedObj, std::function isSafePtr, std::function isSafePtrType, + std::function isSafeGlobalDecl, std::function callback); /// For \p E referring to a ref-countable/-counted pointer/reference we return diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index 9585ceb40f95e..791e70998477f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -29,12 +29,12 @@ namespace { class RawPtrRefCallArgsChecker : public Checker> { BugType Bug; - mutable BugReporter *BR; TrivialFunctionAnalysis TFA; EnsureFunctionAnalysis EFA; protected: + mutable BugReporter *BR; mutable std::optional RTC; public: @@ -46,6 +46,7 @@ class RawPtrRefCallArgsChecker virtual bool isSafePtr(const CXXRecordDecl *Record) const = 0; virtual bool isSafePtrType(const QualType type) const = 0; virtual bool isSafeExpr(const Expr *) const { return false; } + virtual bool isSafeDecl(const Decl *) const { return false; } virtual const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -214,6 +215,7 @@ class RawPtrRefCallArgsChecker Arg, /*StopAtFirstRefCountedObj=*/true, [&](const clang::CXXRecordDecl *Record) { return isSafePtr(Record); }, [&](const clang::QualType T) { return isSafePtrType(T); }, + [&](const clang::Decl *D) { return isSafeDecl(D); }, [&](const clang::Expr *ArgOrigin, bool IsSafe) { if (IsSafe) return true; @@ -479,6 +481,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker { isa(E); } + bool isSafeDecl(const Decl *D) const final { + // Treat NS/CF globals in system header as immortal. + return BR->getSourceManager().isInSystemHeader(D->getLocation()); + } + const char *ptrKind() const final { return "unretained"; } }; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index dd9701fbbb017..c13df47920f72 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -166,10 +166,10 @@ bool isGuardedScopeEmbeddedInGuardianScope(const VarDecl *Guarded, class RawPtrRefLocalVarsChecker : public Checker> { BugType Bug; - mutable BugReporter *BR; EnsureFunctionAnalysis EFA; protected: + mutable BugReporter *BR; mutable std::optional RTC; public: @@ -180,6 +180,7 @@ class RawPtrRefLocalVarsChecker virtual bool isSafePtr(const CXXRecordDecl *) const = 0; virtual bool isSafePtrType(const QualType) const = 0; virtual bool isSafeExpr(const Expr *) const { return false; } + virtual bool isSafeDecl(const Decl *) const { return false; } virtual const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -288,6 +289,7 @@ class RawPtrRefLocalVarsChecker return isSafePtr(Record); }, [&](const clang::QualType Type) { return isSafePtrType(Type); }, + [&](const clang::Decl *D) { return isSafeDecl(D); }, [&](const clang::Expr *InitArgOrigin, bool IsSafe) { if (!InitArgOrigin || IsSafe) return true; @@ -443,6 +445,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { return ento::cocoa::isCocoaObjectRef(E->getType()) && isa(E); } + bool isSafeDecl(const Decl *D) const final { + // Treat NS/CF globals in system header as immortal. + return BR->getSourceManager().isInSystemHeader(D->getLocation()); + } const char *ptrKind() const final { return "unretained"; } }; diff --git a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h index 1e44de8eb62ad..d55b3abd34f4c 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h @@ -34,6 +34,8 @@ void os_log_msg(os_log_t oslog, os_log_type_t type, const char *msg, ...); typedef const struct __attribute__((objc_bridge(NSString))) __CFString * CFStringRef; +extern CFStringRef const kCFURLTagNamesKey; + #ifdef __OBJC__ @class NSString; @interface SystemObject { @@ -41,4 +43,8 @@ typedef const struct __attribute__((objc_bridge(NSString))) __CFString * CFStrin CFStringRef cf_string; } @end + +typedef NSString *NSNotificationName; +extern "C" NSNotificationName NSApplicationDidBecomeActiveNotification; + #endif diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm index 72ba05e9e3a71..f49e7bdb3e79c 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -1,8 +1,11 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -verify %s #import "objc-mock-types.h" +#import "mock-system-header.h" void someFunction(); +extern "C" CFStringRef LocalGlobalCFString; +extern "C" NSString *LocalGlobalNSString; namespace raw_ptr { void foo() { @@ -547,6 +550,29 @@ void foo() { } // autoreleased +namespace ns_global { + +void consumeCFString(CFStringRef); +void consumeNSString(NSString *); + +void cf() { + auto *str = kCFURLTagNamesKey; + consumeCFString(str); + auto *localStr = LocalGlobalCFString; + // expected-warning@-1{{Local variable 'localStr' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + consumeCFString(localStr); +} + +void ns() { + auto *str = NSApplicationDidBecomeActiveNotification; + consumeNSString(str); + auto *localStr = LocalGlobalNSString; + // expected-warning@-1{{Local variable 'localStr' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + consumeNSString(localStr); +} + +} + bool doMoreWorkOpaque(OtherObj*); SomeObj* provide(); diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm b/clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm new file mode 100644 index 0000000000000..5c78b21d6c94f --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-obj-arg.mm @@ -0,0 +1,18 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedCallArgsChecker -verify %s + +#import "mock-types.h" +#import "mock-system-header.h" + +void consumeCFString(CFStringRef); +extern "C" CFStringRef LocalGlobalCFString; +void consumeNSString(NSString *); +extern "C" NSString *LocalGlobalNSString; + +void foo() { + consumeCFString(kCFURLTagNamesKey); + consumeCFString(LocalGlobalCFString); + // expected-warning@-1{{Call argument is unretained and unsafe}} + consumeNSString(NSApplicationDidBecomeActiveNotification); + consumeNSString(LocalGlobalNSString); + // expected-warning@-1{{Call argument is unretained and unsafe}} +} From a8ea080da9fd5a055719d1e094b53b0e864179bd Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Wed, 15 Oct 2025 14:21:18 -0700 Subject: [PATCH 12/13] [alpha.webkit.UnretainedCallArgsChecker] Treat NSStringFromSelector and alike as trivial and returns a retained value (#161135) Treat NSStringFromSelector, NSSelectorFromString, NSStringFromClass, NSClassFromString, NSStringFromProtocol, and NSProtocolFromString as trivial, and treat their return values as a safe pointer origin since the return value of these functions don't need to be retained. (cherry picked from commit b6262825bd52fb9a106ad6feb0358ad3113ecc5f) --- .../Checkers/WebKit/ASTUtils.cpp | 6 +++++- .../Analysis/Checkers/WebKit/objc-mock-types.h | 10 +++++++++- .../Checkers/WebKit/unretained-call-args.mm | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index e45673d7722b7..419d2631fef81 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -164,7 +164,9 @@ bool tryToFindPtrOrigin( auto Name = safeGetName(callee); if (Name == "__builtin___CFStringMakeConstantString" || - Name == "NSClassFromString") + Name == "NSStringFromSelector" || Name == "NSSelectorFromString" || + Name == "NSStringFromClass" || Name == "NSClassFromString" || + Name == "NSStringFromProtocol" || Name == "NSProtocolFromString") return callback(E, true); } else if (auto *CalleeE = call->getCallee()) { if (auto *E = dyn_cast(CalleeE->IgnoreParenCasts())) { @@ -202,6 +204,8 @@ bool tryToFindPtrOrigin( !Selector.getNumArgs()) return callback(E, true); } + if (auto *ObjCProtocol = dyn_cast(E)) + return callback(ObjCProtocol, true); if (auto *ObjCDict = dyn_cast(E)) return callback(ObjCDict, true); if (auto *ObjCArray = dyn_cast(E)) diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h index a5fc3d7f9a932..edf40115afa19 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -98,12 +98,20 @@ typedef CVImageBufferRef CVPixelBufferRef; typedef signed int CVReturn; CVReturn CVPixelBufferCreateWithIOSurface(CFAllocatorRef allocator, IOSurfaceRef surface, CFDictionaryRef pixelBufferAttributes, CF_RETURNS_RETAINED CVPixelBufferRef * pixelBufferOut); +extern "C" NSString *NSStringFromSelector(SEL aSelector); +extern "C" SEL NSSelectorFromString(NSString *aSelectorName); + +extern "C" NSString *NSStringFromClass(Class aClass); +extern "C" Class NSClassFromString(NSString *aClassName); + +extern "C" NSString *NSStringFromProtocol(Protocol *proto); +extern "C" Protocol * NSProtocolFromString(NSString *namestr); + CFRunLoopRef CFRunLoopGetCurrent(void); CFRunLoopRef CFRunLoopGetMain(void); extern CFTypeRef CFRetain(CFTypeRef cf); extern void CFRelease(CFTypeRef cf); #define CFSTR(cStr) ((CFStringRef) __builtin___CFStringMakeConstantString ("" cStr "")) -extern Class NSClassFromString(NSString *aClassName); #if __has_feature(objc_arc) id CFBridgingRelease(CFTypeRef X) { diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 5dc3b38ccb61c..4f231ee8b1c84 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -578,6 +578,24 @@ void foo() { } // autoreleased +namespace sel_string { + +void consumeStr(NSString *); +void consumeSel(SEL); +void consumeClass(Class); +void consumeProtocol(Protocol *); + +void foo() { + consumeStr(NSStringFromSelector(@selector(mutableCopy))); + consumeSel(NSSelectorFromString(@"mutableCopy")); + consumeStr(NSStringFromClass(NSNumber.class)); + consumeClass(NSClassFromString(@"NSNumber")); + consumeStr(NSStringFromProtocol(@protocol(NSCopying))); + consumeProtocol(NSProtocolFromString(@"NSCopying")); +} + +} // namespace sel_string + @interface TestObject : NSObject - (void)doWork:(NSString *)msg, ...; - (void)doWorkOnSelf; From da925f3f3bafb4e465b5edc4b51735fe994c5400 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa Date: Wed, 15 Oct 2025 15:28:58 -0700 Subject: [PATCH 13/13] [alpha.webkit.ForwardDeclChecker] Ignore unary operator when detecting a parameter (#160994) This PR updates the forward declaration checker so that unary operator & and * will be ignored for the purpose of determining if a given function argument is also a function argument of the caller / call-site. (cherry picked from commit e61e6251b692ffe71910bad22b82e41313f003cf) --- .../Checkers/WebKit/ForwardDeclChecker.cpp | 39 ++++++-- .../Checkers/WebKit/PtrTypesSemantics.cpp | 10 +- .../Checkers/WebKit/PtrTypesSemantics.h | 4 + .../Checkers/WebKit/forward-decl-checker.mm | 12 +++ .../Analysis/Checkers/WebKit/mock-types.h | 96 ++++++++++++++++++- 5 files changed, 145 insertions(+), 16 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp index d8539eaaac49f..1d4e6dd572749 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp @@ -263,18 +263,43 @@ class ForwardDeclChecker : public Checker> { void visitCallArg(const Expr *Arg, const ParmVarDecl *Param, const Decl *DeclWithIssue) const { auto *ArgExpr = Arg->IgnoreParenCasts(); - if (auto *InnerCE = dyn_cast(Arg)) { - auto *InnerCallee = InnerCE->getDirectCallee(); - if (InnerCallee && InnerCallee->isInStdNamespace() && - safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { - ArgExpr = InnerCE->getArg(0); - if (ArgExpr) - ArgExpr = ArgExpr->IgnoreParenCasts(); + while (ArgExpr) { + ArgExpr = ArgExpr->IgnoreParenCasts(); + if (auto *InnerCE = dyn_cast(ArgExpr)) { + auto *InnerCallee = InnerCE->getDirectCallee(); + if (InnerCallee && InnerCallee->isInStdNamespace() && + safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { + ArgExpr = InnerCE->getArg(0); + continue; + } + } + if (auto *UO = dyn_cast(ArgExpr)) { + auto OpCode = UO->getOpcode(); + if (OpCode == UO_Deref || OpCode == UO_AddrOf) { + ArgExpr = UO->getSubExpr(); + continue; + } } + break; } + + if (auto *MemberCallExpr = dyn_cast(ArgExpr)) { + if (isOwnerPtrType(MemberCallExpr->getObjectType())) + return; + } + + if (auto *OpCE = dyn_cast(ArgExpr)) { + auto *Method = dyn_cast_or_null(OpCE->getDirectCallee()); + if (Method && isOwnerPtr(safeGetName(Method->getParent()))) { + if (OpCE->getOperator() == OO_Star && OpCE->getNumArgs() == 1) + return; + } + } + if (isNullPtr(ArgExpr) || isa(ArgExpr) || isa(ArgExpr)) return; + if (auto *DRE = dyn_cast(ArgExpr)) { if (auto *ValDecl = DRE->getDecl()) { if (isa(ValDecl)) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 8bb701751e79e..1a7024d559b8d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -138,6 +138,11 @@ bool isCheckedPtr(const std::string &Name) { return Name == "CheckedPtr" || Name == "CheckedRef"; } +bool isOwnerPtr(const std::string &Name) { + return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || + Name == "UniqueRef" || Name == "LazyUniqueRef"; +} + bool isSmartPtrClass(const std::string &Name) { return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) || Name == "WeakPtr" || Name == "WeakPtrFactory" || @@ -210,10 +215,7 @@ bool isRetainPtrOrOSPtrType(const clang::QualType T) { } bool isOwnerPtrType(const clang::QualType T) { - return isPtrOfType(T, [](auto Name) { - return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || - Name == "UniqueRef" || Name == "LazyUniqueRef"; - }); + return isPtrOfType(T, [](auto Name) { return isOwnerPtr(Name); }); } std::optional isUncounted(const QualType T) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 8300a6c051f3e..12e2e2d75b75d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -143,6 +143,10 @@ bool isCheckedPtr(const std::string &Name); /// \returns true if \p Name is RetainPtr or its variant, false if not. bool isRetainPtrOrOSPtr(const std::string &Name); +/// \returns true if \p Name is an owning smar pointer such as Ref, CheckedPtr, +/// and unique_ptr. +bool isOwnerPtr(const std::string &Name); + /// \returns true if \p Name is a smart pointer type name, false if not. bool isSmartPtrClass(const std::string &Name); diff --git a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm index 104b555c1c41d..8aad838b71b35 100644 --- a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm +++ b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm @@ -11,6 +11,8 @@ Obj* provide_obj_ptr(); void receive_obj_ptr(Obj* p = nullptr); +void receive_obj_ref(Obj&); +void receive_obj_rref(Obj&&); sqlite3* open_db(); void close_db(sqlite3*); @@ -38,6 +40,16 @@ return obj; } +void opaque_call_arg(Obj* obj, Obj&& otherObj, const RefPtr& safeObj, WeakPtr weakObj, std::unique_ptr& uniqObj) { + receive_obj_ref(*obj); + receive_obj_ptr(&*obj); + receive_obj_rref(std::move(otherObj)); + receive_obj_ref(*safeObj.get()); + receive_obj_ptr(weakObj.get()); + // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}} + receive_obj_ref(*uniqObj); +} + Obj&& provide_obj_rval(); void receive_obj_rval(Obj&& p); diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index a49faa1d25336..7055a94753a37 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -25,23 +25,23 @@ namespace std { template class unique_ptr { private: - T *t; + void *t; public: unique_ptr() : t(nullptr) { } unique_ptr(T *t) : t(t) { } ~unique_ptr() { if (t) - delete t; + delete static_cast(t); } template unique_ptr(unique_ptr&& u) : t(u.t) { u.t = nullptr; } - T *get() const { return t; } - T *operator->() const { return t; } - T &operator*() const { return *t; } + T *get() const { return static_cast(t); } + T *operator->() const { return get(); } + T &operator*() const { return *get(); } unique_ptr &operator=(T *) { return *this; } explicit operator bool() const { return !!t; } }; @@ -313,4 +313,90 @@ class UniqueRef { UniqueRef &operator=(T &) { return *this; } }; +class WeakPtrImpl { +private: + void* ptr { nullptr }; + mutable unsigned m_refCount { 0 }; + + template friend class CanMakeWeakPtr; + template friend class WeakPtr; + +public: + template + static Ref create(T& t) + { + return adoptRef(*new WeakPtrImpl(t)); + } + + void ref() const { m_refCount++; } + void deref() const { + m_refCount--; + if (!m_refCount) + delete const_cast(this); + } + + template + T* get() { return static_cast(ptr); } + operator bool() const { return !!ptr; } + void clear() { ptr = nullptr; } + +private: + template + WeakPtrImpl(T* t) + : ptr(static_cast(t)) + { } +}; + +template +class CanMakeWeakPtr { +private: + RefPtr impl; + + template friend class CanMakeWeakPtr; + template friend class WeakPtr; + + Ref createWeakPtrImpl() { + if (!impl) + impl = WeakPtrImpl::create(static_cast(*this)); + return *impl; + } + +public: + ~CanMakeWeakPtr() { + if (!impl) + return; + impl->clear(); + impl = nullptr; + } +}; + +template +class WeakPtr { +private: + RefPtr impl; + +public: + WeakPtr(T& t) { + *this = t; + } + WeakPtr(T* t) { + *this = t; + } + + template + WeakPtr operator=(U& obj) { + impl = obj.createWeakPtrImpl(); + } + + template + WeakPtr operator=(U* obj) { + impl = obj ? obj->createWeakPtrImpl() : nullptr; + } + + T* get() { + return impl ? impl->get() : nullptr; + } + +}; + #endif