[analyzer] Add the type of the leaked object to the diagnostic message

[george.karpenkov]

Summary

If the object is a temporary, and there is no variable it binds to, let's at least print out the object type in order to help differentiate it from other temporaries.

rdar://45175098

[NoQ]

clang/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist

3837 ↗

(On Diff #175774)

Should we drop the * here? I.e., MyObj sounds more like a type of an object than MyObj *.

[NoQ]

clang/test/Analysis/osobject-retain-release.cpp

64 ↗

(On Diff #176005)

"OSObject of type OSArray" sounds redundant. Maybe "object of type OSArray"?

Also this warning has always violated the rule of dropping all articles from the message (:

[NoQ]

These aren't critical, feel free to address later :)

lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h

CFRefLeakReportVisitor(SymbolRef sym,
      : CFRefReportVisitor(sym, log) {}
 
   std::shared_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC,
                                                   const ExplodedNode *N,
                                                   BugReport &BR) override;
 };
 
 class CFRefReport : public BugReport {
+protected:
+  SymbolRef Sym;
 
 public:
   CFRefReport(CFRefBug &D, const LangOptions &LOpts,
               const SummaryLogTy &Log, ExplodedNode *n, SymbolRef sym,
               bool registerVisitor = true)
-    : BugReport(D, D.getDescription(), n) {
+    : BugReport(D, D.getDescription(), n), Sym(sym) {
     if (registerVisitor)
       addVisitor(llvm::make_unique<CFRefReportVisitor>(sym, Log));
   }
 
   CFRefReport(CFRefBug &D, const LangOptions &LOpts,
               const SummaryLogTy &Log, ExplodedNode *n, SymbolRef sym,
               StringRef endText)
     : BugReport(D, D.getDescription(), endText, n) {

lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

static bool isNumericLiteralExpression(const Expr *E) {
   // FIXME: This set of cases was copied from SemaExprObjC.
   return isa<IntegerLiteral>(E) ||
          isa<CharacterLiteral>(E) ||
          isa<FloatingLiteral>(E) ||
          isa<ObjCBoolLiteralExpr>(E) ||
          isa<CXXBoolLiteralExpr>(E);
 }
 
+/// If type represents a pointer to CXXRecordDecl,
+/// and is not a typedef, return the decl name.
+/// Otherwise, return the serialization of type.
+static StringRef getPrettyTypeName(QualType QT) {
+  QualType PT = QT->getPointeeType();
+  if (!PT.isNull() && !QT->getAs<TypedefType>())
+    if (const auto *RD = PT->getAsCXXRecordDecl())
+      return RD->getName();
+  return QT.getAsString();
+}
+
 /// Write information about the type state change to {@code os},
 /// return whether the note should be generated.
 static bool shouldGenerateNote(llvm::raw_string_ostream &os,
                                const RefVal *PrevT, const RefVal &CurrV,
                                SmallVector<ArgEffect, 2> &AEffects) {
   // Get the previous type state.
   RefVal PrevV = *PrevT;
 
if (isa<ObjCArrayLiteral>(S)) {
         }
       }
 
       if (CurrV.getObjKind() == RetEffect::CF) {
         os << " returns a Core Foundation object of type "
            << Sym->getType().getAsString() << " with a ";
       } else if (CurrV.getObjKind() == RetEffect::OS) {
         os << " returns an OSObject of type "
-           << Sym->getType().getAsString() << " with a ";
+           << getPrettyTypeName(Sym->getType()) << " with a ";
       } else if (CurrV.getObjKind() == RetEffect::Generalized) {
         os << " returns an object of type " << Sym->getType().getAsString()
            << " with a ";
       } else {
         assert (CurrV.getObjKind() == RetEffect::ObjC);
         QualType T = Sym->getType();
         if (!isa<ObjCObjectPointerType>(T)) {
           os << " returns an Objective-C object with a ";
CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
   llvm::raw_string_ostream os(sbuf);
 
   os << "Object leaked: ";
 
   Optional<std::string> RegionDescription = describeRegion(FirstBinding);
   if (RegionDescription) {
     os << "object allocated and stored into '" << *RegionDescription << '\'';
   } else {
-    os << "allocated object";
+    os << "allocated object of type " << getPrettyTypeName(Sym->getType());
   }
 
   // Get the retain count.
   const RefVal* RV = getRefBinding(EndN->getState(), Sym);
   assert(RV);
 
   if (RV->getKind() == RefVal::ErrorLeakReturned) {
     // FIXME: Per comments in rdar://6320065, "create" only applies to CF
if (D->hasAttr<CFReturnsNotRetainedAttr>()) {
       } else {
         const FunctionDecl *FD = cast<FunctionDecl>(D);
         os << "whose name ('" << *FD
            << "') does not contain 'Copy' or 'Create'.  This violates the naming"
               " convention rules given in the Memory Management Guide for Core"
               " Foundation";
       }
     }
-  }
-  else
+  } else {
     os << " is not referenced later in this execution path and has a retain "
           "count of +" << RV->getCount();
+  }
 
   return std::make_shared<PathDiagnosticEventPiece>(L, os.str());
 }
 
 void CFRefLeakReport::deriveParamLocation(CheckerContext &Ctx, SymbolRef sym) {
   const SourceManager& SMgr = Ctx.getSourceManager();
 
   if (!sym->getOriginRegion())
void CFRefLeakReport::createDescription(CheckerContext &Ctx,
 
   Optional<std::string> RegionDescription = describeRegion(AllocBinding);
   if (RegionDescription) {
     os << " stored into '" << *RegionDescription << '\'';
     if (IncludeAllocationLine) {
       FullSourceLoc SL(AllocStmt->getBeginLoc(), Ctx.getSourceManager());
       os << " (allocated on line " << SL.getSpellingLineNumber() << ")";
     }
+  } else {
+
+    // If we can't figure out the name, just supply the type information.
+    os << " of type " << getPrettyTypeName(Sym->getType());
   }
 }
 
 CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts,
                                  const SummaryLogTy &Log,
                                  ExplodedNode *n, SymbolRef sym,
                                  CheckerContext &Ctx,
                                  bool IncludeAllocationLine)

test/Analysis/Inputs/expected-plists/objc-arc.m.plist

<array>
      <key>location</key>
      <dict>
       <key>line</key><integer>202</integer>
       <key>col</key><integer>1</integer>
       <key>file</key><integer>0</integer>
      </dict>
      <key>depth</key><integer>0</integer>
      <key>extended_message</key>
-     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+     <string>Object leaked: allocated object of type CFStringRef is not referenced later in this execution path and has a retain count of +1</string>
      <key>message</key>
-     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+     <string>Object leaked: allocated object of type CFStringRef is not referenced later in this execution path and has a retain count of +1</string>
     </dict>
    </array>
-   <key>description</key><string>Potential leak of an object</string>
+   <key>description</key><string>Potential leak of an object of type CFStringRef</string>
    <key>category</key><string>Memory (Core Foundation/Objective-C)</string>
    <key>type</key><string>Leak</string>
    <key>check_name</key><string>osx.cocoa.RetainCount</string>
    <!-- This hash is experimental and going to change! -->
    <key>issue_hash_content_of_line_in_context</key><string>e1fbcc142b678b3c2c43737ee35b64d9</string>
   <key>issue_context_kind</key><string>function</string>
   <key>issue_context</key><string>test_objc_arrays</string>
   <key>issue_hash_function_offset</key><string>24</string>

test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist

<array>
          <key>line</key><integer>226</integer>
          <key>col</key><integer>6</integer>
          <key>file</key><integer>0</integer>
         </dict>
        </array>
      </array>
      <key>depth</key><integer>0</integer>
      <key>extended_message</key>
-     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+     <string>Object leaked: allocated object of type MyObj * is not referenced later in this execution path and has a retain count of +1</string>
      <key>message</key>
-     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+     <string>Object leaked: allocated object of type MyObj * is not referenced later in this execution path and has a retain count of +1</string>
     </dict>
    </array>
-   <key>description</key><string>Potential leak of an object</string>
+   <key>description</key><string>Potential leak of an object of type MyObj *</string>
    <key>category</key><string>Memory (Core Foundation/Objective-C)</string>
    <key>type</key><string>Leak</string>
    <key>check_name</key><string>osx.cocoa.RetainCount</string>
    <!-- This hash is experimental and going to change! -->
    <key>issue_hash_content_of_line_in_context</key><string>eef2aef4b58abf21fcfa4bbf69e19c02</string>
   <key>issue_context_kind</key><string>Objective-C method</string>
   <key>issue_context</key><string>test</string>
   <key>issue_hash_function_offset</key><string>2</string>

test/Analysis/objc-radar17039661.m

 // CHECK:         <key>line</key><integer>52</integer>
 // CHECK:         <key>col</key><integer>21</integer>
 // CHECK:         <key>file</key><integer>0</integer>
 // CHECK:        </dict>
 // CHECK:       </array>
 // CHECK:     </array>
 // CHECK:     <key>depth</key><integer>4</integer>
 // CHECK:     <key>extended_message</key>
-// CHECK:     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+// CHECK:     <string>Object leaked: allocated object of type NSNumber * is not referenced later in this execution path and has a retain count of +1</string>
 // CHECK:     <key>message</key>
-// CHECK:     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+// CHECK:     <string>Object leaked: allocated object of type NSNumber * is not referenced later in this execution path and has a retain count of +1</string>
 // CHECK:    </dict>
 // CHECK:   </array>
-// CHECK:   <key>description</key><string>Potential leak of an object</string>
+// CHECK:   <key>description</key><string>Potential leak of an object of type NSNumber *</string>
 // CHECK:   <key>category</key><string>Memory (Core Foundation/Objective-C)</string>
 // CHECK:   <key>type</key><string>Leak</string>
 // CHECK:  <key>location</key>
 // CHECK:  <dict>
 // CHECK:   <key>line</key><integer>52</integer>
 // CHECK:   <key>col</key><integer>5</integer>
 // CHECK:   <key>file</key><integer>0</integer>
 // CHECK:  </dict>
 // CHECK:  </dict>
 // CHECK: </array>

test/Analysis/osobject-retain-release.cpp

 };
 
 void check_custom_iterator_rule(OSArray *arr) {
   OSIterator *it = arr->getIterator();
   it->release();
 }
 
 void check_no_invalidation() {
-  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type OSArray with a +1 retain count}}
   OtherStruct::doNothingToArray(arr);
 } // expected-warning{{Potential leak of an object stored into 'arr'}}
   // expected-note@-1{{Object leaked}}
 
 void check_no_invalidation_other_struct() {
-  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type OSArray with a +1 retain count}}
   OtherStruct other(arr); // expected-warning{{Potential leak}}
                           // expected-note@-1{{Object leaked}}
 }
 
 struct ArrayOwner : public OSObject {
   OSArray *arr;
   ArrayOwner(OSArray *arr) : arr(arr) {}
 
struct ArrayOwner : public OSObject {
   }
 
   OSArray *createArraySourceUnknown();
 
   OSArray *getArraySourceUnknown();
 };
 
 OSArray *generateArray() {
-  return OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  return OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type OSArray with a +1 retain count}}
+                                    // expected-note@-1{{Call to function 'withCapacity' returns an OSObject of type OSArray with a +1 retain count}}
 }
 
 unsigned int check_leak_good_error_message() {
   unsigned int out;
   {
     OSArray *leaked = generateArray(); // expected-note{{Calling 'generateArray'}}
                                        // expected-note@-1{{Returning from 'generateArray'}}
     out = leaked->getCount(); // expected-warning{{Potential leak of an object stored into 'leaked'}}
                               // expected-note@-1{{Object leaked: object allocated and stored into 'leaked' is not referenced later in this execution path and has a retain count of +1}}
   }
   return out;
 }
 
 unsigned int check_leak_msg_temporary() {
-  return generateArray()->getCount();
+  return generateArray()->getCount(); // expected-warning{{Potential leak of an object}}
+                                      // expected-note@-1{{Calling 'generateArray'}}
+                                      // expected-note@-2{{Returning from 'generateArray'}}
+                                      // expected-note@-3{{Object leaked: allocated object of type OSArray is not referenced later in this execution path and has a retain count of +1}}
 }
 
 void check_confusing_getters() {
   OSArray *arr = OSArray::withCapacity(10);
 
   ArrayOwner *AO = ArrayOwner::create(arr);
   AO->getArray();
 
OSArray *arr = OSDynamicCast(OSArray, OSObject::generateObject(1)); // expected-note{{Call to function 'generateObject' returns an OSObject}}
     // expected-warning@-1{{Potential leak of an object}}
     // expected-note@-2{{Object leaked}}
   if (!arr)
     return;
   arr->release();
 }
 
 void use_after_release() {
-  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type OSArray with a +1 retain count}}
   arr->release(); // expected-note{{Object released}}
   arr->getCount(); // expected-warning{{Reference-counted object is used after it is released}}
                    // expected-note@-1{{Reference-counted object is used after it is released}}
 }
 
 void potential_leak() {
-  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type OSArray with a +1 retain count}}
   arr->retain(); // expected-note{{Reference count incremented. The object now has a +2 retain count}}
   arr->release(); // expected-note{{Reference count decremented. The object now has a +1 retain count}}
   arr->getCount();
 } // expected-warning{{Potential leak of an object stored into 'arr'}}
   // expected-note@-1{{Object leaked: object allocated and stored into 'arr' is not referenced later in this execution path and has a retain count of +1}}
 
 void proper_cleanup() {
   OSArray *arr = OSArray::withCapacity(10); // +1

 unsigned int no_warn_ok_release(ArrayOwner *owner) {
   OSArray *arr = owner->getArray(); // +0
   arr->retain(); // +1
   arr->release(); // +0
   return arr->getCount(); // no-warning
 }
 
 unsigned int warn_on_overrelease_with_unknown_source(ArrayOwner *owner) {
-  OSArray *arr = owner->getArraySourceUnknown(); // expected-note{{function call returns an OSObject of type struct OSArray * with a +0 retain count}}
+  OSArray *arr = owner->getArraySourceUnknown(); // expected-note{{function call returns an OSObject of type OSArray with a +0 retain count}}
   arr->release(); // expected-warning{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
                   // expected-note@-1{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
   return arr->getCount();
 }
 
 unsigned int ok_release_with_unknown_source(ArrayOwner *owner) {
   OSArray *arr = owner->getArraySourceUnknown(); // +0
   arr->retain(); // +1
   arr->release(); // +0
   return arr->getCount();
 }

test/Analysis/retain-release-path-notes.m

 }
 
 +(void)test {
   // initX is inlined since we explicitly mark it as interesting
   id x = [[MyObj alloc] initX]; // expected-warning {{Potential leak of an object}}
                                 // expected-note@-1 {{Method returns an instance of MyObj with a +1 retain count}}
                                 // expected-note@-2 {{Calling 'initX'}}
                                 // expected-note@-3 {{Returning from 'initX'}}
-                                // expected-note@-4 {{Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1}}
+                                // expected-note@-4 {{Object leaked: allocated object of type MyObj * is not referenced later in this execution path and has a retain count of +1}}
   // initI is inlined because the allocation happens within initY
   id y = [[MyObj alloc] initY];
                                 // expected-note@-1 {{Calling 'initY'}}
                                 // expected-note@-2 {{Returning from 'initY'}}
 
   // initZ is not inlined
   id z = [[MyObj alloc] initZ]; // expected-warning {{Potential leak of an object}}
                                 // expected-note@-1 {{Object leaked: object allocated and stored into 'y' is not referenced later in this execution path and has a retain count of +1}}

test/Analysis/retaincountchecker-compoundregion.m

 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{{$}}}}
+    CFNumberCreate(allocator, kCFNumberSInt32Type, &width), //expected-warning{{Potential leak of an object of type CFNumberRef}}
+    CFNumberCreate(allocator, kCFNumberSInt32Type, &height), //expected-warning{{Potential leak of an object of type CFNumberRef}}
   };
 }