[DebugInfo] Handle missed DW_FORM_addrx3 and DW_FORM_strx3 cases

[benmxwl-arm]

Summary

This fixes a few places where the addrx3 and strx3 forms were missed.
Previously this meant if one of these forms appeared somewhere various
errors could occur. This now also adds an extra test case for the addrx3
form (which previously failed).

[keith.walker.arm]

This LGTM.

As this is a very simple change covering an obvious omission and has associated tests, and there has beee a few days for any suggestions for improvment to be raised I'm setting "Accept Revision".

[dblaikie]

Looks good except for the unrelated change in DwarfGenerator.

llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h

270

Looks unrelated to the rest of this patch - could you revert this before submitting?

[benmxwl-arm]

Pushed (without the accidental change 👍)

llvm/lib/BinaryFormat/Dwarf.cpp

std::optional<uint8_t> llvm::dwarf::getFixedFormByteSize(dwarf::Form Form,
 
   case DW_FORM_data2:
   case DW_FORM_ref2:
   case DW_FORM_strx2:
   case DW_FORM_addrx2:
     return 2;
 
   case DW_FORM_strx3:
+  case DW_FORM_addrx3:
     return 3;
 
   case DW_FORM_data4:
   case DW_FORM_ref4:
   case DW_FORM_ref_sup4:
   case DW_FORM_strx4:
   case DW_FORM_addrx4:
     return 4;

llvm/lib/CodeGen/AsmPrinter/DIE.cpp

void DIEInteger::emitValue(const AsmPrinter *Asm, dwarf::Form Form) const {
   case dwarf::DW_FORM_data1:
   case dwarf::DW_FORM_strx1:
   case dwarf::DW_FORM_addrx1:
   case dwarf::DW_FORM_ref2:
   case dwarf::DW_FORM_data2:
   case dwarf::DW_FORM_strx2:
   case dwarf::DW_FORM_addrx2:
   case dwarf::DW_FORM_strx3:
+  case dwarf::DW_FORM_addrx3:
   case dwarf::DW_FORM_strp:
   case dwarf::DW_FORM_ref4:
   case dwarf::DW_FORM_data4:
   case dwarf::DW_FORM_ref_sup4:
   case dwarf::DW_FORM_strx4:
   case dwarf::DW_FORM_addrx4:
   case dwarf::DW_FORM_ref8:
   case dwarf::DW_FORM_ref_sig8:

llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp

do {
     case DW_FORM_ref2:
     case DW_FORM_ref4:
     case DW_FORM_ref8:
     case DW_FORM_ref_sig8:
     case DW_FORM_ref_sup4:
     case DW_FORM_ref_sup8:
     case DW_FORM_strx1:
     case DW_FORM_strx2:
+    case DW_FORM_strx3:
     case DW_FORM_strx4:
     case DW_FORM_addrx1:
     case DW_FORM_addrx2:
+    case DW_FORM_addrx3:
     case DW_FORM_addrx4:
     case DW_FORM_sec_offset:
     case DW_FORM_strp:
     case DW_FORM_strp_sup:
     case DW_FORM_line_strp:
     case DW_FORM_GNU_ref_alt:
     case DW_FORM_GNU_strp_alt:
     case DW_FORM_implicit_const:
case DW_FORM_addrx1:
       break;
     case DW_FORM_data2:
     case DW_FORM_ref2:
     case DW_FORM_strx2:
     case DW_FORM_addrx2:
       Value.uval = Data.getU16(OffsetPtr, &Err);
       break;
     case DW_FORM_strx3:
+    case DW_FORM_addrx3:
       Value.uval = Data.getU24(OffsetPtr, &Err);
       break;
     case DW_FORM_data4:
     case DW_FORM_ref4:
     case DW_FORM_ref_sup4:
     case DW_FORM_strx4:
     case DW_FORM_addrx4:
       Value.uval = Data.getRelocatedValue(4, OffsetPtr, nullptr, &Err);

llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

void TestAllForms() {
   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 Addrx3Value = (AddrType)0xABABA000B111C222ULL;
   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;
#define UDATA_1 18446744073709551614ULL
   // 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_addrx3 = static_cast<dwarf::Attribute>(Attr++);
   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_addrx3, DW_FORM_addrx3, Addrx3Value);
     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);
if (Version >= 5) {
     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 ExtractedAddrx3Value = toAddress(DieDG.find(Attr_DW_FORM_addrx3));
+    EXPECT_TRUE(ExtractedAddrx3Value.has_value());
+    EXPECT_EQ(Addrx3Value, *ExtractedAddrx3Value);
+
     auto ExtractedAddrx4Value = toAddress(DieDG.find(Attr_DW_FORM_addrx4));
     EXPECT_TRUE(ExtractedAddrx1Value.has_value());
     EXPECT_EQ(Addrx4Value, *ExtractedAddrx4Value);
   }
 
   //----------------------------------------------------------------------
   // Test block forms
   //----------------------------------------------------------------------

llvm/unittests/DebugInfo/DWARF/DWARFFormValueTest.cpp

testing::Values(
         ParamType(DW_FORM_ref2, 0, 0, DWARF32, SampleU32, 2, true),
         ParamType(DW_FORM_ref4, 0, 0, DWARF32, SampleU32, 4, true),
         ParamType(DW_FORM_ref8, 0, 0, DWARF32, SampleU32, 8, true),
         ParamType(DW_FORM_ref_sig8, 0, 0, DWARF32, SampleU32, 8, true),
         ParamType(DW_FORM_ref_sup4, 0, 0, DWARF32, SampleU32, 4, true),
         ParamType(DW_FORM_ref_sup8, 0, 0, DWARF32, SampleU32, 8, true),
         ParamType(DW_FORM_strx1, 0, 0, DWARF32, SampleU32, 1, true),
         ParamType(DW_FORM_strx2, 0, 0, DWARF32, SampleU32, 2, true),
+        ParamType(DW_FORM_strx3, 0, 0, DWARF32, SampleU32, 3, true),
         ParamType(DW_FORM_strx4, 0, 0, DWARF32, SampleU32, 4, true),
         ParamType(DW_FORM_addrx1, 0, 0, DWARF32, SampleU32, 1, true),
         ParamType(DW_FORM_addrx2, 0, 0, DWARF32, SampleU32, 2, true),
+        ParamType(DW_FORM_addrx3, 0, 0, DWARF32, SampleU32, 3, true),
         ParamType(DW_FORM_addrx4, 0, 0, DWARF32, SampleU32, 4, true),
         ParamType(DW_FORM_sec_offset, 0, 1, DWARF32, SampleU32, 0, false),
         ParamType(DW_FORM_sec_offset, 1, 0, DWARF32, SampleU32, 0, false),
         ParamType(DW_FORM_sec_offset, 1, 1, DWARF32, SampleU32, 4, true),
         ParamType(DW_FORM_sec_offset, 1, 1, DWARF64, SampleU32, 8, true),
         ParamType(DW_FORM_strp, 0, 1, DWARF32, SampleU32, 0, false),
         ParamType(DW_FORM_strp, 1, 0, DWARF32, SampleU32, 0, false),
         ParamType(DW_FORM_strp, 1, 1, DWARF32, SampleU32, 4, true),

llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h

class Generator {
   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);
 
+  private:
     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.