Author: dergachev Date: Thu Apr 25 19:05:18 2019 New Revision: 359264 URL: http://llvm.org/viewvc/llvm-project?rev=359264&view=rev Log: [analyzer] RetainCount: Add a suppression for "the Matching rule".
In the OSObject universe there appears to be another slightly popular contract, apart from "create" and "get", which is "matching". It optionally consumes a "table" parameter and if a table is passed, it fills in the table and returns it at +0; otherwise, it creates a new table, fills it in and returns it at +1. For now suppress false positives by doing a conservative escape on all functions that end with "Matching", which is the naming convention that seems to be followed by all such methods. Differential Revision: https://reviews.llvm.org/D61161 Modified: cfe/trunk/lib/Analysis/RetainSummaryManager.cpp cfe/trunk/test/Analysis/osobject-retain-release.cpp Modified: cfe/trunk/lib/Analysis/RetainSummaryManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RetainSummaryManager.cpp?rev=359264&r1=359263&r2=359264&view=diff ============================================================================== --- cfe/trunk/lib/Analysis/RetainSummaryManager.cpp (original) +++ cfe/trunk/lib/Analysis/RetainSummaryManager.cpp Thu Apr 25 19:05:18 2019 @@ -228,29 +228,36 @@ RetainSummaryManager::isKnownSmartPointe const RetainSummary * RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD, StringRef FName, QualType RetTy) { + assert(TrackOSObjects && + "Requesting a summary for an OSObject but OSObjects are not tracked"); + if (RetTy->isPointerType()) { const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl(); if (PD && isOSObjectSubclass(PD)) { - if (const IdentifierInfo *II = FD->getIdentifier()) { - StringRef FuncName = II->getName(); - if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName)) - return getDefaultSummary(); + if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName)) + return getDefaultSummary(); + + // TODO: Add support for the slightly common *Matching(table) idiom. + // Cf. IOService::nameMatching() etc. - these function have an unusual + // contract of returning at +0 or +1 depending on their last argument. + if (FName.endswith("Matching")) { + return getPersistentStopSummary(); + } - // All objects returned with functions *not* starting with - // get, or iterators, are returned at +1. - if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) || - isOSIteratorSubclass(PD)) { - return getOSSummaryCreateRule(FD); - } else { - return getOSSummaryGetRule(FD); - } + // All objects returned with functions *not* starting with 'get', + // or iterators, are returned at +1. + if ((!FName.startswith("get") && !FName.startswith("Get")) || + isOSIteratorSubclass(PD)) { + return getOSSummaryCreateRule(FD); + } else { + return getOSSummaryGetRule(FD); } } } if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) { const CXXRecordDecl *Parent = MD->getParent(); - if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) { + if (Parent && isOSObjectSubclass(Parent)) { if (FName == "release" || FName == "taggedRelease") return getOSSummaryReleaseRule(FD); 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=359264&r1=359263&r2=359264&view=diff ============================================================================== --- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original) +++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Thu Apr 25 19:05:18 2019 @@ -679,3 +679,26 @@ void test_tagged_retain_no_uaf() { obj->release(); obj->release(); } + +class IOService { +public: + OSObject *somethingMatching(OSObject *table = 0); +}; + +OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc, + OSObject *table = 0) { + // This probably just passes table through. We should probably not make + // ptr1 definitely equal to table, but we should not warn about leaks. + OSObject *ptr1 = svc->somethingMatching(table); // no-warning + + // FIXME: This, however, should follow the Create Rule regardless. + // We should warn about the leak here. + OSObject *ptr2 = svc->somethingMatching(); // no-warning + + if (!table) + table = OSTypeAlloc(OSArray); + + // This function itself ends with "Matching"! Do not warn when we're + // returning from it at +0. + return table; // no-warning +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
