Merge pull request #7836 from adrian-prantl/relax-difile

Allow for mixing source/no-source DIFiles in one CU

Author: adrian-prantl

llvm/include/llvm/MC/MCDwarf.h

@@ -276,7 +276,8 @@ struct MCDwarfLineTableHeader {
   StringMap<unsigned> SourceIdMap;
   std::string CompilationDir;
   MCDwarfFile RootFile;
-  bool HasSource = false;
+  bool HasAnySource = false;
+
 private:
   bool HasAllMD5 = true;
   bool HasAnyMD5 = false;
@@ -316,15 +317,15 @@ struct MCDwarfLineTableHeader {
     RootFile.Checksum = Checksum;
     RootFile.Source = Source;
     trackMD5Usage(Checksum.has_value());
-    HasSource = Source.has_value();
+    HasAnySource |= Source.has_value();
   }
 
   void resetFileTable() {
     MCDwarfDirs.clear();
     MCDwarfFiles.clear();
     RootFile.Name.clear();
     resetMD5Usage();
-    HasSource = false;
+    HasAnySource = false;
   }
 
 private:
@@ -396,7 +397,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(); }

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)[0]) {
+          OS << "         source: ";
+          FileEntry.Source.dump(OS, DumpOptions);
+          OS << '\n';
+        }
       }
     }
   }

llvm/lib/IR/Verifier.cpp

@@ -337,9 +337,6 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
     NoConvergence
   } ConvergenceKind = NoConvergence;
 
-  /// 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;
@@ -621,9 +618,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();
 };
@@ -1420,8 +1414,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);
 
@@ -1501,8 +1493,15 @@ void Verifier::visitDISubprogram(const DISubprogram &N) {
     CheckDI(N.isDistinct(), "subprogram definitions must be distinct", &N);
     CheckDI(Unit, "subprogram definitions must have a compile unit", &N);
     CheckDI(isa<DICompileUnit>(Unit), "invalid unit type", &N, Unit);
-    if (N.getFile())
-      verifySourceDebugInfo(*N.getUnit(), *N.getFile());
+    // There's no good way to cross the CU boundary to insert a nested
+    // DISubprogram definition in one CU into a type defined in another CU.
+    auto *CT = dyn_cast_or_null<DICompositeType>(N.getRawScope());
+    if (CT && CT->getRawIdentifier() &&
+        M.getContext().isODRUniquingDebugTypes())
+      CheckDI(N.getDeclaration(),
+              "definition subprograms cannot be nested within DICompositeType "
+              "when enabling ODR",
+              &N);
   } else {
     // Subprogram declarations (part of the type hierarchy).
     CheckDI(!Unit, "subprogram declarations must not have a compile unit", &N);
@@ -6697,14 +6696,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;

llvm/lib/MC/MCDwarf.cpp

@@ -393,7 +393,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)
@@ -408,7 +408,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 {
@@ -459,7 +459,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);
@@ -471,7 +471,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);
@@ -486,9 +486,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 *>
@@ -605,7 +605,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;
@@ -632,11 +632,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);
@@ -669,8 +664,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;

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

@@ -0,0 +1,37 @@
+; 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:
+
+; Test that DIFiles mixing source and no-source within a DICompileUnit works.
+
+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: !{})