Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ bool tryToFindPtrOrigin(
if (isSingleton(E->getFoundDecl()))
return callback(E, true);
}

if (auto *MemberExpr = dyn_cast<CXXDependentScopeMemberExpr>(CalleeE)) {
auto *Base = MemberExpr->getBase();
auto MemberName = MemberExpr->getMember().getAsString();
bool IsGetter = MemberName == "get" || MemberName == "ptr";
if (Base && isSafePtrType(Base->getType()) && IsGetter)
return callback(E, true);
}
}

// Sometimes, canonical type erroneously turns Ref<T> into T.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,10 @@ class TrivialFunctionAnalysisVisitor
return WithCachedResult(CS, [&]() { return VisitChildren(CS); });
}

bool VisitCoroutineBodyStmt(const CoroutineBodyStmt *CBS) {
return WithCachedResult(CBS, [&]() { return VisitChildren(CBS); });
}

bool VisitReturnStmt(const ReturnStmt *RS) {
// A return statement is allowed as long as the return value is trivial.
if (auto *RV = RS->getRetValue())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ class RawPtrRefLambdaCapturesChecker
llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
llvm::DenseSet<const LambdaExpr *> LambdasToIgnore;
llvm::DenseSet<const ValueDecl *> ProtectedThisDecls;
llvm::DenseSet<const CallExpr *> CallToIgnore;
llvm::DenseSet<const CXXConstructExpr *> ConstructToIgnore;
llvm::DenseMap<const VarDecl *, const LambdaExpr *> LambdaOwnerMap;

QualType ClsType;

Expand Down Expand Up @@ -101,10 +103,60 @@ class RawPtrRefLambdaCapturesChecker
auto *Init = VD->getInit();
if (!Init)
return true;
auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
if (!L)
if (auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts())) {
LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
return true;
}
if (!VD->hasLocalStorage())
return true;
LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
if (auto *E = dyn_cast<ExprWithCleanups>(Init))
Init = E->getSubExpr();
if (auto *E = dyn_cast<CXXBindTemporaryExpr>(Init))
Init = E->getSubExpr();
if (auto *CE = dyn_cast<CallExpr>(Init)) {
if (auto *Callee = CE->getDirectCallee()) {
auto FnName = safeGetName(Callee);
unsigned ArgCnt = CE->getNumArgs();
if (FnName == "makeScopeExit" && ArgCnt == 1) {
auto *Arg = CE->getArg(0);
if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
Arg = E->getSubExpr();
if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
LambdaOwnerMap.insert(std::make_pair(VD, L));
CallToIgnore.insert(CE);
LambdasToIgnore.insert(L);
}
} else if (FnName == "makeVisitor") {
for (unsigned ArgIndex = 0; ArgIndex < ArgCnt; ++ArgIndex) {
auto *Arg = CE->getArg(ArgIndex);
if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
Arg = E->getSubExpr();
if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
LambdaOwnerMap.insert(std::make_pair(VD, L));
CallToIgnore.insert(CE);
LambdasToIgnore.insert(L);
}
}
}
}
} else if (auto *CE = dyn_cast<CXXConstructExpr>(Init)) {
if (auto *Ctor = CE->getConstructor()) {
if (auto *Cls = Ctor->getParent()) {
auto FnName = safeGetName(Cls);
unsigned ArgCnt = CE->getNumArgs();
if (FnName == "ScopeExit" && ArgCnt == 1) {
auto *Arg = CE->getArg(0);
if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
Arg = E->getSubExpr();
if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
LambdaOwnerMap.insert(std::make_pair(VD, L));
ConstructToIgnore.insert(CE);
LambdasToIgnore.insert(L);
}
}
}
}
}
return true;
}

Expand All @@ -114,6 +166,12 @@ class RawPtrRefLambdaCapturesChecker
auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
if (!VD)
return true;
if (auto It = LambdaOwnerMap.find(VD); It != LambdaOwnerMap.end()) {
auto *L = It->second;
Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L),
ClsType);
return true;
}
auto *Init = VD->getInit();
if (!Init)
return true;
Expand Down Expand Up @@ -167,10 +225,14 @@ class RawPtrRefLambdaCapturesChecker
}

bool VisitCallExpr(CallExpr *CE) override {
if (CallToIgnore.contains(CE))
return true;
checkCalleeLambda(CE);
if (auto *Callee = CE->getDirectCallee())
if (auto *Callee = CE->getDirectCallee()) {
if (isVisitFunction(CE, Callee))
return true;
checkParameters(CE, Callee);
else if (auto *CalleeE = CE->getCallee()) {
} else if (auto *CalleeE = CE->getCallee()) {
if (auto *DRE = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
if (auto *Callee = dyn_cast_or_null<FunctionDecl>(DRE->getDecl()))
checkParameters(CE, Callee);
Expand All @@ -179,6 +241,34 @@ class RawPtrRefLambdaCapturesChecker
return true;
}

bool isVisitFunction(CallExpr *CallExpr, FunctionDecl *FnDecl) {
bool IsVisitFn = safeGetName(FnDecl) == "visit";
if (!IsVisitFn)
return false;
bool ArgCnt = CallExpr->getNumArgs();
if (!ArgCnt)
return false;
auto *Ns = FnDecl->getParent();
if (!Ns)
return false;
auto NsName = safeGetName(Ns);
if (NsName != "WTF" && NsName != "std")
return false;
auto *Arg = CallExpr->getArg(0);
if (!Arg)
return false;
auto *DRE = dyn_cast<DeclRefExpr>(Arg->IgnoreParenCasts());
if (!DRE)
return false;
auto *VD = dyn_cast<VarDecl>(DRE->getDecl());
if (!VD)
return false;
if (!LambdaOwnerMap.contains(VD))
return false;
DeclRefExprsToIgnore.insert(DRE);
return true;
}

void checkParameters(CallExpr *CE, FunctionDecl *Callee) {
unsigned ArgIndex = isa<CXXOperatorCallExpr>(CE);
bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
Expand Down Expand Up @@ -280,7 +370,7 @@ class RawPtrRefLambdaCapturesChecker
LambdasToIgnore.insert(L);
}

bool hasProtectedThis(LambdaExpr *L) {
bool hasProtectedThis(const LambdaExpr *L) {
for (const LambdaCapture &OtherCapture : L->captures()) {
if (!OtherCapture.capturesVariable())
continue;
Expand Down Expand Up @@ -378,7 +468,8 @@ class RawPtrRefLambdaCapturesChecker
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}

void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T,
void visitLambdaExpr(const LambdaExpr *L, bool shouldCheckThis,
const QualType T,
bool ignoreParamVarDecl = false) const {
if (TFA.isTrivial(L->getBody()))
return;
Expand Down Expand Up @@ -410,7 +501,7 @@ class RawPtrRefLambdaCapturesChecker
}

void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
const QualType T, LambdaExpr *L) const {
const QualType T, const LambdaExpr *L) const {
assert(CapturedVar);

auto Location = Capture.getLocation();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -std=c++20 -verify %s
// expected-no-diagnostics

template<typename Arg>
void foo(Arg&& arg)
{
[&]{
co_await [&](auto&&... args) {
}(arg);
}();
}
151 changes: 151 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,79 @@ class HashMap {
ValueType* m_table { nullptr };
};

class ScopeExit final {
public:
template<typename ExitFunctionParameter>
explicit ScopeExit(ExitFunctionParameter&& exitFunction)
: m_exitFunction(std::move(exitFunction)) {
}

ScopeExit(ScopeExit&& other)
: m_exitFunction(std::move(other.m_exitFunction))
, m_execute(std::move(other.m_execute)) {
}

~ScopeExit() {
if (m_execute)
m_exitFunction();
}

WTF::Function<void()> take() {
m_execute = false;
return std::move(m_exitFunction);
}

void release() { m_execute = false; }

ScopeExit(const ScopeExit&) = delete;
ScopeExit& operator=(const ScopeExit&) = delete;
ScopeExit& operator=(ScopeExit&&) = delete;

private:
WTF::Function<void()> m_exitFunction;
bool m_execute { true };
};

template<typename ExitFunction> ScopeExit makeScopeExit(ExitFunction&&);
template<typename ExitFunction>
ScopeExit makeScopeExit(ExitFunction&& exitFunction)
{
return ScopeExit(std::move(exitFunction));
}

// Visitor adapted from http://stackoverflow.com/questions/25338795/is-there-a-name-for-this-tuple-creation-idiom

template<class A, class... B> struct Visitor : Visitor<A>, Visitor<B...> {
Visitor(A a, B... b)
: Visitor<A>(a)
, Visitor<B...>(b...)
{
}

using Visitor<A>::operator ();
using Visitor<B...>::operator ();
};

template<class A> struct Visitor<A> : A {
Visitor(A a)
: A(a)
{
}

using A::operator();
};

template<class... F> Visitor<F...> makeVisitor(F... f)
{
return Visitor<F...>(f...);
}

void opaqueFunction();
template <typename Visitor, typename... Variants> void visit(Visitor&& v, Variants&&... values)
{
opaqueFunction();
}

} // namespace WTF

struct A {
Expand Down Expand Up @@ -501,3 +574,81 @@ void RefCountedObj::call() const
};
callLambda(lambda);
}

void scope_exit(RefCountable* obj) {
auto scope = WTF::makeScopeExit([&] {
obj->method();
});
someFunction();
WTF::ScopeExit scope2([&] {
obj->method();
});
someFunction();
}

void doWhateverWith(WTF::ScopeExit& obj);

void scope_exit_with_side_effect(RefCountable* obj) {
auto scope = WTF::makeScopeExit([&] {
obj->method();
// expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
});
doWhateverWith(scope);
}

void scope_exit_static(RefCountable* obj) {
static auto scope = WTF::makeScopeExit([&] {
obj->method();
// expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
});
}

WTF::Function<void()> scope_exit_take_lambda(RefCountable* obj) {
auto scope = WTF::makeScopeExit([&] {
obj->method();
// expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
});
return scope.take();
}

// FIXME: Ideally, we treat release() as a trivial function.
void scope_exit_release(RefCountable* obj) {
auto scope = WTF::makeScopeExit([&] {
obj->method();
// expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
});
scope.release();
}

void make_visitor(RefCountable* obj) {
auto visitor = WTF::makeVisitor([&] {
obj->method();
});
}

void use_visitor(RefCountable* obj) {
auto visitor = WTF::makeVisitor([&] {
obj->method();
});
WTF::visit(visitor, obj);
}

template <typename Visitor, typename ObjectType>
void bad_visit(Visitor&, ObjectType*) {
someFunction();
}

void static_visitor(RefCountable* obj) {
static auto visitor = WTF::makeVisitor([&] {
obj->method();
// expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
});
}

void bad_use_visitor(RefCountable* obj) {
auto visitor = WTF::makeVisitor([&] {
obj->method();
// expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
});
bad_visit(visitor, obj);
}
Loading