Skip to content

Conversation

@labath
Copy link
Collaborator

@labath labath commented Jul 21, 2025

Also known as Function Description Entries. The entries occupy a contiguous piece of the section, so the code is mostly straight-forward.

For more information about the SFrame unwind format, see the specification and the related
RFC.

Also known as Function Description Entries. The entries occupy a
contiguous piece of the section, so the code is mostly straight-forward.

For more information about the SFrame unwind format, see the
[specification](https://sourceware.org/binutils/wiki/sframe) and the
related
[RFC](https://discourse.llvm.org/t/rfc-adding-sframe-support-to-llvm/86900).
@llvmbot
Copy link
Member

llvmbot commented Jul 21, 2025

@llvm/pr-subscribers-llvm-binary-utilities

Author: Pavel Labath (labath)

Changes

Also known as Function Description Entries. The entries occupy a contiguous piece of the section, so the code is mostly straight-forward.

For more information about the SFrame unwind format, see the specification and the related
RFC.


Patch is 30.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149828.diff

8 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/SFrame.h (+13-11)
  • (modified) llvm/include/llvm/BinaryFormat/SFrameConstants.def (+39-2)
  • (modified) llvm/include/llvm/Object/SFrameParser.h (+16-3)
  • (modified) llvm/lib/BinaryFormat/SFrame.cpp (+33)
  • (modified) llvm/lib/Object/SFrameParser.cpp (+55-9)
  • (added) llvm/test/tools/llvm-readobj/ELF/sframe-fde.test (+232)
  • (modified) llvm/test/tools/llvm-readobj/ELF/sframe-header.test (+55-12)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+87-28)
diff --git a/llvm/include/llvm/BinaryFormat/SFrame.h b/llvm/include/llvm/BinaryFormat/SFrame.h
index 98dbe38fb2bc4..3ecaa32f373cf 100644
--- a/llvm/include/llvm/BinaryFormat/SFrame.h
+++ b/llvm/include/llvm/BinaryFormat/SFrame.h
@@ -49,29 +49,27 @@ enum class ABI : uint8_t {
 
 /// SFrame FRE Types. Bits 0-3 of FuncDescEntry.Info.
 enum class FREType : uint8_t {
-  Addr1 = 0,
-  Addr2 = 1,
-  Addr4 = 2,
+#define HANDLE_SFRAME_FRE_TYPE(CODE, NAME) NAME = CODE,
+#include "llvm/BinaryFormat/SFrameConstants.def"
 };
 
 /// SFrame FDE Types. Bit 4 of FuncDescEntry.Info.
 enum class FDEType : uint8_t {
-  PCInc = 0,
-  PCMask = 1,
+#define HANDLE_SFRAME_FDE_TYPE(CODE, NAME) NAME = CODE,
+#include "llvm/BinaryFormat/SFrameConstants.def"
 };
 
 /// Speficies key used for signing return addresses. Bit 5 of
 /// FuncDescEntry.Info.
 enum class AArch64PAuthKey : uint8_t {
-  A = 0,
-  B = 1,
+#define HANDLE_SFRAME_AARCH64_PAUTH_KEY(CODE, NAME) NAME = CODE,
+#include "llvm/BinaryFormat/SFrameConstants.def"
 };
 
-/// Size of stack offsets. Bits 5-6 of FREInfo.Info.
+/// Size of stack offsets. Bits 6-7 of FREInfo.Info.
 enum class FREOffset : uint8_t {
-  B1 = 0,
-  B2 = 1,
-  B4 = 2,
+#define HANDLE_SFRAME_FRE_OFFSET(CODE, NAME) NAME = CODE,
+#include "llvm/BinaryFormat/SFrameConstants.def"
 };
 
 /// Stack frame base register. Bit 0 of FREInfo.Info.
@@ -166,6 +164,10 @@ template <endianness E> using FrameRowEntryAddr4 = FrameRowEntry<uint32_t, E>;
 ArrayRef<EnumEntry<Version>> getVersions();
 ArrayRef<EnumEntry<Flags>> getFlags();
 ArrayRef<EnumEntry<ABI>> getABIs();
+ArrayRef<EnumEntry<FREType>> getFRETypes();
+ArrayRef<EnumEntry<FDEType>> getFDETypes();
+ArrayRef<EnumEntry<AArch64PAuthKey>> getAArch64PAuthKeys();
+ArrayRef<EnumEntry<FREOffset>> getFREOffsets();
 
 } // namespace sframe
 } // namespace llvm
diff --git a/llvm/include/llvm/BinaryFormat/SFrameConstants.def b/llvm/include/llvm/BinaryFormat/SFrameConstants.def
index 643b15f438c86..fddd440e41f32 100644
--- a/llvm/include/llvm/BinaryFormat/SFrameConstants.def
+++ b/llvm/include/llvm/BinaryFormat/SFrameConstants.def
@@ -6,8 +6,11 @@
 //
 //===----------------------------------------------------------------------===//
 
-#if !(defined(HANDLE_SFRAME_VERSION) || defined(HANDLE_SFRAME_FLAG) ||  \
-      defined(HANDLE_SFRAME_ABI))
+#if !(defined(HANDLE_SFRAME_VERSION) || defined(HANDLE_SFRAME_FLAG) ||         \
+      defined(HANDLE_SFRAME_ABI) || defined(HANDLE_SFRAME_FRE_TYPE) ||         \
+      defined(HANDLE_SFRAME_FDE_TYPE) ||                                       \
+      defined(HANDLE_SFRAME_AARCH64_PAUTH_KEY) ||                              \
+      defined(HANDLE_SFRAME_FRE_OFFSET))
 #error "Missing HANDLE_SFRAME definition"
 #endif
 
@@ -23,6 +26,22 @@
 #define HANDLE_SFRAME_ABI(CODE, NAME)
 #endif
 
+#ifndef HANDLE_SFRAME_FRE_TYPE
+#define HANDLE_SFRAME_FRE_TYPE(CODE, NAME)
+#endif
+
+#ifndef HANDLE_SFRAME_FDE_TYPE
+#define HANDLE_SFRAME_FDE_TYPE(CODE, NAME)
+#endif
+
+#ifndef HANDLE_SFRAME_AARCH64_PAUTH_KEY
+#define HANDLE_SFRAME_AARCH64_PAUTH_KEY(CODE, NAME)
+#endif
+
+#ifndef HANDLE_SFRAME_FRE_OFFSET
+#define HANDLE_SFRAME_FRE_OFFSET(CODE, NAME)
+#endif
+
 HANDLE_SFRAME_VERSION(0x01, V1)
 HANDLE_SFRAME_VERSION(0x02, V2)
 
@@ -34,6 +53,24 @@ HANDLE_SFRAME_ABI(0x01, AArch64EndianBig)
 HANDLE_SFRAME_ABI(0x02, AArch64EndianLittle)
 HANDLE_SFRAME_ABI(0x03, AMD64EndianLittle)
 
+HANDLE_SFRAME_FRE_TYPE(0x00, Addr1)
+HANDLE_SFRAME_FRE_TYPE(0x01, Addr2)
+HANDLE_SFRAME_FRE_TYPE(0x02, Addr4)
+
+HANDLE_SFRAME_FDE_TYPE(0, PCInc)
+HANDLE_SFRAME_FDE_TYPE(1, PCMask)
+
+HANDLE_SFRAME_AARCH64_PAUTH_KEY(0, A)
+HANDLE_SFRAME_AARCH64_PAUTH_KEY(1, B)
+
+HANDLE_SFRAME_FRE_OFFSET(0, B1)
+HANDLE_SFRAME_FRE_OFFSET(1, B2)
+HANDLE_SFRAME_FRE_OFFSET(2, B4)
+
 #undef HANDLE_SFRAME_VERSION
 #undef HANDLE_SFRAME_FLAG
 #undef HANDLE_SFRAME_ABI
+#undef HANDLE_SFRAME_FRE_TYPE
+#undef HANDLE_SFRAME_FDE_TYPE
+#undef HANDLE_SFRAME_AARCH64_PAUTH_KEY
+#undef HANDLE_SFRAME_FRE_OFFSET
diff --git a/llvm/include/llvm/Object/SFrameParser.h b/llvm/include/llvm/Object/SFrameParser.h
index cf4fe20e84431..974c757419334 100644
--- a/llvm/include/llvm/Object/SFrameParser.h
+++ b/llvm/include/llvm/Object/SFrameParser.h
@@ -19,11 +19,14 @@ namespace object {
 
 template <endianness E> class SFrameParser {
 public:
-  static Expected<SFrameParser> create(ArrayRef<uint8_t> Contents);
+  static Expected<SFrameParser> create(ArrayRef<uint8_t> Contents,
+                                       uint64_t SectionAddress);
 
   const sframe::Preamble<E> &getPreamble() const { return Header.Preamble; }
   const sframe::Header<E> &getHeader() const { return Header; }
 
+  Expected<ArrayRef<uint8_t>> getAuxHeader() const;
+
   bool usesFixedRAOffset() const {
     return getHeader().ABIArch == sframe::ABI::AMD64EndianLittle;
   }
@@ -31,12 +34,22 @@ template <endianness E> class SFrameParser {
     return false; // Not used in any currently defined ABI.
   }
 
+  using FDERange = ArrayRef<sframe::FuncDescEntry<E>>;
+  Expected<FDERange> fdes() const;
+
+  // Decodes the start address of the given FDE, which must be one of the
+  // objects returned by the `fdes()` function.
+  uint64_t getAbsoluteStartAddress(typename FDERange::iterator FDE) const;
+
 private:
   ArrayRef<uint8_t> Data;
+  uint64_t SectionAddress;
   const sframe::Header<E> &Header;
 
-  SFrameParser(ArrayRef<uint8_t> Data, const sframe::Header<E> &Header)
-      : Data(Data), Header(Header) {}
+  SFrameParser(ArrayRef<uint8_t> Data, uint64_t SectionAddress, const sframe::Header<E> &Header)
+      : Data(Data), SectionAddress(SectionAddress), Header(Header) {}
+
+  uint64_t getFDEBegin() const { return sizeof(Header) + Header.AuxHdrLen + Header.FDEOff; }
 };
 
 extern template class SFrameParser<endianness::big>;
diff --git a/llvm/lib/BinaryFormat/SFrame.cpp b/llvm/lib/BinaryFormat/SFrame.cpp
index 3b436afd32083..f1765d7f3e852 100644
--- a/llvm/lib/BinaryFormat/SFrame.cpp
+++ b/llvm/lib/BinaryFormat/SFrame.cpp
@@ -35,3 +35,36 @@ ArrayRef<EnumEntry<sframe::ABI>> sframe::getABIs() {
   };
   return ArrayRef(ABIs);
 }
+
+ArrayRef<EnumEntry<sframe::FREType>> sframe::getFRETypes() {
+  static constexpr EnumEntry<sframe::FREType> FRETypes[] = {
+#define HANDLE_SFRAME_FRE_TYPE(CODE, NAME) {#NAME, sframe::FREType::NAME},
+#include "llvm/BinaryFormat/SFrameConstants.def"
+  };
+  return ArrayRef(FRETypes);
+}
+
+ArrayRef<EnumEntry<sframe::FDEType>> sframe::getFDETypes() {
+  static constexpr EnumEntry<sframe::FDEType> FDETypes[] = {
+#define HANDLE_SFRAME_FDE_TYPE(CODE, NAME) {#NAME, sframe::FDEType::NAME},
+#include "llvm/BinaryFormat/SFrameConstants.def"
+  };
+  return ArrayRef(FDETypes);
+}
+
+ArrayRef<EnumEntry<sframe::AArch64PAuthKey>> sframe::getAArch64PAuthKeys() {
+  static constexpr EnumEntry<sframe::AArch64PAuthKey> AArch64PAuthKeys[] = {
+#define HANDLE_SFRAME_AARCH64_PAUTH_KEY(CODE, NAME)                            \
+  {#NAME, sframe::AArch64PAuthKey::NAME},
+#include "llvm/BinaryFormat/SFrameConstants.def"
+  };
+  return ArrayRef(AArch64PAuthKeys);
+}
+
+ArrayRef<EnumEntry<sframe::FREOffset>> sframe::getFREOffsets() {
+  static constexpr EnumEntry<sframe::FREOffset> FREOffsets[] = {
+#define HANDLE_SFRAME_FRE_OFFSET(CODE, NAME) {#NAME, sframe::FREOffset::NAME},
+#include "llvm/BinaryFormat/SFrameConstants.def"
+  };
+  return ArrayRef(FREOffsets);
+}
diff --git a/llvm/lib/Object/SFrameParser.cpp b/llvm/lib/Object/SFrameParser.cpp
index 2d74d1d6b3827..aa3c481d35db4 100644
--- a/llvm/lib/Object/SFrameParser.cpp
+++ b/llvm/lib/Object/SFrameParser.cpp
@@ -14,23 +14,34 @@
 using namespace llvm;
 using namespace llvm::object;
 
-template <typename T>
-static Expected<const T &> getDataSliceAs(ArrayRef<uint8_t> Data,
-                                          uint64_t Offset) {
-  static_assert(std::is_trivial_v<T>);
-  if (Data.size() < Offset + sizeof(T)) {
+static Expected<ArrayRef<uint8_t>>
+getDataSlice(ArrayRef<uint8_t> Data, uint64_t Offset, uint64_t Size) {
+  // Check for overflow.
+  if (Offset + Size < Offset || Offset + Size < Size ||
+      Offset + Size > Data.size()) {
     return createStringError(
         formatv("unexpected end of data at offset {0:x} while reading [{1:x}, "
                 "{2:x})",
-                Data.size(), Offset, Offset + sizeof(T))
+                Data.size(), Offset, Offset + Size)
             .str(),
         object_error::unexpected_eof);
   }
-  return *reinterpret_cast<const T *>(Data.data() + Offset);
+  return Data.slice(Offset, Size);
+}
+
+template <typename T>
+static Expected<const T &> getDataSliceAs(ArrayRef<uint8_t> Data,
+                                          uint64_t Offset) {
+  static_assert(std::is_trivial_v<T>);
+  Expected<ArrayRef<uint8_t>> Slice = getDataSlice(Data, Offset, sizeof(T));
+  if (!Slice)
+    return Slice.takeError();
+
+  return *reinterpret_cast<const T *>(Slice->data());
 }
 
 template <endianness E>
-Expected<SFrameParser<E>> SFrameParser<E>::create(ArrayRef<uint8_t> Contents) {
+Expected<SFrameParser<E>> SFrameParser<E>::create(ArrayRef<uint8_t> Contents, uint64_t SectionAddress) {
   Expected<const sframe::Preamble<E> &> Preamble =
       getDataSliceAs<sframe::Preamble<E>>(Contents, 0);
   if (!Preamble)
@@ -48,7 +59,42 @@ Expected<SFrameParser<E>> SFrameParser<E>::create(ArrayRef<uint8_t> Contents) {
       getDataSliceAs<sframe::Header<E>>(Contents, 0);
   if (!Header)
     return Header.takeError();
-  return SFrameParser(Contents, *Header);
+  return SFrameParser(Contents, SectionAddress, *Header);
+}
+
+
+template<endianness E>
+Expected<ArrayRef<uint8_t>> SFrameParser<E>::getAuxHeader() const {
+  return getDataSlice(Data, sizeof(Header), Header.AuxHdrLen);
+}
+
+template<endianness E>
+Expected<ArrayRef<sframe::FuncDescEntry<E>>> SFrameParser<E>::fdes() const {
+  Expected<ArrayRef<uint8_t>> Slice = getDataSlice(
+      Data, getFDEBegin(), Header.NumFDEs * sizeof(sframe::FuncDescEntry<E>));
+  if (!Slice)
+    return Slice.takeError();
+  return ArrayRef(
+      reinterpret_cast<const sframe::FuncDescEntry<E> *>(Slice->data()),
+      Header.NumFDEs);
+}
+
+template<endianness E>
+uint64_t SFrameParser<E>::getAbsoluteStartAddress(typename FDERange::iterator FDE) const {
+  uint64_t Result = SectionAddress + FDE->StartAddress;
+
+  if ((getPreamble().Flags.value() & sframe::Flags::FDEFuncStartPCRel) ==
+      sframe::Flags::FDEFuncStartPCRel) {
+    uintptr_t DataPtr = reinterpret_cast<uintptr_t>(Data.data());
+    uintptr_t FDEPtr = reinterpret_cast<uintptr_t>(&*FDE);
+
+    assert(DataPtr <= FDEPtr);
+    assert(FDEPtr < DataPtr + Data.size());
+
+    Result += FDEPtr - DataPtr;
+  }
+
+  return Result;
 }
 
 template class llvm::object::SFrameParser<endianness::big>;
diff --git a/llvm/test/tools/llvm-readobj/ELF/sframe-fde.test b/llvm/test/tools/llvm-readobj/ELF/sframe-fde.test
new file mode 100644
index 0000000000000..294b28d8f04b4
--- /dev/null
+++ b/llvm/test/tools/llvm-readobj/ELF/sframe-fde.test
@@ -0,0 +1,232 @@
+## Check parsing and dumping of SFrame Function Descriptor Entries.
+# RUN: yaml2obj --docnum=1 %s -o %t.1
+# RUN: llvm-readobj --sframe=.sframe_short --sframe=.sframe_section_relative \
+# RUN:   --sframe=.sframe_fde_relative %t.1 2>&1 | \
+# RUN:   FileCheck %s --strict-whitespace --match-full-lines \
+# RUN:   -DFILE=%t.1 --check-prefix=CASE1
+
+## Check big-endian support.
+# RUN: yaml2obj --docnum=2 %s -o %t.2
+# RUN: llvm-readobj --sframe %t.2 2>&1 | \
+# RUN:   FileCheck %s --strict-whitespace --match-full-lines \
+# RUN:   -DFILE=%t.2 --check-prefix=CASE2
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+Sections:
+  - Name:  .sframe_short
+    Type:  SHT_GNU_SFRAME
+    Flags: [ SHF_ALLOC ]
+    ContentArray: [
+      0xe2, 0xde, 0x02, 0x00,  # Preamble (magic, version, flags)
+      # Header:
+      0x03, 0x42, 0x47, 0x04,  # ABI, Fixed FP offset, Fixed RA Offset, AUX header length
+      0x01, 0x00, 0x00, 0x00,  # Number of FDEs
+      0x10, 0x00, 0x00, 0x00,  # Number of FREs
+      0x00, 0x10, 0x00, 0x00,  # FRE length
+      0x00, 0x00, 0x00, 0x00,  # FDE offset
+      0x00, 0x01, 0x00, 0x00,  # FRE offset
+      0xde, 0xad, 0xbe, 0xef,  # AUX header
+      0x01, 0x02, 0x03, 0x04,  # Short FDE
+    ]
+# CASE1-LABEL:SFrame section '.sframe_short' {
+#       CASE1:  Header {
+#  CASE1-NEXT:    Magic: 0xDEE2
+#  CASE1-NEXT:    Version: V2 (0x2)
+#  CASE1-NEXT:    Flags [ (0x0)
+#  CASE1-NEXT:    ]
+#  CASE1-NEXT:    ABI: AMD64EndianLittle (0x3)
+#  CASE1-NEXT:    CFA fixed FP offset (unused): 66
+#  CASE1-NEXT:    CFA fixed RA offset: 71
+#  CASE1-NEXT:    Auxiliary header length: 4
+#  CASE1-NEXT:    Num FDEs: 1
+#  CASE1-NEXT:    Num FREs: 16
+#  CASE1-NEXT:    FRE subsection length: 4096
+#  CASE1-NEXT:    FDE subsection offset: 0
+#  CASE1-NEXT:    FRE subsection offset: 256
+#  CASE1-NEXT:    Auxiliary header: [0xDE, 0xAD, 0xBE, 0xEF]
+#  CASE1-NEXT:  }
+#  CASE1-NEXT:{{.*}}: warning: '[[FILE]]': unexpected end of data at offset 0x24 while reading [0x20, 0x34)
+#  CASE1-NEXT:}
+
+  - Name:  .sframe_section_relative
+    Type:  SHT_GNU_SFRAME
+    Flags: [ SHF_ALLOC ]
+    ContentArray: [
+      0xe2, 0xde, 0x02, 0x00,  # Preamble (magic, version, flags)
+      # Header:
+      0x03, 0x42, 0x47, 0x00,  # ABI, Fixed FP offset, Fixed RA Offset, AUX header length
+      0x01, 0x00, 0x00, 0x00,  # Number of FDEs
+      0x10, 0x00, 0x00, 0x00,  # Number of FREs
+      0x00, 0x10, 0x00, 0x00,  # FRE length
+      0x04, 0x00, 0x00, 0x00,  # FDE offset
+      0x00, 0x01, 0x00, 0x00,  # FRE offset
+
+      0xff, 0xff, 0xff, 0xff,  # Unused data skipped due to the FDE offset field
+
+      # FDE:
+      0x00, 0xde, 0xad, 0x00,  # Start Address
+      0xbe, 0x01, 0x00, 0x00,  # Size
+      0x10, 0x00, 0x00, 0x00,  # Start FRE Offset
+      0x00, 0x00, 0x00, 0x00,  # Number of FREs
+      0x31, 0xde, 0xad, 0x00,  # Info, RepSize, Padding2
+    ]
+## Also testing:
+## - dead space between the header and the FDE subsection.
+## - PCMask FDE types
+## - unused PAuth key handling
+# CASE1-LABEL:SFrame section '.sframe_section_relative' {
+#       CASE1:  Header {
+#  CASE1-NEXT:    Magic: 0xDEE2
+#  CASE1-NEXT:    Version: V2 (0x2)
+#  CASE1-NEXT:    Flags [ (0x0)
+#  CASE1-NEXT:    ]
+#  CASE1-NEXT:    ABI: AMD64EndianLittle (0x3)
+#  CASE1-NEXT:    CFA fixed FP offset (unused): 66
+#  CASE1-NEXT:    CFA fixed RA offset: 71
+#  CASE1-NEXT:    Auxiliary header length: 0
+#  CASE1-NEXT:    Num FDEs: 1
+#  CASE1-NEXT:    Num FREs: 16
+#  CASE1-NEXT:    FRE subsection length: 4096
+#  CASE1-NEXT:    FDE subsection offset: 4
+#  CASE1-NEXT:    FRE subsection offset: 256
+#  CASE1-NEXT:    Auxiliary header: []
+#  CASE1-NEXT:  }
+#  CASE1-NEXT:  Function Index [
+#  CASE1-NEXT:    FuncDescEntry [0] {
+#  CASE1-NEXT:      PC: 0xADDE24
+#  CASE1-NEXT:      Size: 0x1BE
+#  CASE1-NEXT:      Start FRE Offset: 0x10
+#  CASE1-NEXT:      Num FREs: 0
+#  CASE1-NEXT:      Info: 0x31
+#  CASE1-NEXT:      FRE Type: Addr2 (0x1)
+#  CASE1-NEXT:      FDE Type: PCMask (0x1)
+#  CASE1-NEXT:      PAuth Key (unused): 1
+#  CASE1-NEXT:      Repetitive block size: 0xDE
+#  CASE1-NEXT:      Padding2: 0xAD
+#  CASE1-NEXT:    }
+#  CASE1-NEXT:  ]
+#  CASE1-NEXT:}
+
+  - Name:  .sframe_fde_relative
+    Type:  SHT_GNU_SFRAME
+    Flags: [ SHF_ALLOC ]
+    ContentArray: [
+      0xe2, 0xde, 0x02, 0x04,  # Preamble (magic, version, flags)
+      # Header:
+      0x02, 0x42, 0x47, 0x00,  # ABI, Fixed FP offset, Fixed RA Offset, AUX header length
+      0x01, 0x00, 0x00, 0x00,  # Number of FDEs
+      0x10, 0x00, 0x00, 0x00,  # Number of FREs
+      0x00, 0x10, 0x00, 0x00,  # FRE length
+      0x04, 0x00, 0x00, 0x00,  # FDE offset
+      0x00, 0x01, 0x00, 0x00,  # FRE offset
+
+      0xff, 0xff, 0xff, 0xff,  # Unused data skipped due to the FDE offset field
+
+      # FDE:
+      0x00, 0xde, 0xad, 0x00,  # Start Address
+      0xbe, 0x01, 0x00, 0x00,  # Size
+      0x10, 0x00, 0x00, 0x00,  # Start FRE Offset
+      0x00, 0x00, 0x00, 0x00,  # Number of FREs
+      0x02, 0xde, 0xad, 0x00,  # Info, RepSize, Padding2
+    ]
+## Also testing:
+## - PCInc FDE type
+## - AArch64 PAuth key handling
+# CASE1-LABEL:SFrame section '.sframe_fde_relative' {
+#       CASE1:  Header {
+#  CASE1-NEXT:    Magic: 0xDEE2
+#  CASE1-NEXT:    Version: V2 (0x2)
+#  CASE1-NEXT:    Flags [ (0x4)
+#  CASE1-NEXT:      FDEFuncStartPCRel (0x4){{ *}}
+#  CASE1-NEXT:    ]
+#  CASE1-NEXT:    ABI: AArch64EndianLittle (0x2)
+#  CASE1-NEXT:    CFA fixed FP offset (unused): 66
+#  CASE1-NEXT:    CFA fixed RA offset (unused): 71
+#  CASE1-NEXT:    Auxiliary header length: 0
+#  CASE1-NEXT:    Num FDEs: 1
+#  CASE1-NEXT:    Num FREs: 16
+#  CASE1-NEXT:    FRE subsection length: 4096
+#  CASE1-NEXT:    FDE subsection offset: 4
+#  CASE1-NEXT:    FRE subsection offset: 256
+#  CASE1-NEXT:    Auxiliary header: []
+#  CASE1-NEXT:  }
+#  CASE1-NEXT:  Function Index [
+#  CASE1-NEXT:    FuncDescEntry [0] {
+#  CASE1-NEXT:      PC: 0xADDE78
+#  CASE1-NEXT:      Size: 0x1BE
+#  CASE1-NEXT:      Start FRE Offset: 0x10
+#  CASE1-NEXT:      Num FREs: 0
+#  CASE1-NEXT:      Info: 0x2
+#  CASE1-NEXT:      FRE Type: Addr4 (0x2)
+#  CASE1-NEXT:      FDE Type: PCInc (0x0)
+#  CASE1-NEXT:      PAuth Key: A (0x0)
+#  CASE1-NEXT:      Repetitive block size (unused): 0xDE
+#  CASE1-NEXT:      Padding2: 0xAD
+#  CASE1-NEXT:    }
+#  CASE1-NEXT:  ]
+#  CASE1-NEXT:}
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2MSB
+  Type:  ET_EXEC
+Sections:
+  - Name:  .sframe
+    Type:  SHT_GNU_SFRAME
+    Flags: [ SHF_ALLOC ]
+    ContentArray: [
+      0xde, 0xe2, 0x02, 0x05,  # Preamble (magic, version, flags)
+      # Header:
+      0x01, 0x42, 0x47, 0x00,  # ABI, Fixed FP offset, Fixed RA Offset, AUX header length
+      0x00, 0x00, 0x00, 0x01,  # Number of FDEs
+      0x00, 0x00, 0x00, 0x10,  # Number of FREs
+      0x00, 0x00, 0x10, 0x00,  # FRE length
+      0x00, 0x00, 0x00, 0x00,  # FDE offset
+      0x00, 0x00, 0x01, 0x00,  # FRE offset
+
+      # FDE:
+      0x00, 0xde, 0xad, 0x00,  # Start Address
+      0x00, 0x00, 0x01, 0xbe,  # Size
+      0x00, 0x00, 0x00, 0x10,  # Start FRE Offset
+      0x00, 0x00, 0x00, 0x10,  # Number of FREs
+      0x02, 0xde, 0xad, 0x00,  # Info, RepSize, Padding2
+    ]
+# CASE2-LABEL:SFrame section '.sframe' {
+#       CASE2:  Header {
+#  CASE2-NEXT:    Magic: 0xDEE2
+#  CASE2-NEXT:    Version: V2 (0x2)
+#  CASE2-NEXT:    Flags [ (0x5)
+#  CASE2-NEXT:      FDEFuncStartPCRel (0x4){{ *}}
+#  CASE2-NEXT:      FDESorted (0x1){{ *}}
+#  CASE2-NEXT:    ]
+#  CASE2-NEXT:    ABI: AArch64EndianBig (0x1)
+#  CASE2-NEXT:    CFA fixed FP offset (unused): 66
+#  CASE2-NEXT:    CFA fixed RA offset (unused): 71
+#  CASE2-NEXT:    Auxiliary header length: 0
+#  CASE2-NEXT:    Num FDEs: 1
+#  CASE2-NEXT:    Num FREs: 16
+#  CASE2-NEXT:    FRE subsection length: 4096
+#  CASE2-NEXT:    FDE subsection offset: 0
+#  CASE2-NEXT:    FRE subsection offset: 256
+#  CASE2-NEXT:    Auxiliary header: []
+#  CASE2-NEXT:  }
+#  CASE2-NEXT:  Function Index [
+#  CASE2-NEXT:    FuncDescEntry [0] {
+#  CASE2-NEXT:      PC: 0xDEAD1C
+#  CASE2-NEXT:      Size: 0x1BE
+#  CASE2-NEXT:      Start FRE Offset: 0x10
+#  CASE2-NEXT:      Num FREs: 16
+#  CASE2-NEXT:      Info: 0x2
+#  CASE2-NEXT:      FRE Type: Addr4 (0x2)
+#  CASE2-NEXT:      FDE Type: PCInc (0x0)
+#  CASE2-NEXT:      PAuth Key: A (0x0)
+#  CASE2-NEXT:      Repetitive block size (unused): 0xDE
+#  CASE2-NEXT:      Padding2: 0xAD00
+#  CASE2-NEXT:    }
+#  CASE2-NEXT:  ]
+#  CASE2-NEXT:}
diff --git a/llvm/test/tools/llvm-readobj/ELF/sframe-header.test b/llvm/test/tools/llvm-readobj/ELF/sframe-header.test
index f827296b1c399..e7c0db0d957c1 100644
--- a/llvm/test/tools/llvm-readobj/ELF/sframe-header.test
+++ b/llvm/test/tools/llvm-readobj/ELF/sframe-header.test
@@ -2,7 +2,8 @@
 # RUN: yaml2obj --docnum=1 %s -o %t.1
 # RUN: llvm-readobj --sframe=.sframe_bad_sh_size --sframe=.sframe_1b \
 # RUN:   --sframe=.sframe_bad_magic --sframe=.sframe_bad_version \
-# RUN:   --sframe=.sframe_6b --sframe=.sframe_header %t.1 2>&1 | \
+# RUN:   --sframe=.sframe_6b --sframe=.sframe_short_auxheader \
+# RUN:   --sframe=.sframe_header %t.1 2>&1 | \
 # RUN:   FileCheck %s --strict-whitespace --match-full-lines \
 # RUN:   -DFILE=%t.1 --check-prefix=CASE1
 
@@ -60,24 +61,24 @@ Sections:
       0xe2, 0xde, 0x02, 0x00,  # Preamble (magic, version, flags)
       0x01, 0x02
     ]
-
 # CASE1-LABEL:SFrame sec...
[truncated]

@github-actions
Copy link

github-actions bot commented Jul 21, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

SFrameParser(ArrayRef<uint8_t> Data, uint64_t SectionAddress, const sframe::Header<E> &Header)
: Data(Data), SectionAddress(SectionAddress), Header(Header) {}

uint64_t getFDEBegin() const { return sizeof(Header) + Header.AuxHdrLen + Header.FDEOff; }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibhagatgnu, I'm a little but unsure about this part. This is the interpretation I get from the specification, and I think it matches this endiannes-flipping code in libsframe. However, that's not what happens in the actual parsing code. Can I assume the last part to be an omission/bug?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@labath the SFrame FDE offset is the offset of the FDE sub-section from the end of the SFrame header. So, the omission in the sframe_decode is harmless, but should be done for doc purposes and clarity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand. The value I linked to is used to locate the FDEs, so if sfh_fdeoff is not zero, it will end up looking at the wrong place. Or are you saying that it does not matter in practice because the value will always be zero? That, I suppose is true (and I don't see why would anyone use a value other than zero here), but the spec does allow it so I suppose someone could do it.

I guess my main question is whether I've implemented the FDE location code correctly.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, using the FDEoffset is the right thing to do. You are right that the spec allows non-zero value for fde_offset. So I correct my previous statement: the omission in sframe_decode should be corrected to use the fde_offset to be fully compliant.

}

template <typename ELFT>
void ELFDumper<ELFT>::printSFrameHeader(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved out of line to keep the function size in check.

Type: SHT_GNU_SFRAME
Flags: [ SHF_ALLOC ]
ContentArray: [
0xe2, 0xde, 0x02, 0x00, # Preamble (magic, version, flags)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Aside: perhaps ContentArray should support raw hexadecimal bytes with whitespace ignored (e2de0200, or e2de 0200) https://llvm.org/docs/CommandGuide/llvm-mc.html#cmdoption-llvm-mc-hex

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it existed, I'd use it, but I don't think it makes sense to introduce it just for this. It also doesn't feel like an very good match for ContentArray, as that there will be an array of bytes and it relies on YAML parser to parse the individual bytes. In a way, the regular Content field feels like a better match since it already doesn't use the 0x prefix. "All" that's needed is the ability to ignore comments/whitespace (I don't know how well that fits into the yaml syntax).

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with the change, but please wait for the other reviewers.

P.S. I'm off all of next week. I'm happy for others to review any further changes in my absence.

@labath
Copy link
Collaborator Author

labath commented Jul 25, 2025

Thank you, James. Does anyone else have anything they'd like to add or bring up?

uintptr_t DataPtr = reinterpret_cast<uintptr_t>(Data.data());
uintptr_t FDEPtr = reinterpret_cast<uintptr_t>(&*FDE);

assert(DataPtr <= FDEPtr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sfde_func_start_address is a signed displacement, so the sframe segment could be placed before or after the text segment. I personally favor making it unsigned to double the possible range and which would make this assertion valid, but that isn't as the spec is written today.

Copy link
Collaborator Author

@labath labath Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't check the value of the field -- only its address. The goal is to catch the (UB) case where someone passes an FDE iterator from one object into the parser for another object. I'll add a message to the assertion to make that clearer.

@labath labath merged commit ded255e into llvm:main Jul 30, 2025
9 checks passed
@labath labath deleted the sframe branch July 30, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants