From: Ted Kremenek Date: Sun, 10 May 2009 05:11:21 +0000 (+0000) Subject: analyzer: X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=eaedfeab9eab0d003859aab138784f2c59531408;p=clang analyzer: - Improve -autorelease diagnostics. - Improve VLA diagnostics. - Use "short description" for bug when outputting to TextDiagnostics git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@71383 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp index 23de85709a..db92823781 100644 --- a/lib/Analysis/BugReporter.cpp +++ b/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!"); diff --git a/lib/Analysis/CFRefCount.cpp b/lib/Analysis/CFRefCount.cpp index 7327887212..80ea6e9f5a 100644 --- a/lib/Analysis/CFRefCount.cpp +++ b/lib/Analysis/CFRefCount.cpp @@ -1495,19 +1495,11 @@ public: unsigned Count = 0) { 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"; } }; @@ -1969,7 +1960,11 @@ namespace { public: CFRefReport(CFRefBug& D, const CFRefCount &tf, ExplodedNode *n, SymbolRef sym) - : RangedBugReport(D, D.getDescription(), n), Sym(sym), TF(tf) {} + : RangedBugReport(D, D.getDescription(), n), Sym(sym), TF(tf) {} + + CFRefReport(CFRefBug& D, const CFRefCount &tf, + ExplodedNode *n, SymbolRef sym, const char* endText) + : RangedBugReport(D, D.getDescription(), endText, n), Sym(sym), TF(tf) {} virtual ~CFRefReport() {} @@ -2000,7 +1995,7 @@ namespace { const ExplodedNode* PrevN, BugReporterContext& BRC); }; - + class VISIBILITY_HIDDEN CFRefLeakReport : public CFRefReport { SourceLocation AllocSite; const MemRegion* AllocBinding; @@ -2274,7 +2269,7 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode* 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 *n, SymbolRef sym, GRExprEngine& Eng) @@ -2983,20 +2977,6 @@ void CFRefCount::EvalReturn(ExplodedNodeSet& 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(Sym); - assert(T); - // Change the reference count. RefVal X = *T; @@ -3004,14 +2984,20 @@ void CFRefCount::EvalReturn(ExplodedNodeSet& 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; } @@ -3026,6 +3012,22 @@ void CFRefCount::EvalReturn(ExplodedNodeSet& Dst, // Did we cache out? 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(Sym); + assert(T); + X = *T; // Any leaks or other errors? if (X.isReturnedOwned() && X.getCount() == 0) { @@ -3261,9 +3263,22 @@ CFRefCount::HandleAutoreleaseCounts(GRStateRef state, GenericNodeBuilder Bd, if (ExplodedNode *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(overAutorelease), - *this, N, Sym); + *this, N, Sym, os.str().c_str()); BR->EmitReport(report); } diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp index cca1c68cf7..8f37ad2d65 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/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(), diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m index 78bbb8a776..205bac2c82 100644 --- a/test/Analysis/misc-ps.m +++ b/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) diff --git a/test/Analysis/retain-release.m b/test/Analysis/retain-release.m index acc469a7b7..29a697af8f 100644 --- a/test/Analysis/retain-release.m +++ b/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 }