From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Mon, 1 Jul 2019 23:02:14 +0000 (+0000)
Subject: [analyzer] NonnullGlobalConstants: Don't be confused by a _Nonnull attribute.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=273f8722be00ef1ba622ff40ab2488a135fcbe3b;p=clang

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

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.

Differential Revision: https://reviews.llvm.org/D63956

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

diff --git a/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp b/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
index dd76fd2f12..43dbe57b84 100644
--- a/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
@@ -106,14 +106,21 @@ bool NonnullGlobalConstantsChecker::isGlobalConstString(SVal V) const {
     return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast<TypedefType>(Ty)) {
-    Ty = T->getDecl()->getUnderlyingType();
-
-    // It is sufficient for any intermediate typedef
-    // to be classified const.
-    HasConst = HasConst || Ty.isConstQualified();
-    if (isNonnullType(Ty) && HasConst)
-      return true;
+  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;
 }
diff --git a/test/Analysis/nonnull-global-constants.mm b/test/Analysis/nonnull-global-constants.mm
index 7900b9dd12..9e1a588ba4 100644
--- a/test/Analysis/nonnull-global-constants.mm
+++ b/test/Analysis/nonnull-global-constants.mm
@@ -101,3 +101,15 @@ 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}}
+}