Revert "[Reland][DWARFLinkerParallel] Add limited functionality to DWARFLinkerParallel."

This reverts commit 0229dd0.

This introduces two test failures on s390x.

tools/dsymutil/X86/location-expression.test:

    warning: cann't load line table.
    note: while processing CU1
    /builddir/build/BUILD/llvm-18.0.0.src/test/tools/dsymutil/X86/location-expression.test:20:10: error: CHECK: expected string not found in input
    # CHECK: DW_AT_name{{.*}}"CU1"
             ^
    <stdin>:34:32: note: scanning from here
    0x0000000b: DW_TAG_compile_unit [1] *
                                   ^
    <stdin>:37:2: note: possible intended match here
     DW_AT_name [DW_FORM_strp] ( .debug_str[0x09000000] = )
     ^

tools/dsymutil/X86/tls-variable.test:

    warning: cann't load line table.
    note: while processing CU1
    /builddir/build/BUILD/llvm-18.0.0.src/test/tools/dsymutil/X86/tls-variable.test:19:10: error: CHECK: expected string not found in input
    # CHECK: DW_AT_name{{.*}}"CU1"
             ^
    <stdin>:26:32: note: scanning from here
    0x0000000b: DW_TAG_compile_unit
                                   ^
    <stdin>:29:2: note: possible intended match here
     DW_AT_name ()
     ^

Author: nikic

llvm/include/llvm/CodeGen/DwarfStringPoolEntry.h

@@ -27,34 +27,29 @@ struct DwarfStringPoolEntry {
   bool isIndexed() const { return Index != NotIndexed; }
 };
 
-/// DwarfStringPoolEntry with string keeping externally.
-struct DwarfStringPoolEntryWithExtString : public DwarfStringPoolEntry {
-  StringRef String;
-};
-
 /// DwarfStringPoolEntryRef: Dwarf string pool entry reference.
 ///
 /// Dwarf string pool entry keeps string value and its data.
 /// There are two variants how data are represented:
 ///
-///   1. String data in pool  - StringMapEntry<DwarfStringPoolEntry>.
-///   2. External string data - DwarfStringPoolEntryWithExtString.
+///   1. By value - StringMapEntry<DwarfStringPoolEntry>.
+///   2. By pointer - StringMapEntry<DwarfStringPoolEntry *>.
 ///
-/// The external data variant allows reducing memory usage for the case
-/// when string pool entry does not have data: string entry does not
-/// keep any data and so no need to waste space for the full
-/// DwarfStringPoolEntry. It is recommended to use external variant if not all
-/// entries of dwarf string pool have corresponding DwarfStringPoolEntry.
+/// The "By pointer" variant allows for reducing memory usage for the case
+/// when string pool entry does not have data: it keeps the null pointer
+/// and so no need to waste space for the full DwarfStringPoolEntry.
+/// It is recommended to use "By pointer" variant if not all entries
+/// of dwarf string pool have corresponding DwarfStringPoolEntry.
 
 class DwarfStringPoolEntryRef {
   /// Pointer type for "By value" string entry.
   using ByValStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry> *;
 
-  /// Pointer type for external string entry.
-  using ExtStringEntryPtr = const DwarfStringPoolEntryWithExtString *;
+  /// Pointer type for "By pointer" string entry.
+  using ByPtrStringEntryPtr = const StringMapEntry<DwarfStringPoolEntry *> *;
 
   /// Pointer to the dwarf string pool Entry.
-  PointerUnion<ByValStringEntryPtr, ExtStringEntryPtr> MapEntry = nullptr;
+  PointerUnion<ByValStringEntryPtr, ByPtrStringEntryPtr> MapEntry = nullptr;
 
 public:
   DwarfStringPoolEntryRef() = default;
@@ -66,8 +61,10 @@ class DwarfStringPoolEntryRef {
 
   /// ASSUMPTION: DwarfStringPoolEntryRef keeps pointer to \p Entry,
   /// thus specified entry mustn`t be reallocated.
-  DwarfStringPoolEntryRef(const DwarfStringPoolEntryWithExtString &Entry)
-      : MapEntry(&Entry) {}
+  DwarfStringPoolEntryRef(const StringMapEntry<DwarfStringPoolEntry *> &Entry)
+      : MapEntry(&Entry) {
+    assert(cast<ByPtrStringEntryPtr>(MapEntry)->second != nullptr);
+  }
 
   explicit operator bool() const { return !MapEntry.isNull(); }
 
@@ -91,15 +88,15 @@ class DwarfStringPoolEntryRef {
     if (isa<ByValStringEntryPtr>(MapEntry))
       return cast<ByValStringEntryPtr>(MapEntry)->first();
 
-    return cast<ExtStringEntryPtr>(MapEntry)->String;
+    return cast<ByPtrStringEntryPtr>(MapEntry)->first();
   }
 
   /// \returns the entire string pool entry for convenience.
   const DwarfStringPoolEntry &getEntry() const {
     if (isa<ByValStringEntryPtr>(MapEntry))
       return cast<ByValStringEntryPtr>(MapEntry)->second;
 
-    return *cast<ExtStringEntryPtr>(MapEntry);
+    return *cast<ByPtrStringEntryPtr>(MapEntry)->second;
   }
 
   bool operator==(const DwarfStringPoolEntryRef &X) const {

llvm/include/llvm/DWARFLinker/DWARFLinker.h

@@ -270,11 +270,9 @@ using UnitListTy = std::vector<std::unique_ptr<CompileUnit>>;
 /// and its address map.
 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)
+            const std::vector<std::string> &Warnings)
       : FileName(Name), Dwarf(std::move(Dwarf)),
         Addresses(std::move(Addresses)), Warnings(Warnings) {}
 

llvm/include/llvm/DWARFLinkerParallel/AddressesMap.h

@@ -10,10 +10,8 @@
 #define LLVM_DWARFLINKERPARALLEL_ADDRESSESMAP_H
 
 #include "llvm/ADT/AddressRanges.h"
-#include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDie.h"
 #include "llvm/DebugInfo/DWARF/DWARFExpression.h"
-#include "llvm/DebugInfo/DWARF/DWARFUnit.h"
 #include <cstdint>
 
 namespace llvm {
@@ -64,108 +62,6 @@ class AddressesMap {
 
   /// Erases all data.
   virtual void clear() = 0;
-
-  /// This function checks whether variable has DWARF expression containing
-  /// operation referencing live address(f.e. DW_OP_addr, DW_OP_addrx...).
-  /// \returns first is true if the expression has an operation referencing an
-  /// address.
-  ///          second is the relocation adjustment value if the live address is
-  ///          referenced.
-  std::pair<bool, std::optional<int64_t>>
-  getVariableRelocAdjustment(const DWARFDie &DIE) {
-    assert((DIE.getTag() == dwarf::DW_TAG_variable ||
-            DIE.getTag() == dwarf::DW_TAG_constant) &&
-           "Wrong type of input die");
-
-    const auto *Abbrev = DIE.getAbbreviationDeclarationPtr();
-
-    // Check if DIE has DW_AT_location attribute.
-    DWARFUnit *U = DIE.getDwarfUnit();
-    std::optional<uint32_t> LocationIdx =
-        Abbrev->findAttributeIndex(dwarf::DW_AT_location);
-    if (!LocationIdx)
-      return std::make_pair(false, std::nullopt);
-
-    // Get offset to the DW_AT_location attribute.
-    uint64_t AttrOffset =
-        Abbrev->getAttributeOffsetFromIndex(*LocationIdx, DIE.getOffset(), *U);
-
-    // Get value of the DW_AT_location attribute.
-    std::optional<DWARFFormValue> LocationValue =
-        Abbrev->getAttributeValueFromOffset(*LocationIdx, AttrOffset, *U);
-    if (!LocationValue)
-      return std::make_pair(false, std::nullopt);
-
-    // Check that DW_AT_location attribute is of 'exprloc' class.
-    // Handling value of location expressions for attributes of 'loclist'
-    // class is not implemented yet.
-    std::optional<ArrayRef<uint8_t>> Expr = LocationValue->getAsBlock();
-    if (!Expr)
-      return std::make_pair(false, std::nullopt);
-
-    // Parse 'exprloc' expression.
-    DataExtractor Data(toStringRef(*Expr), U->getContext().isLittleEndian(),
-                       U->getAddressByteSize());
-    DWARFExpression Expression(Data, U->getAddressByteSize(),
-                               U->getFormParams().Format);
-
-    bool HasLocationAddress = false;
-    uint64_t CurExprOffset = 0;
-    for (DWARFExpression::iterator It = Expression.begin();
-         It != Expression.end(); ++It) {
-      DWARFExpression::iterator NextIt = It;
-      ++NextIt;
-
-      const DWARFExpression::Operation &Op = *It;
-      switch (Op.getCode()) {
-      case dwarf::DW_OP_const2u:
-      case dwarf::DW_OP_const4u:
-      case dwarf::DW_OP_const8u:
-      case dwarf::DW_OP_const2s:
-      case dwarf::DW_OP_const4s:
-      case dwarf::DW_OP_const8s:
-        if (NextIt == Expression.end() || !isTlsAddressCode(NextIt->getCode()))
-          break;
-        [[fallthrough]];
-      case dwarf::DW_OP_addr: {
-        HasLocationAddress = true;
-        // Check relocation for the address.
-        if (std::optional<int64_t> RelocAdjustment =
-                getExprOpAddressRelocAdjustment(*U, Op,
-                                                AttrOffset + CurExprOffset,
-                                                AttrOffset + Op.getEndOffset()))
-          return std::make_pair(HasLocationAddress, *RelocAdjustment);
-      } break;
-      case dwarf::DW_OP_constx:
-      case dwarf::DW_OP_addrx: {
-        HasLocationAddress = true;
-        if (std::optional<uint64_t> AddressOffset =
-                DIE.getDwarfUnit()->getIndexedAddressOffset(
-                    Op.getRawOperand(0))) {
-          // Check relocation for the address.
-          if (std::optional<int64_t> RelocAdjustment =
-                  getExprOpAddressRelocAdjustment(
-                      *U, Op, *AddressOffset,
-                      *AddressOffset +
-                          DIE.getDwarfUnit()->getAddressByteSize()))
-            return std::make_pair(HasLocationAddress, *RelocAdjustment);
-        }
-      } break;
-      default: {
-        // Nothing to do.
-      } break;
-      }
-      CurExprOffset = Op.getEndOffset();
-    }
-
-    return std::make_pair(HasLocationAddress, std::nullopt);
-  }
-
-protected:
-  inline bool isTlsAddressCode(uint8_t DW_OP_Code) {
-    return DW_OP_Code == dwarf::DW_OP_form_tls_address ||
-           DW_OP_Code == dwarf::DW_OP_GNU_push_tls_address;
-  }
 };
 
 } // end of namespace dwarflinker_parallel

llvm/include/llvm/DWARFLinkerParallel/DWARFFile.h

@@ -30,7 +30,14 @@ class DWARFFile {
   DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
             std::unique_ptr<AddressesMap> Addresses,
             const std::vector<std::string> &Warnings,
-            UnloadCallbackTy UnloadFunc = nullptr);
+            UnloadCallbackTy UnloadFunc = nullptr)
+      : FileName(Name), Dwarf(std::move(Dwarf)),
+        Addresses(std::move(Addresses)), Warnings(Warnings),
+        UnloadFunc(UnloadFunc) {
+    if (this->Dwarf)
+      Endianess = this->Dwarf->isLittleEndian() ? support::endianness::little
+                                                : support::endianness::big;
+  }
 
   /// Object file name.
   StringRef FileName;
@@ -44,6 +51,9 @@ class DWARFFile {
   /// Warnings for object file.
   const std::vector<std::string> &Warnings;
 
+  /// Endiannes of source DWARF information.
+  support::endianness Endianess = support::endianness::little;
+
   /// Callback to the module keeping object file to unload.
   UnloadCallbackTy UnloadFunc;
 

llvm/include/llvm/DWARFLinkerParallel/StringPool.h

@@ -21,7 +21,7 @@ namespace dwarflinker_parallel {
 
 /// StringEntry keeps data of the string: the length, external offset
 /// and a string body which is placed right after StringEntry.
-using StringEntry = StringMapEntry<std::nullopt_t>;
+using StringEntry = StringMapEntry<DwarfStringPoolEntry *>;
 
 class StringPoolEntryInfo {
 public:
@@ -64,8 +64,6 @@ class StringPool
 
   parallel::PerThreadBumpPtrAllocator &getAllocatorRef() { return Allocator; }
 
-  void clear() { Allocator.Reset(); }
-
 private:
   parallel::PerThreadBumpPtrAllocator Allocator;
 };

llvm/include/llvm/DWARFLinkerParallel/StringTable.h

@@ -0,0 +1,88 @@
+//===- StringTable.h --------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DWARFLINKERPARALLEL_STRINGTABLE_H
+#define LLVM_DWARFLINKERPARALLEL_STRINGTABLE_H
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/DWARFLinkerParallel/StringPool.h"
+
+namespace llvm {
+namespace dwarflinker_parallel {
+
+using StringsVector = SmallVector<StringEntry *>;
+
+/// This class prepares strings for emission into .debug_str table:
+/// translates string if necessary, assigns index and offset, keeps in order.
+class StringTable {
+public:
+  StringTable(StringPool &Strings,
+              std::function<StringRef(StringRef)> StringsTranslator)
+      : Strings(Strings), StringsTranslator(StringsTranslator) {}
+  ~StringTable() {}
+
+  /// Add string to the vector of strings which should be emitted.
+  /// Translate input string if neccessary, assign index and offset.
+  /// \returns updated string entry.
+  StringEntry *add(StringEntry *String) {
+    // Translate string if necessary.
+    if (StringsTranslator)
+      String = Strings.insert(StringsTranslator(String->first())).first;
+
+    // Store String for emission and assign index and offset.
+    if (String->getValue() == nullptr) {
+      DwarfStringPoolEntry *NewEntry =
+          Strings.getAllocatorRef().Allocate<DwarfStringPoolEntry>();
+
+      NewEntry->Symbol = nullptr;
+      NewEntry->Index = StringEntriesForEmission.size();
+
+      if (StringEntriesForEmission.empty())
+        NewEntry->Offset = 0;
+      else {
+        StringEntry *PrevString = StringEntriesForEmission.back();
+        NewEntry->Offset =
+            PrevString->getValue()->Offset + PrevString->getKeyLength() + 1;
+      }
+
+      String->getValue() = NewEntry;
+      StringEntriesForEmission.push_back(String);
+    }
+
+    return String;
+  }
+
+  /// Erase contents of StringsForEmission.
+  void clear() { StringEntriesForEmission.clear(); }
+
+  /// Enumerate all strings in sequential order and call \p Handler for each
+  /// string.
+  void forEach(function_ref<void(DwarfStringPoolEntryRef)> Handler) const {
+    for (const StringEntry *Entry : StringEntriesForEmission)
+      Handler(*Entry);
+  }
+
+  std::function<StringRef(StringRef)> getTranslator() {
+    return StringsTranslator;
+  }
+
+protected:
+  /// List of strings for emission.
+  StringsVector StringEntriesForEmission;
+
+  /// String pool for the translated strings.
+  StringPool &Strings;
+
+  /// Translator for the strings.
+  std::function<StringRef(StringRef)> StringsTranslator;
+};
+
+} // end of namespace dwarflinker_parallel
+} // end namespace llvm
+
+#endif // LLVM_DWARFLINKERPARALLEL_STRINGTABLE_H

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugMacro.h

@@ -22,7 +22,6 @@ class DwarfStreamer;
 
 class DWARFDebugMacro {
   friend DwarfStreamer;
-  friend dwarflinker_parallel::CompileUnit;
 
   /// DWARFv5 section 6.3.1 Macro Information Header.
   enum HeaderFlagMask {

llvm/lib/DWARFLinker/DWARFLinker.cpp

@@ -444,10 +444,8 @@ DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
 
     const DWARFExpression::Operation &Op = *It;
     switch (Op.getCode()) {
-    case dwarf::DW_OP_const2u:
     case dwarf::DW_OP_const4u:
     case dwarf::DW_OP_const8u:
-    case dwarf::DW_OP_const2s:
     case dwarf::DW_OP_const4s:
     case dwarf::DW_OP_const8s:
       if (NextIt == Expression.end() || !isTlsAddressCode(NextIt->getCode()))

llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp

@@ -97,10 +97,8 @@ void CompileUnit::markEverythingAsKept() {
         ++NextIt;
 
         switch (It->getCode()) {
-        case dwarf::DW_OP_const2u:
         case dwarf::DW_OP_const4u:
         case dwarf::DW_OP_const8u:
-        case dwarf::DW_OP_const2s:
         case dwarf::DW_OP_const4s:
         case dwarf::DW_OP_const8s:
           if (NextIt == Expression.end() ||