Author: Fady Farag Date: 2026-09-22T14:56:44-07:00 New Revision: 88dcdc2d619dd09665beb67479a330c36f7251b7
URL: https://github.com/llvm/llvm-project/commit/88dcdc2d619dd09665beb67479a330c36f7251b7 DIFF: https://github.com/llvm/llvm-project/commit/88dcdc2d619dd09665beb67479a330c36f7251b7.diff LOG: [alpha.webkit.UncountedLocalVarsChecker] Don't treat a call through a raw pointer/reference guardian argument as a mutation (#225309) Previously, when a function parameter was used as the guardian of a raw pointer/reference local, `GuardianVisitor` treated any non-const member function call on the parameter, or passing it to a non-const parameter, as a mutation. That is correct for a `RefPtr<T>&` parameter but not for a raw pointer/reference parameter, where such a call operates on the pointee and cannot change the parameter, which caused an unnecessary warning whenever the parameter was used at all. This only treats a raw pointer/reference guardian argument as mutated when it is assigned to, or when the callee can reseat it by receiving it as `Bar*&` or `Bar**`. Added: Modified: clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 2d34ed9e4fae3..d648c7f4f45d6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -52,8 +52,11 @@ bool isRefcountedStringsHack(const VarDecl *V) { struct GuardianVisitor : DynamicRecursiveASTVisitor { const VarDecl *Guardian{nullptr}; + bool GuardianIsRawPtrOrRef{false}; - explicit GuardianVisitor(const VarDecl *Guardian) : Guardian(Guardian) { + explicit GuardianVisitor(const VarDecl *Guardian, + bool GuardianIsRawPtrOrRef = false) + : Guardian(Guardian), GuardianIsRawPtrOrRef(GuardianIsRawPtrOrRef) { assert(Guardian); } @@ -110,6 +113,8 @@ struct GuardianVisitor : DynamicRecursiveASTVisitor { } bool VisitCXXMemberCallExpr(CXXMemberCallExpr *MCE) override { + if (GuardianIsRawPtrOrRef) + return true; auto *Method = MCE->getMethodDecl(); auto ObjType = MCE->getObjectType(); if (ObjType.isConstQualified()) @@ -125,14 +130,29 @@ struct GuardianVisitor : DynamicRecursiveASTVisitor { private: bool mutatesGuardian(const Expr *Arg, const ParmVarDecl *ParmDecl) { Arg = Arg->IgnoreParenCasts(); - if (auto *VarRef = dyn_cast<DeclRefExpr>(Arg)) { - if (VarRef->getDecl() == Guardian) { - auto ArgType = ParmDecl ? ParmDecl->getType() : Arg->getType(); - if (!ArgType.isConstQualified()) - return true; - } + auto ArgType = ParmDecl ? ParmDecl->getType() : Arg->getType(); + bool IsAddressOf = false; + if (auto *UO = dyn_cast<UnaryOperator>(Arg); + UO && UO->getOpcode() == UO_AddrOf) { + Arg = UO->getSubExpr()->IgnoreParenCasts(); + IsAddressOf = true; } - return false; + auto *VarRef = dyn_cast<DeclRefExpr>(Arg); + if (!VarRef || VarRef->getDecl() != Guardian) + return false; + if (GuardianIsRawPtrOrRef && !Guardian->getType()->isPointerType()) + return false; + if (IsAddressOf) { + if (!ArgType->isPointerType()) + return false; + return !ArgType->getPointeeType().isConstQualified(); + } + if (GuardianIsRawPtrOrRef) { + if (!ArgType->isReferenceType()) + return false; + return !ArgType.getNonReferenceType().isConstQualified(); + } + return !ArgType.isConstQualified(); } }; @@ -406,10 +426,12 @@ class RawPtrRefLocalVarsChecker } if (isa<ParmVarDecl>(MaybeGuardian)) { + bool IsRawPtrOrRef = isUnsafePtr(GuardianType).value_or(false); + GuardianVisitor Visitor{MaybeGuardian, IsRawPtrOrRef}; if (auto *FD = dyn_cast<FunctionDecl>(DeclWithIssue)) - return GuardianVisitor{MaybeGuardian}.TraverseStmt(FD->getBody()); + return Visitor.TraverseStmt(FD->getBody()); if (auto *MD = dyn_cast<ObjCMethodDecl>(DeclWithIssue)) - return GuardianVisitor{MaybeGuardian}.TraverseStmt(MD->getBody()); + return Visitor.TraverseStmt(MD->getBody()); } return false; diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 2a3d9f2fefab8..656a755022d1e 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -420,6 +420,90 @@ void foo(RefPtr<RefCountable>& arg) { } // namespace local_assignment_to_guardian +namespace raw_pointer_or_reference_guardian_parameter { + +void consume(RefCountable&); +void consumeConst(const RefCountable&); +void consumePtr(RefCountable*); +void reseat(RefCountable*&); + +void memberCallThroughReference(RefCountable& obj) { + RefCountable& ref = obj; + obj.method(); + ref.method(); +} + +void memberCallThroughPointer(RefCountable* obj) { + RefCountable* ptr = obj; + obj->method(); + ptr->method(); +} + +void passReferenceToNonConstReference(RefCountable& obj) { + RefCountable& ref = obj; + consume(obj); + ref.method(); +} + +void passConstReferenceToConstReference(const RefCountable& obj) { + const RefCountable& ref = obj; + consumeConst(obj); + ref.constMethod(); +} + +void passPointerByValue(RefCountable* obj) { + RefCountable* ptr = obj; + consumePtr(obj); + ptr->method(); +} + +void passPointerByNonConstReference(RefCountable* obj) { + RefCountable* ptr = obj; + // expected-warning@-1{{Local variable 'ptr' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + reseat(obj); + ptr->method(); +} + +void assignToPointer(RefCountable* obj, RefCountable* other) { + RefCountable* ptr = obj; + // expected-warning@-1{{Local variable 'ptr' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + obj = other; + ptr->method(); +} + +void reseatViaPointerToPointer(RefCountable**); +void readViaPointerToConstPointer(RefCountable* const*); + +void passAddressOfPointer(RefCountable* obj) { + RefCountable* ptr = obj; + // expected-warning@-1{{Local variable 'ptr' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + reseatViaPointerToPointer(&obj); + ptr->method(); +} + +void passAddressOfPointerAsConst(RefCountable* obj) { + RefCountable* ptr = obj; + readViaPointerToConstPointer(&obj); + ptr->method(); +} + +void passAddressOfReference(RefCountable& obj) { + RefCountable& ref = obj; + consumePtr(&obj); + ref.method(); +} + +void reseatSmartPointer(RefPtr<RefCountable>*); + +void passAddressOfSmartPointerParameter(RefPtr<RefCountable>& guardian) { + RefCountable* ptr = guardian.get(); + // expected-warning@-1{{Local variable 'ptr' is a raw pointer to RefPtr-capable type 'RefCountable' [alpha.webkit.UncountedLocalVarsChecker]}} + reseatSmartPointer(&guardian); + ptr->method(); +} + +} // namespace raw_pointer_or_reference_guardian_parameter + namespace local_assignment_to_parameter { RefCountable *provide_ref_cntbl(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
