[llvm] Allow for mixing source/no-source DIFiles in one CU (PR #73877)

via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 29 16:10:41 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-ir

Author: Adrian Prantl (adrian-prantl)

<details>
<summary>Changes</summary>

The DWARF proposal that the DW_LNCT_LLVM_source extension is based on
(https://dwarfstd.org/issues/180201.1.html) allows to mix source and
non-source files in the same CU by storing an empty string as a
sentinel value.

This patch implements this feature.

---
Full diff: https://github.com/llvm/llvm-project/pull/73877.diff


7 Files Affected:

- (modified) llvm/include/llvm/MC/MCDwarf.h (+4-4) 
- (modified) llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp (+8-3) 
- (modified) llvm/lib/IR/Verifier.cpp (-18) 
- (modified) llvm/lib/MC/MCDwarf.cpp (+9-14) 
- (removed) llvm/test/Assembler/debug-info-source-invalid.ll (-27) 
- (removed) llvm/test/Assembler/debug-info-source.ll (-41) 
- (added) llvm/test/DebugInfo/Generic/mixed-source.ll (+38) 


``````````diff
diff --git a/llvm/include/llvm/MC/MCDwarf.h b/llvm/include/llvm/MC/MCDwarf.h
index 715714f8e55dbb5..43698533b3b02bc 100644
--- a/llvm/include/llvm/MC/MCDwarf.h
+++ b/llvm/include/llvm/MC/MCDwarf.h
@@ -265,7 +265,7 @@ struct MCDwarfLineTableHeader {
   StringMap<unsigned> SourceIdMap;
   std::string CompilationDir;
   MCDwarfFile RootFile;
-  bool HasSource = false;
+  bool HasAnySource = false;
 private:
   bool HasAllMD5 = true;
   bool HasAnyMD5 = false;
@@ -305,7 +305,7 @@ struct MCDwarfLineTableHeader {
     RootFile.Checksum = Checksum;
     RootFile.Source = Source;
     trackMD5Usage(Checksum.has_value());
-    HasSource = Source.has_value();
+    HasAnySource |= Source.has_value();
   }
 
   void resetFileTable() {
@@ -313,7 +313,7 @@ struct MCDwarfLineTableHeader {
     MCDwarfFiles.clear();
     RootFile.Name.clear();
     resetMD5Usage();
-    HasSource = false;
+    HasAnySource = false;
   }
 
 private:
@@ -385,7 +385,7 @@ class MCDwarfLineTable {
     Header.RootFile.Checksum = Checksum;
     Header.RootFile.Source = Source;
     Header.trackMD5Usage(Checksum.has_value());
-    Header.HasSource = Source.has_value();
+    Header.HasAnySource |= Source.has_value();
   }
 
   void resetFileTable() { Header.resetFileTable(); }
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
index 6f2afe5d50e9c81..42a37a491cf0dd4 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -170,9 +170,14 @@ void DWARFDebugLine::Prologue::dump(raw_ostream &OS,
       if (ContentTypes.HasLength)
         OS << format("         length: 0x%8.8" PRIx64 "\n", FileEntry.Length);
       if (ContentTypes.HasSource) {
-        OS <<        "         source: ";
-        FileEntry.Source.dump(OS, DumpOptions);
-        OS << '\n';
+        auto Source = FileEntry.Source.getAsCString();
+        if (!Source)
+          consumeError(Source.takeError());
+        else if (*Source && (*Source)[0]) {
+          OS << "         source: ";
+          FileEntry.Source.dump(OS, DumpOptions);
+          OS << '\n';
+        }
       }
     }
   }
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 5560c037aa3ee6b..5f466581ea98095 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -329,9 +329,6 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   /// The current source language.
   dwarf::SourceLanguage CurrentSourceLang = dwarf::DW_LANG_lo_user;
 
-  /// Whether source was present on the first DIFile encountered in each CU.
-  DenseMap<const DICompileUnit *, 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;
@@ -620,9 +617,6 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   void verifyAttachedCallBundle(const CallBase &Call,
                                 const OperandBundleUse &BU);
 
-  /// Verify all-or-nothing property of DIFile source attribute within a CU.
-  void verifySourceDebugInfo(const DICompileUnit &U, const DIFile &F);
-
   /// Verify the llvm.experimental.noalias.scope.decl declarations
   void verifyNoAliasScopeDecl();
 };
@@ -1352,8 +1346,6 @@ void Verifier::visitDICompileUnit(const DICompileUnit &N) {
 
   CurrentSourceLang = (dwarf::SourceLanguage)N.getSourceLanguage();
 
-  verifySourceDebugInfo(N, *N.getFile());
-
   CheckDI((N.getEmissionKind() <= DICompileUnit::LastEmissionKind),
           "invalid emission kind", &N);
 
@@ -1442,8 +1434,6 @@ void Verifier::visitDISubprogram(const DISubprogram &N) {
               "definition subprograms cannot be nested within DICompositeType "
               "when enabling ODR",
               &N);
-    if (N.getFile())
-      verifySourceDebugInfo(*N.getUnit(), *N.getFile());
   } else {
     // Subprogram declarations (part of the type hierarchy).
     CheckDI(!Unit, "subprogram declarations must not have a compile unit", &N);
@@ -6590,14 +6580,6 @@ void Verifier::verifyAttachedCallBundle(const CallBase &Call,
   }
 }
 
-void Verifier::verifySourceDebugInfo(const DICompileUnit &U, const DIFile &F) {
-  bool HasSource = F.getSource().has_value();
-  if (!HasSourceDebugInfo.count(&U))
-    HasSourceDebugInfo[&U] = HasSource;
-  CheckDI(HasSource == HasSourceDebugInfo[&U],
-          "inconsistent use of embedded source");
-}
-
 void Verifier::verifyNoAliasScopeDecl() {
   if (NoAliasScopeDecls.empty())
     return;
diff --git a/llvm/lib/MC/MCDwarf.cpp b/llvm/lib/MC/MCDwarf.cpp
index 7925fba876f86cf..f94fc48a033d4e4 100644
--- a/llvm/lib/MC/MCDwarf.cpp
+++ b/llvm/lib/MC/MCDwarf.cpp
@@ -386,7 +386,7 @@ void MCDwarfLineTableHeader::emitV2FileDirTables(MCStreamer *MCOS) const {
 }
 
 static void emitOneV5FileEntry(MCStreamer *MCOS, const MCDwarfFile &DwarfFile,
-                               bool EmitMD5, bool HasSource,
+                               bool EmitMD5, bool HasAnySource,
                                std::optional<MCDwarfLineStr> &LineStr) {
   assert(!DwarfFile.Name.empty());
   if (LineStr)
@@ -401,7 +401,7 @@ static void emitOneV5FileEntry(MCStreamer *MCOS, const MCDwarfFile &DwarfFile,
     MCOS->emitBinaryData(
         StringRef(reinterpret_cast<const char *>(Cksum.data()), Cksum.size()));
   }
-  if (HasSource) {
+  if (HasAnySource) {
     if (LineStr)
       LineStr->emitRef(MCOS, DwarfFile.Source.value_or(StringRef()));
     else {
@@ -452,7 +452,7 @@ void MCDwarfLineTableHeader::emitV5FileDirTables(
   uint64_t Entries = 2;
   if (HasAllMD5)
     Entries += 1;
-  if (HasSource)
+  if (HasAnySource)
     Entries += 1;
   MCOS->emitInt8(Entries);
   MCOS->emitULEB128IntValue(dwarf::DW_LNCT_path);
@@ -464,7 +464,7 @@ void MCDwarfLineTableHeader::emitV5FileDirTables(
     MCOS->emitULEB128IntValue(dwarf::DW_LNCT_MD5);
     MCOS->emitULEB128IntValue(dwarf::DW_FORM_data16);
   }
-  if (HasSource) {
+  if (HasAnySource) {
     MCOS->emitULEB128IntValue(dwarf::DW_LNCT_LLVM_source);
     MCOS->emitULEB128IntValue(LineStr ? dwarf::DW_FORM_line_strp
                                       : dwarf::DW_FORM_string);
@@ -479,9 +479,9 @@ void MCDwarfLineTableHeader::emitV5FileDirTables(
   assert((!RootFile.Name.empty() || MCDwarfFiles.size() >= 1) &&
          "No root file and no .file directives");
   emitOneV5FileEntry(MCOS, RootFile.Name.empty() ? MCDwarfFiles[1] : RootFile,
-                     HasAllMD5, HasSource, LineStr);
+                     HasAllMD5, HasAnySource, LineStr);
   for (unsigned i = 1; i < MCDwarfFiles.size(); ++i)
-    emitOneV5FileEntry(MCOS, MCDwarfFiles[i], HasAllMD5, HasSource, LineStr);
+    emitOneV5FileEntry(MCOS, MCDwarfFiles[i], HasAllMD5, HasAnySource, LineStr);
 }
 
 std::pair<MCSymbol *, MCSymbol *>
@@ -598,7 +598,7 @@ MCDwarfLineTableHeader::tryGetFile(StringRef &Directory, StringRef &FileName,
   // If any files have embedded source, they all must.
   if (MCDwarfFiles.empty()) {
     trackMD5Usage(Checksum.has_value());
-    HasSource = (Source != std::nullopt);
+    HasAnySource |= Source.has_value();
   }
   if (DwarfVersion >= 5 && isRootFile(RootFile, Directory, FileName, Checksum))
     return 0;
@@ -625,11 +625,6 @@ MCDwarfLineTableHeader::tryGetFile(StringRef &Directory, StringRef &FileName,
     return make_error<StringError>("file number already allocated",
                                    inconvertibleErrorCode());
 
-  // If any files have embedded source, they all must.
-  if (HasSource != (Source != std::nullopt))
-    return make_error<StringError>("inconsistent use of embedded source",
-                                   inconvertibleErrorCode());
-
   if (Directory.empty()) {
     // Separate the directory part from the basename of the FileName.
     StringRef tFileName = sys::path::filename(FileName);
@@ -662,8 +657,8 @@ MCDwarfLineTableHeader::tryGetFile(StringRef &Directory, StringRef &FileName,
   File.Checksum = Checksum;
   trackMD5Usage(Checksum.has_value());
   File.Source = Source;
-  if (Source)
-    HasSource = true;
+  if (Source.has_value())
+    HasAnySource = true;
 
   // return the allocated FileNumber.
   return FileNumber;
diff --git a/llvm/test/Assembler/debug-info-source-invalid.ll b/llvm/test/Assembler/debug-info-source-invalid.ll
deleted file mode 100644
index d746e9e25fc3d12..000000000000000
--- a/llvm/test/Assembler/debug-info-source-invalid.ll
+++ /dev/null
@@ -1,27 +0,0 @@
-; RUN: llvm-as < %s 2>&1 >/dev/null | FileCheck %s
-
-; Ensure we reject debug info where the DIFiles of a DICompileUnit mix source
-; and no-source.
-
-define dso_local void @foo() !dbg !5 {
-  ret void
-}
-
-define dso_local void @bar() !dbg !6 {
-  ret void
-}
-
-!llvm.dbg.cu = !{!4}
-!llvm.module.flags = !{!0, !1}
-
-!0 = !{i32 2, !"Dwarf Version", i32 5}
-!1 = !{i32 2, !"Debug Info Version", i32 3}
-
-!2 = !DIFile(filename: "foo.c", directory: "dir", source: "void foo() { }\0A")
-; CHECK: inconsistent use of embedded source
-; CHECK: warning: ignoring invalid debug info
-!3 = !DIFile(filename: "bar.h", directory: "dir")
-
-!4 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2)
-!5 = distinct !DISubprogram(name: "foo", file: !2, unit: !4)
-!6 = distinct !DISubprogram(name: "bar", file: !3, unit: !4)
diff --git a/llvm/test/Assembler/debug-info-source.ll b/llvm/test/Assembler/debug-info-source.ll
deleted file mode 100644
index 381603ef35c3839..000000000000000
--- a/llvm/test/Assembler/debug-info-source.ll
+++ /dev/null
@@ -1,41 +0,0 @@
-; RUN: llvm-as < %s | llvm-dis | llvm-as | llvm-dis | FileCheck %s
-; RUN: verify-uselistorder %s
-
-; Ensure we accept debug info where DIFiles within a DICompileUnit either all
-; have source, or none have source.
-
-define dso_local void @foo() !dbg !6 {
-  ret void
-}
-
-define dso_local void @bar() !dbg !7 {
-  ret void
-}
-
-define dso_local void @baz() !dbg !9 {
-  ret void
-}
-
-define dso_local void @qux() !dbg !11 {
-  ret void
-}
-
-!llvm.dbg.cu = !{!0, !2}
-!llvm.module.flags = !{!4, !5}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)
-; CHECK: !1 = !DIFile(filename: "foo.c", directory: "dir", source: "void foo() { }\0A")
-!1 = !DIFile(filename: "foo.c", directory: "dir", source: "void foo() { }\0A")
-!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3)
-; CHECK: !3 = !DIFile(filename: "qux.h", directory: "dir")
-!3 = !DIFile(filename: "qux.h", directory: "dir")
-!4 = !{i32 2, !"Dwarf Version", i32 5}
-!5 = !{i32 2, !"Debug Info Version", i32 3}
-!6 = distinct !DISubprogram(name: "foo", file: !1, unit: !0)
-!7 = distinct !DISubprogram(name: "bar", file: !8, unit: !0)
-; CHECK: !8 = !DIFile(filename: "bar.h", directory: "dir", source: "void bar() { }\0A")
-!8 = !DIFile(filename: "bar.h", directory: "dir", source: "void bar() { }\0A")
-!9 = distinct !DISubprogram(name: "baz", file: !10, unit: !2)
-; CHECK: !10 = !DIFile(filename: "baz.c", directory: "dir")
-!10 = !DIFile(filename: "baz.c", directory: "dir")
-!11 = distinct !DISubprogram(name: "qux", file: !3, unit: !2)
diff --git a/llvm/test/DebugInfo/Generic/mixed-source.ll b/llvm/test/DebugInfo/Generic/mixed-source.ll
new file mode 100644
index 000000000000000..d8043c155acedbb
--- /dev/null
+++ b/llvm/test/DebugInfo/Generic/mixed-source.ll
@@ -0,0 +1,38 @@
+; RUN: %llc_dwarf -O0 -filetype=obj -o - < %s | llvm-dwarfdump -debug-line - | FileCheck %s
+
+; CHECK: include_directories[  0] = "dir"
+; CHECK-NEXT: file_names[  0]:
+; CHECK-NEXT:            name: "foo.c"
+; CHECK-NEXT:       dir_index: 0
+; CHECK-NEXT:          source: "void foo() { }\n"
+; CHECK-NEXT: file_names[  1]:
+; CHECK-NEXT:            name: "bar.h"
+; CHECK-NEXT:       dir_index: 0
+; CHECK-NOT:           source:
+
+; Ensure we reject debug info where the DIFiles of a DICompileUnit mix source
+; and no-source.
+
+define dso_local void @foo() !dbg !5 {
+  ret void, !dbg !7
+}
+
+define dso_local void @bar() !dbg !6 {
+  ret void, !dbg !8
+}
+
+!llvm.dbg.cu = !{!4}
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 2, !"Dwarf Version", i32 5}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+
+!2 = !DIFile(filename: "foo.c", directory: "dir", source: "void foo() { }\0A")
+!3 = !DIFile(filename: "bar.h", directory: "dir")
+
+!4 = distinct !DICompileUnit(language: DW_LANG_C99, emissionKind: FullDebug, file: !2)
+!5 = distinct !DISubprogram(name: "foo", file: !2, line: 1, type: !9, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !4)
+!6 = distinct !DISubprogram(name: "bar", file: !3, line: 1, type: !9, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !4)
+!7 = !DILocation(line: 1, scope: !5)
+!8 = !DILocation(line: 1, scope: !6)
+!9 = !DISubroutineType(types: !{})

``````````

</details>


https://github.com/llvm/llvm-project/pull/73877


More information about the llvm-commits mailing list