[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

llvm-svn: 359264
This commit is contained in:
Artem Dergachev 2019-04-26 02:05:18 +00:00
parent e264ac6ae1
commit 48e7a2fa8c
2 changed files with 43 additions and 13 deletions

View File

@ -228,29 +228,36 @@ RetainSummaryManager::isKnownSmartPointer(QualType QT) {
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();
// 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);
}
// 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 ((!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);

View File

@ -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
}