r328282 - [analyzer] Trust _Nonnull annotations for system framework
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 22 17:16:03 PDT 2018
Author: george.karpenkov
Date: Thu Mar 22 17:16:03 2018
New Revision: 328282
URL: http://llvm.org/viewvc/llvm-project?rev=328282&view=rev
Log:
[analyzer] Trust _Nonnull annotations for system framework
Changes the analyzer to believe that methods annotated with _Nonnull
from system frameworks indeed return non null objects.
Local methods with such annotation are still distrusted.
rdar://24291919
Differential Revision: https://reviews.llvm.org/D44341
Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
cfe/trunk/test/Analysis/trustnonnullchecker_test.m
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h
Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=328282&r1=328281&r2=328282&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Thu Mar 22 17:16:03 2018
@@ -218,6 +218,14 @@ def NullableReturnedFromNonnullChecker :
} // end "nullability"
+let ParentPackage = APIModeling in {
+
+def TrustNonnullChecker : Checker<"TrustNonnull">,
+ HelpText<"Trust that returns from framework methods annotated with _Nonnull are not null">,
+ DescFile<"TrustNonnullChecker.cpp">;
+
+}
+
//===----------------------------------------------------------------------===//
// Evaluate "builtin" functions.
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h?rev=328282&r1=328281&r2=328282&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h Thu Mar 22 17:16:03 2018
@@ -21,6 +21,8 @@ namespace clang {
class Expr;
class VarDecl;
+class QualType;
+class AttributedType;
namespace ento {
@@ -42,6 +44,25 @@ template <class T> bool containsStmt(con
std::pair<const clang::VarDecl *, const clang::Expr *>
parseAssignment(const Stmt *S);
+// Do not reorder! The getMostNullable method relies on the order.
+// Optimization: Most pointers expected to be unspecified. When a symbol has an
+// unspecified or nonnull type non of the rules would indicate any problem for
+// that symbol. For this reason only nullable and contradicted nullability are
+// stored for a symbol. When a symbol is already contradicted, it can not be
+// casted back to nullable.
+enum class Nullability : char {
+ Contradicted, // Tracked nullability is contradicted by an explicit cast. Do
+ // not report any nullability related issue for this symbol.
+ // This nullability is propagated aggressively to avoid false
+ // positive results. See the comment on getMostNullable method.
+ Nullable,
+ Unspecified,
+ Nonnull
+};
+
+/// Get nullability annotation for a given type.
+Nullability getNullabilityAnnotation(QualType Type);
+
} // end GR namespace
} // end clang namespace
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=328282&r1=328281&r2=328282&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Thu Mar 22 17:16:03 2018
@@ -84,6 +84,7 @@ add_clang_library(clangStaticAnalyzerChe
TaintTesterChecker.cpp
TestAfterDivZeroChecker.cpp
TraversalChecker.cpp
+ TrustNonnullChecker.cpp
UndefBranchChecker.cpp
UndefCapturedBlockVarChecker.cpp
UndefResultChecker.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=328282&r1=328281&r2=328282&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Thu Mar 22 17:16:03 2018
@@ -30,6 +30,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -40,21 +41,6 @@ using namespace clang;
using namespace ento;
namespace {
-// Do not reorder! The getMostNullable method relies on the order.
-// Optimization: Most pointers expected to be unspecified. When a symbol has an
-// unspecified or nonnull type non of the rules would indicate any problem for
-// that symbol. For this reason only nullable and contradicted nullability are
-// stored for a symbol. When a symbol is already contradicted, it can not be
-// casted back to nullable.
-enum class Nullability : char {
- Contradicted, // Tracked nullability is contradicted by an explicit cast. Do
- // not report any nullability related issue for this symbol.
- // This nullability is propagated aggressively to avoid false
- // positive results. See the comment on getMostNullable method.
- Nullable,
- Unspecified,
- Nonnull
-};
/// Returns the most nullable nullability. This is used for message expressions
/// like [receiver method], where the nullability of this expression is either
@@ -345,17 +331,6 @@ NullabilityChecker::NullabilityBugVisito
nullptr);
}
-static Nullability getNullabilityAnnotation(QualType Type) {
- const auto *AttrType = Type->getAs<AttributedType>();
- if (!AttrType)
- return Nullability::Unspecified;
- if (AttrType->getAttrKind() == AttributedType::attr_nullable)
- return Nullability::Nullable;
- else if (AttrType->getAttrKind() == AttributedType::attr_nonnull)
- return Nullability::Nonnull;
- return Nullability::Unspecified;
-}
-
/// Returns true when the value stored at the given location is null
/// and the passed in type is nonnnull.
static bool checkValueAtLValForInvariantViolation(ProgramStateRef State,
@@ -872,7 +847,7 @@ void NullabilityChecker::checkPostObjCMe
// are either item retrieval related or not interesting nullability wise.
// Using this fact, to keep the code easier to read just ignore the return
// value of every instance method of dictionaries.
- if (M.isInstanceMessage() && Name.find("Dictionary") != StringRef::npos) {
+ if (M.isInstanceMessage() && Name.contains("Dictionary")) {
State =
State->set<NullabilityMap>(ReturnRegion, Nullability::Contradicted);
C.addTransition(State);
@@ -880,7 +855,7 @@ void NullabilityChecker::checkPostObjCMe
}
// For similar reasons ignore some methods of Cocoa arrays.
StringRef FirstSelectorSlot = M.getSelector().getNameForSlot(0);
- if (Name.find("Array") != StringRef::npos &&
+ if (Name.contains("Array") &&
(FirstSelectorSlot == "firstObject" ||
FirstSelectorSlot == "lastObject")) {
State =
@@ -893,7 +868,7 @@ void NullabilityChecker::checkPostObjCMe
// encodings are used. Using lossless encodings is so frequent that ignoring
// this class of methods reduced the emitted diagnostics by about 30% on
// some projects (and all of that was false positives).
- if (Name.find("String") != StringRef::npos) {
+ if (Name.contains("String")) {
for (auto Param : M.parameters()) {
if (Param->getName() == "encoding") {
State = State->set<NullabilityMap>(ReturnRegion,
Added: cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp?rev=328282&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp Thu Mar 22 17:16:03 2018
@@ -0,0 +1,52 @@
+//== TrustNonnullChecker.cpp - Checker for trusting annotations -*- 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 methods annotated with _Nonnull
+// which come from system headers actually return a non-null pointer.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.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/CheckerHelpers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class TrustNonnullChecker : public Checker<check::PostCall> {
+public:
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+ // Only trust annotations for system headers for non-protocols.
+ if (!Call.isInSystemHeader())
+ return;
+
+ QualType RetType = Call.getResultType();
+ if (!RetType->isAnyPointerType())
+ return;
+
+ ProgramStateRef State = C.getState();
+ if (getNullabilityAnnotation(RetType) == Nullability::Nonnull)
+ if (auto L = Call.getReturnValue().getAs<Loc>())
+ State = State->assume(*L, /*Assumption=*/true);
+
+ C.addTransition(State);
+ }
+};
+
+} // end empty namespace
+
+
+void ento::registerTrustNonnullChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<TrustNonnullChecker>();
+}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp?rev=328282&r1=328281&r2=328282&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp Thu Mar 22 17:16:03 2018
@@ -15,8 +15,12 @@
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+namespace clang {
+
+namespace ento {
+
// Recursively find any substatements containing macros
-bool clang::ento::containsMacro(const Stmt *S) {
+bool containsMacro(const Stmt *S) {
if (S->getLocStart().isMacroID())
return true;
@@ -31,7 +35,7 @@ bool clang::ento::containsMacro(const St
}
// Recursively find any substatements containing enum constants
-bool clang::ento::containsEnum(const Stmt *S) {
+bool containsEnum(const Stmt *S) {
const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(S);
if (DR && isa<EnumConstantDecl>(DR->getDecl()))
@@ -45,7 +49,7 @@ bool clang::ento::containsEnum(const Stm
}
// Recursively find any substatements containing static vars
-bool clang::ento::containsStaticLocal(const Stmt *S) {
+bool containsStaticLocal(const Stmt *S) {
const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(S);
if (DR)
@@ -61,7 +65,7 @@ bool clang::ento::containsStaticLocal(co
}
// Recursively find any substatements containing __builtin_offsetof
-bool clang::ento::containsBuiltinOffsetOf(const Stmt *S) {
+bool containsBuiltinOffsetOf(const Stmt *S) {
if (isa<OffsetOfExpr>(S))
return true;
@@ -74,7 +78,7 @@ bool clang::ento::containsBuiltinOffsetO
// Extract lhs and rhs from assignment statement
std::pair<const clang::VarDecl *, const clang::Expr *>
-clang::ento::parseAssignment(const Stmt *S) {
+parseAssignment(const Stmt *S) {
const VarDecl *VD = nullptr;
const Expr *RHS = nullptr;
@@ -94,3 +98,18 @@ clang::ento::parseAssignment(const Stmt
return std::make_pair(VD, RHS);
}
+
+Nullability getNullabilityAnnotation(QualType Type) {
+ const auto *AttrType = Type->getAs<AttributedType>();
+ if (!AttrType)
+ return Nullability::Unspecified;
+ if (AttrType->getAttrKind() == AttributedType::attr_nullable)
+ return Nullability::Nullable;
+ else if (AttrType->getAttrKind() == AttributedType::attr_nonnull)
+ return Nullability::Nonnull;
+ return Nullability::Unspecified;
+}
+
+
+} // end namespace ento
+} // end namespace clang
Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h?rev=328282&r1=328281&r2=328282&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h Thu Mar 22 17:16:03 2018
@@ -32,6 +32,8 @@ NSObject<NSObject>
@interface NSString : NSObject<NSCopying>
- (BOOL)isEqualToString : (NSString *)aString;
- (NSString *)stringByAppendingString:(NSString *)aString;
++ (_Nonnull NSString *) generateString;
++ (_Nullable NSString *) generatePossiblyNullString;
@end
void NSSystemFunctionTakingNonnull(NSString *s);
@@ -40,4 +42,11 @@ void NSSystemFunctionTakingNonnull(NSStr
- (void) takesNonnull:(NSString *)s;
@end
+NSString* _Nullable getPossiblyNullString();
+NSString* _Nonnull getString();
+
+ at protocol MyProtocol
+- (_Nonnull NSString *) getString;
+ at end
+
NS_ASSUME_NONNULL_END
Added: cfe/trunk/test/Analysis/trustnonnullchecker_test.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/trustnonnullchecker_test.m?rev=328282&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/trustnonnullchecker_test.m (added)
+++ cfe/trunk/test/Analysis/trustnonnullchecker_test.m Thu Mar 22 17:16:03 2018
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-checker=core,nullability,apiModeling -verify %s
+
+#include "Inputs/system-header-simulator-for-nullability.h"
+
+NSString* getUnknownString();
+
+NSString* _Nonnull trust_nonnull_framework_annotation() {
+ NSString* out = [NSString generateString];
+ if (out) {}
+ return out; // no-warning
+}
+
+NSString* _Nonnull distrust_without_annotation() {
+ NSString* out = [NSString generatePossiblyNullString];
+ if (out) {}
+ return out; // expected-warning{{}}
+}
+
+NSString* _Nonnull nonnull_please_trust_me();
+
+NSString* _Nonnull distrust_local_nonnull_annotation() {
+ NSString* out = nonnull_please_trust_me();
+ if (out) {}
+ return out; // expected-warning{{}}
+}
+
+NSString* _Nonnull trust_c_function() {
+ NSString* out = getString();
+ if (out) {};
+ return out; // no-warning
+}
+
+NSString* _Nonnull distrust_unannoted_function() {
+ NSString* out = getPossiblyNullString();
+ if (out) {};
+ return out; // expected-warning{{}}
+}
+
+NSString * _Nonnull distrustProtocol(id<MyProtocol> o) {
+ NSString* out = [o getString];
+ if (out) {};
+ return out; // expected-warning{{}}
+}
More information about the cfe-commits
mailing list