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

[scott.linder]

Summary

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/D48135.

[scott.linder]

Actually, after thinking about this I agree more with the approach of dropping or somehow warning/failing in the DWARF backend, rather than verifying at the IR level. Conceivably another DI backend which implements embedded source might not have the same restriction as DWARF, so restricting the IR doesn't seem right.

In the case of https://reviews.llvm.org/D53329 the dropping might be surprising, however, as the MD5 handling only seems to warn in the AsmParser.

[dblaikie]

In D54953#1309829, @scott.linder wrote:

Actually, after thinking about this I agree more with the approach of dropping or somehow warning/failing in the DWARF backend, rather than verifying at the IR level. Conceivably another DI backend which implements embedded source might not have the same restriction as DWARF, so restricting the IR doesn't seem right.

Yeah, I'd rather keep the restriction until we have a use case that merits removing it.

I'll leave the sign-off here to @aprantl or @JDevlieghere who have more familiarity/investment in the verifier.

[aprantl]

Is this constraint per-DICompileUnit or per llvm::Module? If it is per DICompileUnit then this implementation is too strict.

lib/IR/Verifier.cpp

1026

is there a less clever and more clear way to write this?

[scott.linder]

The constraint is only per-CU. I've attempted to capture this in the latest patch, although I don't quite understand the interaction of all of the DI* metadata, so I may have still missed cases.

[aprantl]

I'm assuming that with "per-CU" you mean a DICompileUnit, this LGTM.

[scott.linder]

I posted to the commit first by accident; copying here:

I realize now that this is insufficient. The other DI* metadata which have a file: (e.g. DILocalVariable) can point to an arbitrary DIFile, and I don't confirm that it matches the source: of all other files in the same DICompileUnit.

lib/IR/Verifier.cpp

class Verifier : public InstVisitor<Verifier>, VerifierSupport {
 
   /// Whether we've seen a call to @llvm.localescape in this function
   /// already.
   bool SawFrameEscape;
 
   /// 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;
 
   // Maps catchswitches and cleanuppads that unwind to siblings to the
   // terminators that indicate the unwind, used to detect cycles therein.
   MapVector<Instruction *, Instruction *> SiblingFuncletInfo;
 
if (Checksum) {
     case DIFile::CSK_SHA1:
       Size = 40;
       break;
     }
     AssertDI(Checksum->Value.size() == Size, "invalid checksum length", &N);
     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) {
   AssertDI(N.isDistinct(), "compile units must be distinct", &N);
   AssertDI(N.getTag() == dwarf::DW_TAG_compile_unit, "invalid tag", &N);
 
   // Don't bother verifying the compilation directory or producer string
   // as those could be empty.

test/Assembler/debug-info-source-invalid.ll

+; 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")

test/Assembler/debug-info-source.ll

+; 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")

test/Assembler/debug-info.ll

 
 ; CHECK-NEXT: !33 = !DICompositeType(tag: DW_TAG_variant_part, name: "A", scope: !14, size: 64, discriminator: !34)
 ; CHECK-NEXT: !34 = !DIDerivedType(tag: DW_TAG_member, scope: !33, baseType: !35, size: 64, align: 64, flags: DIFlagArtificial)
 ; CHECK-NEXT: !35 = !DIBasicType(name: "u64", size: 64, encoding: DW_ATE_unsigned)
 !36 = !DICompositeType(tag: DW_TAG_variant_part, name: "A", scope: !16, size: 64, discriminator: !37)
 !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)
 !41 = !DIBasicType(name: "u64.be", size: 64, align: 1, encoding: DW_ATE_unsigned, flags: DIFlagBigEndian)
 !42 = !DIBasicType(name: "u64.le", size: 64, align: 1, encoding: DW_ATE_unsigned, flags: DIFlagLittleEndian)