[PATCH] D54953: [DWARFv5] Verify all-or-nothing constraint on DIFile source

[Scott Linder via Phabricator via llvm-commits]

scott.linder created this revision.
scott.linder added reviewers: dblaikie, probinson.
Herald added subscribers: llvm-commits, JDevlieghere, aprantl.

Update IR verifier to check the constraint that DIFile source is present on all files or no files.

This may be the wrong approach, but it prevents a nasty `cantFail` during DWARF emission. After writing this patch I noticed MD5 checksums (which have the same restriction in DWARF) take a different approach of dropping the attribute and warning, see https://reviews.llvm.org/D53329.


Repository:
  rL LLVM

https://reviews.llvm.org/D54953

Files:
  lib/IR/Verifier.cpp
  test/Assembler/debug-info-source-invalid.ll
  test/Assembler/debug-info-source.ll
  test/Assembler/debug-info.ll


Index: test/Assembler/debug-info.ll
===================================================================
--- test/Assembler/debug-info.ll
+++ test/Assembler/debug-info.ll
@@ -90,10 +90,10 @@
 !37 = !DIDerivedType(tag: DW_TAG_member, scope: !36, baseType: !38, size: 64, align: 64, flags: DIFlagArtificial)
 !38 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)
 
-; CHECK-NEXT: !36 = !DIFile(filename: "file", directory: "dir", source: "int source() { }\0A")
-; CHECK-NEXT: !37 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_MD5, checksum: "3a420e2646916a475e68de8d48f779f5", source: "int source() { }\0A")
-!39 = !DIFile(filename: "file", directory: "dir", source: "int source() { }\0A")
-!40 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_MD5, checksum: "3a420e2646916a475e68de8d48f779f5", source: "int source() { }\0A")
+; CHECK-NEXT: !36 = !DIFile(filename: "file", directory: "dir")
+; CHECK-NEXT: !37 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_MD5, checksum: "3a420e2646916a475e68de8d48f779f5")
+!39 = !DIFile(filename: "file", directory: "dir")
+!40 = !DIFile(filename: "file", directory: "dir", checksumkind: CSK_MD5, checksum: "3a420e2646916a475e68de8d48f779f5")
 
 ; CHECK-NEXT: !38 = !DIBasicType(name: "u64.be", size: 64, align: 1, encoding: DW_ATE_unsigned, flags: DIFlagBigEndian)
 ; CHECK-NEXT: !39 = !DIBasicType(name: "u64.le", size: 64, align: 1, encoding: DW_ATE_unsigned, flags: DIFlagLittleEndian)
Index: test/Assembler/debug-info-source.ll
===================================================================
--- /dev/null
+++ test/Assembler/debug-info-source.ll
@@ -0,0 +1,11 @@
+; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
+; RUN: verify-uselistorder %s
+
+; CHECK: !named = !{!0, !0, !1}
+!named = !{!0, !1, !2}
+
+; CHECK: !0 = !DIFile(filename: "file1", directory: "dir1", source: "int source1() { }\0A")
+; CHECK: !1 = !DIFile(filename: "file2", directory: "dir2", source: "int source2() { }\0A")
+!0 = !DIFile(filename: "file1", directory: "dir1", source: "int source1() { }\0A")
+!1 = !DIFile(filename: "file1", directory: "dir1", source: "int source1() { }\0A")
+!2 = !DIFile(filename: "file2", directory: "dir2", source: "int source2() { }\0A")
Index: test/Assembler/debug-info-source-invalid.ll
===================================================================
--- /dev/null
+++ test/Assembler/debug-info-source-invalid.ll
@@ -0,0 +1,7 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+!named = !{!0, !1}
+
+!0 = !DIFile(filename: "file", directory: "dir", source: "int source() { }\0A")
+; CHECK: source must be present for all files or none
+!1 = !DIFile(filename: "file", directory: "dir")
Index: lib/IR/Verifier.cpp
===================================================================
--- lib/IR/Verifier.cpp
+++ lib/IR/Verifier.cpp
@@ -281,6 +281,10 @@
   /// Whether the current function has a DISubprogram attached to it.
   bool HasDebugInfo = false;
 
+  /// Whether source was present on the first DIFile. This is Optional to
+  /// detect when we visit the first file.
+  Optional<bool> HasSourceDebugInfo;
+
   /// Stores the count of how many objects were passed to llvm.localescape for a
   /// given function and the largest index passed to llvm.localrecover.
   DenseMap<Function *, std::pair<unsigned, unsigned>> FrameEscapeInfo;
@@ -1019,6 +1023,11 @@
     AssertDI(Checksum->Value.find_if_not(llvm::isHexDigit) == StringRef::npos,
              "invalid checksum", &N);
   }
+  bool HasSource = !!N.getSource();
+  if (!HasSourceDebugInfo)
+    HasSourceDebugInfo = HasSource;
+  AssertDI(HasSource == *HasSourceDebugInfo,
+           "source must be present for all files or none");
 }
 
 void Verifier::visitDICompileUnit(const DICompileUnit &N) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54953.175490.patch
Type: text/x-patch
Size: 3795 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181127/5c4ab829/attachment.bin>