[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
probinson marked an inline comment as done.
Closed by commit rL11: [DebugInfo] Don't bother with MD5 checksums of 
preprocessed files. (authored by probinson, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47260?vs=148663&id=148668#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47260

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/CodeGen/CGDebugInfo.h
  cfe/trunk/test/CodeGen/md5-checksum-crash.c


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
   DBuilder(CGM.getModule()) {
   for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
 DebugPrefixMap[KV.first] = KV.second;
+  EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+  CGM.getCodeGenOpts().DwarfVersion >= 5;
   CreateCompileUnit();
 }
 
@@ -365,15 +367,21 @@
 CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
   Checksum.clear();
 
-  if (!CGM.getCodeGenOpts().EmitCodeView &&
-  CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!EmitFileChecksums)
 return None;
 
   SourceManager &SM = CGM.getContext().getSourceManager();
   bool Invalid;
-  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
-  if (Invalid)
+  const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+  if (Invalid || !Entry.isFile())
 return None;
+  if (Entry.getFile().hasLineDirectives()) {
+// This must be a preprocessed file; its content won't match the original
+// source; therefore checksumming the text we have is pointless or wrong.
+EmitFileChecksums = false;
+return None;
+  }
+  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
 
   llvm::MD5 Hash;
   llvm::MD5::MD5Result Result;
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.h
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
   CodeGenModule &CGM;
   const codegenoptions::DebugInfoKind DebugKind;
   bool DebugTypeExtRefs;
+  mutable bool EmitFileChecksums;
   llvm::DIBuilder DBuilder;
   llvm::DICompileUnit *TheCU = nullptr;
   ModuleMap *ClangModuleMap = nullptr;
Index: cfe/trunk/test/CodeGen/md5-checksum-crash.c
===
--- cfe/trunk/test/CodeGen/md5-checksum-crash.c
+++ cfe/trunk/test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited 
-dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited 
%s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
   DBuilder(CGM.getModule()) {
   for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
 DebugPrefixMap[KV.first] = KV.second;
+  EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+  CGM.getCodeGenOpts().DwarfVersion >= 5;
   CreateCompileUnit();
 }
 
@@ -365,15 +367,21 @@
 CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
   Checksum.clear();
 
-  if (!CGM.getCodeGenOpts().EmitCodeView &&
-  CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!EmitFileChecksums)
 return None;
 
   SourceManager &SM = CGM.getContext().getSourceManager();
   bool Invalid;
-  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
-  if (Invalid)
+  const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+  if (Invalid || !Entry.isFile())
 return None;
+  if (Entry.getFile().hasLineDirectives()) {
+// This must be a preprocessed file; its content won't match the original
+// source; therefore checksumming the text we have is pointless or wrong.
+EmitFileChecksums = false;
+return None;
+  }
+  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
 
   llvm::MD5 Hash;
   llvm::MD5::MD5Result Result;
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.h
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
   Cod

[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
probinson marked an inline comment as done.
Closed by commit rC11: [DebugInfo] Don't bother with MD5 checksums of 
preprocessed files. (authored by probinson, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47260?vs=148663&id=148667#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47260

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  test/CodeGen/md5-checksum-crash.c


Index: test/CodeGen/md5-checksum-crash.c
===
--- test/CodeGen/md5-checksum-crash.c
+++ test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited 
-dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited 
%s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
   DBuilder(CGM.getModule()) {
   for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
 DebugPrefixMap[KV.first] = KV.second;
+  EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+  CGM.getCodeGenOpts().DwarfVersion >= 5;
   CreateCompileUnit();
 }
 
@@ -365,15 +367,21 @@
 CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
   Checksum.clear();
 
-  if (!CGM.getCodeGenOpts().EmitCodeView &&
-  CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!EmitFileChecksums)
 return None;
 
   SourceManager &SM = CGM.getContext().getSourceManager();
   bool Invalid;
-  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
-  if (Invalid)
+  const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+  if (Invalid || !Entry.isFile())
 return None;
+  if (Entry.getFile().hasLineDirectives()) {
+// This must be a preprocessed file; its content won't match the original
+// source; therefore checksumming the text we have is pointless or wrong.
+EmitFileChecksums = false;
+return None;
+  }
+  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
 
   llvm::MD5 Hash;
   llvm::MD5::MD5Result Result;
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
   CodeGenModule &CGM;
   const codegenoptions::DebugInfoKind DebugKind;
   bool DebugTypeExtRefs;
+  mutable bool EmitFileChecksums;
   llvm::DIBuilder DBuilder;
   llvm::DICompileUnit *TheCU = nullptr;
   ModuleMap *ClangModuleMap = nullptr;


Index: test/CodeGen/md5-checksum-crash.c
===
--- test/CodeGen/md5-checksum-crash.c
+++ test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited %s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
   DBuilder(CGM.getModule()) {
   for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
 DebugPrefixMap[KV.first] = KV.second;
+  EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+  CGM.getCodeGenOpts().DwarfVersion >= 5;
   CreateCompileUnit();
 }
 
@@ -365,15 +367,21 @@
 CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
   Checksum.clear();
 
-  if (!CGM.getCodeGenOpts().EmitCodeView &&
-  CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!EmitFileChecksums)
 return None;
 
   SourceManager &SM = CGM.getContext().getSourceManager();
   bool Invalid;
-  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
- 

[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson marked an inline comment as done.
probinson added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:378
 return None;
+  if (Entry.getFile().hasLineDirectives()) {
+EmitFileChecksums = false;

aprantl wrote:
> Can you add a comment explaining why we are doing this here?
Of course.


https://reviews.llvm.org/D47260



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Minor comment inline.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:378
 return None;
+  if (Entry.getFile().hasLineDirectives()) {
+EmitFileChecksums = false;

Can you add a comment explaining why we are doing this here?


https://reviews.llvm.org/D47260



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files

2018-05-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson updated this revision to Diff 148663.
probinson added a comment.

Upload patch to suppress checksums when we see a preprocessed file.


https://reviews.llvm.org/D47260

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/md5-checksum-crash.c


Index: clang/test/CodeGen/md5-checksum-crash.c
===
--- /dev/null
+++ clang/test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited 
-dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited 
%s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
   CodeGenModule &CGM;
   const codegenoptions::DebugInfoKind DebugKind;
   bool DebugTypeExtRefs;
+  mutable bool EmitFileChecksums;
   llvm::DIBuilder DBuilder;
   llvm::DICompileUnit *TheCU = nullptr;
   ModuleMap *ClangModuleMap = nullptr;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
   DBuilder(CGM.getModule()) {
   for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
 DebugPrefixMap[KV.first] = KV.second;
+  EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+  CGM.getCodeGenOpts().DwarfVersion >= 5;
   CreateCompileUnit();
 }
 
@@ -365,15 +367,19 @@
 CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
   Checksum.clear();
 
-  if (!CGM.getCodeGenOpts().EmitCodeView &&
-  CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!EmitFileChecksums)
 return None;
 
   SourceManager &SM = CGM.getContext().getSourceManager();
   bool Invalid;
-  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
-  if (Invalid)
+  const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+  if (Invalid || !Entry.isFile())
 return None;
+  if (Entry.getFile().hasLineDirectives()) {
+EmitFileChecksums = false;
+return None;
+  }
+  llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
 
   llvm::MD5 Hash;
   llvm::MD5::MD5Result Result;


Index: clang/test/CodeGen/md5-checksum-crash.c
===
--- /dev/null
+++ clang/test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited %s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
   CodeGenModule &CGM;
   const codegenoptions::DebugInfoKind DebugKind;
   bool DebugTypeExtRefs;
+  mutable bool EmitFileChecksums;
   llvm::DIBuilder DBuilder;
   llvm::DICompileUnit *TheCU = nullptr;
   ModuleMap *ClangModuleMap = nullptr;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
   DBuilder(CGM.getModule()) {
   for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
 DebugPrefixMap[KV.first] = KV.second;
+  EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+  CGM.getCodeGenOpts().DwarfVersion >= 5;
   CreateCompileUnit();
 }
 
@@ -365,15 +367,19 @@
 CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
   Checksum.clear();
 
-  if (!CGM.getCodeGenOpts().EmitCodeView &&
-  CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!EmitFileChecksums)
 return None;
 
   SourceManager &SM = CGM.