ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a subscriber: cfe-commits.
ARC implicitly marks indirect parameters passed to a function as autoreleasing
and passing a block that captures those parameters to another function
sometimes causes problems that are hard to debug.
For example, in the code below, a block capturing fillMeIn is passed to
enumerateObjectsUsingBlock, in which an autorelease pool is pushed and popped
before and after the block is invoked:
void doStuff(NSString **fillMeIn) {
[@[@"array"] enumerateObjectsUsingBlock:
^(id obj, NSUInteger idx, BOOL* stop) {
*stop = YES;
*fillMeIn = [@"wow" mutableCopy];
}
];
}
The object assigned to *fillMeIn gets autoreleased inside the block, so it gets
destructed when the autorelease pool is drained, and the program will crash if
it tries to access the NSString returned in fillMeIn after doStuff returns.
To help the users figure out why the program is crashing, this patch adds a new
warning "-Wblock-capture-autoreleasing" which warns about implicitly
autoreleasing indirect parameters captured by blocks and suggests explicitly
specifying ownership qualification.
https://reviews.llvm.org/D25844
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/SemaObjC/arc.m
Index: test/SemaObjC/arc.m
===================================================================
--- test/SemaObjC/arc.m
+++ test/SemaObjC/arc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak
-fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak
-fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class
-Wblock-capture-autoreleasing %s
typedef unsigned long NSUInteger;
typedef const void * CFTypeRef;
@@ -808,3 +808,10 @@
TKAssertEqual(object, nil);
TKAssertEqual(object, (id)nil);
}
+
+void block_capture_autoreleasing(A * __autoreleasing *a, A **b) { //
expected-note {{explicitly specify ownership qualification}}
+ ^{
+ (void)*a;
+ (void)*b; // expected-warning {{block captures indirect parameter
implicitly qualified with __autoreleasing}}
+ }();
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13473,6 +13473,20 @@
}
return false;
}
+
+ // Warn about implicitly autoreleasing indirect parameters captured by
blocks.
+ if (auto *PT = dyn_cast<PointerType>(CaptureType)) {
+ QualType PointeeTy = PT->getPointeeType();
+ if (isa<ObjCObjectPointerType>(PointeeTy.getCanonicalType()) &&
+ PointeeTy.getObjCLifetime() == Qualifiers::OCL_Autoreleasing &&
+ !isa<AttributedType>(PointeeTy)) {
+ if (BuildAndDiagnose) {
+ S.Diag(Loc, diag::warn_block_capture_autoreleasing);
+ S.Diag(Var->getLocation(),
diag::note_explicit_ownership_qualification);
+ }
+ }
+ }
+
const bool HasBlocksAttr = Var->hasAttr<BlocksAttr>();
if (HasBlocksAttr || CaptureType->isReferenceType() ||
(S.getLangOpts().OpenMP && S.IsOpenMPCapturedDecl(Var))) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5073,6 +5073,12 @@
} // end "ARC and @properties" category
+def warn_block_capture_autoreleasing : Warning<
+ "block captures indirect parameter implicitly qualified with
__autoreleasing">,
+ InGroup<BlockCaptureAutoReleasing>, DefaultIgnore;
+def note_explicit_ownership_qualification : Note<
+ "explicitly specify ownership qualification">;
+
def err_arc_atomic_ownership : Error<
"cannot perform atomic operation on a pointer to type %0: type has "
"non-trivial ownership">;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -498,6 +498,7 @@
def ARCRepeatedUseOfWeakMaybe : DiagGroup<"arc-maybe-repeated-use-of-weak">;
def ARCRepeatedUseOfWeak : DiagGroup<"arc-repeated-use-of-weak",
[ARCRepeatedUseOfWeakMaybe]>;
+def BlockCaptureAutoReleasing : DiagGroup<"block-capture-autoreleasing">;
def ObjCBridge : DiagGroup<"bridge-cast">;
def DeallocInCategory:DiagGroup<"dealloc-in-category">;
Index: test/SemaObjC/arc.m
===================================================================
--- test/SemaObjC/arc.m
+++ test/SemaObjC/arc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class -Wblock-capture-autoreleasing %s
typedef unsigned long NSUInteger;
typedef const void * CFTypeRef;
@@ -808,3 +808,10 @@
TKAssertEqual(object, nil);
TKAssertEqual(object, (id)nil);
}
+
+void block_capture_autoreleasing(A * __autoreleasing *a, A **b) { // expected-note {{explicitly specify ownership qualification}}
+ ^{
+ (void)*a;
+ (void)*b; // expected-warning {{block captures indirect parameter implicitly qualified with __autoreleasing}}
+ }();
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13473,6 +13473,20 @@
}
return false;
}
+
+ // Warn about implicitly autoreleasing indirect parameters captured by blocks.
+ if (auto *PT = dyn_cast<PointerType>(CaptureType)) {
+ QualType PointeeTy = PT->getPointeeType();
+ if (isa<ObjCObjectPointerType>(PointeeTy.getCanonicalType()) &&
+ PointeeTy.getObjCLifetime() == Qualifiers::OCL_Autoreleasing &&
+ !isa<AttributedType>(PointeeTy)) {
+ if (BuildAndDiagnose) {
+ S.Diag(Loc, diag::warn_block_capture_autoreleasing);
+ S.Diag(Var->getLocation(), diag::note_explicit_ownership_qualification);
+ }
+ }
+ }
+
const bool HasBlocksAttr = Var->hasAttr<BlocksAttr>();
if (HasBlocksAttr || CaptureType->isReferenceType() ||
(S.getLangOpts().OpenMP && S.IsOpenMPCapturedDecl(Var))) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5073,6 +5073,12 @@
} // end "ARC and @properties" category
+def warn_block_capture_autoreleasing : Warning<
+ "block captures indirect parameter implicitly qualified with __autoreleasing">,
+ InGroup<BlockCaptureAutoReleasing>, DefaultIgnore;
+def note_explicit_ownership_qualification : Note<
+ "explicitly specify ownership qualification">;
+
def err_arc_atomic_ownership : Error<
"cannot perform atomic operation on a pointer to type %0: type has "
"non-trivial ownership">;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -498,6 +498,7 @@
def ARCRepeatedUseOfWeakMaybe : DiagGroup<"arc-maybe-repeated-use-of-weak">;
def ARCRepeatedUseOfWeak : DiagGroup<"arc-repeated-use-of-weak",
[ARCRepeatedUseOfWeakMaybe]>;
+def BlockCaptureAutoReleasing : DiagGroup<"block-capture-autoreleasing">;
def ObjCBridge : DiagGroup<"bridge-cast">;
def DeallocInCategory:DiagGroup<"dealloc-in-category">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
