From 15ca5e7a219a77b22a3652f6317026fa57ce1501 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Wed, 21 Sep 2011 00:35:58 +0000 Subject: [PATCH] [analyzer] Fix a bug where PathDiagnosticLocation did not generate a valid range and add asserts to check validity of locations early on. Ignore invalid ranges in PathDiagnosticPiece (they could be added by checker writers). Addresses radar://10124836 and radar://radar10102244. llvm-svn: 140218 --- .../Core/BugReporter/PathDiagnostic.h | 29 +++++++++++++------ .../StaticAnalyzer/Core/PathDiagnostic.cpp | 15 ++++++---- clang/test/Analysis/retain-release.m | 8 ++++- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h index dd1622b7fbc6..2056d0ebc851 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h @@ -104,21 +104,22 @@ private: PathDiagnosticLocation(SourceLocation L, const SourceManager &sm, Kind kind) : K(kind), S(0), D(0), SM(&sm), - Loc(genLocation(L)), Range(genRange(L)) {} + Loc(genLocation(L)), Range(genRange()) { + assert(Loc.isValid()); + assert(Range.isValid()); + } FullSourceLoc genLocation(SourceLocation L = SourceLocation(), LocationOrAnalysisContext LAC = (AnalysisContext*)0) const; PathDiagnosticRange - genRange(SourceLocation L = SourceLocation(), - LocationOrAnalysisContext LAC = (AnalysisContext*)0) const; + genRange(LocationOrAnalysisContext LAC = (AnalysisContext*)0) const; public: /// Create an invalid location. PathDiagnosticLocation() - : K(SingleLocK), S(0), D(0), SM(0) { - } + : K(SingleLocK), S(0), D(0), SM(0) {} /// Create a location corresponding to the given statement. PathDiagnosticLocation(const Stmt *s, @@ -126,13 +127,17 @@ public: LocationOrAnalysisContext lac) : K(StmtK), S(s), D(0), SM(&sm), Loc(genLocation(SourceLocation(), lac)), - Range(genRange(SourceLocation(), lac)) {} - + Range(genRange(lac)) { + assert(Loc.isValid()); + assert(Range.isValid()); + } /// Create a location corresponding to the given declaration. PathDiagnosticLocation(const Decl *d, const SourceManager &sm) : K(DeclK), S(0), D(d), SM(&sm), Loc(genLocation()), Range(genRange()) { + assert(Loc.isValid()); + assert(Range.isValid()); } /// Create a location corresponding to the given declaration. @@ -291,9 +296,15 @@ public: Kind getKind() const { return kind; } - void addRange(SourceRange R) { ranges.push_back(R); } + void addRange(SourceRange R) { + if (!R.isValid()) + return; + ranges.push_back(R); + } void addRange(SourceLocation B, SourceLocation E) { + if (!B.isValid() || !E.isValid()) + return; ranges.push_back(SourceRange(B,E)); } @@ -338,7 +349,7 @@ public: PathDiagnosticPiece::Kind k, bool addPosRange = true) : PathDiagnosticPiece(s, k), Pos(pos) { - assert(Pos.asLocation().isValid() && + assert(Pos.isValid() && Pos.asLocation().isValid() && "PathDiagnosticSpotPiece's must have a valid location."); if (addPosRange && Pos.hasRange()) addRange(Pos.asRange()); } diff --git a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index 6fbfddbfd7b9..76acfc2e318c 100644 --- a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -228,7 +228,8 @@ PathDiagnosticLocation PathDiagnosticLocation::createSingleLocation( } FullSourceLoc - PathDiagnosticLocation::genLocation(SourceLocation L, LocationOrAnalysisContext LAC) const { + PathDiagnosticLocation::genLocation(SourceLocation L, + LocationOrAnalysisContext LAC) const { assert(isValid()); // Note that we want a 'switch' here so that the compiler can warn us in // case we add more cases. @@ -247,13 +248,13 @@ FullSourceLoc } PathDiagnosticRange - PathDiagnosticLocation::genRange(SourceLocation L, LocationOrAnalysisContext LAC) const { + PathDiagnosticLocation::genRange(LocationOrAnalysisContext LAC) const { assert(isValid()); // Note that we want a 'switch' here so that the compiler can warn us in // case we add more cases. switch (K) { case SingleLocK: - return PathDiagnosticRange(SourceRange(L,L), true); + return PathDiagnosticRange(SourceRange(Loc,Loc), true); case RangeK: break; case StmtK: { @@ -286,8 +287,10 @@ PathDiagnosticRange return SourceRange(L, L); } } - - return S->getSourceRange(); + SourceRange R = S->getSourceRange(); + if (R.isValid()) + return R; + break; } case DeclK: if (const ObjCMethodDecl *MD = dyn_cast(D)) @@ -302,7 +305,7 @@ PathDiagnosticRange } } - return SourceRange(L,L); + return SourceRange(Loc,Loc); } void PathDiagnosticLocation::flatten() { diff --git a/clang/test/Analysis/retain-release.m b/clang/test/Analysis/retain-release.m index 1e50e1ed592d..f1fd4e9642a1 100644 --- a/clang/test/Analysis/retain-release.m +++ b/clang/test/Analysis/retain-release.m @@ -652,6 +652,12 @@ void rdar6704930(unsigned char *s, unsigned int length) { [window release]; [super dealloc]; } + +- (void)radar10102244 { + NSMutableDictionary *dict = [[NSMutableDictionary dictionaryWithCapacity:4] retain]; // expected-warning{{leak}} + if (window) + NSLog(@"%@", window); +} @end //===----------------------------------------------------------------------===// @@ -1444,7 +1450,7 @@ static void rdar_8724287(CFErrorRef error) while (error_to_dump != ((void*)0)) { CFDictionaryRef info; - info = CFErrorCopyUserInfo(error_to_dump); // expected-warning{{Potential leak of an object allocated on line 1447 and stored into 'info'}} + info = CFErrorCopyUserInfo(error_to_dump); // expected-warning{{Potential leak of an object allocated on line}} if (info != ((void*)0)) { }