Skip to content

Commit dadfd4a

Browse files
rniwarniwa
authored
Revert "[webkit.UncountedLambdaCapturesChecker] Ignore trivial functions and [[clang::noescape]]." (#114372)
Reverts #113845. Introduced a test failure.
1 parent 287781c commit dadfd4a

File tree

4 files changed

+33
-240
lines changed

4 files changed

+33
-240
lines changed

‎clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,6 @@ std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
6363
/// class, false if not, std::nullopt if inconclusive.
6464
std::optional<bool> isUncountedPtr(const clang::QualType T);
6565

66-
/// \returns true if \p T is either a raw pointer or reference to an uncounted
67-
/// or unchecked class, false if not, std::nullopt if inconclusive.
68-
std::optional<bool> isUnsafePtr(const QualType T);
69-
7066
/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its
7167
/// variant, false if not.
7268
bool isSafePtrType(const clang::QualType T);

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

Lines changed: 9 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#include "ASTUtils.h"
109
#include "DiagOutputUtils.h"
1110
#include "PtrTypesSemantics.h"
1211
#include "clang/AST/CXXInheritance.h"
@@ -27,7 +26,6 @@ class UncountedLambdaCapturesChecker
2726
BugType Bug{this, "Lambda capture of uncounted variable",
2827
"WebKit coding guidelines"};
2928
mutable BugReporter *BR = nullptr;
30-
TrivialFunctionAnalysis TFA;
3129

3230
public:
3331
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -39,8 +37,6 @@ class UncountedLambdaCapturesChecker
3937
// want to visit those, so we make our own RecursiveASTVisitor.
4038
struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
4139
const UncountedLambdaCapturesChecker *Checker;
42-
llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
43-
4440
explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker)
4541
: Checker(Checker) {
4642
assert(Checker);
@@ -49,100 +45,32 @@ class UncountedLambdaCapturesChecker
4945
bool shouldVisitTemplateInstantiations() const { return true; }
5046
bool shouldVisitImplicitCode() const { return false; }
5147

52-
bool VisitDeclRefExpr(DeclRefExpr *DRE) {
53-
if (DeclRefExprsToIgnore.contains(DRE))
54-
return true;
55-
auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
56-
if (!VD)
57-
return true;
58-
auto *Init = VD->getInit()->IgnoreParenCasts();
59-
auto *L = dyn_cast_or_null<LambdaExpr>(Init);
60-
if (!L)
61-
return true;
48+
bool VisitLambdaExpr(LambdaExpr *L) {
6249
Checker->visitLambdaExpr(L);
6350
return true;
6451
}
65-
66-
// WTF::switchOn(T, F... f) is a variadic template function and couldn't
67-
// be annotated with NOESCAPE. We hard code it here to workaround that.
68-
bool shouldTreatAllArgAsNoEscape(FunctionDecl *Decl) {
69-
auto *NsDecl = Decl->getParent();
70-
if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
71-
return false;
72-
return safeGetName(NsDecl) == "WTF" && safeGetName(Decl) == "switchOn";
73-
}
74-
75-
bool VisitCallExpr(CallExpr *CE) {
76-
checkCalleeLambda(CE);
77-
if (auto *Callee = CE->getDirectCallee()) {
78-
bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
79-
unsigned ArgIndex = 0;
80-
for (auto *Param : Callee->parameters()) {
81-
if (ArgIndex >= CE->getNumArgs())
82-
break;
83-
auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
84-
if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape) {
85-
if (auto *L = dyn_cast_or_null<LambdaExpr>(Arg))
86-
Checker->visitLambdaExpr(L);
87-
}
88-
++ArgIndex;
89-
}
90-
}
91-
return true;
92-
}
93-
94-
void checkCalleeLambda(CallExpr *CE) {
95-
auto *Callee = CE->getCallee();
96-
if (!Callee)
97-
return;
98-
auto *DRE = dyn_cast<DeclRefExpr>(Callee->IgnoreParenCasts());
99-
if (!DRE)
100-
return;
101-
auto *MD = dyn_cast_or_null<CXXMethodDecl>(DRE->getDecl());
102-
if (!MD || CE->getNumArgs() != 1)
103-
return;
104-
auto *Arg = CE->getArg(0)->IgnoreParenCasts();
105-
auto *ArgRef = dyn_cast<DeclRefExpr>(Arg);
106-
if (!ArgRef)
107-
return;
108-
auto *VD = dyn_cast_or_null<VarDecl>(ArgRef->getDecl());
109-
if (!VD)
110-
return;
111-
auto *Init = VD->getInit()->IgnoreParenCasts();
112-
auto *L = dyn_cast_or_null<LambdaExpr>(Init);
113-
if (!L)
114-
return;
115-
DeclRefExprsToIgnore.insert(ArgRef);
116-
Checker->visitLambdaExpr(L, /* ignoreParamVarDecl */ true);
117-
}
11852
};
11953

12054
LocalVisitor visitor(this);
12155
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
12256
}
12357

124-
void visitLambdaExpr(LambdaExpr *L, bool ignoreParamVarDecl = false) const {
125-
if (TFA.isTrivial(L->getBody()))
126-
return;
58+
void visitLambdaExpr(LambdaExpr *L) const {
12759
for (const LambdaCapture &C : L->captures()) {
12860
if (C.capturesVariable()) {
12961
ValueDecl *CapturedVar = C.getCapturedVar();
130-
if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
131-
continue;
13262
QualType CapturedVarQualType = CapturedVar->getType();
133-
auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
134-
if (IsUncountedPtr && *IsUncountedPtr)
135-
reportBug(C, CapturedVar, CapturedVarQualType);
136-
} else if (C.capturesThis()) {
137-
if (ignoreParamVarDecl) // this is always a parameter to this function.
138-
continue;
139-
reportBugOnThisPtr(C);
63+
if (auto *CapturedVarType = CapturedVarQualType.getTypePtrOrNull()) {
64+
auto IsUncountedPtr = isUncountedPtr(CapturedVarQualType);
65+
if (IsUncountedPtr && *IsUncountedPtr)
66+
reportBug(C, CapturedVar, CapturedVarType);
67+
}
14068
}
14169
}
14270
}
14371

14472
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
145-
const QualType T) const {
73+
const Type *T) const {
14674
assert(CapturedVar);
14775

14876
SmallString<100> Buf;
@@ -161,25 +89,7 @@ class UncountedLambdaCapturesChecker
16189
}
16290

16391
printQuotedQualifiedName(Os, Capture.getCapturedVar());
164-
Os << " to ref-counted / CheckedPtr capable type is unsafe.";
165-
166-
PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
167-
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
168-
BR->emitReport(std::move(Report));
169-
}
170-
171-
void reportBugOnThisPtr(const LambdaCapture &Capture) const {
172-
SmallString<100> Buf;
173-
llvm::raw_svector_ostream Os(Buf);
174-
175-
if (Capture.isExplicit()) {
176-
Os << "Captured ";
177-
} else {
178-
Os << "Implicitly captured ";
179-
}
180-
181-
Os << "raw-pointer 'this' to ref-counted / CheckedPtr capable type is "
182-
"unsafe.";
92+
Os << " to uncounted type is unsafe.";
18393

18494
PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager());
18595
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);

‎clang/test/Analysis/Checkers/WebKit/mock-types.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,7 @@ struct RefCountable {
135135
void ref() {}
136136
void deref() {}
137137
void method();
138-
void constMethod() const;
139138
int trivial() { return 123; }
140-
RefCountable* next();
141139
};
142140

143141
template <typename T> T *downcast(T *t) { return t; }

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

Lines changed: 24 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,39 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -verify %s
2-
//#include "mock-types.h"
3-
4-
struct RefCountable {
5-
// static Ref<RefCountable> create();
6-
void ref() {}
7-
void deref() {}
8-
void method();
9-
void constMethod() const;
10-
int trivial() { return 123; }
11-
RefCountable* next();
12-
};
13-
14-
RefCountable* make_obj();
15-
16-
void someFunction();
17-
template <typename Callback> void call(Callback callback) {
18-
someFunction();
19-
callback();
20-
}
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker %s 2>&1 | FileCheck %s --strict-whitespace
2+
#include "mock-types.h"
213

224
void raw_ptr() {
23-
RefCountable* ref_countable = make_obj();
24-
auto foo1 = [ref_countable](){
25-
// expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
26-
ref_countable->method();
27-
};
28-
auto foo2 = [&ref_countable](){
29-
// expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
30-
ref_countable->method();
31-
};
32-
auto foo3 = [&](){
33-
ref_countable->method();
34-
// expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
35-
ref_countable = nullptr;
36-
};
37-
38-
auto foo4 = [=](){
39-
ref_countable->method();
40-
// expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
41-
};
42-
43-
call(foo1);
44-
call(foo2);
45-
call(foo3);
46-
call(foo4);
5+
RefCountable* ref_countable = nullptr;
6+
auto foo1 = [ref_countable](){};
7+
// CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
8+
// CHECK-NEXT:{{^ 6 | }} auto foo1 = [ref_countable](){};
9+
// CHECK-NEXT:{{^ | }} ^
10+
auto foo2 = [&ref_countable](){};
11+
// CHECK: warning: Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
12+
auto foo3 = [&](){ ref_countable = nullptr; };
13+
// CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
14+
// CHECK-NEXT:{{^ 12 | }} auto foo3 = [&](){ ref_countable = nullptr; };
15+
// CHECK-NEXT:{{^ | }} ^
16+
auto foo4 = [=](){ (void) ref_countable; };
17+
// CHECK: warning: Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
4718

4819
// Confirm that the checker respects [[clang::suppress]].
4920
RefCountable* suppressed_ref_countable = nullptr;
5021
[[clang::suppress]] auto foo5 = [suppressed_ref_countable](){};
51-
// no warning.
52-
call(foo5);
22+
// CHECK-NOT: warning: Captured raw-pointer 'suppressed_ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
5323
}
5424

5525
void references() {
5626
RefCountable automatic;
5727
RefCountable& ref_countable_ref = automatic;
58-
auto foo1 = [ref_countable_ref](){ ref_countable_ref.constMethod(); };
59-
// expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
60-
auto foo2 = [&ref_countable_ref](){ ref_countable_ref.method(); };
61-
// expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
62-
auto foo3 = [&](){ ref_countable_ref.method(); };
63-
// expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
64-
auto foo4 = [=](){ ref_countable_ref.constMethod(); };
65-
// expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
6628

67-
call(foo1);
68-
call(foo2);
69-
call(foo3);
70-
call(foo4);
29+
auto foo1 = [ref_countable_ref](){};
30+
// CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
31+
auto foo2 = [&ref_countable_ref](){};
32+
// CHECK: warning: Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
33+
auto foo3 = [&](){ (void) ref_countable_ref; };
34+
// CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
35+
auto foo4 = [=](){ (void) ref_countable_ref; };
36+
// CHECK: warning: Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]
7137
}
7238

7339
void quiet() {
@@ -79,82 +45,5 @@ void quiet() {
7945

8046
auto foo3 = [&]() {};
8147
auto foo4 = [=]() {};
82-
83-
call(foo3);
84-
call(foo4);
85-
8648
RefCountable *ref_countable = nullptr;
8749
}
88-
89-
template <typename Callback>
90-
void map(RefCountable* start, [[clang::noescape]] Callback&& callback)
91-
{
92-
while (start) {
93-
callback(*start);
94-
start = start->next();
95-
}
96-
}
97-
98-
template <typename Callback1, typename Callback2>
99-
void doubleMap(RefCountable* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2)
100-
{
101-
while (start) {
102-
callback1(*start);
103-
callback2(*start);
104-
start = start->next();
105-
}
106-
}
107-
108-
void noescape_lambda() {
109-
RefCountable* someObj = make_obj();
110-
RefCountable* otherObj = make_obj();
111-
map(make_obj(), [&](RefCountable& obj) {
112-
otherObj->method();
113-
});
114-
doubleMap(make_obj(), [&](RefCountable& obj) {
115-
otherObj->method();
116-
}, [&](RefCountable& obj) {
117-
otherObj->method();
118-
// expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
119-
});
120-
([&] {
121-
someObj->method();
122-
})();
123-
}
124-
125-
void lambda_capture_param(RefCountable* obj) {
126-
auto someLambda = [&] {
127-
obj->method();
128-
};
129-
someLambda();
130-
someLambda();
131-
}
132-
133-
struct RefCountableWithLambdaCapturingThis {
134-
void ref() const;
135-
void deref() const;
136-
void nonTrivial();
137-
138-
void method_captures_this_safe() {
139-
auto lambda = [&]() {
140-
nonTrivial();
141-
};
142-
lambda();
143-
}
144-
145-
void method_captures_this_unsafe() {
146-
auto lambda = [&]() {
147-
nonTrivial();
148-
// expected-warning@-1{{Implicitly captured raw-pointer 'this' to ref-counted / CheckedPtr capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
149-
};
150-
call(lambda);
151-
}
152-
};
153-
154-
void trivial_lambda() {
155-
RefCountable* ref_countable = make_obj();
156-
auto trivial_lambda = [&]() {
157-
return ref_countable->trivial();
158-
};
159-
trivial_lambda();
160-
}

0 commit comments

Comments
 (0)