[cfe-commits] r162965 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h lib/StaticAnalyzer/Core/BugReporter.cpp lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp lib/StaticAnalyzer/Core/PathDiagnostic.cpp lib/StaticAnalyzer/Core/PlistDiagnostics.cpp lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/Analysis/plist-html-macros.c

Jordan Rose jordan_rose at apple.com
Thu Aug 30 17:36:27 PDT 2012


Author: jrose
Date: Thu Aug 30 19:36:26 2012
New Revision: 162965

URL: http://llvm.org/viewvc/llvm-project?rev=162965&view=rev
Log:
[analyzer] Ensure that PathDiagnostics profile the same regardless of path.

PathDiagnostics are actually profiled and uniqued independently of the
path on which the bug occurred. This is used to merge diagnostics that
refer to the same issue along different paths, as well as by the plist
diagnostics to reference files created by the HTML diagnostics.

However, there are two problems with the current implementation:

1) The bug description is included in the profile, but some
   PathDiagnosticConsumers prefer abbreviated descriptions and some
   prefer verbose descriptions. Fixed by including both descriptions in
   the PathDiagnostic objects and always using the verbose one in the profile.

2) The "minimal" path generation scheme provides extra information about
   which events came from macros that the "extensive" scheme does not.
   This resulted not only in different locations for the plist and HTML
   diagnostics, but also in diagnostics being uniqued in the plist output
   but not in the HTML output. Fixed by storing the "end path" location
   explicitly in the PathDiagnostic object, rather than trying to find the
   last piece of the path when the diagnostic is requested.

This should hopefully finish unsticking our internal buildbot.

Added:
    cfe/trunk/test/Analysis/plist-html-macros.c
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Thu Aug 30 19:36:26 2012
@@ -163,8 +163,10 @@
 
   const StringRef getDescription() const { return Description; }
 
-  const StringRef getShortDescription() const {
-    return ShortDescription.empty() ? Description : ShortDescription;
+  const StringRef getShortDescription(bool UseFallback = true) const {
+    if (ShortDescription.empty() && UseFallback)
+      return Description;
+    return ShortDescription;
   }
 
   /// Indicates whether or not any path pruning should take place

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h Thu Aug 30 19:36:26 2012
@@ -97,7 +97,6 @@
   virtual PathGenerationScheme getGenerationScheme() const { return Minimal; }
   virtual bool supportsLogicalOpControlFlow() const { return false; }
   virtual bool supportsAllBlockEdges() const { return false; }
-  virtual bool useVerboseDescription() const { return true; }
   
   /// Return true if the PathDiagnosticConsumer supports individual
   /// PathDiagnostics that span multiple files.
@@ -653,14 +652,22 @@
 class PathDiagnostic : public llvm::FoldingSetNode {
   const Decl *DeclWithIssue;
   std::string BugType;
-  std::string Desc;
+  std::string VerboseDesc;
+  std::string ShortDesc;
   std::string Category;
   std::deque<std::string> OtherDesc;
+  PathDiagnosticLocation Loc;
   PathPieces pathImpl;
   llvm::SmallVector<PathPieces *, 3> pathStack;
   
   PathDiagnostic(); // Do not implement.
 public:
+  PathDiagnostic(const Decl *DeclWithIssue, StringRef bugtype,
+                 StringRef verboseDesc, StringRef shortDesc,
+                 StringRef category);
+
+  ~PathDiagnostic();
+  
   const PathPieces &path;
 
   /// Return the path currently used by builders for constructing the 
@@ -683,16 +690,24 @@
   void popActivePath() { if (!pathStack.empty()) pathStack.pop_back(); }
 
   bool isWithinCall() const { return !pathStack.empty(); }
-  
-  //  PathDiagnostic();
-  PathDiagnostic(const Decl *DeclWithIssue,
-                 StringRef bugtype,
-                 StringRef desc,
-                 StringRef category);
-
-  ~PathDiagnostic();
 
-  StringRef getDescription() const { return Desc; }
+  void setEndOfPath(PathDiagnosticPiece *EndPiece) {
+    assert(!Loc.isValid() && "End location already set!");
+    Loc = EndPiece->getLocation();
+    assert(Loc.isValid() && "Invalid location for end-of-path piece");
+    getActivePath().push_back(EndPiece);
+  }
+
+  void resetPath() {
+    pathStack.clear();
+    pathImpl.clear();
+    Loc = PathDiagnosticLocation();
+  }
+  
+  StringRef getVerboseDescription() const { return VerboseDesc; }
+  StringRef getShortDescription() const {
+    return ShortDesc.empty() ? VerboseDesc : ShortDesc;
+  }
   StringRef getBugType() const { return BugType; }
   StringRef getCategory() const { return Category; }
 
@@ -706,15 +721,27 @@
   meta_iterator meta_end() const { return OtherDesc.end(); }
   void addMeta(StringRef s) { OtherDesc.push_back(s); }
 
-  PathDiagnosticLocation getLocation() const;
+  PathDiagnosticLocation getLocation() const {
+    assert(Loc.isValid() && "No end-of-path location set yet!");
+    return Loc;
+  }
 
   void flattenLocations() {
+    Loc.flatten();
     for (PathPieces::iterator I = pathImpl.begin(), E = pathImpl.end(); 
          I != E; ++I) (*I)->flattenLocations();
   }
-  
+
+  /// Profiles the diagnostic, independent of the path it references.
+  ///
+  /// This can be used to merge diagnostics that refer to the same issue
+  /// along different paths.
   void Profile(llvm::FoldingSetNodeID &ID) const;
-  
+
+  /// Profiles the diagnostic, including its path.
+  ///
+  /// Two diagnostics with the same issue along different paths will generate
+  /// different profiles.
   void FullProfile(llvm::FoldingSetNodeID &ID) const;
 };  
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Thu Aug 30 19:36:26 2012
@@ -1898,7 +1898,7 @@
        visitors.push_back((*I)->clone());
 
     // Clear out the active path from any previous work.
-    PD.getActivePath().clear();
+    PD.resetPath();
     originalReportConfigToken = R->getConfigurationChangeToken();
 
     // Generate the very last diagnostic piece - the piece is visible before 
@@ -1915,7 +1915,7 @@
     if (!LastPiece)
       LastPiece = BugReporterVisitor::getDefaultEndPath(PDB, N, *R);
     if (LastPiece)
-      PD.getActivePath().push_back(LastPiece);
+      PD.setEndOfPath(LastPiece);
     else
       return;
 
@@ -2106,9 +2106,8 @@
   OwningPtr<PathDiagnostic>
     D(new PathDiagnostic(exampleReport->getDeclWithIssue(),
                          exampleReport->getBugType().getName(),
-                         PD.useVerboseDescription()
-                         ? exampleReport->getDescription() 
-                         : exampleReport->getShortDescription(),
+                         exampleReport->getDescription(),
+                         exampleReport->getShortDescription(/*Fallback=*/false),
                          BT.getCategory()));
 
   // Generate the full path diagnostic, using the generation scheme
@@ -2128,7 +2127,7 @@
     llvm::tie(Beg, End) = exampleReport->getRanges();
     for ( ; Beg != End; ++Beg)
       piece->addRange(*Beg);
-    D->getActivePath().push_back(piece);
+    D->setEndOfPath(piece);
   }
 
   // Get the meta data.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp Thu Aug 30 19:36:26 2012
@@ -189,7 +189,7 @@
       << (*path.rbegin())->getLocation().asLocation().getExpansionColumnNumber()
       << "</a></td></tr>\n"
          "<tr><td class=\"rowname\">Description:</td><td>"
-      << D.getDescription() << "</td></tr>\n";
+      << D.getVerboseDescription() << "</td></tr>\n";
 
     // Output any other meta data.
 
@@ -209,15 +209,15 @@
     std::string s;
     llvm::raw_string_ostream os(s);
 
-    const std::string& BugDesc = D.getDescription();
+    StringRef BugDesc = D.getVerboseDescription();
     if (!BugDesc.empty())
       os << "\n<!-- BUGDESC " << BugDesc << " -->\n";
 
-    const std::string& BugType = D.getBugType();
+    StringRef BugType = D.getBugType();
     if (!BugType.empty())
       os << "\n<!-- BUGTYPE " << BugType << " -->\n";
 
-    const std::string& BugCategory = D.getCategory();
+    StringRef BugCategory = D.getCategory();
     if (!BugCategory.empty())
       os << "\n<!-- BUGCATEGORY " << BugCategory << " -->\n";
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Thu Aug 30 19:36:26 2012
@@ -104,11 +104,12 @@
 PathDiagnostic::~PathDiagnostic() {}
 
 PathDiagnostic::PathDiagnostic(const Decl *declWithIssue,
-                               StringRef bugtype, StringRef desc,
-                               StringRef category)
+                               StringRef bugtype, StringRef verboseDesc,
+                               StringRef shortDesc, StringRef category)
   : DeclWithIssue(declWithIssue),
     BugType(StripTrailingDots(bugtype)),
-    Desc(StripTrailingDots(desc)),
+    VerboseDesc(StripTrailingDots(verboseDesc)),
+    ShortDesc(StripTrailingDots(shortDesc)),
     Category(StripTrailingDots(category)),
     path(pathImpl) {}
 
@@ -227,8 +228,8 @@
       return false;
     
     // Next, compare by bug description.
-    StringRef XDesc = X->getDescription();
-    StringRef YDesc = Y->getDescription();
+    StringRef XDesc = X->getVerboseDescription();
+    StringRef YDesc = Y->getVerboseDescription();
     if (XDesc < YDesc)
       return true;
     if (XDesc != YDesc)
@@ -271,18 +272,11 @@
   }
 }
 
-static void ProfileDiagnostic(const PathDiagnostic &PD,
-                              llvm::FoldingSetNodeID &NodeID) {
-  NodeID.AddString(PD.getBugType());
-  NodeID.AddString(PD.getDescription());
-  NodeID.Add(PD.getLocation());
-}
-
 void PathDiagnosticConsumer::FilesMade::addDiagnostic(const PathDiagnostic &PD,
                                                       StringRef ConsumerName,
                                                       StringRef FileName) {
   llvm::FoldingSetNodeID NodeID;
-  ProfileDiagnostic(PD, NodeID);
+  NodeID.Add(PD);
   void *InsertPos;
   PDFileEntry *Entry = FindNodeOrInsertPos(NodeID, InsertPos);
   if (!Entry) {
@@ -303,7 +297,7 @@
 PathDiagnosticConsumer::PDFileEntry::ConsumerFiles *
 PathDiagnosticConsumer::FilesMade::getFiles(const PathDiagnostic &PD) {
   llvm::FoldingSetNodeID NodeID;
-  ProfileDiagnostic(PD, NodeID);
+  NodeID.Add(PD);
   void *InsertPos;
   PDFileEntry *Entry = FindNodeOrInsertPos(NodeID, InsertPos);
   if (!Entry)
@@ -627,24 +621,6 @@
   }
 }
 
-PathDiagnosticLocation PathDiagnostic::getLocation() const {
-  assert(path.size() > 0 &&
-         "getLocation() requires a non-empty PathDiagnostic.");
-  
-  PathDiagnosticPiece *p = path.rbegin()->getPtr();
-  
-  while (true) {
-    if (PathDiagnosticCallPiece *cp = dyn_cast<PathDiagnosticCallPiece>(p)) {
-      assert(!cp->path.empty());
-      p = cp->path.rbegin()->getPtr();
-      continue;
-    }
-    break;
-  }
-  
-  return p->getLocation();
-}
-
 //===----------------------------------------------------------------------===//
 // Manipulation of PathDiagnosticCallPieces.
 //===----------------------------------------------------------------------===//
@@ -793,10 +769,9 @@
 }
 
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
-  if (!path.empty())
-    getLocation().Profile(ID);
+  ID.Add(getLocation());
   ID.AddString(BugType);
-  ID.AddString(Desc);
+  ID.AddString(VerboseDesc);
   ID.AddString(Category);
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Thu Aug 30 19:36:26 2012
@@ -47,7 +47,6 @@
     PathGenerationScheme getGenerationScheme() const { return Extensive; }
     bool supportsLogicalOpControlFlow() const { return true; }
     bool supportsAllBlockEdges() const { return true; }
-    virtual bool useVerboseDescription() const { return false; }
     virtual bool supportsCrossFileDiagnostics() const {
       return SupportsCrossFileDiagnostics;
     }
@@ -443,7 +442,7 @@
 
     // Output the bug type and bug category.
     o << "   <key>description</key>";
-    EmitString(o, D->getDescription()) << '\n';
+    EmitString(o, D->getShortDescription()) << '\n';
     o << "   <key>category</key>";
     EmitString(o, D->getCategory()) << '\n';
     o << "   <key>type</key>";

Modified: cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp Thu Aug 30 19:36:26 2012
@@ -41,7 +41,6 @@
   PathGenerationScheme getGenerationScheme() const { return Minimal; }
   bool supportsLogicalOpControlFlow() const { return true; }
   bool supportsAllBlockEdges() const { return true; }
-  virtual bool useVerboseDescription() const { return true; }
   virtual bool supportsCrossFileDiagnostics() const { return true; }
 };
 

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=162965&r1=162964&r2=162965&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Thu Aug 30 19:36:26 2012
@@ -78,7 +78,6 @@
   ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag) : Diag(Diag) {}
   virtual ~ClangDiagPathDiagConsumer() {}
   virtual StringRef getName() const { return "ClangDiags"; }
-  virtual bool useVerboseDescription() const { return false; }
   virtual PathGenerationScheme getGenerationScheme() const { return None; }
 
   void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
@@ -86,7 +85,7 @@
     for (std::vector<const PathDiagnostic*>::iterator I = Diags.begin(),
          E = Diags.end(); I != E; ++I) {
       const PathDiagnostic *PD = *I;
-      StringRef desc = PD->getDescription();
+      StringRef desc = PD->getShortDescription();
       SmallString<512> TmpStr;
       llvm::raw_svector_ostream Out(TmpStr);
       for (StringRef::iterator I=desc.begin(), E=desc.end(); I!=E; ++I) {

Added: cfe/trunk/test/Analysis/plist-html-macros.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plist-html-macros.c?rev=162965&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/plist-html-macros.c (added)
+++ cfe/trunk/test/Analysis/plist-html-macros.c Thu Aug 30 19:36:26 2012
@@ -0,0 +1,31 @@
+// REQUIRES: shell
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
+// (sanity check)
+
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=plist-html -o %t.dir/index.plist %s
+// RUN: ls %t.dir | grep \\.html | count 1
+// RUN: grep \\.html %t.dir/index.plist | count 1
+
+// This tests two things: that the two calls to null_deref below are coalesced
+// into a single bug by both the plist and HTML diagnostics, and that the plist
+// diagnostics have a reference to the HTML diagnostics. (It would be nice to
+// check more carefully that the two actually match, but that's hard to write
+// in a lit RUN line.)
+
+#define CALL_FN(a) null_deref(a)
+
+void null_deref(int *a) {
+  if (a)
+    return;
+  *a = 1; // expected-warning{{null}}
+}
+
+void test1() {
+  CALL_FN(0);
+}
+
+void test2(int *p) {
+  CALL_FN(p);
+}





More information about the cfe-commits mailing list