[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

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 dd1622b7fbc..2056d0ebc85 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 6fbfddbfd7b..76acfc2e318 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<ObjCMethodDecl>(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 1e50e1ed592..f1fd4e9642a 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)) {
         }