This checker crashes on almost each file in our codebase. No test case yet, but here's a stack trace: clang::Type::getTypeClass clang::ReferenceType::classof llvm::isa_impl::doit llvm::isa_impl_cl::doit llvm::isa_impl_wrap::doit llvm::isa_impl_wrap::doit llvm::isa clang::Type::getAs clang::ASTContext::getLValueReferenceType ::TrustNonnullChecker::checkPostCall clang::ento::check::PostCall::_checkCall clang::ento::CheckerFn::operator() ::CheckCallContext::runChecker expandGraphWithCheckers clang::ento::CheckerManager::runCheckersForCallEvent clang::ento::CheckerManager::runCheckersForPostCall clang::ento::ExprEngine::VisitCXXDestructor clang::ento::ExprEngine::ProcessTemporaryDtor clang::ento::ExprEngine::ProcessImplicitDtor clang::ento::ExprEngine::processCFGElement clang::ento::CoreEngine::dispatchWorkItem clang::ento::CoreEngine::ExecuteWorkList ::AnalysisConsumer::ActionExprEngine ::AnalysisConsumer::HandleCode ::AnalysisConsumer::HandleTranslationUnit clang::MultiplexConsumer::HandleTranslationUnit clang::ParseAST clang::FrontendAction::Execute clang::CompilerInstance::ExecuteAction clang::tooling::FrontendActionFactory::runInvocation clang::tooling::ToolInvocation::runInvocation clang::tooling::ToolInvocation::run
Could you fix or revert the patch? On Fri, Mar 23, 2018 at 1:18 AM George Karpenkov via cfe-commits < cfe-commits@lists.llvm.org> wrote: > 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(); > + > +@protocol MyProtocol > +- (_Nonnull NSString *) getString; > +@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{{}} > +} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
