r266219 - [analyzer] Nullability: Suppress diagnostic on bind with cast.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Apr 13 12:59:24 2016
New Revision: 266219

URL: http://llvm.org/viewvc/llvm-project?rev=266219&view=rev
Log:
[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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
    cfe/trunk/test/Analysis/nullability.mm

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=266219&r1=266218&r2=266219&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Wed Apr 13 12:59:24 2016
@@ -1103,26 +1103,48 @@ void NullabilityChecker::checkBind(SVal
     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.
 

Modified: cfe/trunk/test/Analysis/nullability.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability.mm?rev=266219&r1=266218&r2=266219&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/nullability.mm (original)
+++ cfe/trunk/test/Analysis/nullability.mm Wed Apr 13 12:59:24 2016
@@ -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
+}
+
+ at interface ClassWithNonnullIvar : NSObject {
+  Dummy *_nonnullIvar;
+}
+ at end
+
+ at 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;
+}
+ at 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
 }