https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/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. >From 80026828135383a808bfdd604bb50b9737677736 Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Sat, 10 Oct 2026 11:03:06 -0700 Subject: [PATCH] [alpha.webkit.UnretainedLocalVarsChecker] Treat the collection of a fast enumeration as the origin of its element 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. --- .../WebKit/RawPtrRefLocalVarsChecker.cpp | 39 ++++++++- .../Checkers/WebKit/unretained-local-vars.mm | 85 +++++++++++++++++++ 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 232cca1c7a2c4..33935861d153a 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" @@ -214,6 +215,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(); @@ -425,6 +444,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)) return; @@ -503,9 +529,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..372eb3ee874b7 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -574,6 +574,91 @@ 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 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
