NoQ created this revision.

When a block that is being analyzed as top-level captures static local 
variables declared in a function (which itself is not being analyzed) 
surrounding the block, it puts such variables into the unknown memory space, 
similarly to the stack locals of those functions (that may have been moved to 
the heap by the time the block starts executing).

However, because blocks don't move such variables to the heap, but capture them 
by reference instead, we are quite sure that we should put those into the 
static memspace.

This patch fixes a false positive in MacOSXAPIChecker.


https://reviews.llvm.org/D28946

Files:
  lib/StaticAnalyzer/Core/MemRegion.cpp
  test/Analysis/dispatch-once.m

Index: test/Analysis/dispatch-once.m
===================================================================
--- test/Analysis/dispatch-once.m
+++ test/Analysis/dispatch-once.m
@@ -107,3 +107,10 @@
   };
   dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the block variable 'once' for the predicate value.}}
 }
+
+void test_static_var_from_outside_block() {
+  static dispatch_once_t once;
+  ^{
+    dispatch_once(&once, ^{}); // no-warning
+  };
+}
Index: lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- lib/StaticAnalyzer/Core/MemRegion.cpp
+++ lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -776,6 +776,22 @@
   return (const StackFrameContext *)nullptr;
 }
 
+static CanQualType getBlockPointerType(const BlockDecl *BD, ASTContext &C) {
+  // FIXME: The fallback type here is totally bogus -- though it should
+  // never be queried, it will prevent uniquing with the real
+  // BlockCodeRegion. Ideally we'd fix the AST so that we always had a
+  // signature.
+  QualType T;
+  if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten())
+    T = TSI->getType();
+  if (T.isNull())
+    T = C.VoidTy;
+  if (!T->getAs<FunctionType>())
+    T = C.getFunctionNoProtoType(T);
+  T = C.getBlockPointerType(T);
+  return C.getCanonicalType(T);
+}
+
 const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
                                                 const LocationContext *LC) {
   const MemRegion *sReg = nullptr;
@@ -803,7 +819,7 @@
         sReg = getGlobalsRegion();
     }
 
-  // Finally handle static locals.
+  // Finally handle locals.
   } else {
     // FIXME: Once we implement scope handling, we will need to properly lookup
     // 'D' to the proper LocationContext.
@@ -816,9 +832,22 @@
 
     const StackFrameContext *STC = V.get<const StackFrameContext*>();
 
-    if (!STC)
-      sReg = getUnknownRegion();
-    else {
+    if (!STC) {
+      if (D->isStaticLocal()) {
+        const CodeTextRegion *fReg = nullptr;
+        if (const auto *ND = dyn_cast<NamedDecl>(DC))
+          fReg = getFunctionCodeRegion(ND);
+        else if (const auto *BD = dyn_cast<BlockDecl>(DC))
+          fReg = getBlockCodeRegion(BD, getBlockPointerType(BD, getContext()),
+                                    LC->getAnalysisDeclContext());
+        assert(fReg && "Unable to determine code region for a static local!");
+        sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind, fReg);
+      } else {
+        // We're looking at a block-captured local variable, which may be either
+        // still local, or already moved to the heap. So we're not sure.
+        sReg = getUnknownRegion();
+      }
+    } else {
       if (D->hasLocalStorage()) {
         sReg = isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D)
                ? static_cast<const MemRegion*>(getStackArgumentsRegion(STC))
@@ -831,22 +860,9 @@
           sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
                                   getFunctionCodeRegion(cast<NamedDecl>(STCD)));
         else if (const BlockDecl *BD = dyn_cast<BlockDecl>(STCD)) {
-          // FIXME: The fallback type here is totally bogus -- though it should
-          // never be queried, it will prevent uniquing with the real
-          // BlockCodeRegion. Ideally we'd fix the AST so that we always had a
-          // signature.
-          QualType T;
-          if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten())
-            T = TSI->getType();
-          if (T.isNull())
-            T = getContext().VoidTy;
-          if (!T->getAs<FunctionType>())
-            T = getContext().getFunctionNoProtoType(T);
-          T = getContext().getBlockPointerType(T);
-
           const BlockCodeRegion *BTR =
-            getBlockCodeRegion(BD, C.getCanonicalType(T),
-                               STC->getAnalysisDeclContext());
+              getBlockCodeRegion(BD, getBlockPointerType(BD, getContext()),
+                                 STC->getAnalysisDeclContext());
           sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
                                   BTR);
         }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to