https://github.com/rniwa updated
https://github.com/llvm/llvm-project/pull/119932
>From 5c032267f263fb6b7f10d25745d14e63b2f7af59 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 13 Dec 2024 14:34:39 -0800
Subject: [PATCH 1/3] [webkit.UncountedLambdaCapturesChecker] Detect
protectedThis pattern.
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.
---
.../WebKit/UncountedLambdaCapturesChecker.cpp | 79 ++++++++++++++++++-
.../WebKit/uncounted-lambda-captures.cpp | 47 ++++++++---
2 files changed, 113 insertions(+), 13 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 599c2179db0f0e..299038262b8d36 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -96,9 +96,8 @@ class UncountedLambdaCapturesChecker
return true;
auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
- if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg)) {
- Checker->visitLambdaExpr(L, shouldCheckThis());
- }
+ if (auto *Lambda = findLambdaInArg(Arg))
+ Checker->visitLambdaExpr(Lambda, shouldCheckThis());
}
++ArgIndex;
}
@@ -106,6 +105,38 @@ class UncountedLambdaCapturesChecker
return true;
}
+ LambdaExpr* findLambdaInArg(Expr* E) {
+ if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
+ return Lambda;
+ auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E);
+ if (!TempExpr)
+ return nullptr;
+ E = TempExpr->getSubExpr()->IgnoreParenCasts();
+ if (!E)
+ return nullptr;
+ if (auto *Lambda = dyn_cast<LambdaExpr>(E))
+ return Lambda;
+ auto *CE = dyn_cast_or_null<CXXConstructExpr>(E);
+ if (!CE || !CE->getNumArgs())
+ return nullptr;
+ auto *CtorArg = CE->getArg(0)->IgnoreParenCasts();
+ if (!CtorArg)
+ return nullptr;
+ if (auto *Lambda = dyn_cast<LambdaExpr>(CtorArg))
+ return Lambda;
+ auto *DRE = dyn_cast<DeclRefExpr>(CtorArg);
+ if (!DRE)
+ return nullptr;
+ auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
+ if (!VD)
+ return nullptr;
+ auto* Init = VD->getInit();
+ if (!Init)
+ return nullptr;
+ TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
+ return dyn_cast_or_null<LambdaExpr>(TempExpr->getSubExpr());
+ }
+
void checkCalleeLambda(CallExpr *CE) {
auto *Callee = CE->getCallee();
if (!Callee)
@@ -155,11 +186,51 @@ class UncountedLambdaCapturesChecker
} else if (C.capturesThis() && shouldCheckThis) {
if (ignoreParamVarDecl) // this is always a parameter to this function.
continue;
- reportBugOnThisPtr(C);
+ bool hasProtectThis = false;
+ for (const LambdaCapture &OtherCapture : L->captures()) {
+ if (auto *ValueDecl = OtherCapture.getCapturedVar()) {
+ if (protectThis(ValueDecl)) {
+ hasProtectThis = true;
+ break;
+ }
+ }
+ }
+ if (!hasProtectThis)
+ reportBugOnThisPtr(C);
}
}
}
+ bool protectThis(const ValueDecl *ValueDecl) const {
+ auto* VD = dyn_cast<VarDecl>(ValueDecl);
+ if (!VD)
+ return false;
+ auto *Init = VD->getInit()->IgnoreParenCasts();
+ if (!Init)
+ return false;
+ auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Init);
+ if (!BTE)
+ return false;
+ auto *CE = dyn_cast_or_null<CXXConstructExpr>(BTE->getSubExpr());
+ if (!CE)
+ return false;
+ auto* Ctor = CE->getConstructor();
+ if (!Ctor)
+ return false;
+ auto clsName = safeGetName(Ctor->getParent());
+ if (!isRefType(clsName) || !CE->getNumArgs())
+ return false;
+ auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+ while (auto *UO = dyn_cast<UnaryOperator>(Arg)) {
+ auto OpCode = UO->getOpcode();
+ if (OpCode == UO_Deref || OpCode == UO_AddrOf)
+ Arg = UO->getSubExpr();
+ else
+ break;
+ }
+ return isa<CXXThisExpr>(Arg);
+ }
+
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
const QualType T) const {
assert(CapturedVar);
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 65eee9d49106df..106fa02f4a89ef 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -1,18 +1,11 @@
// RUN: %clang_analyze_cc1
-analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
+#include "mock-types.h"
+
struct A {
static void b();
};
-struct RefCountable {
- void ref() {}
- void deref() {}
- void method();
- void constMethod() const;
- int trivial() { return 123; }
- RefCountable* next();
-};
-
RefCountable* make_obj();
void someFunction();
@@ -151,6 +144,42 @@ struct RefCountableWithLambdaCapturingThis {
};
call(lambda);
}
+
+ void method_captures_this_unsafe_capture_local_var_explicitly() {
+ RefCountable* x = make_obj();
+ call([this, protectedThis = RefPtr { this }, x]() {
+ // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or
CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ nonTrivial();
+ x->method();
+ });
+ }
+
+ void method_captures_this_unsafe_capture_local_var_explicitly_with_deref() {
+ RefCountable* x = make_obj();
+ call([this, protectedThis = Ref { *this }, x]() {
+ // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or
CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ nonTrivial();
+ x->method();
+ });
+ }
+
+ void method_captures_this_unsafe_local_var_via_vardecl() {
+ RefCountable* x = make_obj();
+ auto lambda = [this, protectedThis = Ref { *this }, x]() {
+ // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or
CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ nonTrivial();
+ x->method();
+ };
+ call(lambda);
+ }
+
+ void method_captures_this_with_guardian() {
+ auto lambda = [this, protectedThis = Ref { *this }]() {
+ nonTrivial();
+ };
+ call(lambda);
+ }
+
};
struct NonRefCountableWithLambdaCapturingThis {
>From 5b9e8fe00764cee42bdcb9e3031ba84ca4168690 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 13 Dec 2024 19:08:41 -0800
Subject: [PATCH 2/3] Fix formatting
---
.../Checkers/WebKit/UncountedLambdaCapturesChecker.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 299038262b8d36..20de89279ac620 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -105,7 +105,7 @@ class UncountedLambdaCapturesChecker
return true;
}
- LambdaExpr* findLambdaInArg(Expr* E) {
+ LambdaExpr *findLambdaInArg(Expr* E) {
if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
return Lambda;
auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E);
@@ -130,7 +130,7 @@ class UncountedLambdaCapturesChecker
auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
if (!VD)
return nullptr;
- auto* Init = VD->getInit();
+ auto *Init = VD->getInit();
if (!Init)
return nullptr;
TempExpr = dyn_cast<CXXBindTemporaryExpr>(Init->IgnoreParenCasts());
@@ -202,7 +202,7 @@ class UncountedLambdaCapturesChecker
}
bool protectThis(const ValueDecl *ValueDecl) const {
- auto* VD = dyn_cast<VarDecl>(ValueDecl);
+ auto *VD = dyn_cast<VarDecl>(ValueDecl);
if (!VD)
return false;
auto *Init = VD->getInit()->IgnoreParenCasts();
@@ -214,7 +214,7 @@ class UncountedLambdaCapturesChecker
auto *CE = dyn_cast_or_null<CXXConstructExpr>(BTE->getSubExpr());
if (!CE)
return false;
- auto* Ctor = CE->getConstructor();
+ auto *Ctor = CE->getConstructor();
if (!Ctor)
return false;
auto clsName = safeGetName(Ctor->getParent());
>From f8ca92dd5605a8fd46ad26a5e38942def078d14b Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 13 Dec 2024 19:44:15 -0800
Subject: [PATCH 3/3] Fix more formatting
---
.../Checkers/WebKit/UncountedLambdaCapturesChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 20de89279ac620..201b8770adcd77 100644
---
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -105,7 +105,7 @@ class UncountedLambdaCapturesChecker
return true;
}
- LambdaExpr *findLambdaInArg(Expr* E) {
+ LambdaExpr *findLambdaInArg(Expr *E) {
if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
return Lambda;
auto *TempExpr = dyn_cast_or_null<CXXBindTemporaryExpr>(E);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
