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