[analyzer] Refactor PathDiagnosticLocation: Make PathDiagnosticLocation(SourceLocation...) private. Most of the effort here goes to making BugReport refer to a PathDiagnosticLocation instead of FullSourceLocation.

(Another step closer to the goal of having Diagnostics which can recover from invalid SourceLocations.)

llvm-svn: 140182

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

@@ -17,6 +17,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableList.h"
@@ -34,8 +35,6 @@ class ParentMap;
 namespace ento {
 
 class PathDiagnostic;
-class PathDiagnosticPiece;
-class PathDiagnosticClient;
 class ExplodedNode;
 class ExplodedGraph;
 class BugReport;
@@ -70,7 +69,7 @@ protected:
   BugType& BT;
   std::string ShortDescription;
   std::string Description;
-  FullSourceLoc Location;
+  PathDiagnosticLocation Location;
   const ExplodedNode *ErrorNode;
   SmallVector<SourceRange, 4> Ranges;
   ExtraTextList ExtraText;
@@ -91,7 +90,7 @@ public:
     : BT(bt), ShortDescription(shortDesc), Description(desc),
       ErrorNode(errornode), Callbacks(F.getEmptyList()) {}
 
-  BugReport(BugType& bt, StringRef desc, FullSourceLoc l)
+  BugReport(BugType& bt, StringRef desc, PathDiagnosticLocation l)
     : BT(bt), Description(desc), Location(l), ErrorNode(0),
       Callbacks(F.getEmptyList()) {}
 
@@ -124,7 +123,7 @@ public:
   ///  While a bug can span an entire path, usually there is a specific
   ///  location that can be used to identify where the key issue occurred.
   ///  This location is used by clients rendering diagnostics.
-  virtual SourceLocation getLocation() const;
+  virtual PathDiagnosticLocation getLocation(const SourceManager &SM) const;
 
   const Stmt *getStmt() const;
 
@@ -296,31 +295,31 @@ public:
   void EmitReport(BugReport *R);
 
   void EmitBasicReport(StringRef BugName, StringRef BugStr,
-                       SourceLocation Loc,
+                       PathDiagnosticLocation Loc,
                        SourceRange* RangeBeg, unsigned NumRanges);
 
   void EmitBasicReport(StringRef BugName, StringRef BugCategory,
-                       StringRef BugStr, SourceLocation Loc,
+                       StringRef BugStr, PathDiagnosticLocation Loc,
                        SourceRange* RangeBeg, unsigned NumRanges);
 
 
   void EmitBasicReport(StringRef BugName, StringRef BugStr,
-                       SourceLocation Loc) {
+                       PathDiagnosticLocation Loc) {
     EmitBasicReport(BugName, BugStr, Loc, 0, 0);
   }
 
   void EmitBasicReport(StringRef BugName, StringRef BugCategory,
-                       StringRef BugStr, SourceLocation Loc) {
+                       StringRef BugStr, PathDiagnosticLocation Loc) {
     EmitBasicReport(BugName, BugCategory, BugStr, Loc, 0, 0);
   }
 
   void EmitBasicReport(StringRef BugName, StringRef BugStr,
-                       SourceLocation Loc, SourceRange R) {
+                       PathDiagnosticLocation Loc, SourceRange R) {
     EmitBasicReport(BugName, BugStr, Loc, &R, 1);
   }
 
   void EmitBasicReport(StringRef BugName, StringRef Category,
-                       StringRef BugStr, SourceLocation Loc,
+                       StringRef BugStr, PathDiagnosticLocation Loc,
                        SourceRange R) {
     EmitBasicReport(BugName, Category, BugStr, Loc, &R, 1);
   }

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

@@ -29,6 +29,7 @@ class BinaryOperator;
 class CompoundStmt;
 class Decl;
 class LocationContext;
+class MemberExpr;
 class ParentMap;
 class ProgramPoint;
 class SourceManager;
@@ -103,6 +104,12 @@ private:
   FullSourceLoc Loc;
   PathDiagnosticRange Range;
 
+  PathDiagnosticLocation(SourceLocation L, const SourceManager &sm,
+                         Kind kind)
+    : K(kind), R(L, L), S(0), D(0), SM(&sm),
+      Loc(genLocation()), Range(genRange()) {
+  }
+  
   FullSourceLoc
     genLocation(LocationOrAnalysisContext LAC = (AnalysisContext*)0) const;
   PathDiagnosticRange
@@ -118,12 +125,6 @@ public:
       Loc(genLocation()), Range(genRange()) {
   }
 
-  PathDiagnosticLocation(SourceLocation L, const SourceManager &sm,
-                         Kind kind = SingleLocK)
-    : K(kind), R(L, L), S(0), D(0), SM(&sm),
-      Loc(genLocation()), Range(genRange()) {
-  }
-
   PathDiagnosticLocation(const Stmt *s,
                          const SourceManager &sm,
                          LocationOrAnalysisContext lac)
@@ -136,16 +137,30 @@ public:
       Loc(genLocation()), Range(genRange()) {
   }
 
-  // Create a location for the beginning of the statement.
+  static PathDiagnosticLocation create(const Decl *D,
-  static PathDiagnosticLocation createBeginStmt(const Stmt *S,
+                                       const SourceManager &SM) {
-                                                const SourceManager &SM,
+    return PathDiagnosticLocation(D, SM);
-                                                LocationOrAnalysisContext LAC);
+  }
+
+  /// Create a location for the beginning of the declaration.
+  static PathDiagnosticLocation createBegin(const Decl *D,
+                                            const SourceManager &SM);
+
+  /// Create a location for the beginning of the statement.
+  static PathDiagnosticLocation createBegin(const Stmt *S,
+                                            const SourceManager &SM,
+                                            const LocationOrAnalysisContext LAC);
 
   /// Create the location for the operator of the binary expression.
   /// Assumes the statement has a valid location.
   static PathDiagnosticLocation createOperatorLoc(const BinaryOperator *BO,
                                                   const SourceManager &SM);
 
+  /// For member expressions, return the location of the '.' or '->'.
+  /// Assumes the statement has a valid location.
+  static PathDiagnosticLocation createMemberLoc(const MemberExpr *ME,
+                                                const SourceManager &SM);
+
   /// Create a location for the beginning of the compound statement.
   /// Assumes the statement has a valid location.
   static PathDiagnosticLocation createBeginBrace(const CompoundStmt *CS,

clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp

@@ -94,7 +94,7 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G,
       << (Eng.hasEmptyWorkList() ? "yes" : "no");
 
   B.EmitBasicReport("Analyzer Statistics", "Internal Statistics", output.str(),
-      D->getLocation());
+      PathDiagnosticLocation(D, SM));
 
   // Emit warning for each block we bailed out on
   typedef CoreEngine::BlocksExhausted::const_iterator ExhaustedIterator;
@@ -106,7 +106,8 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G,
     const CFGElement &CE = Exit->front();
     if (const CFGStmt *CS = dyn_cast<CFGStmt>(&CE))
       B.EmitBasicReport("Bailout Point", "Internal Statistics", "The analyzer "
-          "stopped analyzing at this point", CS->getStmt()->getLocStart());
+          "stopped analyzing at this point",
+          PathDiagnosticLocation::createBegin(CS->getStmt(), SM, LC));
   }
 }
 

clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

@@ -166,6 +166,9 @@ static void checkObjCDealloc(const ObjCImplementationDecl *D,
     }
   }
 
+  PathDiagnosticLocation DLoc =
+    PathDiagnosticLocation::createBegin(D, BR.getSourceManager());
+
   if (!MD) { // No dealloc found.
 
     const char* name = LOpts.getGC() == LangOptions::NonGC
@@ -176,7 +179,7 @@ static void checkObjCDealloc(const ObjCImplementationDecl *D,
     llvm::raw_string_ostream os(buf);
     os << "Objective-C class '" << D << "' lacks a 'dealloc' instance method";
 
-    BR.EmitBasicReport(name, os.str(), D->getLocStart());
+    BR.EmitBasicReport(name, os.str(), DLoc);
     return;
   }
 
@@ -193,7 +196,7 @@ static void checkObjCDealloc(const ObjCImplementationDecl *D,
        << "' does not send a 'dealloc' message to its super class"
            " (missing [super dealloc])";
 
-    BR.EmitBasicReport(name, os.str(), D->getLocStart());
+    BR.EmitBasicReport(name, os.str(), DLoc);
     return;
   }
 
@@ -257,7 +260,10 @@ static void checkObjCDealloc(const ObjCImplementationDecl *D,
               "but was released in 'dealloc'";
       }
 
-      BR.EmitBasicReport(name, category, os.str(), (*I)->getLocation());
+      PathDiagnosticLocation SDLoc =
+        PathDiagnosticLocation::createBegin((*I), BR.getSourceManager());
+
+      BR.EmitBasicReport(name, category, os.str(), SDLoc);
     }
   }
 }

clang/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp

@@ -66,8 +66,12 @@ static void CompareReturnTypes(const ObjCMethodDecl *MethDerived,
        << "'.  These two types are incompatible, and may result in undefined "
           "behavior for clients of these classes.";
 
+    PathDiagnosticLocation MethDLoc =
+      PathDiagnosticLocation::createBegin(MethDerived,
+                                          BR.getSourceManager());
+
     BR.EmitBasicReport("Incompatible instance method return type",
-                       os.str(), MethDerived->getLocStart());
+                       os.str(), MethDLoc);
   }
 }
 

clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

@@ -12,12 +12,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
+#include "clang/Analysis/AnalysisContext.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
-#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
-#include "clang/AST/StmtVisitor.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace ento;
@@ -34,14 +36,16 @@ static bool isArc4RandomAvailable(const ASTContext &Ctx) {
 namespace {
 class WalkAST : public StmtVisitor<WalkAST> {
   BugReporter &BR;
+  AnalysisContext* AC;
   enum { num_setids = 6 };
   IdentifierInfo *II_setid[num_setids];
 
   const bool CheckRand;
 
 public:
-  WalkAST(BugReporter &br) : BR(br), II_setid(),
+  WalkAST(BugReporter &br, AnalysisContext* ac)
-                 CheckRand(isArc4RandomAvailable(BR.getContext())) {}
+  : BR(br), AC(ac), II_setid(),
+    CheckRand(isArc4RandomAvailable(BR.getContext())) {}
 
   // Statement visitor methods.
   void VisitCallExpr(CallExpr *CE);
@@ -247,8 +251,11 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) {
   ranges.push_back(drInc->getSourceRange());
 
   const char *bugType = "Floating point variable used as loop counter";
+
+  PathDiagnosticLocation FSLoc =
+    PathDiagnosticLocation::createBegin(FS, BR.getSourceManager(), AC);
   BR.EmitBasicReport(bugType, "Security", os.str(),
-                     FS->getLocStart(), ranges.data(), ranges.size());
+                     FSLoc, ranges.data(), ranges.size());
 }
 
 //===----------------------------------------------------------------------===//
@@ -278,11 +285,13 @@ void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
 
   // Issue a warning.
   SourceRange R = CE->getCallee()->getSourceRange();
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
   BR.EmitBasicReport("Potential buffer overflow in call to 'gets'",
                      "Security",
                      "Call to function 'gets' is extremely insecure as it can "
                      "always result in a buffer overflow",
-                     CE->getLocStart(), &R, 1);
+                     CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -314,11 +323,13 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
 
   // Issue a warning.
   SourceRange R = CE->getCallee()->getSourceRange();
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
   BR.EmitBasicReport("Potential buffer overflow in call to 'getpw'",
                      "Security",
                      "The getpw() function is dangerous as it may overflow the "
                      "provided buffer. It is obsoleted by getpwuid().",
-                     CE->getLocStart(), &R, 1);
+                     CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -347,11 +358,13 @@ void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
 
   // Issue a waring.
   SourceRange R = CE->getCallee()->getSourceRange();
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
   BR.EmitBasicReport("Potential insecure temporary file in call 'mktemp'",
                      "Security",
                      "Call to function 'mktemp' is insecure as it always "
                      "creates or uses insecure temporary file.  Use 'mkstemp' instead",
-                     CE->getLocStart(), &R, 1);
+                     CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -366,6 +379,8 @@ void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
 
   // Issue a warning.
   SourceRange R = CE->getCallee()->getSourceRange();
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
   BR.EmitBasicReport("Potential insecure memory buffer bounds restriction in "
                      "call 'strcpy'",
                      "Security",
@@ -373,7 +388,7 @@ void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
 		     "provide bounding of the memory buffer. Replace "
 		     "unbounded copy functions with analogous functions that "
 		     "support length arguments such as 'strncpy'. CWE-119.",
-                     CE->getLocStart(), &R, 1);
+                     CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -388,6 +403,8 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
 
   // Issue a warning.
   SourceRange R = CE->getCallee()->getSourceRange();
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
   BR.EmitBasicReport("Potential insecure memory buffer bounds restriction in "
 		     "call 'strcat'",
 		     "Security",
@@ -395,7 +412,7 @@ void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
 		     "provide bounding of the memory buffer. Replace "
 		     "unbounded copy functions with analogous functions that "
 		     "support length arguments such as 'strncat'. CWE-119.",
-                     CE->getLocStart(), &R, 1);
+                     CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -467,7 +484,9 @@ void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
       << "  Use 'arc4random' instead";
 
   SourceRange R = CE->getCallee()->getSourceRange();
-  BR.EmitBasicReport(os1.str(), "Security", os2.str(),CE->getLocStart(), &R, 1);
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  BR.EmitBasicReport(os1.str(), "Security", os2.str(), CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -490,11 +509,13 @@ void WalkAST::checkCall_random(const CallExpr *CE, const FunctionDecl *FD) {
 
   // Issue a warning.
   SourceRange R = CE->getCallee()->getSourceRange();
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
   BR.EmitBasicReport("'random' is not a secure random number generator",
                      "Security",
                      "The 'random' function produces a sequence of values that "
                      "an adversary may be able to predict.  Use 'arc4random' "
-                     "instead", CE->getLocStart(), &R, 1);
+                     "instead", CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -554,7 +575,9 @@ void WalkAST::checkUncheckedReturnValue(CallExpr *CE) {
       << "', the following code may execute with unexpected privileges";
 
   SourceRange R = CE->getCallee()->getSourceRange();
-  BR.EmitBasicReport(os1.str(), "Security", os2.str(),CE->getLocStart(), &R, 1);
+  PathDiagnosticLocation CELoc =
+    PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  BR.EmitBasicReport(os1.str(), "Security", os2.str(), CELoc, &R, 1);
 }
 
 //===----------------------------------------------------------------------===//
@@ -566,7 +589,7 @@ class SecuritySyntaxChecker : public Checker<check::ASTCodeBody> {
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
                         BugReporter &BR) const {
-    WalkAST walker(BR);
+    WalkAST walker(BR, mgr.getAnalysisContext(D));
     walker.Visit(D->getBody());
   }
 };

clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp

@@ -13,9 +13,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "ClangSACheckers.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
-#include "clang/AST/StmtVisitor.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 
 using namespace clang;
 using namespace ento;
@@ -23,9 +24,10 @@ using namespace ento;
 namespace {
 class WalkAST : public StmtVisitor<WalkAST> {
   BugReporter &BR;
+  AnalysisContext* AC;
 
 public:
-  WalkAST(BugReporter &br) : BR(br) {}
+  WalkAST(BugReporter &br, AnalysisContext* ac) : BR(br), AC(ac) {}
   void VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E);
   void VisitStmt(Stmt *S) { VisitChildren(S); }
   void VisitChildren(Stmt *S);
@@ -59,11 +61,13 @@ void WalkAST::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) {
       return;
 
     SourceRange R = ArgEx->getSourceRange();
+    PathDiagnosticLocation ELoc =
+      PathDiagnosticLocation::createBegin(E, BR.getSourceManager(), AC);
     BR.EmitBasicReport("Potential unintended use of sizeof() on pointer type",
                        "Logic",
                        "The code calls sizeof() on a pointer type. "
                        "This can produce an unexpected result.",
-                       E->getLocStart(), &R, 1);
+                       ELoc, &R, 1);
   }
 }
 
@@ -76,7 +80,7 @@ class SizeofPointerChecker : public Checker<check::ASTCodeBody> {
 public:
   void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
                         BugReporter &BR) const {
-    WalkAST walker(BR);
+    WalkAST walker(BR, mgr.getAnalysisContext(D));
     walker.Visit(D->getBody());
   }
 };

clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

@@ -72,6 +72,7 @@ class DeadStoreObs : public LiveVariables::Observer {
   const CFG &cfg;
   ASTContext &Ctx;
   BugReporter& BR;
+  AnalysisContext* AC;
   ParentMap& Parents;
   llvm::SmallPtrSet<const VarDecl*, 20> Escaped;
   llvm::OwningPtr<ReachableCode> reachableCode;
@@ -81,15 +82,15 @@ class DeadStoreObs : public LiveVariables::Observer {
 
 public:
   DeadStoreObs(const CFG &cfg, ASTContext &ctx,
-               BugReporter& br, ParentMap& parents,
+               BugReporter& br, AnalysisContext* ac, ParentMap& parents,
                llvm::SmallPtrSet<const VarDecl*, 20> &escaped)
-    : cfg(cfg), Ctx(ctx), BR(br), Parents(parents),
+    : cfg(cfg), Ctx(ctx), BR(br), AC(ac), Parents(parents),
       Escaped(escaped), currentBlock(0) {}
 
   virtual ~DeadStoreObs() {}
 
   void Report(const VarDecl *V, DeadStoreKind dsk,
-              SourceLocation L, SourceRange R) {
+              PathDiagnosticLocation L, SourceRange R) {
     if (Escaped.count(V))
       return;
     
@@ -146,9 +147,12 @@ public:
       return;
 
     if (!Live.isLive(VD) && 
-        !(VD->getAttr<UnusedAttr>() || VD->getAttr<BlocksAttr>()))
+        !(VD->getAttr<UnusedAttr>() || VD->getAttr<BlocksAttr>())) {
-      Report(VD, dsk, Ex->getSourceRange().getBegin(),
+
-             Val->getSourceRange());
+      PathDiagnosticLocation ExLoc =
+        PathDiagnosticLocation::createBegin(Ex, BR.getSourceManager(), AC);
+      Report(VD, dsk, ExLoc, Val->getSourceRange());
+    }
   }
 
   void CheckDeclRef(const DeclRefExpr *DR, const Expr *Val, DeadStoreKind dsk,
@@ -293,7 +297,9 @@ public:
                     return;
                 }
 
-              Report(V, DeadInit, V->getLocation(), E->getSourceRange());
+              PathDiagnosticLocation Loc =
+                PathDiagnosticLocation::create(V, BR.getSourceManager());
+              Report(V, DeadInit, Loc, E->getSourceRange());
             }
           }
         }
@@ -344,10 +350,11 @@ public:
                         BugReporter &BR) const {
     if (LiveVariables *L = mgr.getLiveVariables(D)) {
       CFG &cfg = *mgr.getCFG(D);
+      AnalysisContext *AC = mgr.getAnalysisContext(D);
       ParentMap &pmap = mgr.getParentMap(D);
       FindEscaped FS(&cfg);
       FS.getCFG().VisitBlockStmts(FS);
-      DeadStoreObs A(cfg, BR.getContext(), BR, pmap, FS.Escaped);
+      DeadStoreObs A(cfg, BR.getContext(), BR, AC, pmap, FS.Escaped);
       L->runOnAllBlocks(A);
     }
   }

clang/lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp

@@ -175,9 +175,10 @@ void StringRefCheckerVisitor::VisitVarDecl(VarDecl *VD) {
   // Okay, badness!  Report an error.
   const char *desc = "StringRef should not be bound to temporary "
                      "std::string that it outlives";
-
+  PathDiagnosticLocation VDLoc =
+    PathDiagnosticLocation::createBegin(VD, BR.getSourceManager());
   BR.EmitBasicReport(desc, "LLVM Conventions", desc,
-                     VD->getLocStart(), Init->getSourceRange());
+                     VDLoc, Init->getSourceRange());
 }
 
 //===----------------------------------------------------------------------===//
@@ -279,8 +280,10 @@ void ASTFieldVisitor::ReportError(QualType T) {
   // just report warnings when we see an out-of-line method definition for a
   // class, as that heuristic doesn't always work (the complete definition of
   // the class may be in the header file, for example).
+  PathDiagnosticLocation L = PathDiagnosticLocation::createBegin(
+                               FieldChain.front(), BR.getSourceManager());
   BR.EmitBasicReport("AST node allocates heap memory", "LLVM Conventions",
-                     os.str(), FieldChain.front()->getLocStart());
+                     os.str(), L);
 }
 
 //===----------------------------------------------------------------------===//

clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp

@@ -212,8 +212,10 @@ void MallocOverflowSecurityChecker::OutputPossibleOverflows(
        ++i) {
     SourceRange R = i->mulop->getSourceRange();
     BR.EmitBasicReport("MallocOverflowSecurityChecker",
-        "the computation of the size of the memory allocation may overflow",
+      "the computation of the size of the memory allocation may overflow",
-        i->mulop->getOperatorLoc(), &R, 1);
+      PathDiagnosticLocation::createOperatorLoc(i->mulop,
+                                                BR.getSourceManager()),
+      &R, 1);
   }
 }
 

clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp

@@ -67,11 +67,15 @@ void NSAutoreleasePoolChecker::checkPreObjCMessage(ObjCMessage msg,
     return;
                      
   SourceRange R = msg.getSourceRange();
-
+  BugReporter &BR = C.getBugReporter();
+  const LocationContext *LC = C.getPredecessor()->getLocationContext();
+  const SourceManager &SM = BR.getSourceManager();
+  const Expr *E = msg.getMsgOrPropExpr();
+  PathDiagnosticLocation L = PathDiagnosticLocation::createBegin(E, SM, LC);
   C.getBugReporter().EmitBasicReport("Use -drain instead of -release",
     "API Upgrade (Apple)",
     "Use -drain instead of -release when using NSAutoreleasePool "
-    "and garbage collection", R.getBegin(), &R, 1);
+    "and garbage collection", L, &R, 1);
 }
 
 void ento::registerNSAutoreleasePoolChecker(CheckerManager &mgr) {

clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp

@@ -72,8 +72,10 @@ void NSErrorMethodChecker::checkASTDecl(const ObjCMethodDecl *D,
     const char *err = "Method accepting NSError** "
         "should have a non-void return value to indicate whether or not an "
         "error occurred";
+    PathDiagnosticLocation L =
+      PathDiagnosticLocation::create(D, BR.getSourceManager());
     BR.EmitBasicReport("Bad return type when passing NSError**",
-                       "Coding conventions (Apple)", err, D->getLocation());
+                       "Coding conventions (Apple)", err, L);
   }
 }
 
@@ -118,8 +120,10 @@ void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
     const char *err = "Function accepting CFErrorRef* "
         "should have a non-void return value to indicate whether or not an "
         "error occurred";
+    PathDiagnosticLocation L =
+      PathDiagnosticLocation::create(D, BR.getSourceManager());
     BR.EmitBasicReport("Bad return type when passing CFErrorRef*",
-                       "Coding conventions (Apple)", err, D->getLocation());
+                       "Coding conventions (Apple)", err, L);
   }
 }
 

clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp

@@ -159,8 +159,10 @@ static void checkObjCUnusedIvar(const ObjCImplementationDecl *D,
          << "' is never used by the methods in its @implementation "
             "(although it may be used by category methods).";
 
+      PathDiagnosticLocation L =
+        PathDiagnosticLocation::create(I->first, BR.getSourceManager());
       BR.EmitBasicReport("Unused instance variable", "Optimization",
-                         os.str(), I->first->getLocation());
+                         os.str(), L);
     }
 }
 

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

@@ -1756,7 +1756,6 @@ namespace {
   };
 
   class CFRefLeakReport : public CFRefReport {
-    SourceLocation AllocSite;
     const MemRegion* AllocBinding;
 
   public:
@@ -1764,7 +1763,10 @@ namespace {
                     const SummaryLogTy &Log, ExplodedNode *n, SymbolRef sym,
                     ExprEngine &Eng);
 
-    SourceLocation getLocation() const { return AllocSite; }
+    PathDiagnosticLocation getLocation(const SourceManager &SM) const {
+      assert(Location.isValid());
+      return Location;
+    }
   };
 } // end anonymous namespace
 
@@ -2219,18 +2221,20 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts,
   // same SourceLocation.
   const ExplodedNode *AllocNode = 0;
 
+  const SourceManager& SMgr = Eng.getContext().getSourceManager();
+
   llvm::tie(AllocNode, AllocBinding) =  // Set AllocBinding.
     GetAllocationSite(Eng.getStateManager(), getErrorNode(), sym);
 
   // Get the SourceLocation for the allocation site.
   ProgramPoint P = AllocNode->getLocation();
-  AllocSite = cast<PostStmt>(P).getStmt()->getLocStart();
+  const Stmt *AllocStmt = cast<PostStmt>(P).getStmt();
-
+  Location = PathDiagnosticLocation::createBegin(AllocStmt, SMgr,
+                                                  n->getLocationContext());
   // Fill in the description of the bug.
   Description.clear();
   llvm::raw_string_ostream os(Description);
-  SourceManager& SMgr = Eng.getContext().getSourceManager();
+  unsigned AllocLine = SMgr.getExpansionLineNumber(AllocStmt->getLocStart());
-  unsigned AllocLine = SMgr.getExpansionLineNumber(AllocSite);
   os << "Potential leak ";
   if (GCEnabled)
     os << "(when using garbage collection) ";

clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp

@@ -60,11 +60,12 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph &G,
 
   CFG *C = 0;
   ParentMap *PM = 0;
+  const LocationContext *LC = 0;
   // Iterate over ExplodedGraph
   for (ExplodedGraph::node_iterator I = G.nodes_begin(), E = G.nodes_end();
       I != E; ++I) {
     const ProgramPoint &P = I->getLocation();
-    const LocationContext *LC = P.getLocationContext();
+    LC = P.getLocationContext();
 
     // Save the CFG if we don't have it already
     if (!C)
@@ -128,11 +129,13 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph &G,
 
     // We found a block that wasn't covered - find the statement to report
     SourceRange SR;
+    PathDiagnosticLocation DL;
     SourceLocation SL;
     if (const Stmt *S = getUnreachableStmt(CB)) {
       SR = S->getSourceRange();
-      SL = S->getLocStart();
+      DL = PathDiagnosticLocation::createBegin(S, B.getSourceManager(), LC);
-      if (SR.isInvalid() || SL.isInvalid())
+      SL = DL.asLocation();
+      if (SR.isInvalid() || !SL.isValid())
         continue;
     }
     else
@@ -144,7 +147,7 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph &G,
       continue;
 
     B.EmitBasicReport("Unreachable code", "Dead code", "This statement is never"
-        " executed", SL, SR);
+        " executed", DL, SR);
   }
 }
 

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -435,13 +435,13 @@ public:
       return true;
 
     // Create the diagnostic.
-    FullSourceLoc L(S->getLocStart(), BR.getSourceManager());
-
     if (Loc::isLocType(VD->getType())) {
       llvm::SmallString<64> buf;
       llvm::raw_svector_ostream os(buf);
       os << '\'' << VD << "' now aliases '" << MostRecent << '\'';
-
+      PathDiagnosticLocation L =
+        PathDiagnosticLocation::createBegin(S, BR.getSourceManager(),
+                                                   Pred->getLocationContext());
       PD.push_front(new PathDiagnosticEventPiece(L, os.str()));
     }
 
@@ -533,7 +533,9 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
       if (!T)
         continue;
 
-      FullSourceLoc Start(T->getLocStart(), SMgr);
+      PathDiagnosticLocation Start =
+        PathDiagnosticLocation::createBegin(T, SMgr,
+                                                N->getLocationContext());
 
       switch (T->getStmtClass()) {
         default:
@@ -1231,8 +1233,13 @@ BugReport::~BugReport() {
 
 void BugReport::Profile(llvm::FoldingSetNodeID& hash) const {
   hash.AddPointer(&BT);
-  hash.AddInteger(getLocation().getRawEncoding());
   hash.AddString(Description);
+  if (Location.isValid()) {
+    Location.Profile(hash);
+  } else {
+    assert(ErrorNode);
+    hash.AddPointer(GetCurrentOrPreviousStmt(ErrorNode));
+  }
 
   for (SmallVectorImpl<SourceRange>::const_iterator I =
       Ranges.begin(), E = Ranges.end(); I != E; ++I) {
@@ -1280,28 +1287,29 @@ BugReport::getRanges() {
     return std::make_pair(Ranges.begin(), Ranges.end());
 }
 
-SourceLocation BugReport::getLocation() const {
+PathDiagnosticLocation BugReport::getLocation(const SourceManager &SM) const {
   if (ErrorNode) {
-    (Location.isInvalid() &&
+    assert(!Location.isValid() &&
      "Either Location or ErrorNode should be specified but not both.");
 
     if (const Stmt *S = GetCurrentOrPreviousStmt(ErrorNode)) {
+      const LocationContext *LC = ErrorNode->getLocationContext();
+
       // For member expressions, return the location of the '.' or '->'.
       if (const MemberExpr *ME = dyn_cast<MemberExpr>(S))
-        return ME->getMemberLoc();
+        return PathDiagnosticLocation::createMemberLoc(ME, SM);
       // For binary operators, return the location of the operator.
       if (const BinaryOperator *B = dyn_cast<BinaryOperator>(S))
-        return B->getOperatorLoc();
+        return PathDiagnosticLocation::createOperatorLoc(B, SM);
 
-      return S->getLocStart();
+      return PathDiagnosticLocation::createBegin(S, SM, LC);
     }
-
   } else {
     assert(Location.isValid());
     return Location;
   }
 
-  return FullSourceLoc();
+  return PathDiagnosticLocation();
 }
 
 //===----------------------------------------------------------------------===//
@@ -1564,7 +1572,9 @@ static void CompactPathDiagnostic(PathDiagnostic &PD, const SourceManager& SM) {
 
     if (!MacroGroup || ParentInstantiationLoc == MacroStack.back().second) {
       // Create a new macro group and add it to the stack.
-      PathDiagnosticMacroPiece *NewGroup = new PathDiagnosticMacroPiece(Loc);
+      PathDiagnosticMacroPiece *NewGroup =
+        new PathDiagnosticMacroPiece(
+          PathDiagnosticLocation::createSingleLocation(I->getLocation()));
 
       if (MacroGroup)
         MacroGroup->push_back(NewGroup);
@@ -1872,7 +1882,6 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
   BugReport::ranges_iterator Beg, End;
   llvm::tie(Beg, End) = exampleReport->getRanges();
   Diagnostic &Diag = getDiagnostic();
-  FullSourceLoc L(exampleReport->getLocation(), getSourceManager());
   
   // Search the description for '%', as that will be interpretted as a
   // format character by FormatDiagnostics.
@@ -1892,7 +1901,8 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
   }        
 
   {
-    DiagnosticBuilder diagBuilder = Diag.Report(L, ErrorDiag);
+    DiagnosticBuilder diagBuilder = Diag.Report(
+      exampleReport->getLocation(getSourceManager()).asLocation(), ErrorDiag);
     for (BugReport::ranges_iterator I = Beg; I != End; ++I)
       diagBuilder << *I;
   }
@@ -1902,8 +1912,9 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
     return;
 
   if (D->empty()) {
-    PathDiagnosticPiece *piece =
+    PathDiagnosticPiece *piece = new PathDiagnosticEventPiece(
-      new PathDiagnosticEventPiece(L, exampleReport->getDescription());
+                                 exampleReport->getLocation(getSourceManager()),
+                                 exampleReport->getDescription());
 
     for ( ; Beg != End; ++Beg) piece->addRange(*Beg);
     D->push_back(piece);
@@ -1913,20 +1924,19 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
 }
 
 void BugReporter::EmitBasicReport(StringRef name, StringRef str,
-                                  SourceLocation Loc,
+                                  PathDiagnosticLocation Loc,
                                   SourceRange* RBeg, unsigned NumRanges) {
   EmitBasicReport(name, "", str, Loc, RBeg, NumRanges);
 }
 
 void BugReporter::EmitBasicReport(StringRef name,
                                   StringRef category,
-                                  StringRef str, SourceLocation Loc,
+                                  StringRef str, PathDiagnosticLocation Loc,
                                   SourceRange* RBeg, unsigned NumRanges) {
 
   // 'BT' is owned by BugReporter.
   BugType *BT = getBugTypeForName(name, category);
-  FullSourceLoc L = getContext().getFullLoc(Loc);
+  BugReport *R = new BugReport(*BT, str, Loc);
-  BugReport *R = new BugReport(*BT, str, L);
   for ( ; NumRanges > 0 ; --NumRanges, ++RBeg) R->addRange(*RBeg);
   EmitReport(R);
 }

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

@@ -156,9 +156,15 @@ static SourceLocation getValidSourceLocation(const Stmt* S,
 }
 
 PathDiagnosticLocation
-  PathDiagnosticLocation::createBeginStmt(const Stmt *S,
+  PathDiagnosticLocation::createBegin(const Decl *D,
-                                          const SourceManager &SM,
+                                      const SourceManager &SM) {
-                                          LocationOrAnalysisContext LAC) {
+  return PathDiagnosticLocation(D->getLocStart(), SM, SingleLocK);
+}
+
+PathDiagnosticLocation
+  PathDiagnosticLocation::createBegin(const Stmt *S,
+                                      const SourceManager &SM,
+                                      LocationOrAnalysisContext LAC) {
   return PathDiagnosticLocation(getValidSourceLocation(S, LAC),
                                 SM, SingleLocK);
 }
@@ -169,6 +175,12 @@ PathDiagnosticLocation
   return PathDiagnosticLocation(BO->getOperatorLoc(), SM, SingleLocK);
 }
 
+PathDiagnosticLocation
+  PathDiagnosticLocation::createMemberLoc(const MemberExpr *ME,
+                                          const SourceManager &SM) {
+  return PathDiagnosticLocation(ME->getMemberLoc(), SM, SingleLocK);
+}
+
 PathDiagnosticLocation
   PathDiagnosticLocation::createBeginBrace(const CompoundStmt *CS,
                                            const SourceManager &SM) {