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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364869: [analyzer] NonnullGlobalConstants: Dont be 
confused by a _Nonnull attribute. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63956?vs=207139=207433#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63956/new/

https://reviews.llvm.org/D63956

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  cfe/trunk/test/Analysis/nonnull-global-constants.mm


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
@@ -106,14 +106,21 @@
 return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast(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(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(T)) {
+  if (AT->getAttrKind() == attr::TypeNonNull)
+return true;
+  Ty = AT->getModifiedType();
+} else {
+  return false;
+}
   }
   return false;
 }
Index: cfe/trunk/test/Analysis/nonnull-global-constants.mm
===
--- cfe/trunk/test/Analysis/nonnull-global-constants.mm
+++ cfe/trunk/test/Analysis/nonnull-global-constants.mm
@@ -101,3 +101,15 @@
 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}}
+}


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
@@ -106,14 +106,21 @@
 return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast(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(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(T)) {
+  if (AT->getAttrKind() == attr::TypeNonNull)
+return true;
+  Ty = AT->getModifiedType();
+} else {
+  return false;
+}
   }
   return false;
 }
Index: cfe/trunk/test/Analysis/nonnull-global-constants.mm
===
--- cfe/trunk/test/Analysis/nonnull-global-constants.mm
+++ cfe/trunk/test/Analysis/nonnull-global-constants.mm
@@ -101,3 +101,15 @@
 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}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LG!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63956/new/

https://reviews.llvm.org/D63956



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D63956

Files:
  clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  clang/test/Analysis/nonnull-global-constants.mm


Index: clang/test/Analysis/nonnull-global-constants.mm
===
--- clang/test/Analysis/nonnull-global-constants.mm
+++ clang/test/Analysis/nonnull-global-constants.mm
@@ -101,3 +101,15 @@
 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}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
@@ -106,14 +106,21 @@
 return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast(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(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(T)) {
+  if (AT->getAttrKind() == attr::TypeNonNull)
+return true;
+  Ty = AT->getModifiedType();
+} else {
+  return false;
+}
   }
   return false;
 }


Index: clang/test/Analysis/nonnull-global-constants.mm
===
--- clang/test/Analysis/nonnull-global-constants.mm
+++ clang/test/Analysis/nonnull-global-constants.mm
@@ -101,3 +101,15 @@
 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}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
@@ -106,14 +106,21 @@
 return true;
 
   // Look through the typedefs.
-  while (auto *T = dyn_cast(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(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(T)) {
+  if (AT->getAttrKind() == attr::TypeNonNull)
+return true;
+  Ty = AT->getModifiedType();
+} else {
+  return false;
+}
   }
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits