From e264ac6ae19a5dfce26582c7240df402cba269ad Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Fri, 26 Apr 2019 02:05:15 +0000 Subject: [PATCH] [analyzer] RetainCount: Allow offsets in return values. Because RetainCountChecker has custom "local" reasoning about escapes, it has a separate facility to deal with tracked symbols at end of analysis and check them for leaks regardless of whether they're dead or not. This facility iterates over the list of tracked symbols and reports them as leaks, but it needs to treat the return value specially. Some custom allocators tend to return the value with an offset, storing extra metadata at the beginning of the buffer. In this case the return value would be a non-base region. In order to avoid false positives, we still need to find the original symbol within the return value, otherwise it'll be unable to match it to the item in the list of tracked symbols. Differential Revision: https://reviews.llvm.org/D60991 llvm-svn: 359263 --- .../RetainCountChecker/RetainCountChecker.cpp | 6 ++-- clang/test/Analysis/retain-release.mm | 32 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index cdda327..1ccf382 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -970,8 +970,10 @@ ExplodedNode * RetainCountChecker::processReturn(const ReturnStmt *S, return Pred; ProgramStateRef state = C.getState(); - SymbolRef Sym = - state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol(); + // We need to dig down to the symbolic base here because various + // custom allocators do sometimes return the symbol with an offset. + SymbolRef Sym = state->getSValAsScalarOrLoc(RetE, C.getLocationContext()) + .getAsLocSymbol(/*IncludeBaseRegions=*/true); if (!Sym) return Pred; diff --git a/clang/test/Analysis/retain-release.mm b/clang/test/Analysis/retain-release.mm index ba864f8..1c0c199 100644 --- a/clang/test/Analysis/retain-release.mm +++ b/clang/test/Analysis/retain-release.mm @@ -515,3 +515,35 @@ void foo() { } } + +namespace reinterpret_casts { + +void *foo() { + void *p = const_cast( + reinterpret_cast(CFArrayCreate(0, 0, 0, 0))); + void *q = reinterpret_cast( + reinterpret_cast(p) + 1); + // FIXME: Should warn about a leak here. The function should return at +0, + // but it returns at +1 instead. + return q; +} + +void *fooCreate() { + void *p = const_cast( + reinterpret_cast(CFArrayCreate(0, 0, 0, 0))); + void *q = reinterpret_cast( + reinterpret_cast(p) + 1); + // The function follows the Create Rule. + return q; // no-warning +} + +void *fooBar() CF_RETURNS_RETAINED { + void *p = const_cast( + reinterpret_cast(CFArrayCreate(0, 0, 0, 0))); + void *q = reinterpret_cast( + reinterpret_cast(p) + 1); + // The function follows the Create Rule. + return q; // no-warning +} + +} -- 2.7.4