Re: [PATCH] D14919: Fix IssueHash generation

2015-12-01 Thread Gábor Horváth via cfe-commits
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

2015-11-30 Thread Anna Zaks via cfe-commits
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

2015-11-26 Thread Sean Eveson via cfe-commits
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

2015-11-26 Thread Gábor Horváth via cfe-commits
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

2015-11-25 Thread Orbán György via cfe-commits
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 Eveson  wrote:
> 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

2015-11-24 Thread Gábor Horváth via cfe-commits
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

2015-11-24 Thread Gyorgy Orban via cfe-commits
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

2015-11-23 Thread Gyorgy Orban via cfe-commits
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

2015-11-23 Thread Gyorgy Orban via cfe-commits
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) +