NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.
`RetainCountChecker` appears to be using `MemRegion::getString()` to present
the region to the user, which is equivalent to `MemRegion->dump()` and as such
may produce human-unreadable dumps.
Fortunately, for now `RetainCountChecker` only tracks pointer bindings through
local variables, and treats all other bindings as pointer escapes. For local
variables, this worked well.
Before r315736/https://reviews.llvm.org/D38877, however, it used to be possible
to modify retain count of a pointer "in place" after writing it anywhere, eg.:
anyWeirdLocation = x;
SafeCFRetain(anyWeirdLocation);
...which not only caused a leak false positive, but also triggered a dump of
`anyWeirdLocation` (which may be literally any weird location) into the
checker's warning message.
So for now i'm not seeing any other cases where this leaks, but i still want to
add an assertion to make sure this never happens again.
Repository:
rC Clang
https://reviews.llvm.org/D42015
Files:
lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1929,6 +1929,12 @@
isa<CXXBoolLiteralExpr>(E);
}
+static std::string describeRegion(const MemRegion *MR) {
+ // Once we support more storage locations for bindings,
+ // this would need to be improved.
+ return cast<VarRegion>(MR)->getDecl()->getName();
+}
+
/// Returns true if this stack frame is for an Objective-C method that is a
/// property getter or setter whose body has been synthesized by the analyzer.
static bool isSynthesizedAccessor(const StackFrameContext *SFC) {
@@ -2395,7 +2401,7 @@
if (FirstBinding) {
os << "object allocated and stored into '"
- << FirstBinding->getString() << '\'';
+ << describeRegion(FirstBinding) << '\'';
}
else
os << "allocated object";
@@ -2523,7 +2529,7 @@
os << "of an object";
if (AllocBinding) {
- os << " stored into '" << AllocBinding->getString() << '\'';
+ os << " stored into '" << describeRegion(AllocBinding) << '\'';
if (IncludeAllocationLine) {
FullSourceLoc SL(AllocStmt->getLocStart(), Ctx.getSourceManager());
os << " (allocated on line " << SL.getSpellingLineNumber() << ")";
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1929,6 +1929,12 @@
isa<CXXBoolLiteralExpr>(E);
}
+static std::string describeRegion(const MemRegion *MR) {
+ // Once we support more storage locations for bindings,
+ // this would need to be improved.
+ return cast<VarRegion>(MR)->getDecl()->getName();
+}
+
/// Returns true if this stack frame is for an Objective-C method that is a
/// property getter or setter whose body has been synthesized by the analyzer.
static bool isSynthesizedAccessor(const StackFrameContext *SFC) {
@@ -2395,7 +2401,7 @@
if (FirstBinding) {
os << "object allocated and stored into '"
- << FirstBinding->getString() << '\'';
+ << describeRegion(FirstBinding) << '\'';
}
else
os << "allocated object";
@@ -2523,7 +2529,7 @@
os << "of an object";
if (AllocBinding) {
- os << " stored into '" << AllocBinding->getString() << '\'';
+ os << " stored into '" << describeRegion(AllocBinding) << '\'';
if (IncludeAllocationLine) {
FullSourceLoc SL(AllocStmt->getLocStart(), Ctx.getSourceManager());
os << " (allocated on line " << SL.getSpellingLineNumber() << ")";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
