[analyzer] Nullability: allow cast to _Nonnull to suppress warning about returning...

The nullability checker currently allows casts to suppress warnings when a nil
literal is passed as an argument to a parameter annotated as _Nonnull:

  foo((NSString * _Nonnull)nil); // no-warning

It does so by suppressing the diagnostic when the *type* of the argument expression
is _Nonnull -- even when the symbolic value returned is known to be nil.

This commit updates the nullability checker to similarly honor such casts in the analogous
scenario when nil is returned from a function with a _Nonnull return type:

  return (NSString * _Nonnull)nil; // no-warning

This commit also normalizes variable naming between the parameter and return cases and
adds several tests demonstrating the limitations of this suppression mechanism (such as
when nil is cast to _Nonnull and then stored into a local variable without a nullability
qualifier). These tests are marked with FIXMEs.

rdar://problem/23176782

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@256567 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 89b1358a2a5d13fac0a95168b2968a4174507cd6..881ea8e606851f5f61df56ae89d8cda71e9a0350 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -492,12 +492,21 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
 
   NullConstraint Nullness = getNullConstraint(*RetSVal, State);
 
-  Nullability StaticNullability =
+  Nullability RequiredNullability =
       getNullabilityAnnotation(FuncType->getReturnType());
 
+  // If the returned value is null but the type of the expression
+  // generating it is nonnull then we will suppress the diagnostic.
+  // This enables explicit suppression when returning a nil literal in a
+  // function with a _Nonnull return type:
+  //    return (NSString * _Nonnull)0;
+  Nullability RetExprTypeLevelNullability =
+        getNullabilityAnnotation(RetExpr->getType());
+
   if (Filter.CheckNullReturnedFromNonnull &&
       Nullness == NullConstraint::IsNull &&
-      StaticNullability == Nullability::Nonnull) {
+      RetExprTypeLevelNullability != Nullability::Nonnull &&
+      RequiredNullability == Nullability::Nonnull) {
     static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
     ExplodedNode *N = C.generateErrorNode(State, &Tag);
     if (!N)
@@ -518,7 +527,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
     if (Filter.CheckNullableReturnedFromNonnull &&
         Nullness != NullConstraint::IsNotNull &&
         TrackedNullabValue == Nullability::Nullable &&
-        StaticNullability == Nullability::Nonnull) {
+        RequiredNullability == Nullability::Nonnull) {
       static CheckerProgramPointTag Tag(this, "NullableReturnedFromNonnull");
       ExplodedNode *N = C.addTransition(State, C.getPredecessor(), &Tag);
       reportBugIfPreconditionHolds(ErrorKind::NullableReturnedToNonnull, N,
@@ -526,9 +535,10 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
     }
     return;
   }
-  if (StaticNullability == Nullability::Nullable) {
+  if (RequiredNullability == Nullability::Nullable) {
     State = State->set<NullabilityMap>(Region,
-                                       NullabilityState(StaticNullability, S));
+                                       NullabilityState(RequiredNullability,
+                                                        S));
     C.addTransition(State);
   }
 }
@@ -564,13 +574,14 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
 
     NullConstraint Nullness = getNullConstraint(*ArgSVal, State);
 
-    Nullability ParamNullability = getNullabilityAnnotation(Param->getType());
-    Nullability ArgStaticNullability =
+    Nullability RequiredNullability =
+        getNullabilityAnnotation(Param->getType());
+    Nullability ArgExprTypeLevelNullability =
         getNullabilityAnnotation(ArgExpr->getType());
 
     if (Filter.CheckNullPassedToNonnull && Nullness == NullConstraint::IsNull &&
-        ArgStaticNullability != Nullability::Nonnull &&
-        ParamNullability == Nullability::Nonnull) {
+        ArgExprTypeLevelNullability != Nullability::Nonnull &&
+        RequiredNullability == Nullability::Nonnull) {
       ExplodedNode *N = C.generateErrorNode(State);
       if (!N)
         return;
@@ -592,7 +603,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
         continue;
 
       if (Filter.CheckNullablePassedToNonnull &&
-          ParamNullability == Nullability::Nonnull) {
+          RequiredNullability == Nullability::Nonnull) {
         ExplodedNode *N = C.addTransition(State);
         reportBugIfPreconditionHolds(ErrorKind::NullablePassedToNonnull, N,
                                      Region, C, ArgExpr, /*SuppressPath=*/true);
@@ -608,10 +619,10 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
       continue;
     }
     // No tracked nullability yet.
-    if (ArgStaticNullability != Nullability::Nullable)
+    if (ArgExprTypeLevelNullability != Nullability::Nullable)
       continue;
     State = State->set<NullabilityMap>(
-        Region, NullabilityState(ArgStaticNullability, ArgExpr));
+        Region, NullabilityState(ArgExprTypeLevelNullability, ArgExpr));
   }
   if (State != OrigState)
     C.addTransition(State);

diff --git a/test/Analysis/nullability.mm b/test/Analysis/nullability.mm
index 2a96431980f242b4bd6bc0f3dd123c3afc5f8597..d0428d4c8ba6ec5e9802e0285ba9a2534feb7a53 100644 (file)
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -169,9 +169,33 @@ void testObjCMessageResultNullability() {
   }
 }
 
-void testCast() {
+Dummy * _Nonnull testDirectCastNullableToNonnull() {
+  Dummy *p = returnsNullable();
+  takesNonnull((Dummy * _Nonnull)p);  // no-warning
+  return (Dummy * _Nonnull)p;         // no-warning
+}
+
+Dummy * _Nonnull testIndirectCastNullableToNonnull() {
   Dummy *p = (Dummy * _Nonnull)returnsNullable();
-  takesNonnull(p);
+  takesNonnull(p);  // no-warning
+  return p;         // no-warning
+}
+
+Dummy * _Nonnull testDirectCastNilToNonnull() {
+  takesNonnull((Dummy * _Nonnull)0);  // no-warning
+  return (Dummy * _Nonnull)0;         // no-warning
+}
+
+void testIndirectCastNilToNonnullAndPass() {
+  Dummy *p = (Dummy * _Nonnull)0;
+  // FIXME: Ideally the cast above would suppress this warning.
+  takesNonnull(p);  // expected-warning {{Null passed to a callee that requires a non-null argument}}
+}
+
+Dummy * _Nonnull testIndirectCastNilToNonnullAndReturn() {
+  Dummy *p = (Dummy * _Nonnull)0;
+  // FIXME: Ideally the cast above would suppress this warning.
+  return p; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
 }
 
 void testInvalidPropagation() {