[DebugInfo] Handle fixed-width DW_FORM_addrx variants in DWARFFormValue::getAsSectionedAddress()

[benmxwl-arm]

Summary

Previously this would incorrectly return the raw offset into the .debug_addr section for the
DW_FORM_addrx1/2/3/4 forms rather than the actual address.

Note that this was handled correctly in the dump() function so this issue only occurs for users
of this API and not in tools such as llvm-dwarfdump.

[dblaikie]

Could this be refactored to share some support/implementation with the dumping code so that the code is better tested?

(is this really untestable through LLVM tools? If you put an assert(false) in this if block, does it not fire on any LLVM tests?)

[benmxwl-arm]

Some tests call into this function, but I don't see any ones that are (directly) testing the indexed variants. I might have missed something though.
Maybe it'd be possible to write a new test case for DWARFDebugInfoTest.cpp?

Also, I think it would be easy enough to make dump() call this method to get the address.

[benmxwl-arm]

I've updated DWARFFormValue::dump() to make use of getAsSectionedAddress(), which slightly simplifies it.

I've now also added some unit tests for the DWARF5 indexed address forms to DWARFDebugInfoTest. Previously, this was only testing the non-indexed addresses in TestAllForms().

To do this I had to add support for the indexed addresses in the unittest DwarfGenerator, so please let me know if that looks correct.

(One thing to note and [I need to confirm] it seems like DW_FORM_addrx3 currently fails to parse, it looks like it's missed off the DWARFFormValue::skipValue() switch case)

[dblaikie]

Looks great, thanks!

Yeah, if you wanted to address the addrx3 issue, I'm happy to review that. (any particular motivation for this work direction? Got other compilers producing interesting DWARF forms?)

[benmxwl-arm]

Thanks a lot! The motivation here is more just completeness, as I've recently been working on enabling some DWARF5 features in a debugger (looking at the spec and LLVM).

[dblaikie]

In D143073#4105905, @benmxwl-arm wrote:

Thanks a lot! The motivation here is more just completeness, as I've recently been working on enabling some DWARF5 features in a debugger (looking at the spec and LLVM).

Cool - good to know :)

Do you need me to submit this for you, or can you submit it yourself?

[benmxwl-arm]

Do you need me to submit this for you, or can you submit it yourself?

I can submit it myself thanks :)

[nickdesaulniers]

llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp

63

/android0/llvm-project/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp:63:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
  default:
  ^
/android0/llvm-project/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp:63:3: note: insert 'break;' to avoid fall-through
  default:
  ^
  break;

[benmxwl-arm]

Fix pushed: https://github.com/llvm/llvm-project/commit/4b8fdb61afe3444e9b54c9c1d9fbb6cd928b9541

llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp

void DWARFFormValue::dump(raw_ostream &OS, DIDumpOptions DumpOpts) const {
   case DW_FORM_addr:
     dumpSectionedAddress(AddrOS, DumpOpts, {Value.uval, Value.SectionIndex});
     break;
   case DW_FORM_addrx:
   case DW_FORM_addrx1:
   case DW_FORM_addrx2:
   case DW_FORM_addrx3:
   case DW_FORM_addrx4:
-  case DW_FORM_GNU_addr_index: {
-    if (U == nullptr) {
-      OS << "<invalid dwarf unit>";
-      break;
-    }
-    std::optional<object::SectionedAddress> A =
-        U->getAddrOffsetSectionItem(UValue);
-    if (!A || DumpOpts.Verbose)
-      AddrOS << format("indexed (%8.8x) address = ", (uint32_t)UValue);
-    if (A)
-      dumpSectionedAddress(AddrOS, DumpOpts, *A);
-    else
-      OS << "<unresolved>";
-    break;
-  }
+  case DW_FORM_GNU_addr_index:
   case DW_FORM_LLVM_addrx_offset: {
     if (U == nullptr) {
       OS << "<invalid dwarf unit>";
       break;
     }
+    std::optional<object::SectionedAddress> A = getAsSectionedAddress();
+    if (!A || DumpOpts.Verbose) {
+      if (Form == DW_FORM_LLVM_addrx_offset) {
         uint32_t Index = UValue >> 32;
         uint32_t Offset = UValue & 0xffffffff;
-    std::optional<object::SectionedAddress> A =
-        U->getAddrOffsetSectionItem(Index);
-    if (!A || DumpOpts.Verbose)
         AddrOS << format("indexed (%8.8x) + 0x%x address = ", Index, Offset);
-    if (A) {
-      A->Address += Offset;
-      dumpSectionedAddress(AddrOS, DumpOpts, *A);
       } else
+        AddrOS << format("indexed (%8.8x) address = ", (uint32_t)UValue);
+    }
+    if (A)
+      dumpSectionedAddress(AddrOS, DumpOpts, *A);
+    else
       OS << "<unresolved>";
     break;
   }
   case DW_FORM_flag_present:
     OS << "true";
     break;
   case DW_FORM_flag:
   case DW_FORM_data1:
std::optional<uint64_t> DWARFFormValue::getAsAddress() const {
   return std::nullopt;
 }
 
 std::optional<object::SectionedAddress>
 DWARFFormValue::getAsSectionedAddress() const {
   if (!isFormClass(FC_Address))
     return std::nullopt;
   bool AddrOffset = Form == dwarf::DW_FORM_LLVM_addrx_offset;
-  if (Form == DW_FORM_GNU_addr_index || Form == DW_FORM_addrx || AddrOffset) {
+  if (Form == DW_FORM_GNU_addr_index || Form == DW_FORM_addrx ||
+      Form == DW_FORM_addrx1 || Form == DW_FORM_addrx2 ||
+      Form == DW_FORM_addrx3 || Form == DW_FORM_addrx4 || AddrOffset) {
 
     uint32_t Index = AddrOffset ? (Value.uval >> 32) : Value.uval;
     if (!U)
       return std::nullopt;
     std::optional<object::SectionedAddress> SA =
         U->getAddrOffsetSectionItem(Index);
     if (!SA)
       return std::nullopt;

llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

 template <uint16_t Version, class AddrType, class RefAddrType>
 void TestAllForms() {
   Triple Triple = getDefaultTargetTripleForAddrSize(sizeof(AddrType));
   if (!isConfigurationSupported(Triple))
     GTEST_SKIP();
 
   // Test that we can decode all DW_FORM values correctly.
   const AddrType AddrValue = (AddrType)0x0123456789abcdefULL;
+  const AddrType AddrxValue = (AddrType)0x4231abcd4231abcdULL;
+  const AddrType Addrx1Value = (AddrType)0x0000aaaabbbbccccULL;
+  const AddrType Addrx2Value = (AddrType)0xf00123f00456f000ULL;
+  const AddrType Addrx4Value = (AddrType)0xa1b2c3d4e5f6e5d4ULL;
+
   const uint8_t BlockData[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 0};
   const uint32_t BlockSize = sizeof(BlockData);
   const RefAddrType RefAddr = 0x12345678;
   const uint8_t Data1 = 0x01U;
   const uint16_t Data2 = 0x2345U;
   const uint32_t Data4 = 0x6789abcdU;
   const uint64_t Data8 = 0x0011223344556677ULL;
   const uint64_t Data8_2 = 0xAABBCCDDEEFF0011ULL;
#define UDATA_1 18446744073709551614ULL
   const char *Strx4Value = "Indexed4";
 
   auto ExpectedDG = dwarfgen::Generator::create(Triple, Version);
   ASSERT_THAT_EXPECTED(ExpectedDG, Succeeded());
   dwarfgen::Generator *DG = ExpectedDG.get().get();
   dwarfgen::CompileUnit &CU = DG->addCompileUnit();
   dwarfgen::DIE CUDie = CU.getUnitDIE();
 
-  if (Version >= 5)
+  if (Version >= 5) {
     CUDie.addStrOffsetsBaseAttribute();
+    CUDie.addAddrBaseAttribute();
+  }
 
   uint16_t Attr = DW_AT_lo_user;
 
   //----------------------------------------------------------------------
   // Test address forms
   //----------------------------------------------------------------------
   const auto Attr_DW_FORM_addr = static_cast<dwarf::Attribute>(Attr++);
   CUDie.addAttribute(Attr_DW_FORM_addr, DW_FORM_addr, AddrValue);
 
+  const auto Attr_DW_FORM_addrx = static_cast<dwarf::Attribute>(Attr++);
+  const auto Attr_DW_FORM_addrx1 = static_cast<dwarf::Attribute>(Attr++);
+  const auto Attr_DW_FORM_addrx2 = static_cast<dwarf::Attribute>(Attr++);
+  // TODO: Add Attr_DW_FORM_addrx3 test (this form type is currently
+  // unsupported)
+  const auto Attr_DW_FORM_addrx4 = static_cast<dwarf::Attribute>(Attr++);
+
+  if (Version >= 5) {
+    CUDie.addAttribute(Attr_DW_FORM_addrx, DW_FORM_addrx, AddrxValue);
+    CUDie.addAttribute(Attr_DW_FORM_addrx1, DW_FORM_addrx1, Addrx1Value);
+    CUDie.addAttribute(Attr_DW_FORM_addrx2, DW_FORM_addrx2, Addrx2Value);
+    CUDie.addAttribute(Attr_DW_FORM_addrx4, DW_FORM_addrx4, Addrx4Value);
+  }
+
   //----------------------------------------------------------------------
   // Test block forms
   //----------------------------------------------------------------------
   const auto Attr_DW_FORM_block = static_cast<dwarf::Attribute>(Attr++);
   CUDie.addAttribute(Attr_DW_FORM_block, DW_FORM_block, BlockData, BlockSize);
 
   const auto Attr_DW_FORM_block1 = static_cast<dwarf::Attribute>(Attr++);
   CUDie.addAttribute(Attr_DW_FORM_block1, DW_FORM_block1, BlockData, BlockSize);
#define UDATA_1 18446744073709551614ULL
   auto DieDG = U->getUnitDIE(false);
   EXPECT_TRUE(DieDG.isValid());
 
   //----------------------------------------------------------------------
   // Test address forms
   //----------------------------------------------------------------------
   EXPECT_EQ(AddrValue, toAddress(DieDG.find(Attr_DW_FORM_addr), 0));
 
+  if (Version >= 5) {
+    auto ExtractedAddrxValue = toAddress(DieDG.find(Attr_DW_FORM_addrx));
+    EXPECT_TRUE(ExtractedAddrxValue.has_value());
+    EXPECT_EQ(AddrxValue, *ExtractedAddrxValue);
+
+    auto ExtractedAddrx1Value = toAddress(DieDG.find(Attr_DW_FORM_addrx1));
+    EXPECT_TRUE(ExtractedAddrx1Value.has_value());
+    EXPECT_EQ(Addrx1Value, *ExtractedAddrx1Value);
+
+    auto ExtractedAddrx2Value = toAddress(DieDG.find(Attr_DW_FORM_addrx2));
+    EXPECT_TRUE(ExtractedAddrx2Value.has_value());
+    EXPECT_EQ(Addrx2Value, *ExtractedAddrx2Value);
+
+    auto ExtractedAddrx4Value = toAddress(DieDG.find(Attr_DW_FORM_addrx4));
+    EXPECT_TRUE(ExtractedAddrx1Value.has_value());
+    EXPECT_EQ(Addrx4Value, *ExtractedAddrx4Value);
+  }
+
   //----------------------------------------------------------------------
   // Test block forms
   //----------------------------------------------------------------------
   std::optional<DWARFFormValue> FormValue;
   ArrayRef<uint8_t> ExtractedBlockData;
   std::optional<ArrayRef<uint8_t>> BlockDataOpt;
 
   FormValue = DieDG.find(Attr_DW_FORM_block);

llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h

public:
   /// must be one of DW_FORM_strp or DW_FORM_string.
   /// \param P a pointer to the data to store as the attribute value.
   /// \param S the size in bytes of the data pointed to by P .
   void addAttribute(uint16_t Attr, dwarf::Form Form, const void *P, size_t S);
 
   /// Add a DW_AT_str_offsets_base attribute to this DIE.
   void addStrOffsetsBaseAttribute();
 
+  /// Add a DW_AT_addr_base attribute to this DIE.
+  void addAddrBaseAttribute();
+
   /// Add a new child to this DIE object.
   ///
   /// \param Tag the dwarf::Tag to assing to the llvm::DIE object.
   /// \returns the newly created DIE object that is now a child owned by this
   /// object.
   dwarfgen::DIE addChild(dwarf::Tag Tag);
 };
 
class Generator {
   TargetLoweringObjectFile *TLOF; // Owned by TargetMachine;
   std::unique_ptr<AsmPrinter> Asm;
   BumpPtrAllocator Allocator;
   std::unique_ptr<DwarfStringPool> StringPool; // Entries owned by Allocator.
   std::vector<std::unique_ptr<CompileUnit>> CompileUnits;
   std::vector<std::unique_ptr<LineTable>> LineTables;
   DIEAbbrevSet Abbreviations;
 
+  // Mimics llvm::AddressPool, but allows for constant addresses for testing.
+  struct DummyAddressPool {
+    unsigned getIndex(uint64_t Address);
+
+    void emit(AsmPrinter &Asm, MCSection *AddrSection, MCSymbol *StartSym);
+
+    std::vector<uint64_t> AddressValues;
+  } AddressPool;
+
   MCSymbol *StringOffsetsStartSym;
+  MCSymbol *AddrTableStartSym;
 
   SmallString<4096> FileBytes;
   /// The stream we use to generate the DWARF into as an ELF file.
   std::unique_ptr<raw_svector_ostream> Stream;
   /// The DWARF version to generate.
   uint16_t Version;
 
   /// Private constructor, call Generator::Create(...) to get a DWARF generator
public:
   addLineTable(dwarf::DwarfFormat DwarfFormat = dwarf::DwarfFormat::DWARF32);
 
   BumpPtrAllocator &getAllocator() { return Allocator; }
   AsmPrinter *getAsmPrinter() const { return Asm.get(); }
   MCContext *getMCContext() const { return MC.get(); }
   DIEAbbrevSet &getAbbrevSet() { return Abbreviations; }
   DwarfStringPool &getStringPool() { return *StringPool; }
   MCSymbol *getStringOffsetsStartSym() const { return StringOffsetsStartSym; }
+  DummyAddressPool &getAddressPool() { return AddressPool; }
+  MCSymbol *getAddrTableStartSym() const { return AddrTableStartSym; }
 
   /// Save the generated DWARF file to disk.
   ///
   /// \param Path the path to save the ELF file to.
   bool saveFile(StringRef Path);
 };
 
 } // end namespace dwarfgen
 
 } // end namespace llvm
 
 #endif // LLVM_UNITTESTS_DEBUG_INFO_DWARF_DWARFGENERATOR_H

llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp

 unsigned dwarfgen::DIE::computeSizeAndOffsets(unsigned Offset) {
   auto &DG = CU->getGenerator();
   return Die->computeOffsetsAndAbbrevs(DG.getAsmPrinter()->getDwarfFormParams(),
                                        DG.getAbbrevSet(), Offset);
 }
 
 void dwarfgen::DIE::addAttribute(uint16_t A, dwarf::Form Form, uint64_t U) {
   auto &DG = CU->getGenerator();
+  switch (Form) {
+  case DW_FORM_addrx:
+  case DW_FORM_addrx1:
+  case DW_FORM_addrx2:
+  case DW_FORM_addrx3:
+  case DW_FORM_addrx4:
+    U = DG.getAddressPool().getIndex(U);
+  default:
+    break;
+  }
   Die->addValue(DG.getAllocator(), static_cast<dwarf::Attribute>(A), Form,
                 DIEInteger(U));
 }
 
 void dwarfgen::DIE::addAttribute(uint16_t A, dwarf::Form Form, const MCExpr &Expr) {
   auto &DG = CU->getGenerator();
   Die->addValue(DG.getAllocator(), static_cast<dwarf::Attribute>(A), Form,
                 DIEExpr(&Expr));
void dwarfgen::DIE::addStrOffsetsBaseAttribute() {
 
   if (!Asm->MAI->doesDwarfUseRelocationsAcrossSections())
     Expr = MCBinaryExpr::createSub(
         Expr, MCSymbolRefExpr::create(SectionStart, MC), MC);
 
   addAttribute(dwarf::DW_AT_str_offsets_base, DW_FORM_sec_offset, *Expr);
 }
 
+// This is currently fixed to be the first address entry after the header.
+void dwarfgen::DIE::addAddrBaseAttribute() {
+  auto &DG = CU->getGenerator();
+  auto &MC = *DG.getMCContext();
+  AsmPrinter *Asm = DG.getAsmPrinter();
+
+  const MCSymbol *SectionStart =
+      Asm->getObjFileLowering().getDwarfAddrSection()->getBeginSymbol();
+
+  const MCExpr *Expr = MCSymbolRefExpr::create(DG.getAddrTableStartSym(), MC);
+
+  if (!Asm->MAI->doesDwarfUseRelocationsAcrossSections())
+    Expr = MCBinaryExpr::createSub(
+        Expr, MCSymbolRefExpr::create(SectionStart, MC), MC);
+
+  addAttribute(dwarf::DW_AT_addr_base, DW_FORM_sec_offset, *Expr);
+}
+
 dwarfgen::DIE dwarfgen::DIE::addChild(dwarf::Tag Tag) {
   auto &DG = CU->getGenerator();
   return dwarfgen::DIE(CU,
                        &Die->addChild(llvm::DIE::get(DG.getAllocator(), Tag)));
 }
 
 dwarfgen::DIE dwarfgen::CompileUnit::getUnitDIE() {
   return dwarfgen::DIE(this, &DU.getUnitDie());
llvm::Error dwarfgen::Generator::init(Triple TheTriple, uint16_t V) {
 
   // Set the DWARF version correctly on all classes that we use.
   MC->setDwarfVersion(Version);
   Asm->setDwarfVersion(Version);
 
   StringPool = std::make_unique<DwarfStringPool>(Allocator, *Asm, StringRef());
   StringOffsetsStartSym = Asm->createTempSymbol("str_offsets_base");
 
+  AddrTableStartSym = Asm->createTempSymbol("addr_table_base");
+
   return Error::success();
 }
 
+unsigned dwarfgen::Generator::DummyAddressPool::getIndex(uint64_t Address) {
+  AddressValues.push_back(Address);
+  return static_cast<unsigned>(AddressValues.size() - 1);
+}
+
+void dwarfgen::Generator::DummyAddressPool::emit(AsmPrinter &Asm,
+                                                 MCSection *AddrSection,
+                                                 MCSymbol *StartSym) {
+  const uint8_t AddrSize = Asm.getPointerSize();
+
+  // Switch to .debug_addr section
+  Asm.OutStreamer->switchSection(AddrSection);
+
+  if (Asm.getDwarfVersion() >= 5) {
+    // Emit header
+    Asm.emitDwarfUnitLength(AddrSize * AddressValues.size() + 4,
+                            "Length of contribution");
+    Asm.emitInt16(Asm.getDwarfVersion());
+    Asm.emitInt8(AddrSize);
+    Asm.emitInt8(0);
+  }
+
+  if (StartSym)
+    Asm.OutStreamer->emitLabel(StartSym);
+
+  // Emit addresses
+  for (uint64_t Addr : AddressValues)
+    Asm.OutStreamer->emitIntValue(Addr, AddrSize);
+}
+
 StringRef dwarfgen::Generator::generate() {
   // Offset from the first CU in the debug info section is 0 initially.
   uint64_t SecOffset = 0;
 
   // Iterate over each compile unit and set the size and offsets for each
   // DIE within each compile unit. All offsets are CU relative.
   for (auto &CU : CompileUnits) {
     // Set the absolute .debug_info offset for this compile unit.
     CU->setOffset(SecOffset);
     // The DIEs contain compile unit relative offsets.
     unsigned CUOffset = 11;
     CUOffset = CU->getUnitDIE().computeSizeAndOffsets(CUOffset);
     // Update our absolute .debug_info offset.
     SecOffset += CUOffset;
     CU->setLength(CUOffset - 4);
   }
   Abbreviations.Emit(Asm.get(), TLOF->getDwarfAbbrevSection());
 
   StringPool->emitStringOffsetsTableHeader(*Asm, TLOF->getDwarfStrOffSection(),
                                            StringOffsetsStartSym);
   StringPool->emit(*Asm, TLOF->getDwarfStrSection(),
                    TLOF->getDwarfStrOffSection());
 
+  AddressPool.emit(*Asm, TLOF->getDwarfAddrSection(), AddrTableStartSym);
+
   MS->switchSection(TLOF->getDwarfInfoSection());
   for (auto &CU : CompileUnits) {
     uint16_t Version = CU->getVersion();
     auto Length = CU->getLength();
     MC->setDwarfVersion(Version);
     assert(Length != -1U);
     Asm->emitInt32(Length);
     Asm->emitInt16(Version);