From 1fd2a397de02ce79d3dad9a8e4e12aaa51f817fa Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans@hanshq.net>
Date: Tue, 24 Jan 2017 00:08:26 +0000
Subject: [PATCH] Merging r292800:
 ------------------------------------------------------------------------
 r292800 | dergachev | 2017-01-23 08:57:11 -0800 (Mon, 23 Jan 2017) | 13 lines

[analyzer] Fix memory space of static locals seen from nested blocks.

When a block within a function accesses a function's static local variable,
this local is captured by reference rather than copied to the heap.

Therefore this variable's memory space is known: StaticGlobalSpaceRegion.
Used to be UnknownSpaceRegion, same as for stack locals.

Fixes a false positive in MacOSXAPIChecker.

rdar://problem/30105546
Differential revision: https://reviews.llvm.org/D28946

------------------------------------------------------------------------

llvm-svn: 292858
---
 clang/lib/StaticAnalyzer/Core/MemRegion.cpp | 54 +++++++++++++--------
 clang/test/Analysis/dispatch-once.m         |  7 +++
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index c4ba2ae199f88..54774d7501bc8 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -776,6 +776,22 @@ getStackOrCaptureRegionForDeclContext(const LocationContext *LC,
   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 @@ const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
         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 VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
 
     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 @@ const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
           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);
         }
diff --git a/clang/test/Analysis/dispatch-once.m b/clang/test/Analysis/dispatch-once.m
index 7d54147aebe24..2f82718663e5b 100644
--- a/clang/test/Analysis/dispatch-once.m
+++ b/clang/test/Analysis/dispatch-once.m
@@ -107,3 +107,10 @@ void test_block_var_from_outside_block() {
   };
   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
+  };
+}