[clang] Revert "[webkit.UncountedLambdaCapturesChecker] Ignore trivial functions and [[clang::noescape]]." (PR #114372)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 31 00:28:17 PDT 2024
https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/114372
Reverts llvm/llvm-project#113845. Introduced a test failure.
>From ab286462f15736a6e86f0113eab473fb859744be Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <ryosuke.niwa at gmail.com>
Date: Thu, 31 Oct 2024 00:27:46 -0700
Subject: [PATCH] =?UTF-8?q?Revert=20"[webkit.UncountedLambdaCapturesChecke?=
=?UTF-8?q?r]=20Ignore=20trivial=20functions=20and=20=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit 287781c7c9dbd7674cf7cbab8a8fe8a49a4b9317.
---
.../Checkers/WebKit/PtrTypesSemantics.h | 4 -
.../WebKit/UncountedLambdaCapturesChecker.cpp | 108 +-----------
.../Analysis/Checkers/WebKit/mock-types.h | 2 -
.../WebKit/uncounted-lambda-captures.cpp | 159 +++---------------
4 files changed, 33 insertions(+), 240 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 814015c311d61e..4b41ca96e1df1d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -63,10 +63,6 @@ std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
/// class, false if not, std::nullopt if inconclusive.
std::optional<bool> isUncountedPtr(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 d3484d74a2e3eb..998bd4ccee07db 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
@@ -6,7 +6,6 @@
//
//===----------------------------------------------------------------------===//
-#include "ASTUtils.h"
#include "DiagOutputUtils.h"
#include "PtrTypesSemantics.h"
#include "clang/AST/CXXInheritance.h"
@@ -27,7 +26,6 @@ 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,
@@ -39,8 +37,6 @@ 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;
-
explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
: Checker(Checker) {
assert(Checker);
@@ -49,100 +45,32 @@ class UncountedLambdaCapturesChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
- 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()->IgnoreParenCasts();
- auto *L = dyn_cast_or_null<LambdaExpr>(Init);
- if (!L)
- return true;
+ bool VisitLambdaExpr(LambdaExpr *L) {
Checker->visitLambdaExpr(L);
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())
- break;
- auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
- if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
- if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg))
- Checker->visitLambdaExpr(L);
- }
- ++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()->IgnoreParenCasts();
- auto *L = dyn_cast_or_null<LambdaExpr>(Init);
- if (!L)
- return;
- DeclRefExprsToIgnore.insert(ArgRef);
- Checker->visitLambdaExpr(L, /* ignoreParamVarDecl */ true);
- }
};
LocalVisitor visitor(this);
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
- void visitLambdaExpr(LambdaExpr *L, bool ignoreParamVarDecl = false) const {
- if (TFA.isTrivial(L->getBody()))
- return;
+ void visitLambdaExpr(LambdaExpr *L) const {
for (const LambdaCapture &C : L->captures()) {
if (C.capturesVariable()) {
ValueDecl *CapturedVar = C.getCapturedVar();
- if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
- continue;
QualType CapturedVarQualType = CapturedVar->getType();
- auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
- if (IsUncountedPtr && *IsUncountedPtr)
- reportBug(C, CapturedVar, CapturedVarQualType);
- } else if (C.capturesThis()) {
- if (ignoreParamVarDecl) // this is always a parameter to this function.
- continue;
- reportBugOnThisPtr(C);
+ if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
+ auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
+ if (IsUncountedPtr && *IsUncountedPtr)
+ reportBug(C, CapturedVar, CapturedVarType);
+ }
}
}
}
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
- const QualType T) const {
+ const Type *T) const {
assert(CapturedVar);
SmallString<100> Buf;
@@ -161,25 +89,7 @@ class UncountedLambdaCapturesChecker
}
printQuotedQualifiedName(Os, Capture.getCapturedVar());
- Os << " to ref-counted / 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 / CheckedPtr capable type is "
- "unsafe.";
+ Os << " to uncounted 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 30ea7839a223d9..82c79c97a83de6 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -135,9 +135,7 @@ 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 f676367fe74d0b..27e0a74d583cd3 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -1,73 +1,39 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
-//#include "mock-types.h"
-
-struct RefCountable {
-// static Ref<RefCountable> create();
- 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();
-}
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker %s 2>&1 | FileCheck %s --strict-whitespace
+#include "mock-types.h"
void raw_ptr() {
- RefCountable* ref_countable = make_obj();
- auto foo1 = [ref_countable](){
- // expected-warning at -1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- ref_countable->method();
- };
- auto foo2 = [&ref_countable](){
- // expected-warning at -1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- ref_countable->method();
- };
- auto foo3 = [&](){
- ref_countable->method();
- // expected-warning at -1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- ref_countable = nullptr;
- };
-
- auto foo4 = [=](){
- ref_countable->method();
- // expected-warning at -1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- };
-
- call(foo1);
- call(foo2);
- call(foo3);
- call(foo4);
+ 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]
// Confirm that the checker respects [[clang::suppress]].
RefCountable* suppressed_ref_countable = nullptr;
[[clang::suppress]] auto foo5 = [suppressed_ref_countable](){};
- // no warning.
- call(foo5);
+ // CHECK-NOT: warning: Captured raw-pointer 'suppressed_ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
}
void references() {
RefCountable automatic;
RefCountable& ref_countable_ref = automatic;
- auto foo1 = [ref_countable_ref](){ ref_countable_ref.constMethod(); };
- // expected-warning at -1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- auto foo2 = [&ref_countable_ref](){ ref_countable_ref.method(); };
- // expected-warning at -1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- auto foo3 = [&](){ ref_countable_ref.method(); };
- // expected-warning at -1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- auto foo4 = [=](){ ref_countable_ref.constMethod(); };
- // expected-warning at -1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- call(foo1);
- call(foo2);
- call(foo3);
- call(foo4);
+ 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]
}
void quiet() {
@@ -79,82 +45,5 @@ 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 at -1{{Implicitly captured raw-pointer 'otherObj' to ref-counted / 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 at -1{{Implicitly captured raw-pointer 'this' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
- };
- call(lambda);
- }
-};
-
-void trivial_lambda() {
- RefCountable* ref_countable = make_obj();
- auto trivial_lambda = [&]() {
- return ref_countable->trivial();
- };
- trivial_lambda();
-}
More information about the cfe-commits
mailing list