[clang] [WebKit Checkers] Add alpha.webkit.UnborrowedLambdaCapturesChecker (PR #226288)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 24 12:59:54 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: geoffreygaren
<details>
<summary>Changes</summary>
Like alpha.webkit.UnborrowedLocalVarsChecker, but for lambda captures.
This checker is pretty restrictive because, unlike a refcounted object, a lifetime-dependent pointer/reference/view cannot be captured in an escaping closure at all. Still, we permit capturing in a NOESCAPE closure.
---
Full diff: https://github.com/llvm/llvm-project/pull/226288.diff
6 Files Affected:
- (modified) clang/docs/analyzer/checkers.md (+34)
- (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+4)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp (+81-7)
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefSafetyModel.cpp (+8-2)
- (modified) clang/test/Analysis/Checkers/WebKit/mock-canborrow.h (+32)
- (added) clang/test/Analysis/Checkers/WebKit/unborrowed-lambda-captures.cpp (+166)
``````````diff
diff --git a/clang/docs/analyzer/checkers.md b/clang/docs/analyzer/checkers.md
index 343b9482f8a62..f6b6aa3212c12 100644
--- a/clang/docs/analyzer/checkers.md
+++ b/clang/docs/analyzer/checkers.md
@@ -4330,6 +4330,40 @@ These examples do not warn:
> }
> ```
+#### alpha.webkit.UnborrowedLambdaCapturesChecker
+
+The same rule as alpha.webkit.UnborrowedLocalVarsChecker, applied to lambda captures.
+
+Note: It is impossible for an escaping closure to capture a Borrow since Borrow is stack-only.
+
+> ```cpp
+> void takesCallback(const Function<void()>&);
+> void takesNoEscapeCallback([[clang::noescape]] const Function<void()>&);
+>
+> void foo1(Vector<char>& buffer) {
+> takesCallback([data = buffer.data()] { use(data); }); // warn
+>
+> Borrow<Vector<char>> borrowed(buffer);
+> takesCallback([data = borrowed.get().data()] { use(data); }); // warn
+> takesCallback([&borrowed] { use(borrowed.get().data()); }); // warn
+> }
+> ```
+
+A NOESCAPE callee runs the lambda before returning, so a `Borrow` in the enclosing scope protects the capture:
+
+> ```cpp
+> void foo2(Vector<char>& buffer) {
+> Borrow<Vector<char>> borrowed(buffer);
+> takesNoEscapeCallback([data = borrowed.get().data()] { use(data); }); // ok
+> takesNoEscapeCallback([&borrowed] { use(borrowed.get().data()); }); // ok
+>
+> takesNoEscapeCallback([&buffer] {
+> Borrow<Vector<char>> b(buffer);
+> use(b.get().data()); // ok
+> });
+> }
+> ```
+
#### webkit.RetainPtrCtorAdoptChecker
The goal of this rule is to make sure the constructors of RetainPtr and OSObjectPtr as well as adoptNS, adoptCF, and adoptOSObject are used correctly.
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 3d2428bdf92a5..e1a1046efab43 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1810,6 +1810,10 @@ def UnborrowedLocalVarsChecker : Checker<"UnborrowedLocalVarsChecker">,
HelpText<"Check local variables holding a loan on a CanBorrow object that is not guarded by a Borrow.">,
Documentation<HasDocumentation>;
+def UnborrowedLambdaCapturesChecker : Checker<"UnborrowedLambdaCapturesChecker">,
+ HelpText<"Check lambda captures holding a loan on a CanBorrow object that is not guarded by a Borrow.">,
+ Documentation<HasDocumentation>;
+
def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
HelpText<"Check for correct use of RetainPtr/OSObjectPtr constructor, adoptNS, adoptCF, and adoptOSObject">,
Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index 3fc4c38c8bed7..93c1d8224a36d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -551,9 +551,16 @@ class RawPtrRefLambdaCapturesChecker
bool ignoreParamVarDecl = false) const {
if (BR->getSourceManager().isInSystemHeader(L->getBeginLoc()))
return;
+ // FIXME: This check is unsound. Destruction can happen in three places,
+ // and a capture is only safe when all three are trivial: the lambda's
+ // body, the callee that receives it, and the scope that creates it.
if (TFA.isTrivial(L->getBody()))
return;
+ unsigned Index = 0;
for (const LambdaCapture &C : L->captures()) {
+ const Expr *CaptureInit =
+ Index < L->capture_size() ? L->capture_init_begin()[Index] : nullptr;
+ ++Index;
if (C.capturesVariable()) {
ValueDecl *CapturedVar = C.getCapturedVar();
if (ignoreParamVarDecl && isa<ParmVarDecl>(CapturedVar))
@@ -566,22 +573,70 @@ class RawPtrRefLambdaCapturesChecker
continue;
}
QualType CapturedVarQualType = CapturedVar->getType();
- auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType());
+ auto IsUncountedPtr = isUnsafePtr(CapturedVarQualType);
if (C.getCaptureKind() == LCK_ByCopy &&
CapturedVarQualType->isReferenceType())
continue;
- if (IsUncountedPtr && *IsUncountedPtr)
- reportBug(C, CapturedVar, CapturedVarQualType, L);
+ if (!IsUncountedPtr || !*IsUncountedPtr)
+ continue;
+ const Expr *Origin = nullptr;
+ if (Model->checksForInteriorDestruction()) {
+ if (!CaptureInit)
+ continue;
+ if (isCaptureOriginSafeForInteriorDestruction(CaptureInit, Origin))
+ continue;
+ }
+ reportBug(C, CapturedVar, CapturedVarQualType, L, Origin);
} else if (C.capturesThis() && shouldCheckThis) {
- if (ignoreParamVarDecl) // this is always a parameter to this function.
+ if (ignoreParamVarDecl)
+ continue;
+ if (Model->checksForInteriorDestruction())
continue;
reportBugOnThisPtr(C, T);
}
}
}
+ bool isCaptureOriginSafeForInteriorDestruction(const Expr *CaptureInit,
+ const Expr *&Origin) const {
+ return tryToFindPtrOrigin(
+ CaptureInit, /*StopAtFirstRefCountedObj=*/false,
+ Model->checksForInteriorDestruction(),
+ [&](const clang::CXXRecordDecl *Record) {
+ return Model->isSafePtr(Record);
+ },
+ [&](const clang::QualType Type) { return Model->isSafePtrType(Type); },
+ [&](const clang::Decl *D) {
+ return Model->isSafeDecl(D, BR->getSourceManager());
+ },
+ [&](const clang::Expr *CaptureOrigin, bool IsSafe,
+ bool /*OriginDependsOnFullExpressionTemporary*/,
+ bool PtrIsLifetimeBoundToOrigin) {
+ if (!CaptureOrigin)
+ return true;
+ if (isa<CXXThisExpr>(CaptureOrigin))
+ return true;
+ // A Borrow in the enclosing scope does not travel with the lambda,
+ // so a loan taken through it is unguarded once the lambda escapes.
+ QualType OriginType = pointeeType(CaptureOrigin->getType());
+ if (!OriginType.isNull() && isBorrowType(OriginType)) {
+ if (!Origin)
+ Origin = CaptureOrigin;
+ return false;
+ }
+ if (IsSafe)
+ return true;
+ if (Model->isSafeExpr(CaptureOrigin, PtrIsLifetimeBoundToOrigin))
+ return true;
+ if (!Origin)
+ Origin = CaptureOrigin;
+ return false;
+ });
+ }
+
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
- const QualType T, const LambdaExpr *L) const {
+ const QualType T, const LambdaExpr *L,
+ const Expr *Origin) const {
assert(CapturedVar);
auto Location = Capture.getLocation();
@@ -603,8 +658,10 @@ class RawPtrRefLambdaCapturesChecker
Os << " is a ";
else
Os << " contains a ";
- auto *CapturedType = T.getTypePtrOrNull();
- printPointer(Os, CapturedType);
+ if (Model->checksForInteriorDestruction())
+ Model->describeHazard(Os, Origin, T);
+ else
+ printPointer(Os, T.getTypePtrOrNull());
PathDiagnosticLocation BSLoc(Location, BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
@@ -696,6 +753,14 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
makeRetainPtrSafetyModel()) {}
};
+class UnborrowedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
+public:
+ UnborrowedLambdaCapturesChecker()
+ : RawPtrRefLambdaCapturesChecker("Lambda capture of a loan on a "
+ "CanBorrow object",
+ makeBorrowSafetyModel()) {}
+};
+
} // namespace
void ento::registerUncountedLambdaCapturesChecker(CheckerManager &Mgr) {
@@ -724,3 +789,12 @@ bool ento::shouldRegisterUnretainedLambdaCapturesChecker(
const CheckerManager &mgr) {
return true;
}
+
+void ento::registerUnborrowedLambdaCapturesChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<UnborrowedLambdaCapturesChecker>();
+}
+
+bool ento::shouldRegisterUnborrowedLambdaCapturesChecker(
+ const CheckerManager &mgr) {
+ return true;
+}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefSafetyModel.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefSafetyModel.cpp
index d0e8c1bee899e..970ba8ef44fa8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefSafetyModel.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefSafetyModel.cpp
@@ -152,12 +152,18 @@ class BorrowSafetyModel : public PtrRefSafetyModel {
const char *typeName() const override { return "CanBorrow type"; }
void describeHazard(llvm::raw_ostream &Os, const Expr *Origin,
- QualType) const override {
+ QualType SinkType) const override {
+ QualType SinkObject = pointeeType(SinkType);
+ if (!SinkObject.isNull() && isBorrowType(SinkObject)) {
+ Os << "Borrow that does not travel with the lambda";
+ return;
+ }
+
Os << "loan on ";
QualType OriginType = Origin ? pointeeType(Origin->getType()) : QualType();
// Name the borrowed type, not the Borrow<T> guard, when the loan was
- // taken from a Borrow<T> temporary.
+ // taken from a Borrow<T>.
if (!OriginType.isNull() && isBorrowType(OriginType))
OriginType = borrowedType(OriginType);
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-canborrow.h b/clang/test/Analysis/Checkers/WebKit/mock-canborrow.h
index 0bce868fc11ff..7b68bba4999a3 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-canborrow.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-canborrow.h
@@ -223,4 +223,36 @@ class Element {
void inspect() const;
};
+namespace detail {
+class CallableBase {
+public:
+ virtual ~CallableBase() {}
+ virtual void call() = 0;
+};
+
+template <typename F> class Callable : public CallableBase {
+public:
+ Callable(F f) : m_f(f) {}
+ void call() override { m_f(); }
+
+private:
+ F m_f;
+};
+} // namespace detail
+
+class Function {
+public:
+ template <typename F>
+ Function(F f) : m_impl(new detail::Callable<F>(f)) {}
+ ~Function() { delete m_impl; }
+
+ void operator()() const { m_impl->call(); }
+
+private:
+ detail::CallableBase *m_impl { nullptr };
+};
+
+void callEscaping(const Function &);
+void callNoEscape([[clang::noescape]] const Function &);
+
#endif
diff --git a/clang/test/Analysis/Checkers/WebKit/unborrowed-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/unborrowed-lambda-captures.cpp
new file mode 100644
index 0000000000000..7f35625fc521b
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/unborrowed-lambda-captures.cpp
@@ -0,0 +1,166 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnborrowedLambdaCapturesChecker -verify %s
+
+#include "mock-canborrow.h"
+
+void someFunction();
+void use(char *);
+
+namespace loan_shapes {
+
+void init_capture_computing_a_loan() {
+ Vector<char> vec;
+ callEscaping([q = vec.data()] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'q' is a loan on CanBorrow type 'Vector<char>' that is not guarded by a Borrow [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+void reference_to_an_element() {
+ Vector<char> vec;
+ callEscaping([&c = vec[0]] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'c' is a loan on CanBorrow type 'Vector<char>' that is not guarded by a Borrow [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+void loan_on_a_parameter(Vector<char> ¶meter) {
+ callEscaping([q = parameter.data()] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'q' is a loan on CanBorrow type 'Vector<char>' that is not guarded by a Borrow [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+void loan_on_a_nested_container() {
+ Vector<Vector<char>> outer;
+ callEscaping([&inner = outer[0]] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'inner' is a loan on CanBorrow type 'Vector<Vector<char>>' that is not guarded by a Borrow [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+} // namespace loan_shapes
+
+namespace borrow_does_not_travel {
+
+void loan_through_a_borrow() {
+ Vector<char> vec;
+ Borrow<Vector<char>> b(vec);
+ callEscaping([q = b.get().data()] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'q' is a loan on CanBorrow type 'Vector<char>' that is not guarded by a Borrow [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+void element_through_a_borrow() {
+ Vector<char> vec;
+ Borrow<Vector<char>> b(vec);
+ callEscaping([&c = b.get()[0]] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'c' is a loan on CanBorrow type 'Vector<char>' that is not guarded by a Borrow [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+void loan_through_a_borrow_temporary() {
+ Vector<char> vec;
+ callEscaping([q = borrow(vec).get().data()] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'q' is a loan on CanBorrow type 'Vector<char>' that is not guarded by a Borrow [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+void capture_of_the_borrow_by_reference() {
+ Vector<char> vec;
+ Borrow<Vector<char>> b(vec);
+ callEscaping([&b] { someFunction(); });
+ // expected-warning at -1{{Captured variable 'b' is a Borrow that does not travel with the lambda [alpha.webkit.UnborrowedLambdaCapturesChecker]}}
+}
+
+void borrow_inside_the_body() {
+ Vector<char> vec;
+ callEscaping([&vec] {
+ Borrow<Vector<char>> b(vec);
+ use(b.get().data());
+ });
+}
+
+} // namespace borrow_does_not_travel
+
+namespace noescape_borrows_work {
+
+void loan_guarded_for_the_whole_call() {
+ Vector<char> vec;
+ Borrow<Vector<char>> b(vec);
+ callNoEscape([q = b.get().data()] { use(q); });
+}
+
+void borrow_captured_by_reference() {
+ Vector<char> vec;
+ Borrow<Vector<char>> b(vec);
+ callNoEscape([&b] { use(b.get().data()); });
+}
+
+void borrow_inside_the_body() {
+ Vector<char> vec;
+ callNoEscape([&vec] {
+ Borrow<Vector<char>> b(vec);
+ use(b.get().data());
+ });
+}
+
+void nested_container_guarded_at_the_inner() {
+ Vector<Vector<char>> outer;
+ Borrow<Vector<char>> b(outer[0]);
+ callNoEscape([q = b.get().data()] { use(q); });
+}
+
+} // namespace noescape_borrows_work
+
+namespace not_a_loan {
+
+void reference_to_the_container() {
+ Vector<char> vec;
+ callEscaping([&vec] { vec.append('x'); });
+}
+
+void copy_of_an_element() {
+ Vector<char> vec;
+ callEscaping([c = vec[0]] { someFunction(); });
+}
+
+void copy_through_a_reference_variable() {
+ Vector<char> vec;
+ Vector<char> &r = vec;
+ callEscaping([r] { someFunction(); });
+}
+
+void view_on_a_non_container() {
+ NotBorrowable buffer;
+ callEscaping([&c = buffer.at(0)] { someFunction(); });
+}
+
+void unrelated_pointer_parameter(char *unrelated) {
+ callEscaping([unrelated] { someFunction(); });
+}
+
+class Holder {
+public:
+ void capturesThis() { callEscaping([this] { someFunction(); }); }
+
+private:
+ Vector<char> m_vec;
+};
+
+extern const Vector<char> globalConstBuffer;
+
+void loan_on_const_global() {
+ callEscaping([p = globalConstBuffer.data()] { someFunction(); });
+}
+
+} // namespace not_a_loan
+
+namespace known_gaps {
+
+void noescape_parameter() {
+ Vector<char> vec;
+ callNoEscape([q = vec.data()] { someFunction(); });
+}
+
+void trivial_body() {
+ Vector<char> vec;
+ callEscaping([q = vec.data()] {});
+}
+
+void loan_through_a_named_local() {
+ Vector<char> vec;
+ char *p = vec.data();
+ callNoEscape([p] { someFunction(); });
+ callEscaping([p] { someFunction(); });
+}
+
+} // namespace known_gaps
``````````
</details>
https://github.com/llvm/llvm-project/pull/226288
More information about the cfe-commits
mailing list