Author: george.karpenkov Date: Wed Oct 31 10:38:29 2018 New Revision: 345746
URL: http://llvm.org/viewvc/llvm-project?rev=345746&view=rev Log: [analyzer] RetainCountChecker: for now, do not trust the summaries of inlined code Trusting summaries of inlined code would require a more thorough work, as the current approach was causing too many false positives, as the new example in test. The culprit lies in the fact that we currently escape all variables written into a field (but not passed off to unknown functions!), which can result in inconsistent behavior. rdar://45655344 Differential Revision: https://reviews.llvm.org/D53902 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp cfe/trunk/test/Analysis/osobject-retain-release.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=345746&r1=345745&r2=345746&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp Wed Oct 31 10:38:29 2018 @@ -420,13 +420,6 @@ void RetainCountChecker::processSummaryO RetEffect RE = Summ.getRetEffect(); if (SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol()) { - if (const auto *MCall = dyn_cast<CXXMemberCall>(&CallOrMsg)) { - if (Optional<RefVal> updatedRefVal = - refValFromRetEffect(RE, MCall->getResultType())) { - state = setRefBinding(state, Sym, *updatedRefVal); - } - } - if (RE.getKind() == RetEffect::NoRetHard) state = removeRefBinding(state, Sym); } @@ -1103,9 +1096,8 @@ RetainCountChecker::checkRegionChanges(P WhitelistedSymbols.insert(SR->getSymbol()); } - for (InvalidatedSymbols::const_iterator I=invalidated->begin(), - E = invalidated->end(); I!=E; ++I) { - SymbolRef sym = *I; + for (SymbolRef sym : + llvm::make_range(invalidated->begin(), invalidated->end())) { if (WhitelistedSymbols.count(sym)) continue; // Remove any existing reference-count binding. Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=345746&r1=345745&r2=345746&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Wed Oct 31 10:38:29 2018 @@ -101,10 +101,11 @@ static bool isOSObjectRelated(const CXXM return true; for (ParmVarDecl *Param : MD->parameters()) { - QualType PT = Param->getType(); - if (CXXRecordDecl *RD = PT->getPointeeType()->getAsCXXRecordDecl()) - if (isOSObjectSubclass(RD)) - return true; + QualType PT = Param->getType()->getPointeeType(); + if (!PT.isNull()) + if (CXXRecordDecl *RD = PT->getAsCXXRecordDecl()) + if (isOSObjectSubclass(RD)) + return true; } return false; Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=345746&r1=345745&r2=345746&view=diff ============================================================================== --- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original) +++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Wed Oct 31 10:38:29 2018 @@ -2,10 +2,9 @@ struct OSMetaClass; -#define TRUSTED __attribute__((annotate("rc_ownership_trusted_implementation"))) -#define OS_CONSUME TRUSTED __attribute__((annotate("rc_ownership_consumed"))) -#define OS_RETURNS_RETAINED TRUSTED __attribute__((annotate("rc_ownership_returns_retained"))) -#define OS_RETURNS_NOT_RETAINED TRUSTED __attribute__((annotate("rc_ownership_returns_not_retained"))) +#define OS_CONSUME __attribute__((annotate("rc_ownership_consumed"))) +#define OS_RETURNS_RETAINED __attribute__((annotate("rc_ownership_returns_retained"))) +#define OS_RETURNS_NOT_RETAINED __attribute__((annotate("rc_ownership_returns_not_retained"))) #define OSTypeID(type) (type::metaClass) @@ -62,6 +61,37 @@ void check_no_invalidation_other_struct( // expected-note@-1{{Object leaked}} } +struct ArrayOwner : public OSObject { + OSArray *arr; + ArrayOwner(OSArray *arr) : arr(arr) {} + + static ArrayOwner* create(OSArray *arr) { + return new ArrayOwner(arr); + } + + OSArray *getArray() { + return arr; + } + + OSArray *createArray() { + return OSArray::withCapacity(10); + } + + OSArray *createArraySourceUnknown(); + + OSArray *getArraySourceUnknown(); +}; + +void check_confusing_getters() { + OSArray *arr = OSArray::withCapacity(10); + + ArrayOwner *AO = ArrayOwner::create(arr); + AO->getArray(); + + AO->release(); + arr->release(); +} + void check_rc_consumed() { OSArray *arr = OSArray::withCapacity(10); OSArray::consumeArray(arr); @@ -140,31 +170,17 @@ void proper_cleanup() { arr->release(); // 0 } -struct ArrayOwner { - OSArray *arr; - - OSArray *getArray() { - return arr; - } - - OSArray *createArray() { - return OSArray::withCapacity(10); - } - - OSArray *createArraySourceUnknown(); - - OSArray *getArraySourceUnknown(); -}; - unsigned int no_warning_on_getter(ArrayOwner *owner) { OSArray *arr = owner->getArray(); return arr->getCount(); } unsigned int warn_on_overrelease(ArrayOwner *owner) { - OSArray *arr = owner->getArray(); // expected-note{{function call returns an OSObject of type struct OSArray * with a +0 retain count}} - arr->release(); // expected-warning{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}} - // expected-note@-1{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}} + // FIXME: summaries are not applied in case the source of the getter/setter + // is known. + // rdar://45681203 + OSArray *arr = owner->getArray(); + arr->release(); return arr->getCount(); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
