[PATCH] D25844: [Sema][ObjC] Warn about implicitly autoreleasing indirect parameters that are captured by blocks

[Akira Hatanaka via cfe-commits]

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">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25844.75343.patch
Type: text/x-patch
Size: 3190 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161020/33a6ef04/attachment-0001.bin>