r327727 - [analyzer] Fix crashes in RetainCountChecker when underlying region is not a var

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Mar 16 11:16:47 2018
New Revision: 327727

URL: http://llvm.org/viewvc/llvm-project?rev=327727&view=rev
Log:
[analyzer] Fix crashes in RetainCountChecker when underlying region is not a var

For other regions, the error message contains a good indication of the
problem, and there, in general, nothing helpful we can print.
Error pointer to the problematic expression seems enough.

rdar://37323555

Differential Revision: https://reviews.llvm.org/D44409

Added:
    cfe/trunk/test/Analysis/retaincountchecker-compoundregion.m
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=327727&r1=327726&r2=327727&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Mar 16 11:16:47 2018
@@ -1929,10 +1929,12 @@ static bool isNumericLiteralExpression(c
          isa<CXXBoolLiteralExpr>(E);
 }
 
-static std::string describeRegion(const MemRegion *MR) {
+static Optional<std::string> describeRegion(const MemRegion *MR) {
+  if (const auto *VR = dyn_cast_or_null<VarRegion>(MR))
+    return std::string(cast<VarRegion>(MR)->getDecl()->getName());
   // Once we support more storage locations for bindings,
   // this would need to be improved.
-  return cast<VarRegion>(MR)->getDecl()->getName();
+  return None;
 }
 
 /// Returns true if this stack frame is for an Objective-C method that is a
@@ -2399,9 +2401,9 @@ CFRefLeakReportVisitor::getEndPath(BugRe
 
   os << "Object leaked: ";
 
-  if (FirstBinding) {
-    os << "object allocated and stored into '"
-       << describeRegion(FirstBinding) << '\'';
+  Optional<std::string> RegionDescription = describeRegion(FirstBinding);
+  if (RegionDescription) {
+    os << "object allocated and stored into '" << *RegionDescription << '\'';
   }
   else
     os << "allocated object";
@@ -2519,7 +2521,8 @@ void CFRefLeakReport::deriveAllocLocatio
   UniqueingDecl = AllocNode->getLocationContext()->getDecl();
 }
 
-void CFRefLeakReport::createDescription(CheckerContext &Ctx, bool GCEnabled, bool IncludeAllocationLine) {
+void CFRefLeakReport::createDescription(CheckerContext &Ctx, bool GCEnabled,
+                                        bool IncludeAllocationLine) {
   assert(Location.isValid() && UniqueingDecl && UniqueingLocation.isValid());
   Description.clear();
   llvm::raw_string_ostream os(Description);
@@ -2528,8 +2531,9 @@ void CFRefLeakReport::createDescription(
     os << "(when using garbage collection) ";
   os << "of an object";
 
-  if (AllocBinding) {
-    os << " stored into '" << describeRegion(AllocBinding) << '\'';
+  Optional<std::string> RegionDescription = describeRegion(AllocBinding);
+  if (RegionDescription) {
+    os << " stored into '" << *RegionDescription << '\'';
     if (IncludeAllocationLine) {
       FullSourceLoc SL(AllocStmt->getLocStart(), Ctx.getSourceManager());
       os << " (allocated on line " << SL.getSpellingLineNumber() << ")";

Added: cfe/trunk/test/Analysis/retaincountchecker-compoundregion.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retaincountchecker-compoundregion.m?rev=327727&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/retaincountchecker-compoundregion.m (added)
+++ cfe/trunk/test/Analysis/retaincountchecker-compoundregion.m Fri Mar 16 11:16:47 2018
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -fblocks -verify -Wno-objc-root-class %s
+typedef const void *CFTypeRef;
+enum { kCFNumberSInt8Type = 1, kCFNumberSInt16Type = 2,
+       kCFNumberSInt32Type = 3, kCFNumberSInt64Type = 4,
+       kCFNumberFloat32Type = 5, kCFNumberFloat64Type = 6,
+       kCFNumberCharType = 7, kCFNumberShortType = 8,
+       kCFNumberIntType = 9, kCFNumberLongType = 10,
+       kCFNumberLongLongType = 11, kCFNumberFloatType = 12,
+       kCFNumberDoubleType = 13, kCFNumberCFIndexType = 14,
+       kCFNumberNSIntegerType = 15, kCFNumberCGFloatType = 16,
+       kCFNumberMaxType = 16 };
+typedef const struct __CFAllocator * CFAllocatorRef;
+typedef signed long CFIndex;
+typedef CFIndex CFNumberType;
+typedef const struct __CFNumber * CFNumberRef;
+extern CFNumberRef CFNumberCreate(CFAllocatorRef allocator, CFNumberType theType, const void *valuePtr);
+
+void foo(CFAllocatorRef allocator) {
+  int width = 0;
+  int height = 0;
+  CFTypeRef* values = (CFTypeRef[]){
+    CFNumberCreate(allocator, kCFNumberSInt32Type, &width), //expected-warning-re{{Potential leak of an object{{$}}}}
+    CFNumberCreate(allocator, kCFNumberSInt32Type, &height), //expected-warning-re{{Potential leak of an object{{$}}}}
+  };
+}