[dsymutil] Remove paper trail warnings (llvm#67041)

Remove the paper trail warning from dsymutil and the DWARF linker. The
original purpose of this functionality was to leave a paper trail in the
output artifact about missing object files. The current implementation
however has diverged and is the source of a pretty serious bug:

- In the debug map parser, missing object files are not the only
  warnings we emit. When paper trail warnings are enabled, all of them end
  up in the dSYM which wasn't the goal.
  
- When warnings are associated with a object file in the debug map, it
  is skipped by the DWARF linker. This only makes sense if the object file
  is missing and is obviously incorrect for any other type of warning
  (such as a missing symbol).

The combination of the two means that we can generate broken DWARF when
the feature is enabled. AFAIK it was only used by Apple and nobody I
spoke to has relied on it, so rather than fixing the broken behavior I
propose we remove it.

Author: JDevlieghere

llvm/docs/CommandGuide/dsymutil.rst

@@ -100,13 +100,6 @@ OPTIONS
  Specifies an alternate ``path`` to place the dSYM bundle. The default dSYM
  bundle path is created by appending ``.dSYM`` to the executable name.
 
-.. option:: --papertrail
-
- When running dsymutil as part of your build system, it can be desirable for
- warnings to be part of the end product, rather than just being emitted to the
- output stream. When enabled warnings are embedded in the linked DWARF debug
- information.
-
 .. option:: --remarks-drop-without-debug
 
  Drop remarks without valid debug locations. Without this flags, all remarks are kept.

llvm/include/llvm/DWARFLinker/DWARFLinker.h

@@ -99,9 +99,6 @@ class DwarfEmitter {
 public:
   virtual ~DwarfEmitter();
 
-  /// Emit DIE containing warnings.
-  virtual void emitPaperTrailWarningsDie(DIE &Die) = 0;
-
   /// Emit section named SecName with data SecData.
   virtual void emitSectionContents(StringRef SecData, StringRef SecName) = 0;
 
@@ -272,11 +269,9 @@ class DWARFFile {
 public:
   using UnloadCallbackTy = std::function<void(StringRef FileName)>;
   DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
-            std::unique_ptr<AddressesMap> Addresses,
-            const std::vector<std::string> &Warnings,
-            UnloadCallbackTy = nullptr)
+            std::unique_ptr<AddressesMap> Addresses, UnloadCallbackTy = nullptr)
       : FileName(Name), Dwarf(std::move(Dwarf)),
-        Addresses(std::move(Addresses)), Warnings(Warnings) {}
+        Addresses(std::move(Addresses)) {}
 
   /// The object file name.
   StringRef FileName;
@@ -286,9 +281,6 @@ class DWARFFile {
 
   /// Helpful address information(list of valid address ranges, relocations).
   std::unique_ptr<AddressesMap> Addresses;
-
-  /// Warnings for this object file.
-  const std::vector<std::string> &Warnings;
 };
 
 typedef std::map<std::string, std::string> swiftInterfacesMap;
@@ -508,10 +500,6 @@ class DWARFLinker {
       ErrorHandler(Warning, File.FileName, DIE);
   }
 
-  /// Emit warnings as Dwarf compile units to leave a trail after linking.
-  bool emitPaperTrailWarnings(const DWARFFile &File,
-                              OffsetsStringPool &StringPool);
-
   void copyInvariantDebugSection(DWARFContext &Dwarf);
 
   /// Keep information for referenced clang module: already loaded DWARF info

llvm/include/llvm/DWARFLinker/DWARFStreamer.h

@@ -72,9 +72,6 @@ class DwarfStreamer : public DwarfEmitter {
   void emitAbbrevs(const std::vector<std::unique_ptr<DIEAbbrev>> &Abbrevs,
                    unsigned DwarfVersion) override;
 
-  /// Emit DIE containing warnings.
-  void emitPaperTrailWarningsDie(DIE &Die) override;
-
   /// Emit contents of section SecName From Obj.
   void emitSectionContents(StringRef SecData, StringRef SecName) override;
 

llvm/include/llvm/DWARFLinkerParallel/DWARFFile.h

@@ -29,7 +29,6 @@ class DWARFFile {
 
   DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
             std::unique_ptr<AddressesMap> Addresses,
-            const std::vector<std::string> &Warnings,
             UnloadCallbackTy UnloadFunc = nullptr);
 
   /// Object file name.
@@ -41,9 +40,6 @@ class DWARFFile {
   /// Helpful address information(list of valid address ranges, relocations).
   std::unique_ptr<AddressesMap> Addresses;
 
-  /// Warnings for object file.
-  const std::vector<std::string> &Warnings;
-
   /// Callback to the module keeping object file to unload.
   UnloadCallbackTy UnloadFunc;
 

llvm/lib/DWARFLinker/DWARFLinker.cpp

@@ -2584,69 +2584,6 @@ uint64_t DWARFLinker::DIECloner::cloneAllCompileUnits(
   return OutputDebugInfoSize - StartOutputDebugInfoSize;
 }
 
-bool DWARFLinker::emitPaperTrailWarnings(const DWARFFile &File,
-                                         OffsetsStringPool &StringPool) {
-
-  if (File.Warnings.empty())
-    return false;
-
-  DIE *CUDie = DIE::get(DIEAlloc, dwarf::DW_TAG_compile_unit);
-  CUDie->setOffset(11);
-  StringRef Producer;
-  StringRef WarningHeader;
-
-  switch (DwarfLinkerClientID) {
-  case DwarfLinkerClient::Dsymutil:
-    Producer = StringPool.internString("dsymutil");
-    WarningHeader = "dsymutil_warning";
-    break;
-
-  default:
-    Producer = StringPool.internString("dwarfopt");
-    WarningHeader = "dwarfopt_warning";
-    break;
-  }
-
-  StringRef FileName = StringPool.internString(File.FileName);
-  CUDie->addValue(DIEAlloc, dwarf::DW_AT_producer, dwarf::DW_FORM_strp,
-                  DIEInteger(StringPool.getStringOffset(Producer)));
-  DIEBlock *String = new (DIEAlloc) DIEBlock();
-  DIEBlocks.push_back(String);
-  for (auto &C : FileName)
-    String->addValue(DIEAlloc, dwarf::Attribute(0), dwarf::DW_FORM_data1,
-                     DIEInteger(C));
-  String->addValue(DIEAlloc, dwarf::Attribute(0), dwarf::DW_FORM_data1,
-                   DIEInteger(0));
-
-  CUDie->addValue(DIEAlloc, dwarf::DW_AT_name, dwarf::DW_FORM_string, String);
-  for (const auto &Warning : File.Warnings) {
-    DIE &ConstDie = CUDie->addChild(DIE::get(DIEAlloc, dwarf::DW_TAG_constant));
-    ConstDie.addValue(DIEAlloc, dwarf::DW_AT_name, dwarf::DW_FORM_strp,
-                      DIEInteger(StringPool.getStringOffset(WarningHeader)));
-    ConstDie.addValue(DIEAlloc, dwarf::DW_AT_artificial, dwarf::DW_FORM_flag,
-                      DIEInteger(1));
-    ConstDie.addValue(DIEAlloc, dwarf::DW_AT_const_value, dwarf::DW_FORM_strp,
-                      DIEInteger(StringPool.getStringOffset(Warning)));
-  }
-  unsigned Size = 4 /* FORM_strp */ + FileName.size() + 1 +
-                  File.Warnings.size() * (4 + 1 + 4) + 1 /* End of children */;
-  DIEAbbrev Abbrev = CUDie->generateAbbrev();
-  assignAbbrev(Abbrev);
-  CUDie->setAbbrevNumber(Abbrev.getNumber());
-  Size += getULEB128Size(Abbrev.getNumber());
-  // Abbreviation ordering needed for classic compatibility.
-  for (auto &Child : CUDie->children()) {
-    Abbrev = Child.generateAbbrev();
-    assignAbbrev(Abbrev);
-    Child.setAbbrevNumber(Abbrev.getNumber());
-    Size += getULEB128Size(Abbrev.getNumber());
-  }
-  CUDie->setSize(Size);
-  TheDwarfEmitter->emitPaperTrailWarningsDie(*CUDie);
-
-  return true;
-}
-
 void DWARFLinker::copyInvariantDebugSection(DWARFContext &Dwarf) {
   TheDwarfEmitter->emitSectionContents(Dwarf.getDWARFObj().getLocSection().Data,
                                        "debug_loc");
@@ -2711,9 +2648,6 @@ Error DWARFLinker::link() {
         outs() << "OBJECT FILE: " << OptContext.File.FileName << "\n";
     }
 
-    if (emitPaperTrailWarnings(OptContext.File, DebugStrPool))
-      continue;
-
     if (!OptContext.File.Dwarf)
       continue;
 

llvm/lib/DWARFLinker/DWARFStreamer.cpp

@@ -234,18 +234,6 @@ void DwarfStreamer::emitSectionContents(StringRef SecData, StringRef SecName) {
   }
 }
 
-/// Emit DIE containing warnings.
-void DwarfStreamer::emitPaperTrailWarningsDie(DIE &Die) {
-  switchToDebugInfoSection(/* Version */ 2);
-  auto &Asm = getAsmPrinter();
-  Asm.emitInt32(11 + Die.getSize() - 4);
-  Asm.emitInt16(2);
-  Asm.emitInt32(0);
-  Asm.emitInt8(MC->getTargetTriple().isArch64Bit() ? 8 : 4);
-  DebugInfoSectionSize += 11;
-  emitDIE(Die);
-}
-
 /// Emit the debug_str section stored in \p Pool.
 void DwarfStreamer::emitStrings(const NonRelocatableStringpool &Pool) {
   Asm->OutStreamer->switchSection(MOFI->getDwarfStrSection());

llvm/lib/DWARFLinkerParallel/DWARFFile.cpp

@@ -12,7 +12,6 @@
 llvm::dwarflinker_parallel::DWARFFile::DWARFFile(
     StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
     std::unique_ptr<AddressesMap> Addresses,
-    const std::vector<std::string> &Warnings,
     DWARFFile::UnloadCallbackTy UnloadFunc)
     : FileName(Name), Dwarf(std::move(Dwarf)), Addresses(std::move(Addresses)),
-      Warnings(Warnings), UnloadFunc(UnloadFunc) {}
+      UnloadFunc(UnloadFunc) {}

llvm/lib/DWARFLinkerParallel/DWARFLinkerImpl.cpp

@@ -371,27 +371,26 @@ Error DWARFLinkerImpl::LinkContext::loadClangModule(
 
 Error DWARFLinkerImpl::LinkContext::link() {
   InterCUProcessingStarted = false;
-  if (InputDWARFFile.Warnings.empty()) {
-    if (!InputDWARFFile.Dwarf)
-      return Error::success();
+  if (!InputDWARFFile.Dwarf)
+    return Error::success();
 
-    // Preload macro tables, as they can't be loaded asynchronously.
-    InputDWARFFile.Dwarf->getDebugMacinfo();
-    InputDWARFFile.Dwarf->getDebugMacro();
+  // Preload macro tables, as they can't be loaded asynchronously.
+  InputDWARFFile.Dwarf->getDebugMacinfo();
+  InputDWARFFile.Dwarf->getDebugMacro();
 
-    // Link modules compile units first.
-    parallelForEach(ModulesCompileUnits, [&](RefModuleUnit &RefModule) {
-      linkSingleCompileUnit(*RefModule.Unit);
-    });
+  // Link modules compile units first.
+  parallelForEach(ModulesCompileUnits, [&](RefModuleUnit &RefModule) {
+    linkSingleCompileUnit(*RefModule.Unit);
+  });
 
-    // Check for live relocations. If there is no any live relocation then we
-    // can skip entire object file.
-    if (!GlobalData.getOptions().UpdateIndexTablesOnly &&
-        !InputDWARFFile.Addresses->hasValidRelocs()) {
-      if (GlobalData.getOptions().Verbose)
-        outs() << "No valid relocations found. Skipping.\n";
-      return Error::success();
-    }
+  // Check for live relocations. If there is no any live relocation then we
+  // can skip entire object file.
+  if (!GlobalData.getOptions().UpdateIndexTablesOnly &&
+      !InputDWARFFile.Addresses->hasValidRelocs()) {
+    if (GlobalData.getOptions().Verbose)
+      outs() << "No valid relocations found. Skipping.\n";
+    return Error::success();
+  }
 
     OriginalDebugInfoSize = getInputDebugInfoSize();
 
@@ -460,21 +459,8 @@ Error DWARFLinkerImpl::LinkContext::link() {
         linkSingleCompileUnit(*CU, CompileUnit::Stage::Cleaned);
       });
     }
-  }
 
-  if (!InputDWARFFile.Warnings.empty()) {
-    // Create compile unit with paper trail warnings.
-
-    Error ResultErr = Error::success();
-    // We use task group here as PerThreadBumpPtrAllocator should be called from
-    // the threads created by ThreadPoolExecutor.
-    parallel::TaskGroup TGroup;
-    TGroup.spawn([&]() {
-      if (Error Err = cloneAndEmitPaperTrails())
-        ResultErr = std::move(Err);
-    });
-    return ResultErr;
-  } else if (GlobalData.getOptions().UpdateIndexTablesOnly) {
+  if (GlobalData.getOptions().UpdateIndexTablesOnly) {
     // Emit Invariant sections.
 
     if (Error Err = emitInvariantSections())
@@ -715,102 +701,6 @@ void DWARFLinkerImpl::LinkContext::emitFDE(uint32_t CIEOffset,
   Section.OS.write(FDEBytes.data(), FDEBytes.size());
 }
 
-Error DWARFLinkerImpl::LinkContext::cloneAndEmitPaperTrails() {
-  CompileUnits.emplace_back(std::make_unique<CompileUnit>(
-      GlobalData, UniqueUnitID.fetch_add(1), "", InputDWARFFile,
-      getUnitForOffset, Format, Endianness));
-
-  CompileUnit &CU = *CompileUnits.back();
-
-  BumpPtrAllocator Allocator;
-
-  DIEGenerator ParentGenerator(Allocator, CU);
-
-  SectionDescriptor &DebugInfoSection =
-      CU.getOrCreateSectionDescriptor(DebugSectionKind::DebugInfo);
-  OffsetsPtrVector PatchesOffsets;
-
-  uint64_t CurrentOffset = CU.getDebugInfoHeaderSize();
-  DIE *CUDie =
-      ParentGenerator.createDIE(dwarf::DW_TAG_compile_unit, CurrentOffset);
-  CU.setOutUnitDIE(CUDie);
-
-  DebugInfoSection.notePatchWithOffsetUpdate(
-      DebugStrPatch{{CurrentOffset},
-                    GlobalData.getStringPool().insert("dsymutil").first},
-      PatchesOffsets);
-  CurrentOffset += ParentGenerator
-                       .addStringPlaceholderAttribute(dwarf::DW_AT_producer,
-                                                      dwarf::DW_FORM_strp)
-                       .second;
-
-  CurrentOffset +=
-      ParentGenerator
-          .addInplaceString(dwarf::DW_AT_name, InputDWARFFile.FileName)
-          .second;
-
-  size_t SizeAbbrevNumber = ParentGenerator.finalizeAbbreviations(true);
-  CurrentOffset += SizeAbbrevNumber;
-  for (uint64_t *OffsetPtr : PatchesOffsets)
-    *OffsetPtr += SizeAbbrevNumber;
-  for (const auto &Warning : CU.getContaingFile().Warnings) {
-    PatchesOffsets.clear();
-    DIEGenerator ChildGenerator(Allocator, CU);
-
-    DIE *ChildDie =
-        ChildGenerator.createDIE(dwarf::DW_TAG_constant, CurrentOffset);
-    ParentGenerator.addChild(ChildDie);
-
-    DebugInfoSection.notePatchWithOffsetUpdate(
-        DebugStrPatch{
-            {CurrentOffset},
-            GlobalData.getStringPool().insert("dsymutil_warning").first},
-        PatchesOffsets);
-    CurrentOffset += ChildGenerator
-                         .addStringPlaceholderAttribute(dwarf::DW_AT_name,
-                                                        dwarf::DW_FORM_strp)
-                         .second;
-
-    CurrentOffset +=
-        ChildGenerator
-            .addScalarAttribute(dwarf::DW_AT_artificial, dwarf::DW_FORM_flag, 1)
-            .second;
-
-    DebugInfoSection.notePatchWithOffsetUpdate(
-        DebugStrPatch{{CurrentOffset},
-                      GlobalData.getStringPool().insert(Warning).first},
-        PatchesOffsets);
-    CurrentOffset += ChildGenerator
-                         .addStringPlaceholderAttribute(
-                             dwarf::DW_AT_const_value, dwarf::DW_FORM_strp)
-                         .second;
-
-    SizeAbbrevNumber = ChildGenerator.finalizeAbbreviations(false);
-
-    CurrentOffset += SizeAbbrevNumber;
-    for (uint64_t *OffsetPtr : PatchesOffsets)
-      *OffsetPtr += SizeAbbrevNumber;
-
-    ChildDie->setSize(CurrentOffset - ChildDie->getOffset());
-  }
-
-  CurrentOffset += 1; // End of children
-  CUDie->setSize(CurrentOffset - CUDie->getOffset());
-
-  uint64_t UnitSize = 0;
-  UnitSize += CU.getDebugInfoHeaderSize();
-  UnitSize += CUDie->getSize();
-  CU.setUnitSize(UnitSize);
-
-  if (GlobalData.getOptions().NoOutput)
-    return Error::success();
-
-  if (Error Err = CU.emitDebugInfo(*TargetTriple))
-    return Err;
-
-  return CU.emitAbbreviations();
-}
-
 void DWARFLinkerImpl::glueCompileUnitsAndWriteToTheOutput() {
   if (GlobalData.getOptions().NoOutput)
     return;

llvm/lib/DWARFLinkerParallel/DWARFLinkerImpl.h

@@ -294,9 +294,6 @@ class DWARFLinkerImpl : public DWARFLinker {
     void emitFDE(uint32_t CIEOffset, uint32_t AddrSize, uint64_t Address,
                  StringRef FDEBytes, SectionDescriptor &Section);
 
-    /// Clone and emit paper trails.
-    Error cloneAndEmitPaperTrails();
-
     std::function<CompileUnit *(uint64_t)> getUnitForOffset =
         [&](uint64_t Offset) -> CompileUnit * {
       auto CU = llvm::upper_bound(