diff options
4 files changed, 92 insertions, 26 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h b/clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h index cb16a5d0798..486e2aca84a 100644 --- a/clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h +++ b/clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h @@ -636,9 +636,19 @@ public: 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<BehaviorSummary> canEval(const CallExpr *CE, const FunctionDecl *FD, + bool &hasTrustedImplementationAnnotation); bool isTrustedReferenceCountImplementation(const FunctionDecl *FD); diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index 9826e1ce62c..7db1465fa1b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -774,14 +774,17 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { const LocationContext *LCtx = C.getLocationContext(); + using BehaviorSummary = RetainSummaryManager::BehaviorSummary; + Optional<BehaviorSummary> 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 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { 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<DefinedOrUnknownSVal>()) + state = state->assume(*L, /*Assumption=*/true); + + } } C.addTransition(state); diff --git a/clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp b/clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp index 09b744243b0..e9333266ce8 100644 --- a/clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp @@ -65,6 +65,10 @@ static bool isOSObjectSubclass(const Decl *D) { 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 @@ RetainSummaryManager::generateSummary(const FunctionDecl *FD, 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 @@ bool RetainSummaryManager::isTrustedReferenceCountImplementation( return hasRCAnnotation(FD, "rc_ownership_trusted_implementation"); } -bool RetainSummaryManager::canEval(const CallExpr *CE, - const FunctionDecl *FD, - bool &hasTrustedImplementationAnnotation) { +Optional<RetainSummaryManager::BehaviorSummary> +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 @@ bool RetainSummaryManager::canEval(const CallExpr *CE, 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<CXXMethodDecl>(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 * diff --git a/clang/test/Analysis/osobject-retain-release.cpp b/clang/test/Analysis/osobject-retain-release.cpp index 23e92ecaf6b..4e26c03bc26 100644 --- a/clang/test/Analysis/osobject-retain-release.cpp +++ b/clang/test/Analysis/osobject-retain-release.cpp @@ -17,6 +17,8 @@ struct OSObject { virtual void release() {}; virtual ~OSObject(){} + unsigned int foo() { return 42; } + static OSObject *generateObject(int); static const OSMetaClass * const metaClass; @@ -78,8 +80,31 @@ void check_dynamic_cast() { 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(); |
