NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
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.
I don't really understand how this whole facility works in general. In
particular, i don't understand why doesn't it take the function's contract into
account (i.e., should this function generally return at +0 or at +1?), but the
fix still seems to make sense to me.
Repository:
rC Clang
https://reviews.llvm.org/D60991
Files:
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
clang/test/Analysis/retain-release.mm
Index: clang/test/Analysis/retain-release.mm
===================================================================
--- clang/test/Analysis/retain-release.mm
+++ clang/test/Analysis/retain-release.mm
@@ -515,3 +515,35 @@
}
}
+
+namespace reinterpret_casts {
+
+void *foo() {
+ void *p = const_cast<void *>(
+ reinterpret_cast<const void *>(CFArrayCreate(0, 0, 0, 0)));
+ void *q = reinterpret_cast<void *>(
+ reinterpret_cast<char *>(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<void *>(
+ reinterpret_cast<const void *>(CFArrayCreate(0, 0, 0, 0)));
+ void *q = reinterpret_cast<void *>(
+ reinterpret_cast<char *>(p) + 1);
+ // The function follows the Create Rule.
+ return q; // no-warning
+}
+
+void *fooBar() CF_RETURNS_RETAINED {
+ void *p = const_cast<void *>(
+ reinterpret_cast<const void *>(CFArrayCreate(0, 0, 0, 0)));
+ void *q = reinterpret_cast<void *>(
+ reinterpret_cast<char *>(p) + 1);
+ // The function follows the Create Rule.
+ return q; // no-warning
+}
+
+}
Index:
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -970,8 +970,10 @@
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;
Index: clang/test/Analysis/retain-release.mm
===================================================================
--- clang/test/Analysis/retain-release.mm
+++ clang/test/Analysis/retain-release.mm
@@ -515,3 +515,35 @@
}
}
+
+namespace reinterpret_casts {
+
+void *foo() {
+ void *p = const_cast<void *>(
+ reinterpret_cast<const void *>(CFArrayCreate(0, 0, 0, 0)));
+ void *q = reinterpret_cast<void *>(
+ reinterpret_cast<char *>(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<void *>(
+ reinterpret_cast<const void *>(CFArrayCreate(0, 0, 0, 0)));
+ void *q = reinterpret_cast<void *>(
+ reinterpret_cast<char *>(p) + 1);
+ // The function follows the Create Rule.
+ return q; // no-warning
+}
+
+void *fooBar() CF_RETURNS_RETAINED {
+ void *p = const_cast<void *>(
+ reinterpret_cast<const void *>(CFArrayCreate(0, 0, 0, 0)));
+ void *q = reinterpret_cast<void *>(
+ reinterpret_cast<char *>(p) + 1);
+ // The function follows the Create Rule.
+ return q; // no-warning
+}
+
+}
Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -970,8 +970,10 @@
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
