[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315488: [Analyzer] Assume that string-like const globals are
non-nil. (authored by george.karpenkov).
Changed prior to commit:
https://reviews.llvm.org/D38764?vs=118657=118660#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38764
Files:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
cfe/trunk/test/Analysis/nonnull-string-constants.mm
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -57,6 +57,7 @@
NSErrorChecker.cpp
NoReturnFunctionChecker.cpp
NonNullParamChecker.cpp
+ NonnullStringConstantsChecker.cpp
NullabilityChecker.cpp
NumberObjectConversionChecker.cpp
ObjCAtSyncChecker.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
@@ -0,0 +1,134 @@
+//==- NonnullStringConstantsChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This checker adds an assumption that constant string-like globals are
+// non-null, as otherwise they generally do not convey any useful information.
+// The assumption is useful, as many framework use such global const strings,
+// and the analyzer might not be able to infer the global value if the
+// definition is in a separate translation unit.
+// The following types (and their typedef aliases) are considered string-like:
+// - `char* const`
+// - `const CFStringRef` from CoreFoundation
+// - `NSString* const` from Foundation
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class NonnullStringConstantsChecker : public Checker {
+ mutable IdentifierInfo *NSStringII = nullptr;
+ mutable IdentifierInfo *CFStringRefII = nullptr;
+
+public:
+ NonnullStringConstantsChecker() {}
+
+ void checkLocation(SVal l, bool isLoad, const Stmt *S,
+ CheckerContext ) const;
+
+private:
+ void initIdentifierInfo(ASTContext ) const;
+
+ bool isGlobalConstString(SVal V) const;
+
+ bool isStringlike(QualType Ty) const;
+};
+
+} // namespace
+
+/// Lazily initialize cache for required identifier informations.
+void NonnullStringConstantsChecker::initIdentifierInfo(ASTContext ) const {
+ if (NSStringII)
+return;
+
+ NSStringII = ("NSString");
+ CFStringRefII = ("CFStringRef");
+}
+
+/// Add an assumption that const string-like globals are non-null.
+void NonnullStringConstantsChecker::checkLocation(SVal location, bool isLoad,
+ const Stmt *S,
+ CheckerContext ) const {
+ initIdentifierInfo(C.getASTContext());
+ if (!isLoad || !location.isValid())
+return;
+
+ ProgramStateRef State = C.getState();
+ SVal V = State->getSVal(location.castAs());
+
+ if (isGlobalConstString(location)) {
+Optional Constr = V.getAs();
+
+if (Constr) {
+
+ // Assume that the variable is non-null.
+ ProgramStateRef OutputState = State->assume(*Constr, true);
+ C.addTransition(OutputState);
+}
+ }
+}
+
+/// \param V loaded lvalue.
+/// \return whether {@code val} is a string-like const global.
+bool NonnullStringConstantsChecker::isGlobalConstString(SVal V) const {
+ Optional RegionVal = V.getAs();
+ if (!RegionVal)
+return false;
+ auto *Region = dyn_cast(RegionVal->getAsRegion());
+ if (!Region)
+return false;
+ const VarDecl *Decl = Region->getDecl();
+
+ if (!Decl->hasGlobalStorage())
+return false;
+
+ QualType Ty = Decl->getType();
+ bool HasConst = Ty.isConstQualified();
+ if (isStringlike(Ty) && HasConst)
+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.
+
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118657.
george.karpenkov marked 2 inline comments as done.
https://reviews.llvm.org/D38764
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
test/Analysis/nonnull-string-constants.mm
Index: test/Analysis/nonnull-string-constants.mm
===
--- /dev/null
+++ test/Analysis/nonnull-string-constants.mm
@@ -0,0 +1,90 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+// Nullability of const string-like globals, testing
+// NonnullStringConstantsChecker.
+
+void clang_analyzer_eval(bool);
+
+@class NSString;
+typedef const struct __CFString *CFStringRef;
+
+// Global NSString* is non-null.
+extern NSString *const StringConstGlobal;
+void stringConstGlobal() {
+ clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// The logic does not apply to local variables though.
+extern NSString *stringGetter();
+void stringConstLocal() {
+ NSString *const local = stringGetter();
+ clang_analyzer_eval(local); // expected-warning{{UNKNOWN}}
+}
+
+// Global const CFStringRef's are also assumed to be non-null.
+extern const CFStringRef CFStringConstGlobal;
+void cfStringCheckGlobal() {
+ clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// But only "const" ones.
+extern CFStringRef CFStringNonConstGlobal;
+void cfStringCheckMutableGlobal() {
+ clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}}
+}
+
+// char* const is also assumed to be non-null.
+extern const char *const ConstCharStarConst;
+void constCharStarCheckGlobal() {
+ clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}}
+}
+
+// Pointer value can be mutable.
+extern char *const CharStarConst;
+void charStarCheckGlobal() {
+ clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}}
+}
+
+// But the pointer itself should be immutable.
+extern char *CharStar;
+void charStartCheckMutableGlobal() {
+ clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}}
+}
+
+// Type definitions should also work across typedefs, for pointers:
+typedef char *const str;
+extern str globalStr;
+void charStarCheckTypedef() {
+ clang_analyzer_eval(globalStr); // expected-warning{{TRUE}}
+}
+
+// And for types.
+typedef NSString *const NStr;
+extern NStr globalNSString;
+void NSStringCheckTypedef() {
+ clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}}
+}
+
+// Note that constness could be either inside
+// the var declaration, or in a typedef.
+typedef NSString *NStr2;
+extern const NStr2 globalNSString2;
+void NSStringCheckConstTypedef() {
+ clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}}
+}
+
+// Nested typedefs should work as well.
+typedef const CFStringRef str1;
+typedef str1 str2;
+extern str2 globalStr2;
+void testNestedTypedefs() {
+ clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}}
+}
+
+// And for NSString *.
+typedef NSString *const nstr1;
+typedef nstr1 nstr2;
+extern nstr2 nglobalStr2;
+void testNestedTypedefsForNSString() {
+ clang_analyzer_eval(nglobalStr2); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
@@ -0,0 +1,134 @@
+//==- NonnullStringConstantsChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This checker adds an assumption that constant string-like globals are
+// non-null, as otherwise they generally do not convey any useful information.
+// The assumption is useful, as many framework use such global const strings,
+// and the analyzer might not be able to infer the global value if the
+// definition is in a separate translation unit.
+// The following types (and their typedef aliases) are considered string-like:
+// - `char* const`
+// - `const CFStringRef` from CoreFoundation
+// - `NSString* const` from Foundation
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class NonnullStringConstantsChecker : public Checker {
+ mutable IdentifierInfo
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov marked 9 inline comments as done. george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:22 +// Checker uses are defined in the test file: +// - test/Analysis/nonnull-string-constants.mm +// dcoughlin wrote: > We generally don't do this kind of cross-reference in comments since they > tend to get stale fast when things get moved around. There is no tool support > to keep them in sync. :( I guess I'm too spoiled by Javadoc. Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:53 +private: + /// Lazily initialize cache for required identifier informations. + void initIdentifierInfo(ASTContext ) const; dcoughlin wrote: > We usually put the oxygen comments on checkers on the implementation and not > the interface since they aren't API and people reading the code generally > look at the implementations. Can you move them to the implementations? OK. I assume there's no good universal solution here. https://reviews.llvm.org/D38764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.
Looks good to me. Please fix the additional nits mentioned inline and commit!
Also, make sure to do a pass to update the capitalization of variables
throughout the file to match the LLVM coding standards.
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:1
+//==-- RetainCountChecker.cpp - Checks for leaks and other issues -*- C++
-*--//
+//
The file name and description here needs to be updated for this checker.
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:10
+//
+// Class definition for NonnullStringConstantsChecker.
+// This checker adds an assumption that constant string-like globals are
I don't think the comment in the first line here is needed.
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:22
+// Checker uses are defined in the test file:
+// - test/Analysis/nonnull-string-constants.mm
+//
We generally don't do this kind of cross-reference in comments since they tend
to get stale fast when things get moved around. There is no tool support to
keep them in sync.
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:26
+
+//
+// This checker ensures that const string globals are assumed to be non-null.
Can this comment go?
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:53
+private:
+ /// Lazily initialize cache for required identifier informations.
+ void initIdentifierInfo(ASTContext ) const;
We usually put the oxygen comments on checkers on the implementation and not
the interface since they aren't API and people reading the code generally look
at the implementations. Can you move them to the implementations?
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:114
+ // Look through the typedefs.
+ while (const TypedefType *T = dyn_cast(type)) {
+type = T->getDecl()->getUnderlyingType();
To match the coding standards this should be `auto *` as well.
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:131
+
+ if (const ObjCObjectPointerType *T = dyn_cast(type)) {
+return T->getInterfaceDecl()->getIdentifier() == NSStringII;
Similar comment about `auto *`
Comment at: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp:133
+return T->getInterfaceDecl()->getIdentifier() == NSStringII;
+ } else if (const TypedefType *T = dyn_cast(type)) {
+return T->getDecl()->getIdentifier() == CFStringRefII;
Similar comment about `auto *`.
Comment at: test/Analysis/nonnull-string-constants.mm:5
+// Relies on the checker defined in
+// lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp.
+
We generally don't put file paths like this in comments since they tend to go
stale. It is fine to to say that these are tests for the
NonnullStringConstantsChecker though.
https://reviews.llvm.org/D38764
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118522.
george.karpenkov added a comment.
Typo fix.
https://reviews.llvm.org/D38764
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
test/Analysis/nonnull-string-constants.mm
Index: test/Analysis/nonnull-string-constants.mm
===
--- /dev/null
+++ test/Analysis/nonnull-string-constants.mm
@@ -0,0 +1,91 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+// Nullability of const string-like globals.
+// Relies on the checker defined in
+// lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp.
+
+void clang_analyzer_eval(bool);
+
+@class NSString;
+typedef const struct __CFString *CFStringRef;
+
+// Global NSString* is non-null.
+extern NSString *const StringConstGlobal;
+void stringConstGlobal() {
+ clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// The logic does not apply to local variables though.
+extern NSString *stringGetter();
+void stringConstLocal() {
+ NSString *const local = stringGetter();
+ clang_analyzer_eval(local); // expected-warning{{UNKNOWN}}
+}
+
+// Global const CFStringRef's are also assumed to be non-null.
+extern const CFStringRef CFStringConstGlobal;
+void cfStringCheckGlobal() {
+ clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// But only "const" ones.
+extern CFStringRef CFStringNonConstGlobal;
+void cfStringCheckMutableGlobal() {
+ clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}}
+}
+
+// char* const is also assumed to be non-null.
+extern const char *const ConstCharStarConst;
+void constCharStarCheckGlobal() {
+ clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}}
+}
+
+// Pointer value can be mutable.
+extern char *const CharStarConst;
+void charStarCheckGlobal() {
+ clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}}
+}
+
+// But the pointer itself should be immutable.
+extern char *CharStar;
+void charStartCheckMutableGlobal() {
+ clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}}
+}
+
+// Type definitions should also work across typedefs, for pointers:
+typedef char *const str;
+extern str globalStr;
+void charStarCheckTypedef() {
+ clang_analyzer_eval(globalStr); // expected-warning{{TRUE}}
+}
+
+// And for types.
+typedef NSString *const NStr;
+extern NStr globalNSString;
+void NSStringCheckTypedef() {
+ clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}}
+}
+
+// Note that constness could be either inside
+// the var declaration, or in a typedef.
+typedef NSString *NStr2;
+extern const NStr2 globalNSString2;
+void NSStringCheckConstTypedef() {
+ clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}}
+}
+
+// Nested typedefs should work as well.
+typedef const CFStringRef str1;
+typedef str1 str2;
+extern str2 globalStr2;
+void testNestedTypedefs() {
+ clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}}
+}
+
+// And for NSString *.
+typedef NSString *const nstr1;
+typedef nstr1 nstr2;
+extern nstr2 nglobalStr2;
+void testNestedTypedefsForNSString() {
+ clang_analyzer_eval(nglobalStr2); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
@@ -0,0 +1,141 @@
+//==-- RetainCountChecker.cpp - Checks for leaks and other issues -*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Class definition for NonnullStringConstantsChecker.
+// This checker adds an assumption that constant string-like globals are
+// non-null, as otherwise they generally do not convey any useful information.
+// The assumption is useful, as many framework use such global const strings,
+// and the analyzer might not be able to infer the global value if the
+// definition is in a separate translation unit.
+// The following types (and their typedef aliases) are considered string-like:
+// - `char* const`
+// - `const CFStringRef` from CoreFoundation
+// - `NSString* const` from Foundation
+//
+// Checker uses are defined in the test file:
+// - test/Analysis/nonnull-string-constants.mm
+//
+//===--===//
+
+//
+// This checker ensures that const string globals are assumed to be non-null.
+//
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov added a comment. Marking requests as "done". https://reviews.llvm.org/D38764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118521.
george.karpenkov marked 12 inline comments as done.
george.karpenkov added a comment.
Adhering to comments.
https://reviews.llvm.org/D38764
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
test/Analysis/nonnnull-string-constants.mm
Index: test/Analysis/nonnnull-string-constants.mm
===
--- /dev/null
+++ test/Analysis/nonnnull-string-constants.mm
@@ -0,0 +1,91 @@
+// Nullability of const string-like globals.
+// Relies on the checker defined in
+// lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp.
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+@class NSString;
+typedef const struct __CFString *CFStringRef;
+
+// Global NSString* is non-null.
+extern NSString *const StringConstGlobal;
+void stringConstGlobal() {
+ clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// The logic does not apply to local variables though.
+extern NSString *stringGetter();
+void stringConstLocal() {
+ NSString *const local = stringGetter();
+ clang_analyzer_eval(local); // expected-warning{{UNKNOWN}}
+}
+
+// Global const CFStringRef's are also assumed to be non-null.
+extern const CFStringRef CFStringConstGlobal;
+void cfStringCheckGlobal() {
+ clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// But only "const" ones.
+extern CFStringRef CFStringNonConstGlobal;
+void cfStringCheckMutableGlobal() {
+ clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}}
+}
+
+// char* const is also assumed to be non-null.
+extern const char *const ConstCharStarConst;
+void constCharStarCheckGlobal() {
+ clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}}
+}
+
+// Pointer value can be mutable.
+extern char *const CharStarConst;
+void charStarCheckGlobal() {
+ clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}}
+}
+
+// But the pointer itself should be immutable.
+extern char *CharStar;
+void charStartCheckMutableGlobal() {
+ clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}}
+}
+
+// Type definitions should also work across typedefs, for pointers:
+typedef char *const str;
+extern str globalStr;
+void charStarCheckTypedef() {
+ clang_analyzer_eval(globalStr); // expected-warning{{TRUE}}
+}
+
+// And for types.
+typedef NSString *const NStr;
+extern NStr globalNSString;
+void NSStringCheckTypedef() {
+ clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}}
+}
+
+// Note that constness could be either inside
+// the var declaration, or in a typedef.
+typedef NSString *NStr2;
+extern const NStr2 globalNSString2;
+void NSStringCheckConstTypedef() {
+ clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}}
+}
+
+// Nested typedefs should work as well.
+typedef const CFStringRef str1;
+typedef str1 str2;
+extern str2 globalStr2;
+void testNestedTypedefs() {
+ clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}}
+}
+
+// And for NSString *.
+typedef NSString *const nstr1;
+typedef nstr1 nstr2;
+extern nstr2 nglobalStr2;
+void testNestedTypedefsForNSString() {
+ clang_analyzer_eval(nglobalStr2); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/NonnullStringConstantsChecker.cpp
@@ -0,0 +1,141 @@
+//==-- RetainCountChecker.cpp - Checks for leaks and other issues -*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Class definition for NonnullStringConstantsChecker.
+// This checker adds an assumption that constant string-like globals are
+// non-null, as otherwise they generally do not convey any useful information.
+// The assumption is useful, as many framework use such global const strings,
+// and the analyzer might not be able to infer the global value if the
+// definition is in a separate translation unit.
+// The following types (and their typedef aliases) are considered string-like:
+// - `char* const`
+// - `const CFStringRef` from CoreFoundation
+// - `NSString* const` from Foundation
+//
+// Checker uses are defined in the test file:
+// - test/Analysis/nonnull-string-constants.mm
+//
+//===--===//
+
+//
+// This checker ensures that const string globals are assumed to be non-null.
+//
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
dcoughlin added a comment.
Looks like a great start!
There are a bunch of minor nits inline.
The one big thing is that I think your handling of 'const char *' in
`typeIsConstString()` isn't quite right. 'const char *' means that the
pointed-to characters can't be modified but does allow modification of the
variable holding the pointer. I don't think we want to treat such variables as
holding non-null pointers, since anyone could assign them to NULL. On the other
hand, we do want to treat variables of type 'char * const' as holding non-null
pointers since the variable can't be reassigned and (presumably) it was
initialized to a non-null value. In this second case the characters could
potentially be modified, but that doesn't change whether the value of the
pointer itself.
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:1
+
+//
We need to have the license preamble here.
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:3
+//
+// This checker ensures that const string globals are assumed to be non-null.
+//
It would be good to include a motivation for why this is the right thing to do
and even an example of the declarations this will trigger on.
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:17
+
+class NonnilStringConstantsChecker : public Checker {
+ mutable IdentifierInfo *NSStringII = nullptr;
Since this applies to more than just Objective-C, I think it would be better to
use 'null' instead of 'nil' in the name of the checker. Or even remove the
'Nonnil' prefix. What about 'GlobalStringConstantsChecker'?
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:22
+public:
+ NonnilStringConstantsChecker(AnalyzerOptions ) {}
+
Does the constructor need to take an AnalyzerOptions?
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:53
+ SVal V = UnknownVal();
+ if (location.isValid()) {
+V = State->getSVal(location.castAs());
Should we just early return if `location` is not valid?
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:70
+
+bool NonnilStringConstantsChecker::isGlobalConstString(SVal val) const {
+ Optional regionVal = val.getAs();
Can you add doxygen-style comments to the implementations of these methods? I'd
like to break with tradition and have comments for all new code.
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:71
+bool NonnilStringConstantsChecker::isGlobalConstString(SVal val) const {
+ Optional regionVal = val.getAs();
+ if (!regionVal)
Note that we use capital letters for variables and parameters in Clang/LLVM.
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:74
+return false;
+ const VarRegion *region = dyn_cast(regionVal->getAsRegion());
+ if (!region)
The convention Clang uses with dyn_cast and friends to to use 'auto *' in the
variable declaration in order to not repeat the type name:
```
auto *Region = dyn_cast(RegionVal->getAsRegion());
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:93
+// to be classified const.
+hasOuterConst |= type.isConstQualified();
+if (typeIsConstString(type, hasOuterConst))
Do you really want a bit-wise or here?
Comment at: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp:113
+II = T->getDecl()->getIdentifier();
+ }
+
This will leave `II` as uninitialized if your `if/else if` is not exhaustive.
Are you sure that it is?
Comment at: test/Analysis/nonnil-string-constants.mm:41
+// For char* we do not require a pointer itself to be immutable.
+extern const char *CharStarConst;
+void charStarCheckGlobal() {
What is the rationale for treating `const char *v` as non null?
In this scenario `v` can be reassigned, right?
Comment at: test/Analysis/nonnil-string-constants.mm:45
+}
+
+// But the pointed data should be.
I'd also like to see a test for treating `char * const v;` as non null.
Comment at: test/Analysis/nonnil-string-constants.mm:55
+extern str globalStr;
+void charStarCheckTypedef() {
+ clang_analyzer_eval(globalStr); // expected-warning{{TRUE}}
```
typedef const char *str;
extern str globalStr;
```
allows the `globalStr` variable to be written to.
Did you mean:
```
typedef char * const str;
extern str globalStr;
```
https://reviews.llvm.org/D38764
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov updated this revision to Diff 118493.
https://reviews.llvm.org/D38764
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp
test/Analysis/nonnil-string-constants.mm
Index: test/Analysis/nonnil-string-constants.mm
===
--- /dev/null
+++ test/Analysis/nonnil-string-constants.mm
@@ -0,0 +1,80 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+// Nullability of const string-like globals.
+void clang_analyzer_eval(bool);
+
+@class NSString;
+typedef const struct __CFString *CFStringRef;
+
+// Global NSString* is non-null.
+extern NSString *const StringConstGlobal;
+void stringConstGlobal() {
+ clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// The logic does not apply to local variables though.
+extern NSString *stringGetter();
+void stringConstLocal() {
+ NSString *const local = stringGetter();
+ clang_analyzer_eval(local); // expected-warning{{UNKNOWN}}
+}
+
+// Global const CFStringRef's are also assumed to be non-null.
+extern const CFStringRef CFStringConstGlobal;
+void cfStringCheckGlobal() {
+ clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// But only "const" ones.
+extern CFStringRef CFStringNonConstGlobal;
+void cfStringCheckMutableGlobal() {
+ clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}}
+}
+
+// Const char* is also assumed to be non-null.
+extern const char *const ConstCharStarConst;
+void constCharStarCheckGlobal() {
+ clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}}
+}
+
+// For char* we do not require a pointer itself to be immutable.
+extern const char *CharStarConst;
+void charStarCheckGlobal() {
+ clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}}
+}
+
+// But the pointed data should be.
+extern char *CharStar;
+void charStartCheckMutableGlobal() {
+ clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}}
+}
+
+// Type definitions should also work across typedefs, for pointers:
+typedef const char *str;
+extern str globalStr;
+void charStarCheckTypedef() {
+ clang_analyzer_eval(globalStr); // expected-warning{{TRUE}}
+}
+
+// And for types.
+typedef NSString *const NStr;
+extern NStr globalNSString;
+void NSStringCheckTypedef() {
+ clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}}
+}
+
+// Note that constness could be either inside
+// the var declaration, or in a typedef.
+typedef NSString *NStr2;
+extern const NStr2 globalNSString2;
+void NSStringCheckConstTypedef() {
+ clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}}
+}
+
+// Nested typedefs should work as well.
+typedef const CFStringRef str1;
+typedef str1 str2;
+extern str2 globalStr2;
+void testNestedTypedefs() {
+ clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp
@@ -0,0 +1,121 @@
+
+//
+// This checker ensures that const string globals are assumed to be non-null.
+//
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class NonnilStringConstantsChecker : public Checker {
+ mutable IdentifierInfo *NSStringII = nullptr;
+ mutable IdentifierInfo *CFStringRefII = nullptr;
+
+public:
+ NonnilStringConstantsChecker(AnalyzerOptions ) {}
+
+ void checkLocation(SVal l, bool isLoad, const Stmt *S,
+ CheckerContext ) const;
+
+private:
+ /// Lazily initialize cache for required identifier informations.
+ void initIdentifierInfo(ASTContext ) const;
+ bool typeIsConstString(QualType type, bool isConstQualified) const;
+ bool isGlobalConstString(SVal val) const;
+};
+
+} // namespace
+
+void NonnilStringConstantsChecker::initIdentifierInfo(ASTContext ) const {
+ if (NSStringII)
+return;
+
+ NSStringII = ("NSString");
+ CFStringRefII = ("CFStringRef");
+}
+
+void NonnilStringConstantsChecker::checkLocation(SVal location, bool isLoad,
+ const Stmt *S,
+ CheckerContext ) const {
+ initIdentifierInfo(C.getASTContext());
+ if (!isLoad)
+return;
+
+ ProgramStateRef State = C.getState();
+ SVal V = UnknownVal();
+ if (location.isValid()) {
+V = State->getSVal(location.castAs());
+ }
+
+ if (isGlobalConstString(location)) {
+Optional Constr =
[PATCH] D38764: [Analyzer] Assume const string-like globals are non-null
george.karpenkov created this revision.
Herald added subscribers: szepet, xazax.hun, mgorny.
https://reviews.llvm.org/D38764
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp
test/Analysis/nonnil-string-constants.mm
Index: test/Analysis/nonnil-string-constants.mm
===
--- /dev/null
+++ test/Analysis/nonnil-string-constants.mm
@@ -0,0 +1,80 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+// Nullability of const string-like globals.
+void clang_analyzer_eval(bool);
+
+@class NSString;
+typedef const struct __CFString *CFStringRef;
+
+// Global NSString* is non-null.
+extern NSString *const StringConstGlobal;
+void stringConstGlobal() {
+ clang_analyzer_eval(StringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// The logic does not apply to local variables though.
+extern NSString *stringGetter();
+void stringConstLocal() {
+ NSString *const local = stringGetter();
+ clang_analyzer_eval(local); // expected-warning{{UNKNOWN}}
+}
+
+// Global const CFStringRef's are also assumed to be non-null.
+extern const CFStringRef CFStringConstGlobal;
+void cfStringCheckGlobal() {
+ clang_analyzer_eval(CFStringConstGlobal); // expected-warning{{TRUE}}
+}
+
+// But only "const" ones.
+extern CFStringRef CFStringNonConstGlobal;
+void cfStringCheckMutableGlobal() {
+ clang_analyzer_eval(CFStringNonConstGlobal); // expected-warning{{UNKNOWN}}
+}
+
+// Const char* is also assumed to be non-null.
+extern const char *const ConstCharStarConst;
+void constCharStarCheckGlobal() {
+ clang_analyzer_eval(ConstCharStarConst); // expected-warning{{TRUE}}
+}
+
+// For char* we do not require a pointer itself to be immutable.
+extern const char *CharStarConst;
+void charStarCheckGlobal() {
+ clang_analyzer_eval(CharStarConst); // expected-warning{{TRUE}}
+}
+
+// But the pointed data should be.
+extern char *CharStar;
+void charStartCheckMutableGlobal() {
+ clang_analyzer_eval(CharStar); // expected-warning{{UNKNOWN}}
+}
+
+// Type definitions should also work across typedefs, for pointers:
+typedef const char *str;
+extern str globalStr;
+void charStarCheckTypedef() {
+ clang_analyzer_eval(globalStr); // expected-warning{{TRUE}}
+}
+
+// And for types.
+typedef NSString *const NStr;
+extern NStr globalNSString;
+void NSStringCheckTypedef() {
+ clang_analyzer_eval(globalNSString); // expected-warning{{TRUE}}
+}
+
+// Note that constness could be either inside
+// the var declaration, or in a typedef.
+typedef NSString *NStr2;
+extern const NStr2 globalNSString2;
+void NSStringCheckConstTypedef() {
+ clang_analyzer_eval(globalNSString2); // expected-warning{{TRUE}}
+}
+
+// Nested typedefs should work as well.
+typedef const CFStringRef str1;
+typedef str1 str2;
+extern str2 globalStr2;
+void testNestedTypedefs() {
+ clang_analyzer_eval(globalStr2); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/NonnilStringConstantsChecker.cpp
@@ -0,0 +1,120 @@
+
+//
+// This checker ensures that const string globals are assumed to be non-null.
+//
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class NonnilStringConstantsChecker : public Checker {
+ mutable IdentifierInfo *NSStringII;
+ mutable IdentifierInfo *CFStringRefII;
+
+public:
+ NonnilStringConstantsChecker(AnalyzerOptions ) {}
+
+ void checkLocation(SVal l, bool isLoad, const Stmt *S,
+ CheckerContext ) const;
+
+private:
+ /// Lazily initialize cache for required identifier informations.
+ void initIdentifierInfo(ASTContext ) const;
+ bool typeIsConstString(QualType type, bool isConstQualified) const;
+ bool isGlobalConstString(SVal val) const;
+};
+
+} // namespace
+
+void NonnilStringConstantsChecker::checkLocation(SVal location, bool isLoad,
+ const Stmt *S,
+ CheckerContext ) const {
+ initIdentifierInfo(C.getASTContext());
+ if (!isLoad)
+return;
+
+ ProgramStateRef State = C.getState();
+ SVal V = UnknownVal();
+ if (location.isValid()) {
+V = State->getSVal(location.castAs());
+ }
+
+ if (isGlobalConstString(location)) {
+Optional Constr = V.getAs();
+
+if (Constr) {
+
+ // Assume that the variable is non-null.
+ ProgramStateRef OutputState = State->assume(*Constr, true);
+
