From 1eb408116bbfa3856e471f917c0901864607064b Mon Sep 17 00:00:00 2001
From: Devin Coughlin <dcoughlin@apple.com>
Date: Wed, 13 Apr 2016 17:59:24 +0000
Subject: [PATCH] [analyzer] Nullability: Suppress diagnostic on bind with
 cast.

Update the nullability checker to allow an explicit cast to nonnull to
suppress a warning on an assignment of nil to a nonnull:

id _Nonnull x = (id _Nonnull)nil; // no-warning

This suppression as already possible for diagnostics on returns and
function/method arguments.

rdar://problem/25381178

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

diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 0941b5240e..d7ec6b10c6 100644
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -1103,26 +1103,48 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
     ValNullability = getNullabilityAnnotation(Sym->getType());
 
   Nullability LocNullability = getNullabilityAnnotation(LocType);
+
+  // If the type of the RHS expression is nonnull, don't warn. This
+  // enables explicit suppression with a cast to nonnull.
+  Nullability ValueExprTypeLevelNullability = Nullability::Unspecified;
+  const Expr *ValueExpr = matchValueExprForBind(S);
+  if (ValueExpr) {
+    ValueExprTypeLevelNullability =
+      getNullabilityAnnotation(lookThroughImplicitCasts(ValueExpr)->getType());
+  }
+
+  bool NullAssignedToNonNull = (LocNullability == Nullability::Nonnull &&
+                                RhsNullness == NullConstraint::IsNull);
   if (Filter.CheckNullPassedToNonnull &&
-      RhsNullness == NullConstraint::IsNull &&
+      NullAssignedToNonNull &&
       ValNullability != Nullability::Nonnull &&
-      LocNullability == Nullability::Nonnull &&
+      ValueExprTypeLevelNullability != Nullability::Nonnull &&
       !isARCNilInitializedLocal(C, S)) {
     static CheckerProgramPointTag Tag(this, "NullPassedToNonnull");
     ExplodedNode *N = C.generateErrorNode(State, &Tag);
     if (!N)
       return;
 
-    const Stmt *ValueExpr = matchValueExprForBind(S);
-    if (!ValueExpr)
-      ValueExpr = S;
+
+    const Stmt *ValueStmt = S;
+    if (ValueExpr)
+      ValueStmt = ValueExpr;
 
     reportBugIfInvariantHolds("Null is assigned to a pointer which is "
                               "expected to have non-null value",
                               ErrorKind::NilAssignedToNonnull, N, nullptr, C,
-                              ValueExpr);
+                              ValueStmt);
+    return;
+  }
+
+  // If null was returned from a non-null function, mark the nullability
+  // invariant as violated even if the diagnostic was suppressed.
+  if (NullAssignedToNonNull) {
+    State = State->set<InvariantViolated>(true);
+    C.addTransition(State);
     return;
   }
+
   // Intentionally missing case: '0' is bound to a reference. It is handled by
   // the DereferenceChecker.
 
diff --git a/test/Analysis/nullability.mm b/test/Analysis/nullability.mm
index d4deecd9b5..c6d6519d90 100644
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -174,6 +174,47 @@ void testIndirectCastNilToNonnullAndPass() {
   takesNonnull(p);  // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
 }
 
+void testDirectCastNilToNonnullAndAssignToLocalInInitializer() {
+  Dummy * _Nonnull nonnullLocalWithAssignmentInInitializer = (Dummy * _Nonnull)0; // no-warning
+  (void)nonnullLocalWithAssignmentInInitializer;
+
+  // Since we've already had an invariant violation along this path,
+  // we shouldn't warn here.
+  nonnullLocalWithAssignmentInInitializer = 0;
+  (void)nonnullLocalWithAssignmentInInitializer;
+
+}
+
+void testDirectCastNilToNonnullAndAssignToLocal(Dummy * _Nonnull p) {
+  Dummy * _Nonnull nonnullLocalWithAssignment = p;
+  nonnullLocalWithAssignment = (Dummy * _Nonnull)0; // no-warning
+  (void)nonnullLocalWithAssignment;
+
+  // Since we've already had an invariant violation along this path,
+  // we shouldn't warn here.
+  nonnullLocalWithAssignment = 0;
+  (void)nonnullLocalWithAssignment;
+}
+
+void testDirectCastNilToNonnullAndAssignToParam(Dummy * _Nonnull p) {
+  p = (Dummy * _Nonnull)0; // no-warning
+}
+
+@interface ClassWithNonnullIvar : NSObject {
+  Dummy *_nonnullIvar;
+}
+@end
+
+@implementation ClassWithNonnullIvar
+-(void)testDirectCastNilToNonnullAndAssignToIvar {
+  _nonnullIvar = (Dummy * _Nonnull)0; // no-warning;
+
+  // Since we've already had an invariant violation along this path,
+  // we shouldn't warn here.
+  _nonnullIvar = 0;
+}
+@end
+
 void testIndirectNilPassToNonnull() {
   Dummy *p = 0;
   takesNonnull(p);  // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
@@ -479,9 +520,7 @@ void callMethodInSystemHeader() {
 }
 
 -(id _Nonnull)methodWithNilledOutInternal {
-  // The cast below should (but does not yet) suppress the warning on the
-  // assignment.
-  _nilledOutInternal = (id _Nonnull)nil; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+  _nilledOutInternal = (id _Nonnull)nil;
 
   return nil; // no-warning
 }
-- 
2.40.0