Index: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h @@ -636,9 +636,19 @@ InitializeMethodSummaries(); } - bool canEval(const CallExpr *CE, - const FunctionDecl *FD, - bool &hasTrustedImplementationAnnotation); + enum class BehaviorSummary { + // Function does not return. + NoOp, + + // Function returns the first argument. + Identity, + + // Function either returns zero, or the input parameter. + IdentityOrZero + }; + + Optional canEval(const CallExpr *CE, const FunctionDecl *FD, + bool &hasTrustedImplementationAnnotation); bool isTrustedReferenceCountImplementation(const FunctionDecl *FD); Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -774,14 +774,17 @@ const LocationContext *LCtx = C.getLocationContext(); + using BehaviorSummary = RetainSummaryManager::BehaviorSummary; + Optional BSmr = + SmrMgr.canEval(CE, FD, hasTrustedImplementationAnnotation); + // See if it's one of the specific functions we know how to eval. - if (!SmrMgr.canEval(CE, FD, hasTrustedImplementationAnnotation)) + if (!BSmr) return false; // Bind the return value. - // For now, all the functions which we can evaluate and which take - // at least one argument are identities. - if (CE->getNumArgs() >= 1) { + if (BSmr == BehaviorSummary::Identity || + BSmr == BehaviorSummary::IdentityOrZero) { SVal RetVal = state->getSVal(CE->getArg(0), LCtx); // If the receiver is unknown or the function has @@ -793,7 +796,24 @@ RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount()); } - state = state->BindExpr(CE, LCtx, RetVal, false); + state = state->BindExpr(CE, LCtx, RetVal, /*Invalidate=*/false); + + if (BSmr == BehaviorSummary::IdentityOrZero) { + // Add a branch where the output is zero. + ProgramStateRef NullOutputState = C.getState(); + + // Assume that output is zero on the other branch. + NullOutputState = NullOutputState->BindExpr( + CE, LCtx, C.getSValBuilder().makeNull(), /*Invalidate=*/false); + + C.addTransition(NullOutputState); + + // And on the original branch assume that both input and + // output are non-zero. + if (auto L = RetVal.getAs()) + state = state->assume(*L, /*Assumption=*/true); + + } } C.addTransition(state); Index: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp @@ -65,6 +65,10 @@ return isSubclass(D, "OSObject"); } +static bool isOSObjectDynamicCast(StringRef S) { + return S == "safeMetaCast"; +} + static bool isOSIteratorSubclass(const Decl *D) { return isSubclass(D, "OSIterator"); } @@ -231,6 +235,9 @@ if (TrackOSObjects && PD && isOSObjectSubclass(PD)) { if (const IdentifierInfo *II = FD->getIdentifier()) { + if (isOSObjectDynamicCast(II->getName())) + return getDefaultSummary(); + // All objects returned with functions starting with "get" are getters. if (II->getName().startswith("get")) { @@ -515,20 +522,21 @@ return hasRCAnnotation(FD, "rc_ownership_trusted_implementation"); } -bool RetainSummaryManager::canEval(const CallExpr *CE, - const FunctionDecl *FD, - bool &hasTrustedImplementationAnnotation) { +Optional +RetainSummaryManager::canEval(const CallExpr *CE, const FunctionDecl *FD, + bool &hasTrustedImplementationAnnotation) { IdentifierInfo *II = FD->getIdentifier(); if (!II) - return false; + return None; StringRef FName = II->getName(); FName = FName.substr(FName.find_first_not_of('_')); QualType ResultTy = CE->getCallReturnType(Ctx); if (ResultTy->isObjCIdType()) { - return II->isStr("NSMakeCollectable"); + if (II->isStr("NSMakeCollectable")) + return BehaviorSummary::Identity; } else if (ResultTy->isPointerType()) { // Handle: (CF|CG|CV)Retain // CFAutorelease @@ -536,31 +544,34 @@ if (cocoa::isRefType(ResultTy, "CF", FName) || cocoa::isRefType(ResultTy, "CG", FName) || cocoa::isRefType(ResultTy, "CV", FName)) - return isRetain(FD, FName) || isAutorelease(FD, FName) || - isMakeCollectable(FName); - - // Process OSDynamicCast: should just return the first argument. - // For now, treating the cast as a no-op, and disregarding the case where - // the output becomes null due to the type mismatch. - if (TrackOSObjects && FName == "safeMetaCast") { - return true; + if (isRetain(FD, FName) || isAutorelease(FD, FName) || + isMakeCollectable(FName)) + return BehaviorSummary::Identity; + + // safeMetaCast is called by OSDynamicCast. + // We assume that OSDynamicCast is either an identity (cast is OK, + // the input was non-zero), + // or that it returns zero (when the cast failed, or the input + // was zero). + if (TrackOSObjects && isOSObjectDynamicCast(FName)) { + return BehaviorSummary::IdentityOrZero; } const FunctionDecl* FDD = FD->getDefinition(); if (FDD && isTrustedReferenceCountImplementation(FDD)) { hasTrustedImplementationAnnotation = true; - return true; + return BehaviorSummary::Identity; } } if (const auto *MD = dyn_cast(FD)) { const CXXRecordDecl *Parent = MD->getParent(); if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) - return FName == "release" || FName == "retain"; + if (FName == "release" || FName == "retain") + return BehaviorSummary::NoOp; } - return false; - + return None; } const RetainSummary * Index: cfe/trunk/test/Analysis/osobject-retain-release.cpp =================================================================== --- cfe/trunk/test/Analysis/osobject-retain-release.cpp +++ cfe/trunk/test/Analysis/osobject-retain-release.cpp @@ -17,6 +17,8 @@ virtual void release() {}; virtual ~OSObject(){} + unsigned int foo() { return 42; } + static OSObject *generateObject(int); static const OSMetaClass * const metaClass; @@ -78,8 +80,31 @@ arr->release(); } +unsigned int check_dynamic_cast_no_null_on_orig(OSObject *obj) { + OSArray *arr = OSDynamicCast(OSArray, obj); + if (arr) { + return arr->getCount(); + } else { + + // The fact that dynamic cast has failed should not imply that + // the input object was null. + return obj->foo(); // no-warning + } +} + +void check_dynamic_cast_null_branch(OSObject *obj) { + OSArray *arr1 = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject}} + OSArray *arr = OSDynamicCast(OSArray, obj); + if (!arr) // expected-note{{Taking true branch}} + return; // expected-warning{{Potential leak}} + // expected-note@-1{{Object leaked}} + arr1->release(); +} + void check_dynamic_cast_null_check() { - OSArray *arr = OSDynamicCast(OSArray, OSObject::generateObject(1)); + OSArray *arr = OSDynamicCast(OSArray, OSObject::generateObject(1)); // expected-note{{Call to function 'generateObject' returns an OSObject}} + // expected-warning@-1{{Potential leak of an object}} + // expected-note@-2{{Object leaked}} if (!arr) return; arr->release();