Author: Ryosuke Niwa Date: 2026-10-11T08:11:45-07:00 New Revision: 308aa0cff3fbaea7c3dcc97244c13fbfbafcc513
URL: https://github.com/llvm/llvm-project/commit/308aa0cff3fbaea7c3dcc97244c13fbfbafcc513 DIFF: https://github.com/llvm/llvm-project/commit/308aa0cff3fbaea7c3dcc97244c13fbfbafcc513.diff LOG: [alpha.webkit.UnretainedLocalVarsChecker] Treat the collection of a fast enumeration as the origin of its element (#230818) alpha.webkit.UnretainedLocalVarsChecker reported every element variable of an Objective-C fast enumeration such as "for (T *x in collection)" since the variable has no initializer, even when the collection was kept alive by a RetainPtr local variable. The element of a fast enumeration is kept alive by its collection, which can't be mutated during the enumeration. So treat the collection as the initial value of the element, as in "T *x = collection;". Because the collection is evaluated before the loop body is entered, a guardian local variable only needs to outlive the loop body; it can be declared in the same scope as the loop as long as the loop body doesn't mutate it. The collection is a full-expression of its own so a temporary smart pointer in the collection is destroyed before the loop body is entered and isn't considered safe. Authored with Claude Code. Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 51511197072b5..70c8a7f88384b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -15,6 +15,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/AST/ParentMapContext.h" +#include "clang/AST/StmtObjC.h" #include "clang/Analysis/DomainSpecific/CocoaConventions.h" #include "clang/Basic/SourceLocation.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" @@ -222,6 +223,24 @@ bool isGuardedScopeEmbeddedInGuardianScope(const VarDecl *Guarded, return false; } +// Returns the fast enumeration statement whose element is declared as V as in +// "for (T *V in Collection)". +static const ObjCForCollectionStmt * +findObjCForCollectionStmtOfElement(const VarDecl *V) { + ASTContext &Ctx = V->getASTContext(); + for (auto &VarParent : Ctx.getParents(*V)) { + auto *DS = VarParent.get<DeclStmt>(); + if (!DS || !DS->isSingleDecl()) + continue; + for (auto &DeclStmtParent : Ctx.getParents(*DS)) { + auto *FCS = DeclStmtParent.get<ObjCForCollectionStmt>(); + if (FCS && FCS->getElement() == DS) + return FCS; + } + } + return nullptr; +} + static const VarDecl *findAssignedVar(const Expr *DestExpr) { while (DestExpr) { DestExpr = DestExpr->IgnoreParenCasts(); @@ -434,6 +453,13 @@ class RawPtrRefLocalVarsChecker std::optional<bool> IsUncountedPtr = isUnsafePtr(SinkType); if (IsUncountedPtr && *IsUncountedPtr) { const Expr *Origin = nullptr; + // The element of a fast enumeration is kept alive by the collection, + // which can't be mutated during the enumeration. Treat the collection as + // the initial value of the element. + if (!Value) { + if (auto *FCS = findObjCForCollectionStmtOfElement(V)) + Value = FCS->getCollection(); + } if (Value) { if (isPtrOriginSafe(V, Value, DeclWithIssue, Origin, SinkType)) return; @@ -514,9 +540,16 @@ class RawPtrRefLocalVarsChecker if (auto *Record = GuardianType->getAsCXXRecordDecl()) { if (MaybeGuardian->isLocalVarDecl() && (Model->isSafePtr(Record) || - isRefcountedStringsHack(MaybeGuardian)) && - isGuardedScopeEmbeddedInGuardianScope(V, MaybeGuardian)) - return true; + isRefcountedStringsHack(MaybeGuardian))) { + // The collection of a fast enumeration is evaluated before the loop + // body is entered so the guardian only needs to outlive the body. + if (auto *FCS = findObjCForCollectionStmtOfElement(V)) { + GuardianVisitor Visitor(MaybeGuardian); + return Visitor.TraverseStmt(const_cast<Stmt *>(FCS->getBody())); + } + if (isGuardedScopeEmbeddedInGuardianScope(V, MaybeGuardian)) + return true; + } } } diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm index 03946b40ea816..acec4b36947e2 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -574,6 +574,108 @@ void ns() { } +namespace fast_enumeration { + +NSArray *provideArray(); + +void iterateLocal() { + RetainPtr<NSArray> array = provideArray(); + for (SomeObj *obj in array.get()) + [obj doWork]; +} + +void iterateLocalInNestedScope() { + RetainPtr<NSArray> array = provideArray(); + { + for (SomeObj *obj in array.get()) { + for (SomeObj *otherObj in array.get()) + [otherObj doWork]; + [obj doWork]; + } + } +} + +void iterateLocalWithRangeBasedFor() { + RetainPtr<NSArray> array = provideArray(); + for (SomeObj *obj : array.get()) + [obj doWork]; +} + +void iterateLocalWithId() { + RetainPtr<NSArray> array = provideArray(); + for (id obj in array.get()) + [obj doWork]; +} + +void iterateLocalClearedInBody() { + RetainPtr<NSArray> array = provideArray(); + for (SomeObj *obj in array.get()) { + // expected-warning@-1{{Local variable 'obj' is a raw pointer to RetainPtr-capable type 'SomeObj' [alpha.webkit.UnretainedLocalVarsChecker]}} + array = nullptr; + [obj doWork]; + } +} + +void iterateLocalClearedAfterLoop() { + RetainPtr<NSArray> array = provideArray(); + for (SomeObj *obj in array.get()) + [obj doWork]; + array = nullptr; +} + +void iterateParam(NSArray *array) { + for (SomeObj *obj in array) + [obj doWork]; +} + +void iterateParamReassigned(NSArray *array) { + for (SomeObj *obj in array) { + // expected-warning@-1{{Local variable 'obj' is a raw pointer to RetainPtr-capable type 'SomeObj' [alpha.webkit.UnretainedLocalVarsChecker]}} + array = provideArray(); + // expected-warning@-1{{Parameter 'array' is a raw pointer to RetainPtr-capable type 'NSArray' [alpha.webkit.UnretainedLocalVarsChecker]}} + [obj doWork]; + } +} + +void iterateTemporary() { + for (SomeObj *obj in retainPtr(provideArray()).get()) + // expected-warning@-1{{Local variable 'obj' is a raw pointer to RetainPtr-capable type 'SomeObj' [alpha.webkit.UnretainedLocalVarsChecker]}} + [obj doWork]; +} + +void iterateReturnValue() { + for (SomeObj *obj in provideArray()) + // expected-warning@-1{{Local variable 'obj' is a raw pointer to RetainPtr-capable type 'SomeObj' [alpha.webkit.UnretainedLocalVarsChecker]}} + [obj doWork]; +} + +void iterateLocalWithStructuredBinding() { + RetainPtr<NSArray> array = provideArray(); + for (auto [obj] in array.get()) + // expected-warning@-1{{'auto' deduced as 'id' in declaration of ''}} + (void)obj; +} + +void iterateLocalWithStructuredBindingClearedInBody() { + RetainPtr<NSArray> array = provideArray(); + for (auto [obj] in array.get()) { + // expected-warning@-1{{'auto' deduced as 'id' in declaration of ''}} + // expected-warning@-2{{Local variable '[obj]' is a raw pointer to RetainPtr-capable type}} + array = nullptr; + (void)obj; + } +} + +void iterateRawLocal() { + NSArray *array = provideArray(); + // expected-warning@-1{{Local variable 'array' is a raw pointer to RetainPtr-capable type 'NSArray' [alpha.webkit.UnretainedLocalVarsChecker]}} + for (SomeObj *obj in array) + // expected-warning@-1{{Local variable 'obj' is a raw pointer to RetainPtr-capable type 'SomeObj' [alpha.webkit.UnretainedLocalVarsChecker]}} + [obj doWork]; +} + +} // namespace fast_enumeration + bool doMoreWorkOpaque(OtherObj*); SomeObj* provide(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
