[PATCH] D42015: [analyzer] NFC: RetainCount: Don't dump() regions to the user.

[Artem Dergachev via Phabricator via cfe-commits]

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() << ")";


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D42015.129717.patch
Type: text/x-patch
Size: 1363 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180112/3eab3ee6/attachment.bin>