[dsymutil] Add preliminary support for DWARF 5.

[JDevlieghere]

Summary

Currently dsymutil will silently fail when processing binaries with
Dwarf 5 debug info. This patch adds rudimentary support for Dwarf 5 in
dsymutil.

• Recognize relocations in the debug_addr section.
• Recognize (a subset of) Dwarf 5 form values.
• Emits valid Dwarf 5 compile unit header chains.

To simplify things (and avoid having to emit indexed sections) I decided
to emit the relocated addresses directly in the debug info section.

• DW_FORM_strx gets relocated and rewritten to DW_FORM_strp
• DW_FORM_addrx gets relocated and rewritten to DW_FORM_addr

Obviously there's a lot of work left, but this should be a step in the
right direction.

rdar://62345491

[avl]

If I understood correctly, It looks like this change is not enough to work properly for DWARF5.
Currently, relocation is checked using offsets to the attribute:

bool DwarfLinkerForBinary::AddressManager::hasLiveAddressRange(
    const DWARFDie &DIE, CompileUnit::DIEInfo &MyInfo) {
  const auto *Abbrev = DIE.getAbbreviationDeclarationPtr();

  Optional<uint32_t> LowPcIdx = Abbrev->findAttributeIndex(dwarf::DW_AT_low_pc);
  if (!LowPcIdx)
    return false;

  uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
  uint64_t LowPcOffset, LowPcEndOffset;
  std::tie(LowPcOffset, LowPcEndOffset) =
      getAttributeOffsets(Abbrev, *LowPcIdx, Offset, *DIE.getDwarfUnit());

  return hasValidRelocationAt(LowPcOffset, LowPcEndOffset, MyInfo);   <<<< check relocation inside debug_info
}

i.e. we check whether relocation exists between LowPcOffset, LowPcEndOffset of dwarf::DW_AT_low_pc inside debug_info table.

If we would read relocations for "debug_addr" table instead of the "debug_info" table then we would check the wrong relocations. To work properly we would need to recognize offset inside "debug_addr". Something like this(I did not check whether it could be compiled or whether it is full enough):

Optional<uint32_t> LowPcIdx = Abbrev->findAttributeIndex(dwarf::DW_AT_low_pc);
if (!LowPcIdx)
  return false;

if ( Abbrev->getFormByIndex(*LowPcIdx) == DW_FORM_addr ) {
  uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
  uint64_t LowPcOffset, LowPcEndOffset;
  std::tie(LowPcOffset, LowPcEndOffset) =
    getAttributeOffsets(Abbrev, *LowPcIdx, Offset, *DIE.getDwarfUnit());

  return hasValidRelocationAt(LowPcOffset, LowPcEndOffset, MyInfo); <<<< check relocations inside debug_info
} else ( Abbrev->getFormByIndex(*LowPcIdx) == DW_FORM_addrx) {
  Optional<DWARFFormValue> OffsetValue = DIE.find(dwarf::DW_AT_low_pc);
  if (OffsetValue) {
    uint64_t LowPcOffset = OffsetValue->getRawUValue();
    uint64_t LowPcEndOffset = LowPcOffset + 4;
    return hasValidRelocationAt(LowPcOffset, LowPcEndOffset, MyInfo); <<<< check relocations inside debug_addr
  }
}

i.e. in case when we do "findValidRelocs(Section, Obj, DMO)" for "debug_addr" then we need to use other offsets(LowPcOffset/LowPcEndOffset) inside hasLiveMemoryLocation/hasLiveAddressRange.

[JDevlieghere]

In D94323#2487899, @avl wrote:

If I understood correctly, It looks like this change is not enough to work properly for DWARF5.

Yeah, I knew that when I uploaded the patch but I was planning to put up a series of smaller patches but then I changed my mind. I went with a slightly different approach than what you suggested, but the idea is the essentially the same. At first I tried to shoehorn them into the ValidRelocs list but the separate vector simplifies things a lot.

[JDevlieghere]

llvm/lib/DWARFLinker/DWARFStreamer.cpp

231 ↗

(On Diff #315558)

Note to self: remove this when updating the diff.

[avl]

llvm/include/llvm/DWARFLinker/DWARFLinker.h

88–89 ↗

(On Diff #315558)

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

688

it is also necessary to update how dwarf::DW_AT_location is handled for debug_addr. It looks like we need to parse location expression, extract addresses, and check whether it points to live code. I think it needs comment in this review if it is supposed to be done in some follow-up review.

707

I would propose to use single unified searching function here. i.e. to use the same AddressManager::hasValidRelocationAt() for both tables(debug_info and debug_abbrev). Like in this review - D93106 .

[avl]

llvm/tools/dsymutil/DwarfLinkerForBinary.h

163 ↗

(On Diff #315558)

nit: probably it would be better to name argument not Addr, but DebugAddrOffset or something like that. To make it clear that specified operand is not an address but offset in the debug_addr table.

[avl]

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

614

hasValidDebugAddrRelocationAt() and hasValidDebugInfoRelocationAt() have different implementations - first uses binary search and second sequentially searches from the first till last. My previous comment was about using one implementation for both cases. But, let it would be done with D93106.

757

it looks like we need to add ValidReloc.Addend here, the same as in applyValidRelocs():

return It->Mapping->getValue().BinaryAddress + It->Addend;

Also, not for this review, but as a general thing: probably it would be better to make relocateIndexedAddr() to be more general(since it appears in AddressManager interface). i.e. make relocateIndexedAddr to be able to relocate any address, not only one from debug_addr section:

class AddressManager {
public:

uint64_t relocateAddr(const DWARFFormValue&);
}

llvm/tools/dsymutil/DwarfLinkerForBinary.h

154 ↗

(On Diff #315884)

The comment is about debug_addr section but the function works with debug_info section.

163 ↗

(On Diff #315884)

/// This function must be called with offsets in strictly ascending order
/// because it never looks back at relocations it already 'went past'.

above comment is old. This function could not be called with offsets in strictly ascending order. DIEs from debug_info are evaluated in strictly ascending order, but addresses which they are referred may be in any order. The implementation of hasValidDebugAddrRelocationAt uses lower_bound search function and does not relate on order of offsets.

it looks like comments for hasValidDebugAddrRelocationAt and hasValidDebugInfoRelocationAt somehow mixed up.

[avl]

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

756

It looks incorrect to return offset in debug_addr section as the value of relocated address. It might be better to make return value Expected and return Error in case offset is not found.

[JDevlieghere]

Address @avl's review comments.

[JDevlieghere]

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

614

Ah, I misunderstood. Yeah I figured we could unify this after that patch lands.

[avl]

LGTM with small comment inside.

llvm/tools/dsymutil/DwarfLinkerForBinary.h

152 ↗

(On Diff #316124)

llvm/test/tools/dsymutil/X86/dwarf5.test

+Test DWARF5 support in dsymutil. Currently this still generates an empty dSYM.
+
+$ cat dwarf5.c
+__attribute__ ((optnone))
+int foo() {
+  volatile i;
+  return i;
+}
+
+int main(int argc, char** argv) {
+  return foo();
+}
+
+$ clang -gdwarf-5 dwarf5.c -c -o dwarf5.o
+$ clang dwarf5.o -o dwarf5.out
+
+RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/dwarf5/dwarf5.out -o %t.dSYM 2>&1 | FileCheck %s
+CHECK: warning: Unsupported attribute form in cloneAttribute. Dropping.

llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

bool DwarfLinkerForBinary::AddressManager::findValidRelocs(
   // Sort the relocations by offset. We will walk the DIEs linearly in
   // the file, this allows us to just keep an index in the relocation
   // array that we advance during our walk, rather than resorting to
   // some associative container. See DwarfLinkerForBinary::NextValidReloc.
   llvm::sort(ValidRelocs);
   return true;
 }
 
-/// Look for relocations in the debug_info section that match
-/// entries in the debug map. These relocations will drive the Dwarf
-/// link by indicating which DIEs refer to symbols present in the
-/// linked binary.
+/// Look for relocations in either the debug_info or the debug_addr sections
+/// that match entries in the debug map. These relocations will drive the Dwarf
+/// link by indicating which DIEs refer to symbols present in the linked
+/// binary.
 /// \returns whether there are any valid relocations in the debug info.
 bool DwarfLinkerForBinary::AddressManager::findValidRelocsInDebugInfo(
     const object::ObjectFile &Obj, const DebugMapObject &DMO) {
   // Find the debug_info section.
   for (const object::SectionRef &Section : Obj.sections()) {
     StringRef SectionName;
     if (Expected<StringRef> NameOrErr = Section.getName())
       SectionName = *NameOrErr;
     else
       consumeError(NameOrErr.takeError());
 
     SectionName = SectionName.substr(SectionName.find_first_not_of("._"));
-    if (SectionName != "debug_info")
-      continue;
-    return findValidRelocs(Section, Obj, DMO);
+    if (SectionName == "debug_info" || SectionName == "debug_addr")
+      if(findValidRelocs(Section, Obj, DMO))
+        return true;
   }
   return false;
 }
 
 /// Checks that there is a relocation against an actual debug
 /// map entry between \p StartOffset and \p NextOffset.
 ///
 /// This function must be called with offsets in strictly ascending
 /// order because it never looks back at relocations it already 'went past'.
 /// \returns true and sets Info.InDebugMap if it is the case.
 bool DwarfLinkerForBinary::AddressManager::hasValidRelocationAt(
     uint64_t StartOffset, uint64_t EndOffset, CompileUnit::DIEInfo &Info) {
   assert(NextValidReloc == 0 ||
          StartOffset > ValidRelocs[NextValidReloc - 1].Offset);
   if (NextValidReloc >= ValidRelocs.size())
     return false;
 
   uint64_t RelocOffset = ValidRelocs[NextValidReloc].Offset;
 
bool DwarfLinkerForBinary::AddressManager::hasLiveMemoryLocation(
   if (!LocationIdx)
     return false;
 
   uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
   uint64_t LocationOffset, LocationEndOffset;
   std::tie(LocationOffset, LocationEndOffset) =
       getAttributeOffsets(Abbrev, *LocationIdx, Offset, *DIE.getDwarfUnit());
 
   return hasValidRelocationAt(LocationOffset, LocationEndOffset, MyInfo);
 }
 
 bool DwarfLinkerForBinary::AddressManager::hasLiveAddressRange(
     const DWARFDie &DIE, CompileUnit::DIEInfo &MyInfo) {
   const auto *Abbrev = DIE.getAbbreviationDeclarationPtr();
 
   Optional<uint32_t> LowPcIdx = Abbrev->findAttributeIndex(dwarf::DW_AT_low_pc);
   if (!LowPcIdx)
     return false;
 
   uint64_t Offset = DIE.getOffset() + getULEB128Size(Abbrev->getCode());
   uint64_t LowPcOffset, LowPcEndOffset;
   std::tie(LowPcOffset, LowPcEndOffset) =
       getAttributeOffsets(Abbrev, *LowPcIdx, Offset, *DIE.getDwarfUnit());
 
   return hasValidRelocationAt(LowPcOffset, LowPcEndOffset, MyInfo);
 }
 /// Apply the valid relocations found by findValidRelocs() to
 /// the buffer \p Data, taking into account that Data is at \p BaseOffset
 /// in the debug_info section.
 ///
 /// Like for findValidRelocs(), this function must be called with
 /// monotonic \p BaseOffset values.
 ///
 /// \returns whether any reloc has been applied.
 bool DwarfLinkerForBinary::AddressManager::applyValidRelocs(
     MutableArrayRef<char> Data, uint64_t BaseOffset, bool IsLittleEndian) {
bool DwarfLinkerForBinary::AddressManager::applyValidRelocs(
   return Applied;
 }
 
 bool linkDwarf(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
                const DebugMap &DM, LinkOptions Options) {
   DwarfLinkerForBinary Linker(OutFile, BinHolder, std::move(Options));
   return Linker.link(DM);
 }
 
 } // namespace dsymutil
 } // namespace llvm