From a5b27b8fe57e8a60e56c1edfaa8a2f37c51e3a6c Mon Sep 17 00:00:00 2001
From: Devin Coughlin <dcoughlin@apple.com>
Date: Tue, 29 Dec 2015 23:44:19 +0000
Subject: [PATCH] [analyzer] Suppress nullability warning for _Nonnull locals
 zero-initialized by ObjC ARC.

Prevent the analyzer from warning when a _Nonnnull local variable is implicitly
zero-initialized because of Objective-C automated reference counting. This avoids false
positives in cases where a _Nonnull local variable cannot be initialized with an
initialization expression, such as:
  NSString * _Nonnull s; // no-warning
  @autoreleasepool {
    s = ...;
  }

The nullability checker will still warn when a _Nonnull local variable is explicitly
initialized with nil.

This suppression introduces the potential for false negatives if the local variable
is used before it is assigned a _Nonnull value. Based on a discussion with Anna Zaks,
Jordan Rose, and John McCall, I've added a FIXME to treat implicitly zero-initialized
_Nonnull locals as uninitialized in Sema's UninitializedValues analysis to avoid these
false negatives.

rdar://problem/23522311

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@256603 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../Checkers/NullabilityChecker.cpp           | 45 ++++++++++++++++++-
 test/Analysis/nullability.mm                  | 11 ++++-
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 881ea8e606..bb86ea401d 100644
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -897,6 +897,48 @@ static const Expr * matchValueExprForBind(const Stmt *S) {
   return nullptr;
 }
 
+/// Returns true if \param S is a DeclStmt for a local variable that
+/// ObjC automated reference counting initialized with zero.
+static bool isARCNilInitializedLocal(CheckerContext &C, const Stmt *S) {
+  // We suppress diagnostics for ARC zero-initialized _Nonnull locals. This
+  // prevents false positives when a _Nonnull local variable cannot be
+  // initialized with an initialization expression:
+  //    NSString * _Nonnull s; // no-warning
+  //    @autoreleasepool {
+  //      s = ...
+  //    }
+  //
+  // FIXME: We should treat implicitly zero-initialized _Nonnull locals as
+  // uninitialized in Sema's UninitializedValues analysis to warn when a use of
+  // the zero-initialized definition will unexpectedly yield nil.
+
+  // Locals are only zero-initialized when automated reference counting
+  // is turned on.
+  if (!C.getASTContext().getLangOpts().ObjCAutoRefCount)
+    return false;
+
+  auto *DS = dyn_cast<DeclStmt>(S);
+  if (!DS || !DS->isSingleDecl())
+    return false;
+
+  auto *VD = dyn_cast<VarDecl>(DS->getSingleDecl());
+  if (!VD)
+    return false;
+
+  // Sema only zero-initializes locals with ObjCLifetimes.
+  if(!VD->getType().getQualifiers().hasObjCLifetime())
+    return false;
+
+  const Expr *Init = VD->getInit();
+  assert(Init && "ObjC local under ARC without initializer");
+
+  // Return false if the local is explicitly initialized (e.g., with '= nil').
+  if (!isa<ImplicitValueInitExpr>(Init))
+    return false;
+
+  return true;
+}
+
 /// Propagate the nullability information through binds and warn when nullable
 /// pointer or null symbol is assigned to a pointer with a nonnull type.
 void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
@@ -928,7 +970,8 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
   if (Filter.CheckNullPassedToNonnull &&
       RhsNullness == NullConstraint::IsNull &&
       ValNullability != Nullability::Nonnull &&
-      LocNullability == Nullability::Nonnull) {
+      LocNullability == Nullability::Nonnull &&
+      !isARCNilInitializedLocal(C, S)) {
     static CheckerProgramPointTag Tag(this, "NullPassedToNonnull");
     ExplodedNode *N = C.generateErrorNode(State, &Tag);
     if (!N)
diff --git a/test/Analysis/nullability.mm b/test/Analysis/nullability.mm
index d0428d4c8b..2c29d0088e 100644
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,nullability -verify %s
+// RUN: %clang_cc1 -fobjc-arc -analyze -analyzer-checker=core,nullability -verify %s
 
 #define nil 0
 #define BOOL int
@@ -278,3 +278,12 @@ Dummy *_Nonnull testDefensiveInlineChecks(Dummy * p) {
 
   return p;
 }
+
+void testObjCARCImplicitZeroInitialization() {
+  TestObject * _Nonnull implicitlyZeroInitialized; // no-warning
+  implicitlyZeroInitialized = getNonnullTestObject();
+}
+
+void testObjCARCExplicitZeroInitialization() {
+  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+}
-- 
2.40.0