Merge pull request #7511 from apple/cherrypick/rebranch/697b34fd966b

JDevlieghere · web-flow · commit e4c469cb2920 · 2023-09-22T14:04:20.000-07:00
[dsymutil] Remove paper trail warnings (llvm#67041)
diff --git a/llvm/docs/CommandGuide/dsymutil.rst b/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.
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/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;
 
@@ -267,10 +264,9 @@ using UnitListTy = std::vector<std::unique_ptr<CompileUnit>>;
 class DWARFFile {
 public:
   DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
-            std::unique_ptr<AddressesMap> Addresses,
-            const std::vector<std::string> &Warnings)
+            std::unique_ptr<AddressesMap> Addresses)
       : FileName(Name), Dwarf(std::move(Dwarf)),
-        Addresses(std::move(Addresses)), Warnings(Warnings) {}
+        Addresses(std::move(Addresses)) {}
 
   /// The object file name.
   StringRef FileName;
@@ -280,9 +276,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;
@@ -502,10 +495,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
diff --git a/llvm/include/llvm/DWARFLinker/DWARFStreamer.h b/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;
 
diff --git a/llvm/include/llvm/DWARFLinkerParallel/DWARFFile.h b/llvm/include/llvm/DWARFLinkerParallel/DWARFFile.h
@@ -29,11 +29,9 @@ class DWARFFile {
 
   DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
             std::unique_ptr<AddressesMap> Addresses,
-            const std::vector<std::string> &Warnings,
             UnloadCallbackTy UnloadFunc = nullptr)
       : FileName(Name), Dwarf(std::move(Dwarf)),
-        Addresses(std::move(Addresses)), Warnings(Warnings),
-        UnloadFunc(UnloadFunc) {
+        Addresses(std::move(Addresses)), UnloadFunc(UnloadFunc) {
     if (this->Dwarf)
       Endianess = this->Dwarf->isLittleEndian() ? support::endianness::little
                                                 : support::endianness::big;
@@ -48,9 +46,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;
-
   /// Endiannes of source DWARF information.
   support::endianness Endianess = support::endianness::little;
 
diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -2561,69 +2561,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");
@@ -2687,9 +2624,6 @@ Error DWARFLinker::link() {
         outs() << "OBJECT FILE: " << OptContext.File.FileName << "\n";
     }
 
-    if (emitPaperTrailWarnings(OptContext.File, DebugStrPool))
-      continue;
-
     if (!OptContext.File.Dwarf)
       continue;
 
diff --git a/llvm/lib/DWARFLinker/DWARFStreamer.cpp b/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());
diff --git a/llvm/test/tools/dsymutil/X86/papertrail-warnings.test b/llvm/test/tools/dsymutil/X86/papertrail-warnings.test
diff --git a/llvm/test/tools/dsymutil/cmdline.test b/llvm/test/tools/dsymutil/cmdline.test
@@ -21,7 +21,6 @@ CHECK: -object-prefix-map <prefix=remapped>
 CHECK: -oso-prepend-path <path>
 CHECK: -out <filename>
 CHECK: {{-o <filename>}}
-CHECK: -papertrail
 CHECK: -remarks-drop-without-debug
 CHECK: -remarks-output-format <format>
 CHECK: -remarks-prepend-path <path>
diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -234,8 +234,7 @@ DwarfLinkerForBinary::loadObject(const DebugMapObject &Obj,
   if (ErrorOrObj) {
     Res = std::make_unique<OutDWARFFile>(
         Obj.getObjectFilename(), DWARFContext::create(*ErrorOrObj),
-        std::make_unique<AddressesMap>(*this, *ErrorOrObj, Obj),
-        Obj.empty() ? Obj.getWarnings() : EmptyWarnings);
+        std::make_unique<AddressesMap>(*this, *ErrorOrObj, Obj));
 
     Error E = RL.link(*ErrorOrObj);
     if (Error NewE = handleErrors(
@@ -768,8 +767,7 @@ bool DwarfLinkerForBinary::linkImpl(
                                    OnCUDieLoaded);
     } else {
       ObjectsForLinking.push_back(std::make_unique<OutDwarfFile>(
-          Obj->getObjectFilename(), nullptr, nullptr,
-          Obj->empty() ? Obj->getWarnings() : EmptyWarnings));
+          Obj->getObjectFilename(), nullptr, nullptr));
       GeneralLinker->addObjectFile(*ObjectsForLinking.back());
     }
   }
diff --git a/llvm/tools/dsymutil/MachODebugMapParser.cpp b/llvm/tools/dsymutil/MachODebugMapParser.cpp
@@ -28,11 +28,9 @@ class MachODebugMapParser {
 public:
   MachODebugMapParser(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
                       StringRef BinaryPath, ArrayRef<std::string> Archs,
-                      StringRef PathPrefix = "",
-                      bool PaperTrailWarnings = false, bool Verbose = false)
+                      StringRef PathPrefix = "", bool Verbose = false)
       : BinaryPath(std::string(BinaryPath)), Archs(Archs.begin(), Archs.end()),
-        PathPrefix(std::string(PathPrefix)),
-        PaperTrailWarnings(PaperTrailWarnings), BinHolder(VFS, Verbose),
+        PathPrefix(std::string(PathPrefix)), BinHolder(VFS, Verbose),
         CurrentDebugMapObject(nullptr), SkipDebugMapObject(false) {}
 
   /// Parses and returns the DebugMaps of the input binary. The binary contains
@@ -50,7 +48,6 @@ class MachODebugMapParser {
   std::string BinaryPath;
   SmallVector<StringRef, 1> Archs;
   std::string PathPrefix;
-  bool PaperTrailWarnings;
 
   /// Owns the MemoryBuffer for the main binary.
   BinaryHolder BinHolder;
@@ -137,13 +134,6 @@ class MachODebugMapParser {
                          << MachOUtils::getArchName(
                                 Result->getTriple().getArchName())
                          << ") " << File << " " << Msg << "\n";
-
-    if (PaperTrailWarnings) {
-      if (!File.empty())
-        Result->addDebugMapObject(File, sys::TimePoint<std::chrono::seconds>());
-      if (Result->end() != Result->begin())
-        (*--Result->end())->addWarning(Msg.str());
-    }
   }
 };
 
@@ -704,13 +694,11 @@ namespace dsymutil {
 llvm::ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
 parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
               StringRef InputFile, ArrayRef<std::string> Archs,
-              StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
-              bool InputIsYAML) {
+              StringRef PrependPath, bool Verbose, bool InputIsYAML) {
   if (InputIsYAML)
     return DebugMap::parseYAMLDebugMap(InputFile, PrependPath, Verbose);
 
-  MachODebugMapParser Parser(VFS, InputFile, Archs, PrependPath,
-                             PaperTrailWarnings, Verbose);
+  MachODebugMapParser Parser(VFS, InputFile, Archs, PrependPath, Verbose);
   return Parser.parse();
 }
 
diff --git a/llvm/tools/dsymutil/Options.td b/llvm/tools/dsymutil/Options.td
@@ -69,10 +69,6 @@ def yaml_input: Flag<["-", "--"], "y">,
   HelpText<"Treat the input file is a YAML debug map rather than a binary.">,
   Group<grp_general>;
 
-def papertrail: F<"papertrail">,
-  HelpText<"Embed warnings in the linked DWARF debug info.">,
-  Group<grp_general>;
-
 def assembly: Flag<["-", "--"], "S">,
   HelpText<"Output textual assembly instead of a binary dSYM companion file.">,
   Group<grp_general>;
diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp
@@ -106,7 +106,6 @@ struct DsymutilOptions {
   bool DumpStab = false;
   bool Flat = false;
   bool InputIsYAMLDebugMap = false;
-  bool PaperTrailWarnings = false;
   bool ForceKeepFunctionForStatic = false;
   std::string SymbolMap;
   std::string OutputFile;
@@ -197,11 +196,6 @@ static Error verifyOptions(const DsymutilOptions &Options) {
         "cannot use -o with multiple inputs in flat mode.",
         errc::invalid_argument);
 
-  if (Options.PaperTrailWarnings && Options.InputIsYAMLDebugMap)
-    return make_error<StringError>(
-        "paper trail warnings are not supported for YAML input.",
-        errc::invalid_argument);
-
   if (!Options.ReproducerPath.empty() &&
       Options.ReproMode != ReproducerMode::Use)
     return make_error<StringError>(
@@ -303,7 +297,6 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
   Options.DumpStab = Args.hasArg(OPT_symtab);
   Options.Flat = Args.hasArg(OPT_flat);
   Options.InputIsYAMLDebugMap = Args.hasArg(OPT_yaml_input);
-  Options.PaperTrailWarnings = Args.hasArg(OPT_papertrail);
 
   if (Expected<DWARFVerify> Verify = getVerifyKind(Args)) {
     Options.Verify = *Verify;
@@ -389,9 +382,6 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
   if (Options.DumpDebugMap || Options.LinkOpts.Verbose)
     Options.LinkOpts.Threads = 1;
 
-  if (getenv("RC_DEBUG_OPTIONS"))
-    Options.PaperTrailWarnings = true;
-
   if (opt::Arg *RemarksPrependPath = Args.getLastArg(OPT_remarks_prepend_path))
     Options.LinkOpts.RemarksPrependPath = RemarksPrependPath->getValue();
 
@@ -671,8 +661,8 @@ int main(int argc, char **argv) {
 
     auto DebugMapPtrsOrErr =
         parseDebugMap(Options.LinkOpts.VFS, InputFile, Options.Archs,
-                      Options.LinkOpts.PrependPath, Options.PaperTrailWarnings,
-                      Options.LinkOpts.Verbose, Options.InputIsYAMLDebugMap);
+                      Options.LinkOpts.PrependPath, Options.LinkOpts.Verbose,
+                      Options.InputIsYAMLDebugMap);
 
     if (auto EC = DebugMapPtrsOrErr.getError()) {
       WithColor::error() << "cannot parse the debug map for '" << InputFile
diff --git a/llvm/tools/dsymutil/dsymutil.h b/llvm/tools/dsymutil/dsymutil.h
@@ -37,8 +37,7 @@ class BinaryHolder;
 ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
 parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
               StringRef InputFile, ArrayRef<std::string> Archs,
-              StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
-              bool InputIsYAML);
+              StringRef PrependPath, bool Verbose, bool InputIsYAML);
 
 /// Dump the symbol table.
 bool dumpStab(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
diff --git a/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp b/llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
@@ -322,17 +322,15 @@ Error linkDebugInfoImpl(object::ObjectFile &File, const Options &Options,
   DebugInfoLinker->setUpdateIndexTablesOnly(!Options.DoGarbageCollection);
 
   std::vector<std::unique_ptr<OutDwarfFile>> ObjectsForLinking(1);
-  std::vector<std::string> EmptyWarnings;
 
   // Add object files to the DWARFLinker.
   std::unique_ptr<DWARFContext> Context = DWARFContext::create(File);
   std::unique_ptr<ObjFileAddressMap<AddressMapBase>> AddressesMap(
       std::make_unique<ObjFileAddressMap<AddressMapBase>>(*Context, Options,
                                                           File));
 
-  ObjectsForLinking[0] =
-      std::make_unique<OutDwarfFile>(File.getFileName(), std::move(Context),
-                                     std::move(AddressesMap), EmptyWarnings);
+  ObjectsForLinking[0] = std::make_unique<OutDwarfFile>(
+      File.getFileName(), std::move(Context), std::move(AddressesMap));
 
   uint16_t MaxDWARFVersion = 0;
   std::function<void(const DWARFUnit &Unit)> OnCUDieLoaded =
