Index: clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp @@ -1,4 +1,4 @@ -//== TrustNonnullChecker.cpp - Checker for trusting annotations -*- C++ -*--==// +//== TrustNonnullChecker.cpp -------- API nullability modelling -*- C++ -*--==// // // The LLVM Compiler Infrastructure // @@ -7,12 +7,20 @@ // //===----------------------------------------------------------------------===// // -// This checker adds an assumption that methods annotated with _Nonnull +// This checker adds nullability-related assumptions: +// +// 1. Methods annotated with _Nonnull // which come from system headers actually return a non-null pointer. // +// 2. NSDictionary key is non-null after the keyword subscript operation +// on read if and only if the resulting expression is non-null. +// +// 3. NSMutableDictionary index is non-null after a write operation. +// //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "SelectorExtras.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" @@ -22,9 +30,103 @@ using namespace clang; using namespace ento; +/// Records implications between symbols. +/// The semantics is: +/// (key != 0) => (value != 0) +/// These implications are then read during the evaluation of the assumption, +/// and the appropriate antecedents are applied. +REGISTER_MAP_WITH_PROGRAMSTATE(ImplicationMap, SymbolRef, SymbolRef) + namespace { -class TrustNonnullChecker : public Checker { +class TrustNonnullChecker : public Checker { + Selector ObjectForKeyedSubscriptSel; + Selector ObjectForKeySel; + Selector SetObjectForKeyedSubscriptSel; + Selector SetObjectForKeySel; + +public: + TrustNonnullChecker(ASTContext &Ctx) + : ObjectForKeyedSubscriptSel( + getKeywordSelector(Ctx, "objectForKeyedSubscript")), + ObjectForKeySel(getKeywordSelector(Ctx, "objectForKey")), + SetObjectForKeyedSubscriptSel( + getKeywordSelector(Ctx, "setObject", "forKeyedSubscript")), + SetObjectForKeySel(getKeywordSelector(Ctx, "setObject", "forKey")) {} + + ProgramStateRef evalAssume(ProgramStateRef State, + SVal Cond, + bool Assumption) const { + if (const SymbolRef Antecedent = Cond.getAsSymbol()) + State = addConsequentForCheck(State, Antecedent, Assumption); + + return State; + } + + void checkPostCall(const CallEvent &Call, CheckerContext &C) const { + // Only trust annotations for system headers for non-protocols. + if (!Call.isInSystemHeader()) + return; + + ProgramStateRef State = C.getState(); + + if (isNonNullPtr(Call, C)) + if (auto L = Call.getReturnValue().getAs()) + State = State->assume(*L, /*Assumption=*/true); + + C.addTransition(State); + } + + void checkPostObjCMessage(const ObjCMethodCall &Msg, + CheckerContext &C) const { + const ObjCInterfaceDecl *ID = Msg.getReceiverInterface(); + if (!ID) + return; + + ProgramStateRef State = C.getState(); + + // Index to setter for NSMutableDictionary is assumed to be non-null, + // as an exception is thrown otherwise. + if (interfaceHasSuperclass(ID, "NSMutableDictionary") && + (Msg.getSelector() == SetObjectForKeyedSubscriptSel || + Msg.getSelector() == SetObjectForKeySel)) { + if (auto L = Msg.getArgSVal(1).getAs()) + State = State->assume(*L, /*Assumption=*/true); + } + + // Record an implication: index is non-null if the output is non-null. + if (interfaceHasSuperclass(ID, "NSDictionary") && + (Msg.getSelector() == ObjectForKeyedSubscriptSel || + Msg.getSelector() == ObjectForKeySel)) { + SymbolRef ArgS = Msg.getArgSVal(0).getAsSymbol(); + SymbolRef RetS = Msg.getReturnValue().getAsSymbol(); + + // Emulate an implication: the argument is non-null if + // the return value is non-null. + if (ArgS && RetS) + State = State->set(RetS, ArgS); + } + + C.addTransition(State); + } + + void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + // Remove the implication if either symbol is dead. + for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), + E = SymReaper.dead_end(); I != E; ++I) { + if (const SymbolRef *SImpl = State->get(*I)) + State = State->remove(*SImpl); + + State = State->remove(*I); + } + C.addTransition(State); + } + private: /// \returns Whether we trust the result of the method call to be /// a non-null pointer. @@ -66,19 +168,61 @@ return false; } -public: - void checkPostCall(const CallEvent &Call, CheckerContext &C) const { - // Only trust annotations for system headers for non-protocols. - if (!Call.isInSystemHeader()) - return; + /// \return Whether \p ID has a superclass by the name \p ClassName. + bool interfaceHasSuperclass(const ObjCInterfaceDecl *ID, + StringRef ClassName) const { + if (ID->getIdentifier()->getName() == ClassName) + return true; - ProgramStateRef State = C.getState(); + if (const ObjCInterfaceDecl *Super = ID->getSuperClass()) + return interfaceHasSuperclass(Super, ClassName); - if (isNonNullPtr(Call, C)) - if (auto L = Call.getReturnValue().getAs()) - State = State->assume(*L, /*Assumption=*/true); + return false; + } - C.addTransition(State); + + /// Add assumptions resulting from stored implications. + /// Implication semantics: + /// (antecedent != 0) => (consequent != 0) + ProgramStateRef addConsequentForCheck(ProgramStateRef State, + SymbolRef Antecedent, + bool Assumption) const { + SValBuilder &SVB = State->getStateManager().getSValBuilder(); + + // Parse comparison to zero. + if (const SymIntExpr *SIE = dyn_cast(Antecedent)) + return matchNonNullComparision(State, + SIE->getOpcode(), + SIE->getLHS(), SIE->getRHS(), Assumption); + + if (const IntSymExpr *ISE = dyn_cast(Antecedent)) + return matchNonNullComparision(State, + ISE->getOpcode(), + ISE->getRHS(), ISE->getLHS(), Assumption); + + if (const SymbolRef *Consequent = State->get(Antecedent)) { + + // Note that the "assume" call below is recursive and calls this method + // again. We are removing the implication from map + // first to avoid infinite recursion. + State = State->remove(Antecedent); + SVal ConsequentS = SVB.makeSymbolVal(*Consequent); + State = State->assume(ConsequentS.castAs(), Assumption); + } + return State; + } + + /// A helper matcher to see whether the comparison states that the symbol + /// is non-null. + ProgramStateRef matchNonNullComparision(ProgramStateRef State, + BinaryOperator::Opcode Op, + SymbolRef LHS, + const llvm::APSInt &RHS, + bool Assumption) const { + if (((Op == BO_NE && Assumption) || (Op == BO_EQ && !Assumption)) && + RHS == 0) + return addConsequentForCheck(State, LHS, /*Assumption=*/true); + return State; } }; @@ -86,5 +230,5 @@ void ento::registerTrustNonnullChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); + Mgr.registerChecker(Mgr.getASTContext()); } Index: clang/test/Analysis/Inputs/system-header-simulator-for-nullability.h =================================================================== --- clang/test/Analysis/Inputs/system-header-simulator-for-nullability.h +++ clang/test/Analysis/Inputs/system-header-simulator-for-nullability.h @@ -9,6 +9,8 @@ NS_ASSUME_NONNULL_BEGIN typedef struct _NSZone NSZone; +typedef unsigned long NSUInteger; +@class NSCoder, NSEnumerator; @protocol NSObject + (instancetype)alloc; @@ -24,6 +26,22 @@ - (id)mutableCopyWithZone:(nullable NSZone *)zone; @end +@protocol NSCoding +- (void)encodeWithCoder:(NSCoder *)aCoder; +@end + +@protocol NSSecureCoding +@required ++ (BOOL)supportsSecureCoding; +@end + +typedef struct { + unsigned long state; + id *itemsPtr; + unsigned long *mutationsPtr; + unsigned long extra[5]; +} NSFastEnumerationState; + __attribute__((objc_root_class)) @interface NSObject @@ -52,3 +70,36 @@ @end NS_ASSUME_NONNULL_END + +@interface NSDictionary : NSObject + +- (NSUInteger)count; +- (id)objectForKey:(id)aKey; +- (NSEnumerator *)keyEnumerator; +- (id)objectForKeyedSubscript:(id)aKey; + +@end + +@interface NSDictionary (NSDictionaryCreation) + ++ (id)dictionary; ++ (id)dictionaryWithObject:(id)object forKey:(id )key; ++ (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(NSUInteger)cnt; + +@end + +@interface NSMutableDictionary : NSDictionary + +- (void)removeObjectForKey:(id)aKey; +- (void)setObject:(id)anObject forKey:(id )aKey; + +@end + +@interface NSMutableDictionary (NSExtendedMutableDictionary) + +- (void)addEntriesFromDictionary:(NSDictionary *)otherDictionary; +- (void)removeAllObjects; +- (void)setDictionary:(NSDictionary *)otherDictionary; +- (void)setObject:(id)obj forKeyedSubscript:(id )key __attribute__((availability(macosx,introduced=10.8))); + +@end Index: clang/test/Analysis/analyzer-config.c =================================================================== --- clang/test/Analysis/analyzer-config.c +++ clang/test/Analysis/analyzer-config.c @@ -25,6 +25,7 @@ // CHECK-NEXT: inline-lambdas = true // CHECK-NEXT: ipa = dynamic-bifurcate // CHECK-NEXT: ipa-always-inline-size = 3 +// CHECK-NEXT: leak-diagnostics-reference-allocation = false // CHECK-NEXT: max-inlinable-size = 100 // CHECK-NEXT: max-nodes = 225000 // CHECK-NEXT: max-times-inline-large = 32 @@ -35,4 +36,4 @@ // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: widen-loops = false // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 23 +// CHECK-NEXT: num-entries = 24 Index: clang/test/Analysis/analyzer-config.cpp =================================================================== --- clang/test/Analysis/analyzer-config.cpp +++ clang/test/Analysis/analyzer-config.cpp @@ -40,6 +40,7 @@ // CHECK-NEXT: inline-lambdas = true // CHECK-NEXT: ipa = dynamic-bifurcate // CHECK-NEXT: ipa-always-inline-size = 3 +// CHECK-NEXT: leak-diagnostics-reference-allocation = false // CHECK-NEXT: max-inlinable-size = 100 // CHECK-NEXT: max-nodes = 225000 // CHECK-NEXT: max-times-inline-large = 32 @@ -50,4 +51,4 @@ // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: widen-loops = false // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 30 +// CHECK-NEXT: num-entries = 31 Index: clang/test/Analysis/trustnonnullchecker_test.m =================================================================== --- clang/test/Analysis/trustnonnullchecker_test.m +++ clang/test/Analysis/trustnonnullchecker_test.m @@ -67,3 +67,92 @@ return out; // expected-warning{{}} } +// If the return value is non-nil, the index is non-nil. +NSString *_Nonnull retImpliesIndex(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (obj) + return s; // no-warning + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexOtherMethod(NSString *s, + NSDictionary *dic) { + id obj = [dic objectForKey:s]; + if (s) {} + if (obj) + return s; // no-warning + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexOnRHS(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (nil != obj) + return s; // no-warning + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexReverseCheck(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (!obj) + return @"foo"; + return s; // no-warning +} + +NSString *_Nonnull retImpliesIndexReverseCheckOnRHS(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (nil == obj) + return @"foo"; + return s; // no-warning +} + +NSString *_Nonnull retImpliesIndexWrongBranch(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (!obj) + return s; // expected-warning{{}} + return @"foo"; +} + +NSString *_Nonnull retImpliesIndexWrongBranchOnRHS(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (s) {} + if (nil == obj) + return s; // expected-warning{{}} + return @"foo"; +} + +// The return value could still be nil for a non-nil index. +NSDictionary *_Nonnull indexDoesNotImplyRet(NSString *s, + NSDictionary *dic) { + id obj = dic[s]; + if (obj) {} + if (s) + return obj; // expected-warning{{}} + return [[NSDictionary alloc] init]; +} + +NSString *_Nonnull checkAssumeOnMutableDictionary(NSMutableDictionary *d, + NSString *k, + NSString *val) { + d[k] = val; + if (k) {} + return k; // no-warning +} + +NSString *_Nonnull checkAssumeOnMutableDictionaryOtherMethod(NSMutableDictionary *d, + NSString *k, + NSString *val) { + [d setObject:val forKey:k]; + if (k) {} + return k; // no-warning +}