[analyzer] MacOSXAPIChecker: Improve warning messages for __block vars in dispatch_once().

[NoQ]

Summary

The checker already warns for __block-storage variables being used as a dispatch_once() predicate, however it refers to them as local which is not quite accurate, so we fix that.

Also add tests to make sure it works.

FIXME: The analyzer sets stack memory space for __block variables when they are referenced outside the block (eg. test_block_var_from_outside_block() line 108). Will try to fix in a separate patch; i'm not relying on the memory space in this patch.

[dcoughlin]

LGTM.

We should probably be warning any time the address of a block variable is taken since the address is not stable -- but that's a job for a different checker or possibly even Sema.

[NoQ]

FIXME: The analyzer sets stack memory space for __block variables when they are referenced outside the block (eg. test_block_var_from_outside_block() line 108). Will try to fix in a separate patch; i'm not relying on the memory space in this patch.

That's actually correct, never mind.

lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp

if (TrimmedFName != FName)
       FName = TrimmedFName;
   }
 
   SmallString<256> S;
   llvm::raw_svector_ostream os(S);
   bool SuggestStatic = false;
   os << "Call to '" << FName << "' uses";
   if (const VarRegion *VR = dyn_cast<VarRegion>(RB)) {
-    // We filtered out globals earlier, so it must be a local variable.
+    // We filtered out globals earlier, so it must be a local variable
+    // or a block variable which is under UnknownSpaceRegion.
     if (VR != R)
       os << " memory within";
-    os << " the local variable '" << VR->getDecl()->getName() << '\'';
+    if (VR->getDecl()->hasAttr<BlocksAttr>())
+      os << " the block variable '";
+    else
+      os << " the local variable '";
+    os << VR->getDecl()->getName() << '\'';
     SuggestStatic = true;
   } else if (const ObjCIvarRegion *IVR = getParentIvarRegion(R)) {
     if (IVR != R)
       os << " memory within";
     os << " the instance variable '" << IVR->getDecl()->getName() << '\'';
   } else if (isa<HeapSpaceRegion>(RS)) {
     os << " heap-allocated memory";
   } else if (isa<UnknownSpaceRegion>(RS)) {
     // Presence of an IVar superregion has priority over this branch, because
     // ObjC objects are on the heap even if the core doesn't realize this.
+    // Presence of a block variable base region has priority over this branch,
+    // because block variables are known to be either on stack or on heap
+    // (might actually move between the two, hence UnknownSpace).
     return;
   } else {
     os << " stack allocated memory";
   }
   os << " for the predicate value.  Using such transient memory for "
         "the predicate is potentially dangerous.";
   if (SuggestStatic)
     os << "  Perhaps you intended to declare the variable as 'static'?";

test/Analysis/dispatch-once.m

void test_ivar_from_external_obj(Object *o) {
   dispatch_once(&o->once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the instance variable 'once' for the predicate value.}}
 }
 void test_ivar_struct_from_external_obj(Object *o) {
   dispatch_once(&o->s.once, ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 's' for the predicate value.}}
 }
 void test_ivar_array_from_external_obj(Object *o) {
   dispatch_once(&o->once_array[1], ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 'once_array' for the predicate value.}}
 }
+
+void test_block_var_from_block() {
+  __block dispatch_once_t once;
+  ^{
+    dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the block variable 'once' for the predicate value.}}
+  };
+}
+
+void use_block_var(dispatch_once_t *once);
+
+void test_block_var_from_outside_block() {
+  __block dispatch_once_t once;
+  ^{
+    use_block_var(&once);
+  };
+  dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the block variable 'once' for the predicate value.}}
+}