[clang] [alpha.webkit.UnretainedLambdaCapturesChecker] Add the support for protectedSelf (PR #132363)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 21 02:52:32 PDT 2025
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/132363
>From cf415a9cf9933bc4e55d3c8da9b27551b09061fe Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 21 Mar 2025 02:32:49 -0700
Subject: [PATCH 1/2] [alpha.webkit.UnretainedLambdaCapturesChecker] Add the
support for protectedSelf
This PR adds the support for treating capturing of "self" as safe if the lambda simultaneously
captures "protectedSelf", which is a RetainPtr of "self".
This PR also fixes a bug that the checker wasn't generating a warning when "self" is implicitly
captured. Note when "self" is implicitly captured, we use the lambda's getBeginLoc as a fallback
source location.
In addition, this PR also fixes a bug that the checker wasn't generating warnings for lambda
caotures when ARC is enabled. While ARC protects variables in the stack, it does not
automatically extend the lifetime of a lambda capture. So we now call RetainTypeChecker's
isUnretained with ignoreARC set to true.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 2 +-
.../WebKit/RawPtrRefLambdaCapturesChecker.cpp | 58 +++++++++++++++----
.../WebKit/unretained-lambda-captures-arc.mm | 8 +++
.../WebKit/unretained-lambda-captures.mm | 16 +++++
4 files changed, 73 insertions(+), 11 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index b4d2353a03cd2..a161cf644b7d9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -147,7 +147,7 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
bool isCtorOfRetainPtr(const clang::FunctionDecl *F) {
const std::string &FunctionName = safeGetName(F);
return FunctionName == "RetainPtr" || FunctionName == "adoptNS" ||
- FunctionName == "adoptCF";
+ FunctionName == "adoptCF" || FunctionName == "retainPtr";
}
bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index 8496a75c1b84f..e8dcf3aaf518e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -36,6 +36,7 @@ class RawPtrRefLambdaCapturesChecker
: Bug(this, description, "WebKit coding guidelines") {}
virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
+ virtual bool isPtrType(const std::string &) const = 0;
virtual const char *ptrKind(QualType QT) const = 0;
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -68,6 +69,15 @@ class RawPtrRefLambdaCapturesChecker
return DynamicRecursiveASTVisitor::TraverseCXXMethodDecl(CXXMD);
}
+ bool TraverseObjCMethodDecl(ObjCMethodDecl *OCMD) override {
+ llvm::SaveAndRestore SavedDecl(ClsType);
+ if (OCMD && OCMD->isInstanceMethod()) {
+ if (auto *ImplParamDecl = OCMD->getSelfDecl())
+ ClsType = ImplParamDecl->getType();
+ }
+ return DynamicRecursiveASTVisitor::TraverseObjCMethodDecl(OCMD);
+ }
+
bool VisitTypedefDecl(TypedefDecl *TD) override {
if (Checker->RTC)
Checker->RTC->visitTypedef(TD);
@@ -287,7 +297,7 @@ class RawPtrRefLambdaCapturesChecker
if (!Ctor)
return false;
auto clsName = safeGetName(Ctor->getParent());
- if (isRefType(clsName) && CE->getNumArgs()) {
+ if (Checker->isPtrType(clsName) && CE->getNumArgs()) {
Arg = CE->getArg(0)->IgnoreParenCasts();
continue;
}
@@ -307,6 +317,12 @@ class RawPtrRefLambdaCapturesChecker
Arg = CE->getArg(0)->IgnoreParenCasts();
continue;
}
+ if (auto *Callee = CE->getDirectCallee()) {
+ if (isCtorOfSafePtr(Callee) && CE->getNumArgs() == 1) {
+ Arg = CE->getArg(0)->IgnoreParenCasts();
+ continue;
+ }
+ }
}
if (auto *OpCE = dyn_cast<CXXOperatorCallExpr>(Arg)) {
auto OpCode = OpCE->getOperator();
@@ -315,7 +331,7 @@ class RawPtrRefLambdaCapturesChecker
if (!Callee)
return false;
auto clsName = safeGetName(Callee->getParent());
- if (!isRefType(clsName) || !OpCE->getNumArgs())
+ if (!Checker->isPtrType(clsName) || !OpCE->getNumArgs())
return false;
Arg = OpCE->getArg(0)->IgnoreParenCasts();
continue;
@@ -330,8 +346,15 @@ class RawPtrRefLambdaCapturesChecker
}
break;
} while (Arg);
- if (auto *DRE = dyn_cast<DeclRefExpr>(Arg))
- return ProtectedThisDecls.contains(DRE->getDecl());
+ if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+ auto *Decl = DRE->getDecl();
+ if (auto *ImplicitParam = dyn_cast<ImplicitParamDecl>(Decl)) {
+ auto kind = ImplicitParam->getParameterKind();
+ return kind == ImplicitParamKind::ObjCSelf ||
+ kind == ImplicitParamKind::CXXThis;
+ }
+ return ProtectedThisDecls.contains(Decl);
+ }
return isa<CXXThisExpr>(Arg);
}
};
@@ -351,10 +374,16 @@ class RawPtrRefLambdaCapturesChecker
ValueDecl *CapturedVar = C.getCapturedVar();
if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
continue;
+ if (auto *ImplicitParam = dyn_cast<ImplicitParamDecl>(CapturedVar)) {
+ auto kind = ImplicitParam->getParameterKind();
+ if ((kind == ImplicitParamKind::ObjCSelf ||
+ kind == ImplicitParamKind::CXXThis) && !shouldCheckThis)
+ continue;
+ }
QualType CapturedVarQualType = CapturedVar->getType();
auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
if (IsUncountedPtr && *IsUncountedPtr)
- reportBug(C, CapturedVar, CapturedVarQualType);
+ reportBug(C, CapturedVar, CapturedVarQualType, L);
} else if (C.capturesThis() && shouldCheckThis) {
if (ignoreParamVarDecl) // this is always a parameter to this function.
continue;
@@ -364,11 +393,12 @@ class RawPtrRefLambdaCapturesChecker
}
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
- const QualType T) const {
+ const QualType T, LambdaExpr *L) const {
assert(CapturedVar);
- if (isa<ImplicitParamDecl>(CapturedVar) && !Capture.getLocation().isValid())
- return; // Ignore implicit captruing of self.
+ auto Location = Capture.getLocation();
+ if (isa<ImplicitParamDecl>(CapturedVar) && !Location.isValid())
+ Location = L->getBeginLoc();
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
@@ -387,7 +417,7 @@ class RawPtrRefLambdaCapturesChecker
printQuotedQualifiedName(Os, CapturedVar);
Os << " to " << ptrKind(T) << " type is unsafe.";
- PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
+ PathDiagnosticLocation BSLoc(Location, BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
BR->emitReport(std::move(Report));
}
@@ -429,6 +459,10 @@ class UncountedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
return result2;
}
+ virtual bool isPtrType(const std::string &Name) const final {
+ return isRefType(Name) || isCheckedPtr(Name);
+ }
+
const char *ptrKind(QualType QT) const final {
if (isUncounted(QT))
return "uncounted";
@@ -445,7 +479,11 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
}
std::optional<bool> isUnsafePtr(QualType QT) const final {
- return RTC->isUnretained(QT);
+ return RTC->isUnretained(QT, /* ignoreARC */ true);
+ }
+
+ virtual bool isPtrType(const std::string &Name) const final {
+ return isRetainPtr(Name);
}
const char *ptrKind(QualType QT) const final { return "unretained"; }
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 4e3d9c2708d96..3b718e1e3d76f 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm
@@ -123,19 +123,23 @@ explicit CallableWrapper(CallableType& callable)
void raw_ptr() {
SomeObj* obj = make_obj();
auto foo1 = [obj](){
+ // expected-warning at -1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
[obj doWork];
};
call(foo1);
auto foo2 = [&obj](){
+ // expected-warning at -1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
[obj doWork];
};
auto foo3 = [&](){
[obj doWork];
+ // expected-warning at -1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
obj = nullptr;
};
auto foo4 = [=](){
[obj doWork];
+ // expected-warning at -1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
};
auto cf = make_cf();
@@ -218,6 +222,7 @@ void noescape_lambda() {
[otherObj doWork];
}, [&](SomeObj *obj) {
[otherObj doWork];
+ // expected-warning at -1{{Implicitly captured raw-pointer 'otherObj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
});
([&] {
[someObj doWork];
@@ -242,11 +247,13 @@ void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf)
{
callFunction([&]() {
[obj doWork];
+ // expected-warning at -1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
CFArrayAppendValue(cf, nullptr);
// expected-warning at -1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
});
callFunctionOpaque([&]() {
[obj doWork];
+ // expected-warning at -1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
CFArrayAppendValue(cf, nullptr);
// expected-warning at -1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
});
@@ -262,6 +269,7 @@ -(void)run;
@implementation ObjWithSelf
-(void)doWork {
auto doWork = [&] {
+ // expected-warning at -1{{Implicitly captured raw-pointer 'self' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
someFunction();
[delegate doWork];
};
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm
index 073eff9386baa..33be0eaaae914 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm
@@ -279,17 +279,33 @@ @interface ObjWithSelf : NSObject {
RetainPtr<id> delegate;
}
-(void)doWork;
+-(void)doMoreWork;
-(void)run;
@end
@implementation ObjWithSelf
-(void)doWork {
auto doWork = [&] {
+ // expected-warning at -1{{Implicitly captured raw-pointer 'self' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}}
+ someFunction();
+ [delegate doWork];
+ };
+ auto doExtraWork = [&, protectedSelf = retainPtr(self)] {
someFunction();
[delegate doWork];
};
callFunctionOpaque(doWork);
+ callFunctionOpaque(doExtraWork);
}
+
+-(void)doMoreWork {
+ auto doWork = [self, protectedSelf = retainPtr(self)] {
+ someFunction();
+ [self doWork];
+ };
+ callFunctionOpaque(doWork);
+}
+
-(void)run {
someFunction();
}
>From 895d0e97caf176a5111dafa40a55ec2f33896f32 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 21 Mar 2025 02:52:17 -0700
Subject: [PATCH 2/2] Fix formatting.
---
.../Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index e8dcf3aaf518e..8ecb6da45e016 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -377,7 +377,8 @@ class RawPtrRefLambdaCapturesChecker
if (auto *ImplicitParam = dyn_cast<ImplicitParamDecl>(CapturedVar)) {
auto kind = ImplicitParam->getParameterKind();
if ((kind == ImplicitParamKind::ObjCSelf ||
- kind == ImplicitParamKind::CXXThis) && !shouldCheckThis)
+ kind == ImplicitParamKind::CXXThis) &&
+ !shouldCheckThis)
continue;
}
QualType CapturedVarQualType = CapturedVar->getType();
More information about the cfe-commits
mailing list