[analyzer] Nullability: Treat nil _Nonnull ivar as invariant violation.

Treat a _Nonnull ivar that is nil as an invariant violation in a similar
fashion to how a nil _Nonnull parameter is treated as a precondition violation.

This avoids warning on defensive returns of nil on defensive internal
checks, such as the following common idiom:

@class InternalImplementation
@interface PublicClass {
  InternalImplementation * _Nonnull _internal;
}
-(id _Nonnull)foo;
@end

@implementation PublicClass
-(id _Nonnull)foo {
  if (!_internal)
    return nil; // no-warning

  return [_internal foo];
}
@end

rdar://problem/24485171

llvm-svn: 266157

1 files changed, 56 insertions, 14 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index d8a224ea2f4..0941b5240e9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -356,29 +356,70 @@ static Nullability getNullabilityAnnotation(QualType Type) {
   return Nullability::Unspecified;
 }
 
-template <typename ParamVarDeclRange>
+/// Returns true when the value stored at the given location is null
+/// and the passed in type is nonnnull.
+static bool checkValueAtLValForInvariantViolation(ProgramStateRef State,
+                                                  SVal LV, QualType T) {
+  if (getNullabilityAnnotation(T) != Nullability::Nonnull)
+    return false;
+
+  auto RegionVal = LV.getAs<loc::MemRegionVal>();
+  if (!RegionVal)
+    return false;
+
+  auto StoredVal =
+  State->getSVal(RegionVal->getRegion()).getAs<DefinedOrUnknownSVal>();
+  if (!StoredVal)
+    return false;
+
+  if (getNullConstraint(*StoredVal, State) == NullConstraint::IsNull)
+    return true;
+
+  return false;
+}
+
 static bool
-checkParamsForPreconditionViolation(const ParamVarDeclRange &Params,
+checkParamsForPreconditionViolation(ArrayRef<ParmVarDecl *> Params,
                                     ProgramStateRef State,
                                     const LocationContext *LocCtxt) {
   for (const auto *ParamDecl : Params) {
     if (ParamDecl->isParameterPack())
       break;
 
-    if (getNullabilityAnnotation(ParamDecl->getType()) != Nullability::Nonnull)
-      continue;
+    SVal LV = State->getLValue(ParamDecl, LocCtxt);
+    if (checkValueAtLValForInvariantViolation(State, LV,
+                                              ParamDecl->getType())) {
+      return true;
+    }
+  }
+  return false;
+}
 
-    auto RegVal = State->getLValue(ParamDecl, LocCtxt)
-                      .template getAs<loc::MemRegionVal>();
-    if (!RegVal)
-      continue;
+static bool
+checkSelfIvarsForInvariantViolation(ProgramStateRef State,
+                                    const LocationContext *LocCtxt) {
+  auto *MD = dyn_cast<ObjCMethodDecl>(LocCtxt->getDecl());
+  if (!MD || !MD->isInstanceMethod())
+    return false;
 
-    auto ParamValue = State->getSVal(RegVal->getRegion())
-                          .template getAs<DefinedOrUnknownSVal>();
-    if (!ParamValue)
-      continue;
+  const ImplicitParamDecl *SelfDecl = LocCtxt->getSelfDecl();
+  if (!SelfDecl)
+    return false;
+
+  SVal SelfVal = State->getSVal(State->getRegion(SelfDecl, LocCtxt));
+
+  const ObjCObjectPointerType *SelfType =
+      dyn_cast<ObjCObjectPointerType>(SelfDecl->getType());
+  if (!SelfType)
+    return false;
+
+  const ObjCInterfaceDecl *ID = SelfType->getInterfaceDecl();
+  if (!ID)
+    return false;
 
-    if (getNullConstraint(*ParamValue, State) == NullConstraint::IsNull) {
+  for (const auto *IvarDecl : ID->ivars()) {
+    SVal LV = State->getLValue(IvarDecl, SelfVal);
+    if (checkValueAtLValForInvariantViolation(State, LV, IvarDecl->getType())) {
       return true;
     }
   }
@@ -405,7 +446,8 @@ static bool checkInvariantViolation(ProgramStateRef State, ExplodedNode *N,
   else
     return false;
 
-  if (checkParamsForPreconditionViolation(Params, State, LocCtxt)) {
+  if (checkParamsForPreconditionViolation(Params, State, LocCtxt) ||
+      checkSelfIvarsForInvariantViolation(State, LocCtxt)) {
     if (!N->isSink())
       C.addTransition(State->set<InvariantViolated>(true), N);
     return true;