analyzer:

- Improve -autorelease diagnostics.
- Improve VLA diagnostics.
- Use "short description" for bug when outputting to TextDiagnostics

llvm-svn: 71383

clang/lib/Analysis/BugReporter.cpp

@@ -1576,7 +1576,7 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
   Diagnostic& Diag = getDiagnostic();
   FullSourceLoc L(R.getLocation(), getSourceManager());  
   unsigned ErrorDiag = Diag.getCustomDiagID(Diagnostic::Warning,
-                                            R.getDescription().c_str());
+                                            R.getShortDescription().c_str());
 
   switch (End-Beg) {
     default: assert(0 && "Don't handle this many ranges yet!");

clang/lib/Analysis/CFRefCount.cpp

@@ -1496,18 +1496,10 @@ public:
     return RefVal(NotOwned, o, Count, 0, t);
   }
   
-  static RefVal makeReturnedOwned(unsigned Count) {
-    return RefVal(ReturnedOwned, Count);
-  }
-  
-  static RefVal makeReturnedNotOwned() {
-    return RefVal(ReturnedNotOwned);
-  }
-  
   // Comparison, profiling, and pretty-printing.
   
   bool operator==(const RefVal& X) const {
-    return kind == X.kind && Cnt == X.Cnt && T == X.T;
+    return kind == X.kind && Cnt == X.Cnt && T == X.T && ACnt == X.ACnt;
   }
   
   RefVal operator-(size_t i) const {
@@ -1929,8 +1921,7 @@ namespace {
       CFRefBug(tf, "Object sent -autorelease too many times") {}
     
     const char *getDescription() const {
-      return "Object will be sent more -release messages from its containing "
-             "autorelease pools than it has retain counts";
+      return "Object sent -autorelease too many times";
     }
   };
   
@@ -1971,6 +1962,10 @@ namespace {
                 ExplodedNode<GRState> *n, SymbolRef sym)
       : RangedBugReport(D, D.getDescription(), n), Sym(sym), TF(tf) {}
 
+    CFRefReport(CFRefBug& D, const CFRefCount &tf,
+                ExplodedNode<GRState> *n, SymbolRef sym, const char* endText)
+      : RangedBugReport(D, D.getDescription(), endText, n), Sym(sym), TF(tf) {}    
+    
     virtual ~CFRefReport() {}
     
     CFRefBug& getBugType() {
@@ -2274,7 +2269,7 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
               return 0;
             
             assert(PrevV.getAutoreleaseCount() < CurrV.getAutoreleaseCount());            
-            os << "Object added to autorelease pool.";
+            os << "Object sent -autorelease message";
             break;
           }
           
@@ -2500,7 +2495,6 @@ CFRefLeakReport::getEndPath(BugReporterContext& BRC,
   return new PathDiagnosticEventPiece(L, os.str());
 }
 
-
 CFRefLeakReport::CFRefLeakReport(CFRefBug& D, const CFRefCount &tf,
                                  ExplodedNode<GRState> *n,
                                  SymbolRef sym, GRExprEngine& Eng)
@@ -2983,20 +2977,6 @@ void CFRefCount::EvalReturn(ExplodedNodeSet<GRState>& Dst,
   if (!T)
     return;
   
-  // Update the autorelease counts.
-  static unsigned autoreleasetag = 0;
-  GenericNodeBuilder Bd(Builder, S, &autoreleasetag);
-  bool stop = false;
-  llvm::tie(Pred, state) = HandleAutoreleaseCounts(state , Bd, Pred, Eng, Sym,
-                                                   *T, stop);
-  
-  if (stop)
-    return;
-
-  // Get the updated binding.
-  T = state.get<RefBindings>(Sym);
-  assert(T);
-  
   // Change the reference count.  
   RefVal X = *T;  
   
@@ -3004,14 +2984,20 @@ void CFRefCount::EvalReturn(ExplodedNodeSet<GRState>& Dst,
     case RefVal::Owned: { 
       unsigned cnt = X.getCount();
       assert (cnt > 0);
-      X = RefVal::makeReturnedOwned(cnt - 1);
+      X.setCount(cnt - 1);
+      X = X ^ RefVal::ReturnedOwned;
       break;
     }
       
     case RefVal::NotOwned: {
       unsigned cnt = X.getCount();
-      X = cnt ? RefVal::makeReturnedOwned(cnt - 1)
-              : RefVal::makeReturnedNotOwned();
+      if (cnt) {
+        X.setCount(cnt - 1);
+        X = X ^ RefVal::ReturnedOwned;
+      }
+      else {
+        X = X ^ RefVal::ReturnedNotOwned;
+      }
       break;
     }
       
@@ -3027,6 +3013,22 @@ void CFRefCount::EvalReturn(ExplodedNodeSet<GRState>& Dst,
   if (!Pred)
     return;
   
+  // Update the autorelease counts.
+  static unsigned autoreleasetag = 0;
+  GenericNodeBuilder Bd(Builder, S, &autoreleasetag);
+  bool stop = false;
+  llvm::tie(Pred, state) = HandleAutoreleaseCounts(state , Bd, Pred, Eng, Sym,
+                                                   X, stop);
+  
+  // Did we cache out?
+  if (!Pred || stop)
+    return;
+  
+  // Get the updated binding.
+  T = state.get<RefBindings>(Sym);
+  assert(T);
+  X = *T;
+    
   // Any leaks or other errors?
   if (X.isReturnedOwned() && X.getCount() == 0) {
     const Decl *CD = &Eng.getStateManager().getCodeDecl();
@@ -3261,9 +3263,22 @@ CFRefCount::HandleAutoreleaseCounts(GRStateRef state, GenericNodeBuilder Bd,
 
   if (ExplodedNode<GRState> *N = Bd.MakeNode(state, Pred)) {
     N->markAsSink();
+    
+    std::string sbuf;
+    llvm::raw_string_ostream os(sbuf);
+    os << "Object over-autoreleased: object was sent -autorelease " ;
+    if (V.getAutoreleaseCount() > 1)
+      os << V.getAutoreleaseCount() << " times";
+    os << " but the object has ";
+    if (V.getCount() == 0)
+      os << "zero (locally visible)";
+    else
+      os << "+" << V.getCount();
+    os << " retain counts";
+    
     CFRefReport *report =
       new CFRefReport(*static_cast<CFRefBug*>(overAutorelease),
-                      *this, N, Sym);
+                      *this, N, Sym, os.str().c_str());
     BR->EmitReport(report);
   }
   

clang/lib/Analysis/GRExprEngineInternalChecks.cpp

@@ -453,8 +453,8 @@ public:
       std::string shortBuf;
       llvm::raw_string_ostream os_short(shortBuf);
       os_short << "Variable-length array '" << VD->getNameAsString() << "' "
-               << (isUndefined ? " garbage value for array size"
-                   : " has zero elements (undefined behavior)");
+               << (isUndefined ? "garbage value for array size"
+                   : "has zero elements (undefined behavior)");
 
       RangedBugReport *report = new RangedBugReport(*this,
                                                     os_short.str().c_str(),

clang/test/Analysis/misc-ps.m

@@ -103,12 +103,12 @@ void check_zero_sized_VLA(int x) {
   if (x)
     return;
 
-  int vla[x]; // expected-warning{{VLAs with no elements have undefined behavior}}
+  int vla[x]; // expected-warning{{Variable-length array 'vla' has zero elements (undefined behavior)}}
 }
 
 void check_uninit_sized_VLA() {
   int x;
-  int vla[x]; // expected-warning{{The expression used to specify the number of elements in the variable-length array (VLA) 'vla' evaluates to an undefined or garbage value}}
+  int vla[x]; // expected-warning{{Variable-length array 'vla' garbage value for array size}}
 }
 
 // sizeof(void)

clang/test/Analysis/retain-release.m

@@ -291,21 +291,21 @@ void f13_autorelease() {
 void f13_autorelease_b() {
   CFMutableArrayRef A = CFArrayCreateMutable(0, 10, &kCFTypeArrayCallBacks);
   [(id) A autorelease];
-  [(id) A autorelease]; // expected-warning{{Object will be sent more -release messages from its containing autorelease pools than it has retain counts}}
+  [(id) A autorelease]; // expected-warning{{Object sent -autorelease too many times}}
 }
 
 CFMutableArrayRef f13_autorelease_c() {
   CFMutableArrayRef A = CFArrayCreateMutable(0, 10, &kCFTypeArrayCallBacks);
   [(id) A autorelease];
   [(id) A autorelease]; 
-  return A; // expected-warning{{Object will be sent more -release messages from its containing autorelease pools than it has retain counts}}
+  return A; // expected-warning{{Object sent -autorelease too many times}}
 }
 
 CFMutableArrayRef f13_autorelease_d() {
   CFMutableArrayRef A = CFArrayCreateMutable(0, 10, &kCFTypeArrayCallBacks);
   [(id) A autorelease];
   [(id) A autorelease]; 
-  CFMutableArrayRef B = CFArrayCreateMutable(0, 10, &kCFTypeArrayCallBacks); // expected-warning{{Object will be sent more -release messages}}
+  CFMutableArrayRef B = CFArrayCreateMutable(0, 10, &kCFTypeArrayCallBacks); // expected-warning{{Object sent -autorelease too many times}}
   CFRelease(B); // no-warning
 }