[analyzer] NonnullGlobalConstants: Don't be confused with a _Nonnull attribute.

[NoQ]

Summary

The NonnullGlobalConstants checker models the rule "it doesn't make sense to make a constant global pointer and initialize it to null"; it makes sure that whatever it's initialized with is known to be non-null.

Ironically, annotating the type of the pointer as _Nonnull breaks the checker.

Fix handling of the _Nonnull annotation so that it was instead one more reason to believe that the value is non-null.

[xazax.hun]

LG!

cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp

if (!Decl->hasGlobalStorage())
     return false;
 
   QualType Ty = Decl->getType();
   bool HasConst = Ty.isConstQualified();
   if (isNonnullType(Ty) && HasConst)
     return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast<TypedefType>(Ty)) {
-    Ty = T->getDecl()->getUnderlyingType();
-
+  while (const Type *T = Ty.getTypePtr()) {
+    if (const auto *TT = dyn_cast<TypedefType>(T)) {
+      Ty = TT->getDecl()->getUnderlyingType();
       // It is sufficient for any intermediate typedef
       // to be classified const.
       HasConst = HasConst || Ty.isConstQualified();
       if (isNonnullType(Ty) && HasConst)
         return true;
+    } else if (const auto *AT = dyn_cast<AttributedType>(T)) {
+      if (AT->getAttrKind() == attr::TypeNonNull)
+        return true;
+      Ty = AT->getModifiedType();
+    } else {
+      return false;
+    }
   }
   return false;
 }
 
 /// \return whether {@code type} is extremely unlikely to be null
 bool NonnullGlobalConstantsChecker::isNonnullType(QualType Ty) const {
 
   if (Ty->isPointerType() && Ty->getPointeeType()->isCharType())

cfe/trunk/test/Analysis/nonnull-global-constants.mm

void testNonnullBool() {
   clang_analyzer_eval(kBool); // expected-warning{{TRUE}}
 }
 
 // And again, only for const one.
 extern CFBooleanRef kBoolMutable;
 void testNonnullNonconstBool() {
   clang_analyzer_eval(kBoolMutable); // expected-warning{{UNKNOWN}}
 }
+
+// If it's annotated as nonnull, it doesn't even need to be const.
+extern CFStringRef _Nonnull str3;
+void testNonnullNonconstCFString() {
+  clang_analyzer_eval(str3); // expected-warning{{TRUE}}
+}
+
+// This one's nonnull for two reasons.
+extern const CFStringRef _Nonnull str4;
+void testNonnullNonnullCFString() {
+  clang_analyzer_eval(str4); // expected-warning{{TRUE}}
+}