[clang] [alpha.webkit.UncountedLocalVarsChecker] Detect an assignment to a guardian argument (PR #198695)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Wed May 27 13:54:46 PDT 2026
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/198695
>From 3ba95214ec71ee3337cff21b1a5621a6154cc214 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 19 May 2026 19:14:58 -0700
Subject: [PATCH 1/5] [alpha.webkit.UncountedLocalVarsChecker] Detect an
assignment to a guardian argument
A function parameter of type RefPtr<T>& should not be used as a guardian variable of
a raw pointer/reference variable if the function body contains an assignment to it
since such an assignment can shorten the lifetime of the guarded object.
---
.../Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp | 14 ++++++++++++--
.../Checkers/WebKit/uncounted-local-vars.cpp | 13 +++++++++++++
.../Checkers/WebKit/unretained-local-vars.mm | 7 +++++++
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index f720f8e15ed03..aaa3260bc8046 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -340,8 +340,18 @@ class RawPtrRefLocalVarsChecker
// Parameters are guaranteed to be safe for the duration of
// the call by another checker.
- if (isa<ParmVarDecl>(MaybeGuardian))
- return true;
+ if (isa<ParmVarDecl>(MaybeGuardian)) {
+ if (auto *FD = dyn_cast<FunctionDecl>(DeclWithIssue)) {
+ GuardianVisitor guardianVisitor(MaybeGuardian);
+ if (guardianVisitor.TraverseStmt(FD->getBody()))
+ return true;
+ }
+ if (auto *MD = dyn_cast<ObjCMethodDecl>(DeclWithIssue)) {
+ GuardianVisitor guardianVisitor(MaybeGuardian);
+ if (guardianVisitor.TraverseStmt(MD->getBody()))
+ return true;
+ }
+ }
}
}
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 22af4d2116808..f06a1eaa66a0c 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -343,6 +343,19 @@ void baz() {
} // namespace local_assignment_basic
+namespace local_assignment_to_guardian_parameter {
+
+RefCountable *provide_ref_cntbl();
+
+void foo(RefPtr<RefCountable>& arg) {
+ RefCountable* ptr = arg.get();
+ // expected-warning at -1{{Local variable 'ptr' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+ arg = nullptr;
+ ptr->method();
+}
+
+} // namespace local_assignment_to_guardian
+
namespace local_assignment_to_parameter {
RefCountable *provide_ref_cntbl();
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
index f49e7bdb3e79c..04f403eda206f 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm
@@ -594,4 +594,11 @@ - (void)storeSomeObj {
auto *obj = [self getSomeObj];
[obj doWork];
}
+
+- (void)assignToGuardianArg:(RetainPtr<SomeObj>&)obj {
+ SomeObj* ptr = obj.get();
+ // expected-warning at -1{{Local variable 'ptr' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}}
+ obj = nullptr;
+ [ptr doWork];
+}
@end
>From 064da18d1693054546c5a80e5b28a26accdd6b7c Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 20 May 2026 15:42:22 -0700
Subject: [PATCH 2/5] Remove the stale comment.
---
.../Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index aaa3260bc8046..ed3a5a1c6ced2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -338,8 +338,6 @@ class RawPtrRefLocalVarsChecker
}
}
- // Parameters are guaranteed to be safe for the duration of
- // the call by another checker.
if (isa<ParmVarDecl>(MaybeGuardian)) {
if (auto *FD = dyn_cast<FunctionDecl>(DeclWithIssue)) {
GuardianVisitor guardianVisitor(MaybeGuardian);
>From 5840ddf7f25005ec47cbef41512eba335b0e8808 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 27 May 2026 13:49:44 -0700
Subject: [PATCH 3/5] Build & test fix
---
.../Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 68f424db7f0db..7720029934411 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -86,6 +86,8 @@ struct GuardianVisitor : DynamicRecursiveASTVisitor {
auto *Callee = CE->getDirectCallee();
if (!Callee)
return false;
+ if (isPtrConversion(Callee))
+ return true;
unsigned ArgIndex = 0;
unsigned ArgOffset = isa<CXXOperatorCallExpr>(CE);
for (auto *Arg : CE->arguments()) {
@@ -327,13 +329,14 @@ class RawPtrRefLocalVarsChecker
std::optional<bool> IsUncountedPtr = isUnsafePtr(V->getType());
if (IsUncountedPtr && *IsUncountedPtr) {
- if (isPtrOriginSafe(V, Value))
+ if (isPtrOriginSafe(V, Value, DeclWithIssue))
return;
reportBug(V, Value, nullptr, DeclWithIssue);
}
}
- bool isPtrOriginSafe(const VarDecl *V, const Expr *Value) const {
+ bool isPtrOriginSafe(const VarDecl *V, const Expr *Value,
+ const Decl *DeclWithIssue) const {
return tryToFindPtrOrigin(
Value, /*StopAtFirstRefCountedObj=*/false,
[&](const clang::CXXRecordDecl *Record) { return isSafePtr(Record); },
>From 74acc322c147f3ac24bfec5debbd6682975c3198 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 27 May 2026 13:51:29 -0700
Subject: [PATCH 4/5] Address review comments
---
.../Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 7720029934411..404f9291237ef 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -384,13 +384,11 @@ class RawPtrRefLocalVarsChecker
if (isa<ParmVarDecl>(MaybeGuardian)) {
if (auto *FD = dyn_cast<FunctionDecl>(DeclWithIssue)) {
- GuardianVisitor guardianVisitor(MaybeGuardian);
- if (guardianVisitor.TraverseStmt(FD->getBody()))
+ if (GuardianVisitor{MaybeGuardian}.TraverseStmt(FD->getBody()))
return true;
}
if (auto *MD = dyn_cast<ObjCMethodDecl>(DeclWithIssue)) {
- GuardianVisitor guardianVisitor(MaybeGuardian);
- if (guardianVisitor.TraverseStmt(MD->getBody()))
+ if (GuardianVisitor{MaybeGuardian}.TraverseStmt(MD->getBody()))
return true;
}
}
>From 907ffb316fb9d635e830ef8c630239a45b496b71 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 27 May 2026 13:54:19 -0700
Subject: [PATCH 5/5] Fix formatting.
---
.../Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 404f9291237ef..56df6742eda9c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -376,19 +376,20 @@ class RawPtrRefLocalVarsChecker
if (MaybeGuardian->isLocalVarDecl() &&
(isSafePtr(MaybeGuardianArgCXXRecord) ||
isRefcountedStringsHack(MaybeGuardian)) &&
- isGuardedScopeEmbeddedInGuardianScope(
- V, MaybeGuardian))
+ isGuardedScopeEmbeddedInGuardianScope(V, MaybeGuardian))
return true;
}
}
if (isa<ParmVarDecl>(MaybeGuardian)) {
if (auto *FD = dyn_cast<FunctionDecl>(DeclWithIssue)) {
- if (GuardianVisitor{MaybeGuardian}.TraverseStmt(FD->getBody()))
+ if (GuardianVisitor{MaybeGuardian}.TraverseStmt(
+ FD->getBody()))
return true;
}
if (auto *MD = dyn_cast<ObjCMethodDecl>(DeclWithIssue)) {
- if (GuardianVisitor{MaybeGuardian}.TraverseStmt(MD->getBody()))
+ if (GuardianVisitor{MaybeGuardian}.TraverseStmt(
+ MD->getBody()))
return true;
}
}
More information about the cfe-commits
mailing list