diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 31bebdb07dbdc..5412fea6f1323 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -222,7 +222,13 @@ std::optional<bool> isUncheckedPtr(const QualType T) {
std::optional<bool> isUnsafePtr(const QualType T) {
if (T->isPointerType() || T->isReferenceType()) {
if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
- return isUncounted(CXXRD) || isUnchecked(CXXRD);
+ auto isUncountedPtr = isUncounted(CXXRD);
+ auto isUncheckedPtr = isUnchecked(CXXRD);
+ if (isUncountedPtr && isUncheckedPtr)
+ return *isUncountedPtr || *isUncheckedPtr;
+ if (isUncountedPtr)
+ return *isUncountedPtr;
+ return isUncheckedPtr;
}
}
return false;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 7c6c0a63f22ab..681ea55c61355 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -75,6 +75,10 @@ std::optional<bool> isUncountedPtr(const clang::QualType T);
/// class, false if not, std::nullopt if inconclusive.
std::optional<bool> isUncheckedPtr(const clang::QualType T);
+/// \returns true if \p T is either a raw pointer or reference to an uncounted
+/// or unchecked class, false if not, std::nullopt if inconclusive.
+std::optional<bool> isUnsafePtr(const QualType T);
+
/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its
/// variant, false if not.
bool isSafePtrType(const clang::QualType T);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
index 998bd4ccee07d..914ff6d44270d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "ASTUtils.h"
#include "DiagOutputUtils.h"
#include "PtrTypesSemantics.h"
#include "clang/AST/CXXInheritance.h"
@@ -26,6 +27,7 @@ class UncountedLambdaCapturesChecker
BugType Bug{this, "Lambda capture of uncounted variable",
"WebKit coding guidelines"};
mutable BugReporter *BR = nullptr;
+ TrivialFunctionAnalysis TFA;
public:
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -37,6 +39,11 @@ class UncountedLambdaCapturesChecker
// want to visit those, so we make our own RecursiveASTVisitor.
struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
const UncountedLambdaCapturesChecker *Checker;
+ llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
+ QualType ClsType;
+
+ using Base = RecursiveASTVisitor<LocalVisitor>;
+
explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
: Checker(Checker) {
assert(Checker);
@@ -45,32 +52,118 @@ class UncountedLambdaCapturesChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
- bool VisitLambdaExpr(LambdaExpr *L) {
- Checker->visitLambdaExpr(L);
+ bool TraverseCXXMethodDecl(CXXMethodDecl *CXXMD) {
+ llvm::SaveAndRestore SavedDecl(ClsType);
+ ClsType = CXXMD->getThisType();
+ return Base::TraverseCXXMethodDecl(CXXMD);
+ }
+
+ bool shouldCheckThis() {
+ auto result = !ClsType.isNull() ? isUnsafePtr(ClsType) : std::nullopt;
+ return result && *result;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+ if (DeclRefExprsToIgnore.contains(DRE))
+ return true;
+ auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
+ if (!VD)
+ return true;
+ auto *Init = VD->getInit();
+ if (!Init)
+ return true;
+ auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
+ if (!L)
+ return true;
+ Checker->visitLambdaExpr(L, shouldCheckThis());
return true;
}
+
+ // WTF::switchOn(T, F... f) is a variadic template function and couldn't
+ // be annotated with NOESCAPE. We hard code it here to workaround that.
+ bool shouldTreatAllArgAsNoEscape(FunctionDecl *Decl) {
+ auto *NsDecl = Decl->getParent();
+ if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
+ return false;
+ return safeGetName(NsDecl) == "WTF" && safeGetName(Decl) == "switchOn";
+ }
+
+ bool VisitCallExpr(CallExpr *CE) {
+ checkCalleeLambda(CE);
+ if (auto *Callee = CE->getDirectCallee()) {
+ bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
+ unsigned ArgIndex = 0;
+ for (auto *Param : Callee->parameters()) {
+ if (ArgIndex >= CE->getNumArgs())
+ 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());
+ }
+ }
+ ++ArgIndex;
+ }
+ }
+ return true;
+ }
+
+ void checkCalleeLambda(CallExpr *CE) {
+ auto *Callee = CE->getCallee();
+ if (!Callee)
+ return;
+ auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts());
+ if (!DRE)
+ return;
+ auto *MD = dyn_cast_or_null<CXXMethodDecl>(DRE->getDecl());
+ if (!MD || CE->getNumArgs() != 1)
+ return;
+ auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+ auto *ArgRef = dyn_cast<DeclRefExpr>(Arg);
+ if (!ArgRef)
+ return;
+ auto *VD = dyn_cast_or_null<VarDecl>(ArgRef->getDecl());
+ if (!VD)
+ return;
+ auto *Init = VD->getInit();
+ if (!Init)
+ return;
+ auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
+ if (!L)
+ return;
+ DeclRefExprsToIgnore.insert(ArgRef);
+ Checker->visitLambdaExpr(L, shouldCheckThis(),
+ /* ignoreParamVarDecl */ true);
+ }
};
LocalVisitor visitor(this);
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
- void visitLambdaExpr(LambdaExpr *L) const {
+ void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis,
+ bool ignoreParamVarDecl = false) const {
+ if (TFA.isTrivial(L->getBody()))
+ return;
for (const LambdaCapture &C : L->captures()) {
if (C.capturesVariable()) {
ValueDecl *CapturedVar = C.getCapturedVar();
+ if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
+ continue;
QualType CapturedVarQualType = CapturedVar->getType();
- if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
- auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
- if (IsUncountedPtr && *IsUncountedPtr)
- reportBug(C, CapturedVar, CapturedVarType);
- }
+ auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
+ if (IsUncountedPtr && *IsUncountedPtr)
+ reportBug(C, CapturedVar, CapturedVarQualType);
+ } else if (C.capturesThis() && shouldCheckThis) {
+ if (ignoreParamVarDecl) // this is always a parameter to this function.
+ continue;
+ reportBugOnThisPtr(C);
}
}
}
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
- const Type *T) const {
+ const QualType T) const {
assert(CapturedVar);
SmallString<100> Buf;
@@ -89,7 +182,25 @@ class UncountedLambdaCapturesChecker
}
printQuotedQualifiedName(Os, Capture.getCapturedVar());
- Os << " to uncounted type is unsafe.";
+ Os << " to ref-counted type or CheckedPtr-capable type is unsafe.";
+
+ PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
+ auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+ BR->emitReport(std::move(Report));
+ }
+
+ void reportBugOnThisPtr(const LambdaCapture &Capture) const {
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ if (Capture.isExplicit()) {
+ Os << "Captured ";
+ } else {
+ Os << "Implicitly captured ";
+ }
+
+ Os << "raw-pointer 'this' to ref-counted type or CheckedPtr-capable type "
+ "is unsafe.";
PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 9c9326f0f11cf..4a55ddab64d80 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -135,7 +135,9 @@ struct RefCountable {
void ref() {}
void deref() {}
void method();
+ void constMethod() const;
int trivial() { return 123; }
+ RefCountable* next();
};
template <typename T> T *downcast(T *t) { return t; }
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 27e0a74d583cd..9bfcdea04755d 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -1,39 +1,71 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker %s 2>&1 | FileCheck %s --strict-whitespace
-#include "mock-types.h"
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
+
+struct RefCountable {
+ void ref() {}
+ void deref() {}
+ void method();
+ void constMethod() const;
+ int trivial() { return 123; }
+ RefCountable* next();
+};
+
+RefCountable* make_obj();
+
+void someFunction();
+template <typename Callback> void call(Callback callback) {
+ someFunction();
+ callback();
+}
void raw_ptr() {
- RefCountable* ref_countable = nullptr;
- auto foo1 = [ref_countable](){};
- // CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
- // CHECK-NEXT:{{^ 6 | }} auto foo1 = [ref_countable](){};
- // CHECK-NEXT:{{^ | }} ^
- auto foo2 = [&ref_countable](){};
- // CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
- auto foo3 = [&](){ ref_countable = nullptr; };
- // CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
- // CHECK-NEXT:{{^ 12 | }} auto foo3 = [&](){ ref_countable = nullptr; };
- // CHECK-NEXT:{{^ | }} ^
- auto foo4 = [=](){ (void) ref_countable; };
- // CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+ RefCountable* ref_countable = make_obj();
+ auto foo1 = [ref_countable](){
+ // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ ref_countable->method();
+ };
+ auto foo2 = [&ref_countable](){
+ // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ ref_countable->method();
+ };
+ auto foo3 = [&](){
+ ref_countable->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ ref_countable = nullptr;
+ };
+
+ auto foo4 = [=](){
+ ref_countable->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ };
+
+ call(foo1);
+ call(foo2);
+ call(foo3);
+ call(foo4);
// Confirm that the checker respects [[clang::suppress]].
RefCountable* suppressed_ref_countable = nullptr;
[[clang::suppress]] auto foo5 = [suppressed_ref_countable](){};
- // CHECK-NOT: warning: Captured raw-pointer 'suppressed_ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+ // no warning.
+ call(foo5);
}
void references() {
RefCountable automatic;
RefCountable& ref_countable_ref = automatic;
+ auto foo1 = [ref_countable_ref](){ ref_countable_ref.constMethod(); };
+ // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ auto foo2 = [&ref_countable_ref](){ ref_countable_ref.method(); };
+ // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ auto foo3 = [&](){ ref_countable_ref.method(); };
+ // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ auto foo4 = [=](){ ref_countable_ref.constMethod(); };
+ // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- auto foo1 = [ref_countable_ref](){};
- // CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
- auto foo2 = [&ref_countable_ref](){};
- // CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
- auto foo3 = [&](){ (void) ref_countable_ref; };
- // CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
- auto foo4 = [=](){ (void) ref_countable_ref; };
- // CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
+ call(foo1);
+ call(foo2);
+ call(foo3);
+ call(foo4);
}
void quiet() {
@@ -45,5 +77,100 @@ void quiet() {
auto foo3 = [&]() {};
auto foo4 = [=]() {};
+
+ call(foo3);
+ call(foo4);
+
RefCountable *ref_countable = nullptr;
}
+
+template <typename Callback>
+void map(RefCountable* start, [[clang::noescape]] Callback&& callback)
+{
+ while (start) {
+ callback(*start);
+ start = start->next();
+ }
+}
+
+template <typename Callback1, typename Callback2>
+void doubleMap(RefCountable* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2)
+{
+ while (start) {
+ callback1(*start);
+ callback2(*start);
+ start = start->next();
+ }
+}
+
+void noescape_lambda() {
+ RefCountable* someObj = make_obj();
+ RefCountable* otherObj = make_obj();
+ map(make_obj(), [&](RefCountable& obj) {
+ otherObj->method();
+ });
+ doubleMap(make_obj(), [&](RefCountable& obj) {
+ otherObj->method();
+ }, [&](RefCountable& obj) {
+ otherObj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ ([&] {
+ someObj->method();
+ })();
+}
+
+void lambda_capture_param(RefCountable* obj) {
+ auto someLambda = [&] {
+ obj->method();
+ };
+ someLambda();
+ someLambda();
+}
+
+struct RefCountableWithLambdaCapturingThis {
+ void ref() const;
+ void deref() const;
+ void nonTrivial();
+
+ void method_captures_this_safe() {
+ auto lambda = [&]() {
+ nonTrivial();
+ };
+ lambda();
+ }
+
+ void method_captures_this_unsafe() {
+ auto lambda = [&]() {
+ nonTrivial();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ };
+ call(lambda);
+ }
+};
+
+struct NonRefCountableWithLambdaCapturingThis {
+ void nonTrivial();
+
+ void method_captures_this_safe() {
+ auto lambda = [&]() {
+ nonTrivial();
+ };
+ lambda();
+ }
+
+ void method_captures_this_unsafe() {
+ auto lambda = [&]() {
+ nonTrivial();
+ };
+ call(lambda);
+ }
+};
+
+void trivial_lambda() {
+ RefCountable* ref_countable = make_obj();
+ auto trivial_lambda = [&]() {
+ return ref_countable->trivial();
+ };
+ trivial_lambda();
+}