Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -218,6 +218,14 @@ } // 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. //===----------------------------------------------------------------------===// Index: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h @@ -21,6 +21,8 @@ class Expr; class VarDecl; +class QualType; +class AttributedType; namespace ento { @@ -42,6 +44,25 @@ std::pair 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 Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -84,6 +84,7 @@ TaintTesterChecker.cpp TestAfterDivZeroChecker.cpp TraversalChecker.cpp + TrustNonnullChecker.cpp UndefBranchChecker.cpp UndefCapturedBlockVarChecker.cpp UndefResultChecker.cpp Index: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -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 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 @@ nullptr); } -static Nullability getNullabilityAnnotation(QualType Type) { - const auto *AttrType = Type->getAs(); - 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 @@ // 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(ReturnRegion, Nullability::Contradicted); C.addTransition(State); @@ -880,7 +855,7 @@ } // 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 @@ // 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(ReturnRegion, Index: lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp =================================================================== --- /dev/null +++ lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp @@ -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 { +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()) + State = State->assume(*L, /*Assumption=*/true); + + C.addTransition(State); + } +}; + +} // end empty namespace + + +void ento::registerTrustNonnullChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp =================================================================== --- lib/StaticAnalyzer/Core/CheckerHelpers.cpp +++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp @@ -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 @@ } // Recursively find any substatements containing enum constants -bool clang::ento::containsEnum(const Stmt *S) { +bool containsEnum(const Stmt *S) { const DeclRefExpr *DR = dyn_cast(S); if (DR && isa(DR->getDecl())) @@ -45,7 +49,7 @@ } // Recursively find any substatements containing static vars -bool clang::ento::containsStaticLocal(const Stmt *S) { +bool containsStaticLocal(const Stmt *S) { const DeclRefExpr *DR = dyn_cast(S); if (DR) @@ -61,7 +65,7 @@ } // Recursively find any substatements containing __builtin_offsetof -bool clang::ento::containsBuiltinOffsetOf(const Stmt *S) { +bool containsBuiltinOffsetOf(const Stmt *S) { if (isa(S)) return true; @@ -74,7 +78,7 @@ // Extract lhs and rhs from assignment statement std::pair -clang::ento::parseAssignment(const Stmt *S) { +parseAssignment(const Stmt *S) { const VarDecl *VD = nullptr; const Expr *RHS = nullptr; @@ -94,3 +98,18 @@ return std::make_pair(VD, RHS); } + +Nullability getNullabilityAnnotation(QualType Type) { + const auto *AttrType = Type->getAs(); + 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 Index: test/Analysis/Inputs/system-header-simulator-for-nullability.h =================================================================== --- test/Analysis/Inputs/system-header-simulator-for-nullability.h +++ test/Analysis/Inputs/system-header-simulator-for-nullability.h @@ -32,6 +32,8 @@ @interface NSString : NSObject - (BOOL)isEqualToString : (NSString *)aString; - (NSString *)stringByAppendingString:(NSString *)aString; ++ (_Nonnull NSString *) generateString; ++ (_Nullable NSString *) generatePossiblyNullString; @end void NSSystemFunctionTakingNonnull(NSString *s); @@ -40,4 +42,11 @@ - (void) takesNonnull:(NSString *)s; @end +NSString* _Nullable getPossiblyNullString(); +NSString* _Nonnull getString(); + +@protocol MyProtocol +- (_Nonnull NSString *) getString; +@end + NS_ASSUME_NONNULL_END Index: test/Analysis/trustnonnullchecker_test.m =================================================================== --- /dev/null +++ test/Analysis/trustnonnullchecker_test.m @@ -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 o) { + NSString* out = [o getString]; + if (out) {}; + return out; // expected-warning{{}} +}