Skip to content

Commit a71f9e6

Browse files
authored
[webkit.UncountedLambdaCapturesChecker] Detect protectedThis pattern. (#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.
1 parent 4307198 commit a71f9e6

File tree

2 files changed

+128
-2
lines changed

2 files changed

+128
-2
lines changed

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

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ class UncountedLambdaCapturesChecker
115115
if (ArgIndex >= CE->getNumArgs())
116116
return true;
117117
auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
118-
if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
118+
if (auto *L = findLambdaInArg(Arg)) {
119119
LambdasToIgnore.insert(L);
120120
if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape)
121121
Checker->visitLambdaExpr(L, shouldCheckThis());
@@ -126,6 +126,38 @@ class UncountedLambdaCapturesChecker
126126
return true;
127127
}
128128

129+
LambdaExpr *findLambdaInArg(Expr *E) {
130+
if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
131+
return Lambda;
132+
auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E);
133+
if (!TempExpr)
134+
return nullptr;
135+
E = TempExpr->getSubExpr()->IgnoreParenCasts();
136+
if (!E)
137+
return nullptr;
138+
if (auto *Lambda = dyn_cast<LambdaExpr>(E))
139+
return Lambda;
140+
auto *CE = dyn_cast_or_null<CXXConstructExpr>(E);
141+
if (!CE || !CE->getNumArgs())
142+
return nullptr;
143+
auto *CtorArg = CE->getArg(0)->IgnoreParenCasts();
144+
if (!CtorArg)
145+
return nullptr;
146+
if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg))
147+
return Lambda;
148+
auto *DRE = dyn_cast<DeclRefExpr>(CtorArg);
149+
if (!DRE)
150+
return nullptr;
151+
auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
152+
if (!VD)
153+
return nullptr;
154+
auto *Init = VD->getInit();
155+
if (!Init)
156+
return nullptr;
157+
TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
158+
return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr());
159+
}
160+
129161
void checkCalleeLambda(CallExpr *CE) {
130162
auto *Callee = CE->getCallee();
131163
if (!Callee)
@@ -180,11 +212,53 @@ class UncountedLambdaCapturesChecker
180212
} else if (C.capturesThis() && shouldCheckThis) {
181213
if (ignoreParamVarDecl) // this is always a parameter to this function.
182214
continue;
183-
reportBugOnThisPtr(C);
215+
bool hasProtectThis = false;
216+
for (const LambdaCapture &OtherCapture : L->captures()) {
217+
if (!OtherCapture.capturesVariable())
218+
continue;
219+
if (auto *ValueDecl = OtherCapture.getCapturedVar()) {
220+
if (protectThis(ValueDecl)) {
221+
hasProtectThis = true;
222+
break;
223+
}
224+
}
225+
}
226+
if (!hasProtectThis)
227+
reportBugOnThisPtr(C);
184228
}
185229
}
186230
}
187231

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

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

Lines changed: 52 additions & 0 deletions
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)