Re: [PATCH] D14919: Fix IssueHash generation
xazax.hun closed this revision. xazax.hun added a comment. Committed in r254394. http://reviews.llvm.org/D14919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14919: Fix IssueHash generation
zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM. http://reviews.llvm.org/D14919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14919: Fix IssueHash generation
seaneveson added a comment. In http://reviews.llvm.org/D14919#296597, @o.gyorgy wrote: > I did not create a test checker for the NormalizeLine error in the patch. > Should I add a test checker for this? I was wondering what sort of source code causes the crash, but I do think it would be good to have a regression test. > Do you have any suggestions where to put it if required? Maybe in bug_hash_test.cpp, although I'm not sure its necessary to check the plist output when testing for a crash. http://reviews.llvm.org/D14919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14919: Fix IssueHash generation
xazax.hun added a comment. In http://reviews.llvm.org/D14919#297168, @seaneveson wrote: > I was wondering what sort of source code causes the crash, but I do think it > would be good to have a regression test. It is possible to generate bug reports without associated declaration, altough it is not advies. I think the reason why it is possible, to give flexibility. E.g.: one could report issues regarding documentation. http://reviews.llvm.org/D14919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14919: Fix IssueHash generation
I did not create a test checker for the NormalizeLine error in the patch. Should I add a test checker for this? Do you have any suggestions where to put it if required? GetEnclosingDeclContextSignature already checks for nullptr, this should have been done in the NormalizeLine function also, but in that case it is not enough because the Lexer needs the LangOpts. On Tue, Nov 24, 2015 at 5:18 PM, Sean Evesonwrote: > seaneveson added a subscriber: seaneveson. > seaneveson added a comment. > >> If the Decl *D pointer is nullptr the NormalizeLine would crash while >> getting the LangOptions. > > > Do you have a reproducible test case for this? > > > http://reviews.llvm.org/D14919 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14919: Fix IssueHash generation
xazax.hun added inline comments. Comment at: include/clang/StaticAnalyzer/Core/IssueHash.h:42 @@ -41,1 +41,3 @@ + llvm::StringRef BugType, const Decl *D, + const LangOptions& LangOpts); Please put & next to he variable name, here and some other places. http://reviews.llvm.org/D14919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14919: Fix IssueHash generation
o.gyorgy updated this revision to Diff 41024. o.gyorgy added a comment. Some small format changes. Based on the review. 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: typeBad deallocator // CHECK-NEXT: check_nameunix.MismatchedDeallocator // CHECK-NEXT: -// CHECK-NEXT: issue_hash_content_of_line_in_contextf21ac032efaa3d1459a5ed31f0ad44f0 +// CHECK-NEXT: issue_hash_content_of_line_in_contextf689fbd54138491e228f0f89bb02bfb2 // CHECK-NEXT: issue_context_kindfunction // CHECK-NEXT: issue_contextmainPlusHeader // CHECK-NEXT: issue_hash_function_offset2 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: // CHECK-NEXT: depth0 // CHECK-NEXT: extended_message -// CHECK-NEXT: debug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: message -// CHECK-NEXT: debug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: descriptiondebug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: descriptiondebug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: categorydebug // CHECK-NEXT: typeDump hash components // CHECK-NEXT: check_namedebug.DumpBugHash // CHECK-NEXT: -// CHECK-NEXT: issue_hash_content_of_line_in_contextf8ee38da3de42e209c4afa886b5531ab +// CHECK-NEXT: issue_hash_content_of_line_in_contextf5471f52854dc14167fe96db50c4ba5f // CHECK-NEXT: issue_context_kindfunction // CHECK-NEXT: issue_contextf // CHECK-NEXT: issue_hash_function_offset0 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 , FullSourceLoc , - const Decl *D) { + const LangOptions ) { static StringRef Whitespaces = " \t\n"; - const LangOptions = 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,20 +174,23 @@ std::string clang::GetIssueString(const SourceManager , FullSourceLoc , StringRef CheckerName, StringRef BugType, - const Decl *D) { + const Decl *D, + const LangOptions ) { 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)
[PATCH] D14919: Fix IssueHash generation
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: typeBad deallocator // CHECK-NEXT: check_nameunix.MismatchedDeallocator // CHECK-NEXT: -// CHECK-NEXT: issue_hash_content_of_line_in_contextf21ac032efaa3d1459a5ed31f0ad44f0 +// CHECK-NEXT: issue_hash_content_of_line_in_contextf689fbd54138491e228f0f89bb02bfb2 // CHECK-NEXT: issue_context_kindfunction // CHECK-NEXT: issue_contextmainPlusHeader // CHECK-NEXT: issue_hash_function_offset2 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: // CHECK-NEXT: depth0 // CHECK-NEXT: extended_message -// CHECK-NEXT: debug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: message -// CHECK-NEXT: debug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: descriptiondebug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: descriptiondebug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: categorydebug // CHECK-NEXT: typeDump hash components // CHECK-NEXT: check_namedebug.DumpBugHash // CHECK-NEXT: -// CHECK-NEXT: issue_hash_content_of_line_in_contextf8ee38da3de42e209c4afa886b5531ab +// CHECK-NEXT: issue_hash_content_of_line_in_contextf5471f52854dc14167fe96db50c4ba5f // CHECK-NEXT: issue_context_kindfunction // CHECK-NEXT: issue_contextf // CHECK-NEXT: issue_hash_function_offset0 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 , FullSourceLoc , - const Decl *D) { + const LangOptions ) { static StringRef Whitespaces = " \t\n"; - const LangOptions = 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 ,
Re: [PATCH] D14919: Fix IssueHash generation
o.gyorgy updated this revision to Diff 40920. o.gyorgy added a comment. Regenerate patch with context and clang format HTMLDiagnostics.cpp. 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: typeBad deallocator // CHECK-NEXT: check_nameunix.MismatchedDeallocator // CHECK-NEXT: -// CHECK-NEXT: issue_hash_content_of_line_in_contextf21ac032efaa3d1459a5ed31f0ad44f0 +// CHECK-NEXT: issue_hash_content_of_line_in_contextf689fbd54138491e228f0f89bb02bfb2 // CHECK-NEXT: issue_context_kindfunction // CHECK-NEXT: issue_contextmainPlusHeader // CHECK-NEXT: issue_hash_function_offset2 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: // CHECK-NEXT: depth0 // CHECK-NEXT: extended_message -// CHECK-NEXT: debug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: message -// CHECK-NEXT: debug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: debug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: descriptiondebug.DumpBugHash$int f()$28$namespaceAA{$debug +// CHECK-NEXT: descriptiondebug.DumpBugHash$int f()$28$constexprintf(){return5;}$debug // CHECK-NEXT: categorydebug // CHECK-NEXT: typeDump hash components // CHECK-NEXT: check_namedebug.DumpBugHash // CHECK-NEXT: -// CHECK-NEXT: issue_hash_content_of_line_in_contextf8ee38da3de42e209c4afa886b5531ab +// CHECK-NEXT: issue_hash_content_of_line_in_contextf5471f52854dc14167fe96db50c4ba5f // CHECK-NEXT: issue_context_kindfunction // CHECK-NEXT: issue_contextf // CHECK-NEXT: issue_hash_function_offset0 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 , FullSourceLoc , - const Decl *D) { + const LangOptions ) { static StringRef Whitespaces = " \t\n"; - const LangOptions = 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,20 +174,23 @@ std::string clang::GetIssueString(const SourceManager , FullSourceLoc , StringRef CheckerName, StringRef BugType, - const Decl *D) { + const Decl *D, + const LangOptions ) { 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) +