[DebugInfo] Do not truncate 64-bit values when dumping CIEs and FDEs.

[ikudrin]

Summary

This fixes printing long values that might reside in CIE and FDE, including offsets, lengths, and addresses.

[ikudrin]

Ping (and to reduce the noise, for all other revisions in the stack as well, please).

[aprantl]

Mechanically looks fine to me by @probinson should also take a look and see if the concerns in the other review were addressed.

[probinson]

Apologies for the slow response. Overall this seems cleaner than D73714, now that I better understand the subtleties between .debug_frame and .eh_frame specifications.

The commit message should include a link to the Linux spec that you cited in the other review, which will help future readers understand the differences between the two formats.

A couple of minor things inline that I would like to see addressed.

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h

191

Because this will not return the value from the data, it should have a comment along the lines of:
// Returns the CIE identifier to be used by the requested format.

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

342

This looks like you are printing LinkedCIEOffset twice?

[ikudrin]

• Updated to reflect changes in the parent revisions.

[ikudrin]

In D73887#1872416, @probinson wrote:

The commit message should include a link to the Linux spec that you cited in the other review, which will help future readers understand the differences between the two formats.

This revision is mostly about 64-bit values. The links to the specs are added to the comment for getCIEId(), in D73886, which is a parent revision for this one, and to the commit message for D73627.

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h

191

I've moved the method (now a function) getCIEId() to D73886. The comment is added there.

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

342

Thanks for catching that! It was an issue in our implementation, I've prepared D74613 to fix that.

[ikudrin]

Ping.

[jhenderson]

llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp

20

For completeness, you probably want DWARF32 versions of these too.

22

clang-format-recognised style is /*IsDWARF64=*/true.

Same principle applies throughout.

23

I believe there's no need for the ULL suffix here and elsewhere below.

50

As the TestCIE block is practically identical in all test cases, it might be good to pull it into a function e.g. createCIE(uint64_t Length, uint64_t Offset). This will also make clearer what is actually important to each test case.

[ikudrin]

Thanks, @jhenderson!

• Simplified tests by moving similar code to helper functions.
• Added a test for DWARF32 CIE.
• Removed unneeded suffixes.
• Removed spaces in the comments for arguments.

[ikudrin]

llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp

22

A side note. It looks like clang-format recognizes both styles, and both are used in the LLVM codebase, but the condensed variant is way more widespread and also is mentioned in the LLVM Coding Standards (not as a requirement, just as an example).

[jhenderson]

I don't claim to have checked all the functionality matches the spec, but otherwise this looks good to me.

[jhenderson]

P.S. please remember to mark inline comments as "Done" using the checkbox once you address something. If you do that before uploading a patch, they will get automatically submitted when the patch is uploaded.

llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp

22

When you say "recognises" I think what happens is that it treats the old style as a normal inline comment, so applies the normal comment formatting rules. That is based purely on observation though, so I could be completely wrong!

[ikudrin]

llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp

22

Well, maybe it does not need to recognize them in a special way if the result of the formatting is the same as expected. Anyway, I agree that it is better to stick with a more widespread style despite my personal taste.

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h

 };
 
 /// An entry in either debug_frame or eh_frame. This entry can be a CIE or an
 /// FDE.
 class FrameEntry {
 public:
   enum FrameKind { FK_CIE, FK_FDE };
 
-  FrameEntry(FrameKind K, uint64_t Offset, uint64_t Length, uint64_t CodeAlign,
-             int64_t DataAlign, Triple::ArchType Arch)
-      : Kind(K), Offset(Offset), Length(Length),
+  FrameEntry(FrameKind K, bool IsDWARF64, uint64_t Offset, uint64_t Length,
+             uint64_t CodeAlign, int64_t DataAlign, Triple::ArchType Arch)
+      : Kind(K), IsDWARF64(IsDWARF64), Offset(Offset), Length(Length),
         CFIs(CodeAlign, DataAlign, Arch) {}
 
   virtual ~FrameEntry() {}
 
   FrameKind getKind() const { return Kind; }
   uint64_t getOffset() const { return Offset; }
   uint64_t getLength() const { return Length; }
   const CFIProgram &cfis() const { return CFIs; }
   CFIProgram &cfis() { return CFIs; }
 
   /// Dump the instructions in this CFI fragment
   virtual void dump(raw_ostream &OS, const MCRegisterInfo *MRI,
                     bool IsEH) const = 0;
 
 protected:
   const FrameKind Kind;
 
+  const bool IsDWARF64;
+
   /// Offset of this entry in the section.
   const uint64_t Offset;
 
   /// Entry length as specified in DWARF.
   const uint64_t Length;
 
   CFIProgram CFIs;
 };
 
 /// DWARF Common Information Entry (CIE)
 class CIE : public FrameEntry {
 public:
   // CIEs (and FDEs) are simply container classes, so the only sensible way to
   // create them is by providing the full parsed contents in the constructor.
-  CIE(uint64_t Offset, uint64_t Length, uint8_t Version,
+  CIE(bool IsDWARF64, uint64_t Offset, uint64_t Length, uint8_t Version,
       SmallString<8> Augmentation, uint8_t AddressSize,
       uint8_t SegmentDescriptorSize, uint64_t CodeAlignmentFactor,
       int64_t DataAlignmentFactor, uint64_t ReturnAddressRegister,
       SmallString<8> AugmentationData, uint32_t FDEPointerEncoding,
       uint32_t LSDAPointerEncoding, Optional<uint64_t> Personality,
       Optional<uint32_t> PersonalityEnc, Triple::ArchType Arch)
-      : FrameEntry(FK_CIE, Offset, Length, CodeAlignmentFactor,
+      : FrameEntry(FK_CIE, IsDWARF64, Offset, Length, CodeAlignmentFactor,
                    DataAlignmentFactor, Arch),
         Version(Version), Augmentation(std::move(Augmentation)),
         AddressSize(AddressSize), SegmentDescriptorSize(SegmentDescriptorSize),
         CodeAlignmentFactor(CodeAlignmentFactor),
         DataAlignmentFactor(DataAlignmentFactor),
         ReturnAddressRegister(ReturnAddressRegister),
         AugmentationData(std::move(AugmentationData)),
         FDEPointerEncoding(FDEPointerEncoding),
         LSDAPointerEncoding(LSDAPointerEncoding), Personality(Personality),
         PersonalityEnc(PersonalityEnc) {}
 
   static bool classof(const FrameEntry *FE) { return FE->getKind() == FK_CIE; }
 
   StringRef getAugmentationString() const { return Augmentation; }
   uint64_t getCodeAlignmentFactor() const { return CodeAlignmentFactor; }
   int64_t getDataAlignmentFactor() const { return DataAlignmentFactor; }
   uint8_t getVersion() const { return Version; }
   uint64_t getReturnAddressRegister() const { return ReturnAddressRegister; }
   Optional<uint64_t> getPersonalityAddress() const { return Personality; }
   Optional<uint32_t> getPersonalityEncoding() const { return PersonalityEnc; }
 
private:
   const uint32_t LSDAPointerEncoding;
   const Optional<uint64_t> Personality;
   const Optional<uint32_t> PersonalityEnc;
 };
 
 /// DWARF Frame Description Entry (FDE)
 class FDE : public FrameEntry {
 public:
-  FDE(uint64_t Offset, uint64_t Length, uint64_t CIEPointer,
+  FDE(bool IsDWARF64, uint64_t Offset, uint64_t Length, uint64_t CIEPointer,
       uint64_t InitialLocation, uint64_t AddressRange, CIE *Cie,
       Optional<uint64_t> LSDAAddress, Triple::ArchType Arch)
-      : FrameEntry(FK_FDE, Offset, Length,
+      : FrameEntry(FK_FDE, IsDWARF64, Offset, Length,
                    Cie ? Cie->getCodeAlignmentFactor() : 0,
                    Cie ? Cie->getDataAlignmentFactor() : 0,
                    Arch),
         CIEPointer(CIEPointer), InitialLocation(InitialLocation),
         AddressRange(AddressRange), LinkedCIE(Cie), LSDAAddress(LSDAAddress) {}
 
   ~FDE() override = default;
 

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

constexpr uint64_t getCIEId(bool IsDWARF64, bool IsEH) {
   if (IsEH)
     return 0;
   if (IsDWARF64)
     return DW64_CIE_ID;
   return DW_CIE_ID;
 }
 
 void CIE::dump(raw_ostream &OS, const MCRegisterInfo *MRI, bool IsEH) const {
-  OS << format("%08x %08x %08x CIE", (uint32_t)Offset, (uint32_t)Length,
-               IsEH ? 0 : DW_CIE_ID)
-     << "\n";
+  OS << format("%08" PRIx64, Offset)
+     << format(" %0*" PRIx64, IsDWARF64 ? 16 : 8, Length)
+     << format(" %0*" PRIx64, IsDWARF64 && !IsEH ? 16 : 8,
+               getCIEId(IsDWARF64, IsEH))
+     << " CIE\n";
   OS << format("  Version:               %d\n", Version);
   OS << "  Augmentation:          \"" << Augmentation << "\"\n";
   if (Version >= 4) {
     OS << format("  Address size:          %u\n", (uint32_t)AddressSize);
     OS << format("  Segment desc size:     %u\n",
                  (uint32_t)SegmentDescriptorSize);
   }
   OS << format("  Code alignment factor: %u\n", (uint32_t)CodeAlignmentFactor);
   OS << format("  Data alignment factor: %d\n", (int32_t)DataAlignmentFactor);
   OS << format("  Return address column: %d\n", (int32_t)ReturnAddressRegister);
   if (Personality)
     OS << format("  Personality Address: %016" PRIx64 "\n", *Personality);
   if (!AugmentationData.empty()) {
     OS << "  Augmentation data:    ";
     for (uint8_t Byte : AugmentationData)
       OS << ' ' << hexdigit(Byte >> 4) << hexdigit(Byte & 0xf);
     OS << "\n";
   }
   OS << "\n";
   CFIs.dump(OS, MRI, IsEH);
   OS << "\n";
 }
 
 void FDE::dump(raw_ostream &OS, const MCRegisterInfo *MRI, bool IsEH) const {
-  OS << format("%08x %08x %08x", (uint32_t)Offset, (uint32_t)Length,
-               (uint32_t)CIEPointer)
+  OS << format("%08" PRIx64, Offset)
+     << format(" %0*" PRIx64, IsDWARF64 ? 16 : 8, Length)
+     << format(" %0*" PRIx64, IsDWARF64 && !IsEH ? 16 : 8, CIEPointer)
      << " FDE cie=";
   if (LinkedCIE)
-    OS << format("%08x", (uint32_t)(LinkedCIE->getOffset()));
+    OS << format("%08" PRIx64, LinkedCIE->getOffset());
   else
     OS << "<invalid offset>";
-  OS << format(" pc=%08x...%08x\n", (uint32_t)InitialLocation,
-               (uint32_t)InitialLocation + (uint32_t)AddressRange);
+  OS << format(" pc=%08" PRIx64 "...%08" PRIx64 "\n", InitialLocation,
+               InitialLocation + AddressRange);
   if (LSDAAddress)
     OS << format("  LSDA Address: %016" PRIx64 "\n", *LSDAAddress);
   CFIs.dump(OS, MRI, IsEH);
   OS << "\n";
 }
 
 DWARFDebugFrame::DWARFDebugFrame(Triple::ArchType Arch,
     bool IsEH, uint64_t EHFrameAddress)
     : Arch(Arch), IsEH(IsEH), EHFrameAddress(EHFrameAddress) {}
 
 DWARFDebugFrame::~DWARFDebugFrame() = default;
if (Id == getCIEId(IsDWARF64, IsEH)) {
                         "Parsing augmentation data at %" PRIx64 " failed");
 
           AugmentationData = Data.getData().slice(StartAugmentationOffset,
                                                   EndAugmentationOffset);
         }
       }
 
       auto Cie = std::make_unique<CIE>(
-          StartOffset, Length, Version, AugmentationString, AddressSize,
-          SegmentDescriptorSize, CodeAlignmentFactor, DataAlignmentFactor,
-          ReturnAddressRegister, AugmentationData, FDEPointerEncoding,
-          LSDAPointerEncoding, Personality, PersonalityEncoding, Arch);
+          IsDWARF64, StartOffset, Length, Version, AugmentationString,
+          AddressSize, SegmentDescriptorSize, CodeAlignmentFactor,
+          DataAlignmentFactor, ReturnAddressRegister, AugmentationData,
+          FDEPointerEncoding, LSDAPointerEncoding, Personality,
+          PersonalityEncoding, Arch);
       CIEs[StartOffset] = Cie.get();
       Entries.emplace_back(std::move(Cie));
     } else {
       // FDE
       uint64_t CIEPointer = Id;
       uint64_t InitialLocation = 0;
       uint64_t AddressRange = 0;
       Optional<uint64_t> LSDAAddress;
if (Id == getCIEId(IsDWARF64, IsEH)) {
             ReportError(StartOffset,
                         "Parsing augmentation data at %" PRIx64 " failed");
         }
       } else {
         InitialLocation = Data.getRelocatedAddress(&Offset);
         AddressRange = Data.getRelocatedAddress(&Offset);
       }
 
-      Entries.emplace_back(new FDE(StartOffset, Length, CIEPointer,
-                                   InitialLocation, AddressRange,
-                                   Cie, LSDAAddress, Arch));
+      Entries.emplace_back(new FDE(IsDWARF64, StartOffset, Length, CIEPointer,
+                                   InitialLocation, AddressRange, Cie,
+                                   LSDAAddress, Arch));
     }
 
     if (Error E =
             Entries.back()->cfis().parse(Data, &Offset, EndStructureOffset)) {
       report_fatal_error(toString(std::move(E)));
     }
 
     if (Offset != EndStructureOffset)

llvm/test/tools/llvm-dwarfdump/X86/debug_frame_offset.test

 RUN: llc -filetype=obj %p/../../dsymutil/Inputs/frame-dw2.ll -o - \
 RUN:   | llvm-dwarfdump -debug-frame=0x00000014 - | FileCheck %s
 CHECK: .debug_frame contents:
 CHECK-NEXT: 00000014 00000014 00000000 FDE cie=00000000 pc=00000000...0000001d
 CHECK-NEXT:   DW_CFA_advance_loc: 1
 CHECK-NOT: pc
 
 RUN: llvm-dwarfdump %p/../../dsymutil/Inputs/basic1.macho.x86_64.o \
 RUN:   -eh-frame=0x00000018 | FileCheck %s --check-prefix=EH
 EH: .eh_frame contents:
-EH-NEXT: 00000018 00000024 0000001c FDE cie=00000000 pc=fffffd00...fffffd24
+EH-NEXT: 00000018 00000024 0000001c FDE cie=00000000 pc=fffffffffffffd00...fffffffffffffd24
 EH-NEXT:   DW_CFA_advance_loc: 1
 EH-NOT: pc
 EH-NOT: CIE

llvm/test/tools/llvm-objdump/eh_frame-arm64.test

 # RUN: llvm-objdump -dwarf=frames %p/Inputs/eh_frame.macho-arm64 2>/dev/null | FileCheck %s
 
 # CHECK: .eh_frame contents:
 
 # CHECK: 00000000 00000010 00000000 CIE
 # CHECK:   Version:               1
 # CHECK:   Augmentation:          "zR"
 # CHECK:   Code alignment factor: 1
 # CHECK:   Data alignment factor: -8
 # CHECK:   Return address column: 30
 # CHECK:   Augmentation data:     10
 
 # CHECK:   DW_CFA_def_cfa: reg31 +0
 
-# CHECK: 00000014 00000020 00000018 FDE cie=00000000 pc=ffffffe4...00000004
+# CHECK: 00000014 00000020 00000018 FDE cie=00000000 pc=ffffffffffffffe4...00000004
 # CHECK:   DW_CFA_advance_loc: 8
 # CHECK:   DW_CFA_def_cfa_offset: +16
 # CHECK:   DW_CFA_offset: reg30 -8
 # CHECK:   DW_CFA_offset: reg29 -16
 # CHECK:   DW_CFA_nop:
 # CHECK:   DW_CFA_nop:
 # CHECK:   DW_CFA_nop:
 # CHECK:   DW_CFA_nop:

llvm/unittests/DebugInfo/DWARF/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   AsmPrinter
   DebugInfoDWARF
   MC
   Object
   ObjectYAML
   Support
   )
 
 add_llvm_unittest(DebugInfoDWARFTests
   DwarfGenerator.cpp
   DwarfUtils.cpp
   DWARFAcceleratorTableTest.cpp
   DWARFDebugArangeSetTest.cpp
+  DWARFDebugFrameTest.cpp
   DWARFDebugInfoTest.cpp
   DWARFDebugLineTest.cpp
   DWARFDieTest.cpp
   DWARFFormValueTest.cpp
   DWARFLocationExpressionTest.cpp
   )
 
 target_link_libraries(DebugInfoDWARFTests PRIVATE LLVMTestingSupport)

llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp

+//===- llvm/unittest/DebugInfo/DWARFDebugFrameTest.cpp --------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/BinaryFormat/Dwarf.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugFrame.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+dwarf::CIE createCIE(bool IsDWARF64, uint64_t Offset, uint64_t Length) {
+  return dwarf::CIE(IsDWARF64, Offset, Length,
+                    /*Version=*/3,
+                    /*Augmentation=*/StringRef(),
+                    /*AddressSize=*/8,
+                    /*SegmentDescriptorSize=*/0,
+                    /*CodeAlignmentFactor=*/1,
+                    /*DataAlignmentFactor=*/-8,
+                    /*ReturnAddressRegister=*/16,
+                    /*AugmentationData=*/StringRef(),
+                    /*FDEPointerEncoding=*/dwarf::DW_EH_PE_absptr,
+                    /*LSDAPointerEncoding=*/dwarf::DW_EH_PE_omit,
+                    /*Personality=*/None,
+                    /*PersonalityEnc=*/None,
+                    /*Arch=*/Triple::x86_64);
+}
+
+void expectDumpResult(const dwarf::CIE &TestCIE, bool IsEH,
+                      StringRef ExpectedFirstLine) {
+  std::string Output;
+  raw_string_ostream OS(Output);
+  TestCIE.dump(OS, /*MRI=*/nullptr, IsEH);
+  OS.flush();
+  StringRef FirstLine = StringRef(Output).split('\n').first;
+  EXPECT_EQ(FirstLine, ExpectedFirstLine);
+}
+
+void expectDumpResult(const dwarf::FDE &TestFDE, bool IsEH,
+                      StringRef ExpectedFirstLine) {
+  std::string Output;
+  raw_string_ostream OS(Output);
+  TestFDE.dump(OS, /*MRI=*/nullptr, IsEH);
+  OS.flush();
+  StringRef FirstLine = StringRef(Output).split('\n').first;
+  EXPECT_EQ(FirstLine, ExpectedFirstLine);
+}
+
+TEST(DWARFDebugFrame, DumpDWARF32CIE) {
+  dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/false,
+                                 /*Offset=*/0x1111abcd,
+                                 /*Length=*/0x2222abcd);
+  expectDumpResult(TestCIE, /*IsEH=*/false, "1111abcd 2222abcd ffffffff CIE");
+}
+
+TEST(DWARFDebugFrame, DumpDWARF64CIE) {
+  dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/true,
+                                 /*Offset=*/0x1111abcdabcd,
+                                 /*Length=*/0x2222abcdabcd);
+  expectDumpResult(TestCIE, /*IsEH=*/false,
+                   "1111abcdabcd 00002222abcdabcd ffffffffffffffff CIE");
+}
+
+TEST(DWARFDebugFrame, DumpEHCIE) {
+  dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/false,
+                                 /*Offset=*/0x1000,
+                                 /*Length=*/0x20);
+  expectDumpResult(TestCIE, /*IsEH=*/true, "00001000 00000020 00000000 CIE");
+}
+
+TEST(DWARFDebugFrame, DumpEH64CIE) {
+  dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/true,
+                                 /*Offset=*/0x1000,
+                                 /*Length=*/0x20);
+  expectDumpResult(TestCIE, /*IsEH=*/true,
+                   "00001000 0000000000000020 00000000 CIE");
+}
+
+TEST(DWARFDebugFrame, DumpDWARF64FDE) {
+  dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/true,
+                                 /*Offset=*/0x1111abcdabcd,
+                                 /*Length=*/0x2222abcdabcd);
+  dwarf::FDE TestFDE(/*IsDWARF64=*/true,
+                     /*Offset=*/0x3333abcdabcd,
+                     /*Length=*/0x4444abcdabcd,
+                     /*CIEPointer=*/0x1111abcdabcd,
+                     /*InitialLocation=*/0x5555abcdabcd,
+                     /*AddressRange=*/0x111111111111,
+                     /*Cie=*/&TestCIE,
+                     /*LSDAAddress=*/None,
+                     /*Arch=*/Triple::x86_64);
+  expectDumpResult(TestFDE, /*IsEH=*/false,
+                   "3333abcdabcd 00004444abcdabcd 00001111abcdabcd FDE "
+                   "cie=1111abcdabcd pc=5555abcdabcd...6666bcdebcde");
+}
+
+TEST(DWARFDebugFrame, DumpEH64FDE) {
+  dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/true,
+                                 /*Offset=*/0x1111ab9a000c,
+                                 /*Length=*/0x20);
+  dwarf::FDE TestFDE(/*IsDWARF64=*/true,
+                     /*Offset=*/0x1111abcdabcd,
+                     /*Length=*/0x2222abcdabcd,
+                     /*CIEPointer=*/0x33abcd,
+                     /*InitialLocation=*/0x4444abcdabcd,
+                     /*AddressRange=*/0x111111111111,
+                     /*Cie=*/&TestCIE,
+                     /*LSDAAddress=*/None,
+                     /*Arch=*/Triple::x86_64);
+  expectDumpResult(TestFDE, /*IsEH=*/true,
+                   "1111abcdabcd 00002222abcdabcd 0033abcd FDE "
+                   "cie=1111ab9a000c pc=4444abcdabcd...5555bcdebcde");
+}
+
+} // end anonymous namespace