https://github.com/sdefresne created
https://github.com/llvm/llvm-project/pull/144388
Add three new warning that reports when blocks captures 'this', a raw pointer
(i.e. not a pointer to a reference counted object) or a reference.
Those warnings can be used in Objective-C++ to detect blocks that create
potentially unsafe capture (as the captured pointer can be dangling by the time
the block is captured).
To reduce the false positive, no warning is emitted if the block is detected as
not escaping.
The three warnings are:
- -Wblocks-capturing-this
- -Wblocks-capturing-reference
- -Wblocks-capturing-raw-pointer
Fixes issue #143924.
>From f41dc700d51726563a19aa8fa87b81075ba1859e Mon Sep 17 00:00:00 2001
From: Sylvain Defresne <sdefre...@chromium.org>
Date: Mon, 16 Jun 2025 17:09:24 +0200
Subject: [PATCH] Add warning for blocks capturing {'this', raw pointers,
references}
Add three new warning that reports when blocks captures 'this', a
raw pointer (i.e. not a pointer to a reference counted object) or
a reference.
Those warnings can be used in Objective-C++ to detect blocks that
create potentially unsafe capture (as the captured pointer can be
dangling by the time the block is captured).
To reduce the false positive, no warning is emitted if the block
is detected as not escaping.
The three warnings are:
- -Wblocks-capturing-this
- -Wblocks-capturing-reference
- -Wblocks-capturing-raw-pointer
Fixes issue #143924.
---
.../clang/Basic/DiagnosticSemaKinds.td | 11 ++
clang/include/clang/Sema/Sema.h | 19 +++-
clang/lib/Sema/SemaDecl.cpp | 44 ++++++--
clang/lib/Sema/SemaDeclObjC.cpp | 2 +-
clang/lib/Sema/SemaExpr.cpp | 13 +++
clang/lib/Sema/SemaExprObjC.cpp | 6 +-
.../warn-blocks-capturing-pointers.mm | 103 ++++++++++++++++++
7 files changed, 184 insertions(+), 14 deletions(-)
create mode 100644 clang/test/SemaObjCXX/warn-blocks-capturing-pointers.mm
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 979ff60b73b75..4b33c40658028 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1641,6 +1641,17 @@ def warn_implicitly_retains_self : Warning <
"block implicitly retains 'self'; explicitly mention 'self' to indicate "
"this is intended behavior">,
InGroup<DiagGroup<"implicit-retain-self">>, DefaultIgnore;
+def warn_blocks_capturing_this : Warning<"block implicitly captures 'this'">,
+ InGroup<DiagGroup<"blocks-capturing-this">>,
+ DefaultIgnore;
+def warn_blocks_capturing_reference
+ : Warning<"block implicitly captures a C++ reference">,
+ InGroup<DiagGroup<"blocks-capturing-reference">>,
+ DefaultIgnore;
+def warn_blocks_capturing_raw_pointer
+ : Warning<"block implicitly captures a raw pointer">,
+ InGroup<DiagGroup<"blocks-capturing-raw-pointer">>,
+ DefaultIgnore;
def warn_arc_possible_repeated_use_of_weak : Warning <
"weak %select{variable|property|implicit property|instance variable}0 %1 may
"
"be accessed multiple times in this %select{function|method|block|lambda}2 "
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 29452bb37260d..7a6ec7c7a3f2c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8227,6 +8227,22 @@ class Sema final : public SemaBase {
}
};
+ /// Store information about a diagnosable block catpure.
+ struct BlockCapture {
+ /// Enumeration representing types of block captures that may be
+ /// diagnosable because they could be problematic.
+ enum CaptureType {
+ Self,
+ This,
+ Reference,
+ RawPointer,
+ };
+
+ SourceLocation Loc;
+ const BlockDecl *BD;
+ CaptureType Type;
+ };
+
/// Check an argument list for placeholders that we won't try to
/// handle later.
bool CheckArgsForPlaceholders(MultiExprArg args);
@@ -8243,8 +8259,7 @@ class Sema final : public SemaBase {
/// List of SourceLocations where 'self' is implicitly retained inside a
/// block.
- llvm::SmallVector<std::pair<SourceLocation, const BlockDecl *>, 1>
- ImplicitlyRetainedSelfLocs;
+ llvm::SmallVector<BlockCapture, 1> DiagnosableBlockCaptures;
/// Do an explicit extend of the given block pointer if we're in ARC.
void maybeExtendBlockObject(ExprResult &E);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5cffd82e3372e..713cbf9e3ef2c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16149,7 +16149,7 @@ class ExitFunctionBodyRAII {
bool IsLambda = false;
};
-static void diagnoseImplicitlyRetainedSelf(Sema &S) {
+static void diagnoseBlockCaptures(Sema &S) {
llvm::DenseMap<const BlockDecl *, bool> EscapeInfo;
auto IsOrNestedInEscapingBlock = [&](const BlockDecl *BD) {
@@ -16170,13 +16170,35 @@ static void diagnoseImplicitlyRetainedSelf(Sema &S) {
return It->second = R;
};
- // If the location where 'self' is implicitly retained is inside a escaping
- // block, emit a diagnostic.
- for (const std::pair<SourceLocation, const BlockDecl *> &P :
- S.ImplicitlyRetainedSelfLocs)
- if (IsOrNestedInEscapingBlock(P.second))
- S.Diag(P.first, diag::warn_implicitly_retains_self)
- << FixItHint::CreateInsertion(P.first, "self->");
+ for (const auto &C : S.DiagnosableBlockCaptures) {
+ // Do not emit a diagnostic if the location is invalid.
+ if (!C.Loc.isValid())
+ continue;
+
+ // Do not emit a diagnostic if the block is not escaping.
+ if (!IsOrNestedInEscapingBlock(C.BD))
+ continue;
+
+ // Emit the correct warning depending on the type of capture.
+ switch (C.Type) {
+ case Sema::BlockCapture::Self:
+ S.Diag(C.Loc, diag::warn_implicitly_retains_self)
+ << FixItHint::CreateInsertion(C.Loc, "self->");
+ break;
+
+ case Sema::BlockCapture::This:
+ S.Diag(C.Loc, diag::warn_blocks_capturing_this);
+ break;
+
+ case Sema::BlockCapture::Reference:
+ S.Diag(C.Loc, diag::warn_blocks_capturing_reference);
+ break;
+
+ case Sema::BlockCapture::RawPointer:
+ S.Diag(C.Loc, diag::warn_blocks_capturing_raw_pointer);
+ break;
+ }
+ }
}
static bool methodHasName(const FunctionDecl *FD, StringRef Name) {
@@ -16511,6 +16533,10 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt
*Body,
}
}
+ // Warn about block captures, unless this is a lambda function.
+ if (!isLambdaCallOperator(FD))
+ diagnoseBlockCaptures(*this);
+
assert((FD == getCurFunctionDecl(/*AllowLambdas=*/true)) &&
"Function parsing confused");
} else if (ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(dcl)) {
@@ -16563,7 +16589,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt
*Body,
FSI->ObjCWarnForNoInitDelegation = false;
}
- diagnoseImplicitlyRetainedSelf(*this);
+ diagnoseBlockCaptures(*this);
} else {
// Parsing the function declaration failed in some way. Pop the fake
scope
// we pushed on.
diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 9dbc3efbca405..b6191bbe9aebf 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -367,7 +367,7 @@ HasExplicitOwnershipAttr(Sema &S, ParmVarDecl *Param) {
/// and user declared, in the method definition's AST.
void SemaObjC::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, Decl *D) {
ASTContext &Context = getASTContext();
- SemaRef.ImplicitlyRetainedSelfLocs.clear();
+ SemaRef.DiagnosableBlockCaptures.clear();
assert((SemaRef.getCurMethodDecl() == nullptr) && "Methodparsing confused");
ObjCMethodDecl *MDecl = dyn_cast_or_null<ObjCMethodDecl>(D);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 413eff4aa294a..80200fbc00b19 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16536,6 +16536,11 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation
CaretLoc,
// Set the captured variables on the block.
SmallVector<BlockDecl::Capture, 4> Captures;
for (Capture &Cap : BSI->Captures) {
+ if (Cap.isThisCapture()) {
+ DiagnosableBlockCaptures.push_back(
+ Sema::BlockCapture{Cap.getLocation(), BD, Sema::BlockCapture::This});
+ }
+
if (Cap.isInvalid() || Cap.isThisCapture())
continue;
// Cap.getVariable() is always a VarDecl because
@@ -16595,6 +16600,14 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation
CaretLoc,
}
}
+ QualType Type = Cap.getCaptureType();
+ if (Type->isPointerOrReferenceType()) {
+ DiagnosableBlockCaptures.push_back(Sema::BlockCapture{
+ Cap.getLocation(), BD,
+ Type->isReferenceType() ? Sema::BlockCapture::Reference
+ : Sema::BlockCapture::RawPointer});
+ }
+
BlockDecl::Capture NewCap(Var, Cap.isBlockCapture(), Cap.isNested(),
CopyExpr);
Captures.push_back(NewCap);
diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp
index 3505d9f38d23c..8c8ccbefa46ed 100644
--- a/clang/lib/Sema/SemaExprObjC.cpp
+++ b/clang/lib/Sema/SemaExprObjC.cpp
@@ -4913,8 +4913,10 @@ ExprResult SemaObjC::BuildIvarRefExpr(Scope *S,
SourceLocation Loc,
SemaRef.getCurFunction()->recordUseOfWeak(Result);
}
if (getLangOpts().ObjCAutoRefCount && !SemaRef.isUnevaluatedContext())
- if (const BlockDecl *BD = SemaRef.CurContext->getInnermostBlockDecl())
- SemaRef.ImplicitlyRetainedSelfLocs.push_back({Loc, BD});
+ if (const BlockDecl *BD = SemaRef.CurContext->getInnermostBlockDecl()) {
+ SemaRef.DiagnosableBlockCaptures.push_back(
+ Sema::BlockCapture{Loc, BD, Sema::BlockCapture::Self});
+ }
return Result;
}
diff --git a/clang/test/SemaObjCXX/warn-blocks-capturing-pointers.mm
b/clang/test/SemaObjCXX/warn-blocks-capturing-pointers.mm
new file mode 100644
index 0000000000000..2bf1ea5261196
--- /dev/null
+++ b/clang/test/SemaObjCXX/warn-blocks-capturing-pointers.mm
@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 -x objective-c++ -std=c++11 -fobjc-arc -fblocks
-Wblocks-capturing-this -Wblocks-capturing-reference
-Wblocks-capturing-raw-pointer -Wimplicit-retain-self -verify %s
+
+typedef void (^BlockTy)();
+
+void noescapeFunc(__attribute__((noescape)) BlockTy);
+void escapeFunc(BlockTy);
+
+@interface Root @end
+
+@interface I : Root
+- (void)foo;
+@end
+
+class F {
+ public:
+ void Foo() const;
+ void FooAsync(F* p, F& r, I* i) {
+ escapeFunc(
+ ^{
+ Foo(); // expected-warning {{block implicitly captures 'this'}}
+ p->Foo(); // expected-warning {{block implicitly captures a raw
pointer}}
+ r.Foo(); // expected-warning {{block implicitly captures a C++
reference}}
+ [i foo];
+ });
+
+ ([=, &r]() {
+ escapeFunc(
+ ^{
+ Foo(); // expected-warning {{block implicitly captures 'this'}}
+ p->Foo(); // expected-warning {{block implicitly captures a raw
pointer}}
+ r.Foo(); // expected-warning {{block implicitly captures a C++
reference}}
+ [i foo];
+ });
+ })();
+
+ escapeFunc(
+ ^{
+ ([=]() {
+ Foo(); // expected-warning {{block implicitly captures 'this'}}
+ p->Foo(); // expected-warning {{block implicitly captures a raw
pointer}}
+ r.Foo(); // expected-warning {{block implicitly captures a C++
reference}}
+ [i foo];
+ })();
+ });
+
+ noescapeFunc(
+ ^{
+ Foo();
+ p->Foo();
+ r.Foo();
+ [i foo];
+ });
+ }
+};
+
+@implementation I {
+ int _bar;
+}
+
+- (void)doSomethingWithPtr:(F*)p
+ ref:(F&)r
+ obj:(I*)i {
+ escapeFunc(
+ ^{
+ (void)_bar; // expected-warning {{block implicitly retains 'self';
explicitly mention 'self' to indicate this is intended behavior}}
+ p->Foo(); // expected-warning {{block implicitly captures a raw
pointer}}
+ r.Foo(); // expected-warning {{block implicitly captures a C++
reference}}
+ [i foo];
+ });
+
+ ([=, &r]() {
+ escapeFunc(
+ ^{
+ (void)_bar; // expected-warning {{block implicitly retains 'self';
explicitly mention 'self' to indicate this is intended behavior}}
+ p->Foo(); // expected-warning {{block implicitly captures a raw
pointer}}
+ r.Foo(); // expected-warning {{block implicitly captures a C++
reference}}
+ [i foo];
+ });
+ })();
+
+ escapeFunc(
+ ^{
+ ([=]() {
+ (void)_bar; // expected-warning {{block implicitly retains 'self';
explicitly mention 'self' to indicate this is intended behavior}}
+ p->Foo(); // expected-warning {{block implicitly captures a raw
pointer}}
+ r.Foo(); // expected-warning {{block implicitly captures a C++
reference}}
+ [i foo];
+ })();
+ });
+
+ noescapeFunc(
+ ^{
+ (void)_bar;
+ p->Foo();
+ r.Foo();
+ [i foo];
+ });
+}
+
+- (void)foo {
+}
+
+@end
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
