Skip to content

Commit f55b40e

Browse files
committedFeb 7, 2025
[webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. (llvm#120528)
In WebKit, we often capture this as Ref or RefPtr in addition to this itself so that the object lives as long as a capturing lambda stays alive. Detect this pattern and treat it as safe. This PR also makes the check for a lambda being passed as a function argument more robust by handling CXXBindTemporaryExpr, CXXConstructExpr, and DeclRefExpr referring to the lambda.

File tree

2 files changed

+128
-2
lines changed

2 files changed

+128
-2
lines changed
 

‎clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp

+76-2
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ class UncountedLambdaCapturesChecker
118118
if (ArgIndex >= CE->getNumArgs())
119119
return true;
120120
auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
121-
if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
121+
if (auto *L = findLambdaInArg(Arg)) {
122122
LambdasToIgnore.insert(L);
123123
if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape)
124124
Checker->visitLambdaExpr(L, shouldCheckThis());
@@ -129,6 +129,38 @@ class UncountedLambdaCapturesChecker
129129
return true;
130130
}
131131

132+
LambdaExpr *findLambdaInArg(Expr *E) {
133+
if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
134+
return Lambda;
135+
auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E);
136+
if (!TempExpr)
137+
return nullptr;
138+
E = TempExpr->getSubExpr()->IgnoreParenCasts();
139+
if (!E)
140+
return nullptr;
141+
if (auto *Lambda = dyn_cast<LambdaExpr>(E))
142+
return Lambda;
143+
auto *CE = dyn_cast_or_null<CXXConstructExpr>(E);
144+
if (!CE || !CE->getNumArgs())
145+
return nullptr;
146+
auto *CtorArg = CE->getArg(0)->IgnoreParenCasts();
147+
if (!CtorArg)
148+
return nullptr;
149+
if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg))
150+
return Lambda;
151+
auto *DRE = dyn_cast<DeclRefExpr>(CtorArg);
152+
if (!DRE)
153+
return nullptr;
154+
auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
155+
if (!VD)
156+
return nullptr;
157+
auto *Init = VD->getInit();
158+
if (!Init)
159+
return nullptr;
160+
TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
161+
return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr());
162+
}
163+
132164
void checkCalleeLambda(CallExpr *CE) {
133165
auto *Callee = CE->getCallee();
134166
if (!Callee)
@@ -183,11 +215,53 @@ class UncountedLambdaCapturesChecker
183215
} else if (C.capturesThis() && shouldCheckThis) {
184216
if (ignoreParamVarDecl) // this is always a parameter to this function.
185217
continue;
186-
reportBugOnThisPtr(C);
218+
bool hasProtectThis = false;
219+
for (const LambdaCapture &OtherCapture : L->captures()) {
220+
if (!OtherCapture.capturesVariable())
221+
continue;
222+
if (auto *ValueDecl = OtherCapture.getCapturedVar()) {
223+
if (protectThis(ValueDecl)) {
224+
hasProtectThis = true;
225+
break;
226+
}
227+
}
228+
}
229+
if (!hasProtectThis)
230+
reportBugOnThisPtr(C);
187231
}
188232
}
189233
}
190234

235+
bool protectThis(const ValueDecl *ValueDecl) const {
236+
auto *VD = dyn_cast<VarDecl>(ValueDecl);
237+
if (!VD)
238+
return false;
239+
auto *Init = VD->getInit()->IgnoreParenCasts();
240+
if (!Init)
241+
return false;
242+
auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Init);
243+
if (!BTE)
244+
return false;
245+
auto *CE = dyn_cast_or_null<CXXConstructExpr>(BTE->getSubExpr());
246+
if (!CE)
247+
return false;
248+
auto *Ctor = CE->getConstructor();
249+
if (!Ctor)
250+
return false;
251+
auto clsName = safeGetName(Ctor->getParent());
252+
if (!isRefType(clsName) || !CE->getNumArgs())
253+
return false;
254+
auto *Arg = CE->getArg(0)->IgnoreParenCasts();
255+
while (auto *UO = dyn_cast<UnaryOperator>(Arg)) {
256+
auto OpCode = UO->getOpcode();
257+
if (OpCode == UO_Deref || OpCode == UO_AddrOf)
258+
Arg = UO->getSubExpr();
259+
else
260+
break;
261+
}
262+
return isa<CXXThisExpr>(Arg);
263+
}
264+
191265
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
192266
const QualType T) const {
193267
assert(CapturedVar);

‎clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp

+52
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,58 @@ struct RefCountableWithLambdaCapturingThis {
207207
};
208208
call(lambda);
209209
}
210+
211+
void method_captures_this_unsafe_capture_local_var_explicitly() {
212+
RefCountable* x = make_obj();
213+
call([this, protectedThis = RefPtr { this }, x]() {
214+
// expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
215+
nonTrivial();
216+
x->method();
217+
});
218+
}
219+
220+
void method_captures_this_with_other_protected_var() {
221+
RefCountable* x = make_obj();
222+
call([this, protectedX = RefPtr { x }]() {
223+
// expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
224+
nonTrivial();
225+
protectedX->method();
226+
});
227+
}
228+
229+
void method_captures_this_unsafe_capture_local_var_explicitly_with_deref() {
230+
RefCountable* x = make_obj();
231+
call([this, protectedThis = Ref { *this }, x]() {
232+
// expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
233+
nonTrivial();
234+
x->method();
235+
});
236+
}
237+
238+
void method_captures_this_unsafe_local_var_via_vardecl() {
239+
RefCountable* x = make_obj();
240+
auto lambda = [this, protectedThis = Ref { *this }, x]() {
241+
// expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
242+
nonTrivial();
243+
x->method();
244+
};
245+
call(lambda);
246+
}
247+
248+
void method_captures_this_with_guardian() {
249+
auto lambda = [this, protectedThis = Ref { *this }]() {
250+
nonTrivial();
251+
};
252+
call(lambda);
253+
}
254+
255+
void method_captures_this_with_guardian_refPtr() {
256+
auto lambda = [this, protectedThis = RefPtr { &*this }]() {
257+
nonTrivial();
258+
};
259+
call(lambda);
260+
}
261+
210262
};
211263

212264
struct NonRefCountableWithLambdaCapturingThis {

0 commit comments

Comments
 (0)
Please sign in to comment.