Skip to content

Commit 512f483

Browse files
committed
[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 llvm-svn: 364869
1 parent 35fdec1 commit 512f483

File tree

2 files changed

+27
-8
lines changed

2 files changed

+27
-8
lines changed

clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,21 @@ bool NonnullGlobalConstantsChecker::isGlobalConstString(SVal V) const {
106106
return true;
107107

108108
// Look through the typedefs.
109-
while (auto *T = dyn_cast<TypedefType>(Ty)) {
110-
Ty = T->getDecl()->getUnderlyingType();
111-
112-
// It is sufficient for any intermediate typedef
113-
// to be classified const.
114-
HasConst = HasConst || Ty.isConstQualified();
115-
if (isNonnullType(Ty) && HasConst)
116-
return true;
109+
while (const Type *T = Ty.getTypePtr()) {
110+
if (const auto *TT = dyn_cast<TypedefType>(T)) {
111+
Ty = TT->getDecl()->getUnderlyingType();
112+
// It is sufficient for any intermediate typedef
113+
// to be classified const.
114+
HasConst = HasConst || Ty.isConstQualified();
115+
if (isNonnullType(Ty) && HasConst)
116+
return true;
117+
} else if (const auto *AT = dyn_cast<AttributedType>(T)) {
118+
if (AT->getAttrKind() == attr::TypeNonNull)
119+
return true;
120+
Ty = AT->getModifiedType();
121+
} else {
122+
return false;
123+
}
117124
}
118125
return false;
119126
}

clang/test/Analysis/nonnull-global-constants.mm

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,15 @@ void testNonnullBool() {
101101
void testNonnullNonconstBool() {
102102
clang_analyzer_eval(kBoolMutable); // expected-warning{{UNKNOWN}}
103103
}
104+
105+
// If it's annotated as nonnull, it doesn't even need to be const.
106+
extern CFStringRef _Nonnull str3;
107+
void testNonnullNonconstCFString() {
108+
clang_analyzer_eval(str3); // expected-warning{{TRUE}}
109+
}
110+
111+
// This one's nonnull for two reasons.
112+
extern const CFStringRef _Nonnull str4;
113+
void testNonnullNonnullCFString() {
114+
clang_analyzer_eval(str4); // expected-warning{{TRUE}}
115+
}

0 commit comments

Comments
 (0)