o.gyorgy created this revision.
o.gyorgy added reviewers: zaks.anna, xazax.hun, dcoughlin, jordan_rose.
o.gyorgy added a subscriber: cfe-commits.

Fixing IssueHash generation.
There were some problems with the current version.

If the Decl *D pointer is nullptr the NormalizeLine would crash while getting 
the LangOptions. The required LangOptions can be provided by the HTML or Plist 
Diagnostics.

It is possible that the column number provided by the find_first_not_of is 0 in 
that case the Lexer reads up a wrong source line (translateLineCol requires 
that column numbering should start at 1).

I think there should be an assert checking the line and col values in the 
sourcemanagers translateLineCol. The same way as in translateFileLineCol which 
does this check.

http://reviews.llvm.org/D14919

Files:
  include/clang/StaticAnalyzer/Core/IssueHash.h
  lib/Basic/SourceManager.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lib/StaticAnalyzer/Core/IssueHash.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  test/Analysis/bug_hash_test.cpp
  test/Analysis/diagnostics/report-issues-within-main-file.cpp

Index: test/Analysis/diagnostics/report-issues-within-main-file.cpp
===================================================================
--- test/Analysis/diagnostics/report-issues-within-main-file.cpp
+++ test/Analysis/diagnostics/report-issues-within-main-file.cpp
@@ -949,7 +949,7 @@
 // CHECK-NEXT:   <key>type</key><string>Bad deallocator</string>
 // CHECK-NEXT:   <key>check_name</key><string>unix.MismatchedDeallocator</string>
 // CHECK-NEXT:   <!-- This hash is experimental and going to change! -->
-// CHECK-NEXT:   <key>issue_hash_content_of_line_in_context</key><string>f21ac032efaa3d1459a5ed31f0ad44f0</string>
+// CHECK-NEXT:   <key>issue_hash_content_of_line_in_context</key><string>f689fbd54138491e228f0f89bb02bfb2</string>
 // CHECK-NEXT:  <key>issue_context_kind</key><string>function</string>
 // CHECK-NEXT:  <key>issue_context</key><string>mainPlusHeader</string>
 // CHECK-NEXT:  <key>issue_hash_function_offset</key><string>2</string>
Index: test/Analysis/bug_hash_test.cpp
===================================================================
--- test/Analysis/bug_hash_test.cpp
+++ test/Analysis/bug_hash_test.cpp
@@ -288,17 +288,17 @@
 // CHECK-NEXT:     </array>
 // CHECK-NEXT:     <key>depth</key><integer>0</integer>
 // CHECK-NEXT:     <key>extended_message</key>
-// CHECK-NEXT:     <string>debug.DumpBugHash$int f()$28$namespaceAA{$debug</string>
+// CHECK-NEXT:     <string>debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug</string>
 // CHECK-NEXT:     <key>message</key>
-// CHECK-NEXT:     <string>debug.DumpBugHash$int f()$28$namespaceAA{$debug</string>
+// CHECK-NEXT:     <string>debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug</string>
 // CHECK-NEXT:    </dict>
 // CHECK-NEXT:   </array>
-// CHECK-NEXT:   <key>description</key><string>debug.DumpBugHash$int f()$28$namespaceAA{$debug</string>
+// CHECK-NEXT:   <key>description</key><string>debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug</string>
 // CHECK-NEXT:   <key>category</key><string>debug</string>
 // CHECK-NEXT:   <key>type</key><string>Dump hash components</string>
 // CHECK-NEXT:   <key>check_name</key><string>debug.DumpBugHash</string>
 // CHECK-NEXT:   <!-- This hash is experimental and going to change! -->
-// CHECK-NEXT:   <key>issue_hash_content_of_line_in_context</key><string>f8ee38da3de42e209c4afa886b5531ab</string>
+// CHECK-NEXT:   <key>issue_hash_content_of_line_in_context</key><string>f5471f52854dc14167fe96db50c4ba5f</string>
 // CHECK-NEXT:  <key>issue_context_kind</key><string>function</string>
 // CHECK-NEXT:  <key>issue_context</key><string>f</string>
 // CHECK-NEXT:  <key>issue_hash_function_offset</key><string>0</string>
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -399,7 +399,7 @@
                     *SM);
     const Decl *DeclWithIssue = D->getDeclWithIssue();
     EmitString(o, GetIssueHash(*SM, L, D->getCheckName(), D->getBugType(),
-                               DeclWithIssue))
+                               DeclWithIssue, LangOpts))
         << '\n';
 
     // Output information about the semantic context where
Index: lib/StaticAnalyzer/Core/IssueHash.cpp
===================================================================
--- lib/StaticAnalyzer/Core/IssueHash.cpp
+++ lib/StaticAnalyzer/Core/IssueHash.cpp
@@ -127,14 +127,13 @@
 }
 
 static std::string NormalizeLine(const SourceManager &SM, FullSourceLoc &L,
-                                 const Decl *D) {
+                                 const LangOptions &LangOpts) {
   static StringRef Whitespaces = " \t\n";
 
-  const LangOptions &Opts = D->getASTContext().getLangOpts();
   StringRef Str = GetNthLineOfFile(SM.getBuffer(L.getFileID(), L),
                                    L.getExpansionLineNumber());
   unsigned col = Str.find_first_not_of(Whitespaces);
-
+  col++;
   SourceLocation StartOfLine =
       SM.translateLineCol(SM.getFileID(L), L.getExpansionLineNumber(), col);
   llvm::MemoryBuffer *Buffer =
@@ -145,7 +144,7 @@
   const char *BufferPos = SM.getCharacterData(StartOfLine);
 
   Token Token;
-  Lexer Lexer(SM.getLocForStartOfFile(SM.getFileID(StartOfLine)), Opts,
+  Lexer Lexer(SM.getLocForStartOfFile(SM.getFileID(StartOfLine)), LangOpts,
               Buffer->getBufferStart(), BufferPos, Buffer->getBufferEnd());
 
   size_t NextStart = 0;
@@ -175,13 +174,14 @@
 std::string clang::GetIssueString(const SourceManager &SM,
                                   FullSourceLoc &IssueLoc,
                                   StringRef CheckerName, StringRef BugType,
-                                  const Decl *D) {
+                                  const Decl *D,
+                                  const LangOptions &LangOpts) {
   static StringRef Delimiter = "$";
 
   return (llvm::Twine(CheckerName) + Delimiter +
           GetEnclosingDeclContextSignature(D) + Delimiter +
           llvm::utostr(IssueLoc.getExpansionColumnNumber()) + Delimiter +
-          NormalizeLine(SM, IssueLoc, D) + Delimiter + BugType)
+          NormalizeLine(SM, IssueLoc, LangOpts) + Delimiter + BugType)
       .str();
 }
 
@@ -188,7 +188,9 @@
 SmallString<32> clang::GetIssueHash(const SourceManager &SM,
                                     FullSourceLoc &IssueLoc,
                                     StringRef CheckerName, StringRef BugType,
-                                    const Decl *D) {
+                                    const Decl *D,
+                                    const LangOptions &LangOpts) {
+
   return GetHashOfContent(
-      GetIssueString(SM, IssueLoc, CheckerName, BugType, D));
+      GetIssueString(SM, IssueLoc, CheckerName, BugType, D, LangOpts));
 }
Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -255,7 +255,7 @@
     os  << "\n<!-- FUNCTIONNAME " <<  declName << " -->\n";
 
     os << "\n<!-- ISSUEHASHCONTENTOFLINEINCONTEXT "
-       << GetIssueHash(SMgr, L, D.getCheckName(), D.getBugType(), DeclWithIssue)
+       << GetIssueHash(SMgr, L, D.getCheckName(), D.getBugType(), DeclWithIssue, PP.getLangOpts())
        << " -->\n";
 
     os << "\n<!-- BUGLINE "
Index: lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
+++ lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
@@ -230,11 +230,12 @@
     if (!N)
       return;
 
+    const LangOptions &Opts = C.getLangOpts();
     const SourceManager &SM = C.getSourceManager();
     FullSourceLoc FL(S->getLocStart(), SM);
     std::string HashContent =
         GetIssueString(SM, FL, getCheckName().getName(), BT->getCategory(),
-                       C.getLocationContext()->getDecl());
+                       C.getLocationContext()->getDecl(), Opts);
 
     C.emitReport(llvm::make_unique<BugReport>(*BT, HashContent, N));
   }
Index: lib/Basic/SourceManager.cpp
===================================================================
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1718,7 +1718,7 @@
                                                unsigned Col) const {
   // Lines are used as a one-based index into a zero-based array. This assert
   // checks for possible buffer underruns.
-  assert(Line != 0 && "Passed a zero-based line");
+  assert(Line && Col && "Line and column should start from 1!");
 
   if (FID.isInvalid())
     return SourceLocation();
Index: include/clang/StaticAnalyzer/Core/IssueHash.h
===================================================================
--- include/clang/StaticAnalyzer/Core/IssueHash.h
+++ include/clang/StaticAnalyzer/Core/IssueHash.h
@@ -15,6 +15,7 @@
 class Decl;
 class SourceManager;
 class FullSourceLoc;
+class LangOptions;
 
 /// \brief Get an MD5 hash to help identify bugs.
 ///
@@ -37,13 +38,14 @@
 llvm::SmallString<32> GetIssueHash(const SourceManager &SM,
                                    FullSourceLoc &IssueLoc,
                                    llvm::StringRef CheckerName,
-                                   llvm::StringRef BugType, const Decl *D);
+                                   llvm::StringRef BugType, const Decl *D,
+                                   const LangOptions& LangOpts);
 
 /// \brief Get the string representation of issue hash. See GetIssueHash() for
 /// more information.
 std::string GetIssueString(const SourceManager &SM, FullSourceLoc &IssueLoc,
                            llvm::StringRef CheckerName, llvm::StringRef BugType,
-                           const Decl *D);
+                           const Decl *D, const LangOptions& LangOpts);
 } // namespace clang
 
 #endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to