[clang] [alpha.webkit.UnretainedLambdaCapturesChecker] Add the support for protectedSelf (PR #132518)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 21 21:33:50 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Ryosuke Niwa (rniwa)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/132518.diff
3 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+1-1)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp (+50-11)
- (modified) clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm (+16)
``````````diff
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..da403d591a2e2 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);
@@ -275,10 +285,10 @@ class RawPtrRefLambdaCapturesChecker
auto *VD = dyn_cast<VarDecl>(ValueDecl);
if (!VD)
return false;
- auto *Init = VD->getInit()->IgnoreParenCasts();
+ auto *Init = VD->getInit();
if (!Init)
return false;
- const Expr *Arg = Init;
+ const Expr *Arg = Init->IgnoreParenCasts();
do {
if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Arg))
Arg = BTE->getSubExpr()->IgnoreParenCasts();
@@ -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,17 @@ 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 +394,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 +418,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 +460,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";
@@ -448,6 +483,10 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
return RTC->isUnretained(QT);
}
+ 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.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();
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/132518
More information about the cfe-commits
mailing list