-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[Object] Beginnings of SFrame parser and dumper #147294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-llvm-binary-utilities Author: Pavel Labath (labath) ChangesThis PR adds the SFrameParser class and uses it from llvm-readobj to llvm-readobj uses the same sframe flag syntax as GNU readelf, but I have Depends on #147264. Note to reviewers: This PR consists of two commits. The first patch is equivalent to #147264. For reviewing, I recommend viewing only the changes in the second commit. Patch is 27.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/147294.diff 14 Files Affected:
diff --git a/llvm/include/llvm/BinaryFormat/SFrame.h b/llvm/include/llvm/BinaryFormat/SFrame.h
new file mode 100644
index 0000000000000..fa2b2b4db4ebd
--- /dev/null
+++ b/llvm/include/llvm/BinaryFormat/SFrame.h
@@ -0,0 +1,171 @@
+//===-- llvm/BinaryFormat/SFrame.h ---SFrame Data Structures ----*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file
+/// This file contains data-structure definitions and constants to support
+/// unwinding based on .sframe sections. This only supports SFRAME_VERSION_2
+/// as described at https://sourceware.org/binutils/docs/sframe-spec.html
+///
+/// Naming conventions follow the spec document. #defines converted to constants
+/// and enums for better C++ compatibility.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_BINARYFORMAT_SFRAME_H
+#define LLVM_BINARYFORMAT_SFRAME_H
+
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/DataTypes.h"
+#include "llvm/ADT/ArrayRef.h"
+
+namespace llvm {
+
+template <typename T> struct EnumEntry;
+
+namespace sframe {
+
+constexpr uint16_t SFRAME_MAGIC = 0xDEE2;
+
+enum : uint8_t {
+#define HANDLE_SFRAME_VERSION(CODE, NAME) SFRAME_##NAME = CODE,
+#include "llvm/BinaryFormat/SFrameConstants.def"
+};
+
+/// sframe_preable.sfp_flags flags.
+enum : uint8_t {
+#define HANDLE_SFRAME_HEADER_FLAG(CODE, NAME) SFRAME_F_##NAME = CODE,
+#include "llvm/BinaryFormat/SFrameConstants.def"
+};
+
+/// Possible values for sframe_header.sfh_abi_arch.
+enum : uint8_t {
+#define HANDLE_SFRAME_ABI(CODE, NAME) SFRAME_ABI_##NAME = CODE,
+#include "llvm/BinaryFormat/SFrameConstants.def"
+};
+
+/// SFrame FRE Types. Bits 0-3 of sframe_func_desc_entry.sfde_func_info.
+enum : uint8_t {
+ SFRAME_FRE_TYPE_ADDR1 = 0,
+ SFRAME_FRE_TYPE_ADDR2 = 1,
+ SFRAME_FRE_TYPE_ADDR4 = 2,
+};
+
+/// SFrame FDE Types. Bit 4 of sframe_func_desc_entry.sfde_func_info.
+enum : uint8_t {
+ SFRAME_FDE_TYPE_PCINC = 0,
+ SFRAME_FDE_TYPE_PCMASK = 1,
+};
+
+/// Speficies key used for signing return addresses. Bit 5 of
+/// sframe_func_desc_entry.sfde_func_info.
+enum : uint8_t {
+ SFRAME_AARCH64_PAUTH_KEY_A = 0,
+ SFRAME_AARCH64_PAUTH_KEY_B = 1,
+};
+
+/// Size of stack offsets. Bits 5-6 of sframe_fre_info.fre_info.
+enum : uint8_t {
+ SFRAME_FRE_OFFSET_1B = 0,
+ SFRAME_FRE_OFFSET_2B = 1,
+ SFRAME_FRE_OFFSET_4B = 2,
+};
+
+/// Stack frame base register. Bit 0 of sframe_fre_info.fre_info.
+enum : uint8_t { SFRAME_BASE_REG_FP = 0, SFRAME_BASE_REG_SP = 1 };
+
+LLVM_PACKED_START
+
+struct sframe_preamble {
+ uint16_t sfp_magic;
+ uint8_t sfp_version;
+ uint8_t sfp_flags;
+};
+
+struct sframe_header {
+ sframe_preamble sfh_preamble;
+ uint8_t sfh_abi_arch;
+ int8_t sfh_cfa_fixed_fp_offset;
+ int8_t sfh_cfa_fixed_ra_offset;
+ uint8_t sfh_auxhdr_len;
+ uint32_t sfh_num_fdes;
+ uint32_t sfh_num_fres;
+ uint32_t sfh_fre_len;
+ uint32_t sfh_fdeoff;
+ uint32_t sfh_freoff;
+};
+
+struct sframe_func_desc_entry {
+ int32_t sfde_func_start_address;
+ uint32_t sfde_func_size;
+ uint32_t sfde_func_start_fre_off;
+ uint32_t sfde_func_num_fres;
+ uint8_t sfde_func_info;
+ uint8_t sfde_func_rep_size;
+ uint16_t sfde_func_padding2;
+
+ uint8_t getPAuthKey() const { return (sfde_func_info >> 5) & 1; }
+ uint8_t getFDEType() const { return (sfde_func_info >> 4) & 1; }
+ uint8_t getFREType() const { return sfde_func_info & 0xf; }
+ void setPAuthKey(uint8_t P) { setFuncInfo(P, getFDEType(), getFREType()); }
+ void setFDEType(uint8_t D) { setFuncInfo(getPAuthKey(), D, getFREType()); }
+ void setFREType(uint8_t R) { setFuncInfo(getPAuthKey(), getFDEType(), R); }
+ void setFuncInfo(uint8_t PAuthKey, uint8_t FDEType, uint8_t FREType) {
+ sfde_func_info =
+ ((PAuthKey & 1) << 5) | ((FDEType & 1) << 4) | (FREType & 0xf);
+ }
+};
+
+struct sframe_fre_info {
+ uint8_t fre_info;
+
+ bool isReturnAddressSigned() const { return fre_info >> 7; }
+ uint8_t getOffsetSize() const { return (fre_info >> 5) & 3; }
+ uint8_t getOffsetCount() const { return (fre_info >> 1) & 0xf; }
+ uint8_t getBaseRegister() const { return fre_info & 1; }
+ void setReturnAddressSigned(bool RA) {
+ setFREInfo(RA, getOffsetSize(), getOffsetCount(), getBaseRegister());
+ }
+ void setOffsetSize(uint8_t Sz) {
+ setFREInfo(isReturnAddressSigned(), Sz, getOffsetCount(),
+ getBaseRegister());
+ }
+ void setOffsetCount(uint8_t N) {
+ setFREInfo(isReturnAddressSigned(), getOffsetSize(), N, getBaseRegister());
+ }
+ void setBaseRegister(uint8_t Reg) {
+ setFREInfo(isReturnAddressSigned(), getOffsetSize(), getOffsetCount(), Reg);
+ }
+ void setFREInfo(bool RA, uint8_t Sz, uint8_t N, uint8_t Reg) {
+ fre_info = ((RA & 1) << 7) | ((Sz & 3) << 5) | ((N & 0xf) << 1) | (Reg & 1);
+ }
+};
+
+struct sframe_frame_row_entry_addr1 {
+ uint8_t sfre_start_address;
+ sframe_fre_info sfre_info;
+};
+
+struct sframe_frame_row_entry_addr2 {
+ uint16_t sfre_start_address;
+ sframe_fre_info sfre_info;
+};
+
+struct sframe_frame_row_entry_addr4 {
+ uint32_t sfre_start_address;
+ sframe_fre_info sfre_info;
+};
+
+LLVM_PACKED_END
+
+ArrayRef<EnumEntry<uint8_t>> getVersions();
+ArrayRef<EnumEntry<uint8_t>> getHeaderFlags();
+ArrayRef<EnumEntry<uint8_t>> getABIs();
+
+} // namespace sframe
+} // namespace llvm
+
+#endif // LLVM_BINARYFORMAT_SFRAME_H
diff --git a/llvm/include/llvm/BinaryFormat/SFrameConstants.def b/llvm/include/llvm/BinaryFormat/SFrameConstants.def
new file mode 100644
index 0000000000000..d6c131330e2c0
--- /dev/null
+++ b/llvm/include/llvm/BinaryFormat/SFrameConstants.def
@@ -0,0 +1,38 @@
+//===- SFrameConstants.def --------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#if !(defined(HANDLE_SFRAME_VERSION) || defined(HANDLE_SFRAME_HEADER_FLAG) || \
+ defined(HANDLE_SFRAME_ABI))
+#error "Missing HANDLE_SFRAME definition"
+#endif
+
+#ifndef HANDLE_SFRAME_VERSION
+#define HANDLE_SFRAME_VERSION(CODE, NAME)
+#endif
+
+#ifndef HANDLE_SFRAME_HEADER_FLAG
+#define HANDLE_SFRAME_HEADER_FLAG(CODE, NAME)
+#endif
+
+#ifndef HANDLE_SFRAME_ABI
+#define HANDLE_SFRAME_ABI(CODE, NAME)
+#endif
+
+HANDLE_SFRAME_VERSION(0x01, VERSION_1)
+HANDLE_SFRAME_VERSION(0x02, VERSION_2)
+
+HANDLE_SFRAME_HEADER_FLAG(0x01, FDE_SORTED)
+HANDLE_SFRAME_HEADER_FLAG(0x02, FRAME_POINTER)
+
+HANDLE_SFRAME_ABI(0x01, AARCH64_ENDIAN_BIG)
+HANDLE_SFRAME_ABI(0x02, AARCH64_ENDIAN_LITTLE)
+HANDLE_SFRAME_ABI(0x03, AMD64_ENDIAN_LITTLE)
+
+#undef HANDLE_SFRAME_VERSION
+#undef HANDLE_SFRAME_HEADER_FLAG
+#undef HANDLE_SFRAME_ABI
diff --git a/llvm/include/llvm/Object/SFrameParser.h b/llvm/include/llvm/Object/SFrameParser.h
new file mode 100644
index 0000000000000..c297650d82558
--- /dev/null
+++ b/llvm/include/llvm/Object/SFrameParser.h
@@ -0,0 +1,47 @@
+//===- SFrameParser.h -------------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_OBJECT_SFRAME_H
+#define LLVM_OBJECT_SFRAME_H
+
+#include "llvm/BinaryFormat/SFrame.h"
+#include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/Error.h"
+#include <cstdint>
+
+namespace llvm {
+namespace object {
+
+class SFrameParser {
+public:
+ static Expected<SFrameParser> create(ArrayRef<uint8_t> Contents);
+
+ const sframe::sframe_preamble &getPreamble() const {
+ return Header.sfh_preamble;
+ }
+ const sframe::sframe_header &getHeader() const { return Header; }
+
+ bool usesFixedRAOffset() const {
+ return Header.sfh_abi_arch == sframe::SFRAME_ABI_AMD64_ENDIAN_LITTLE;
+ }
+ bool usesFixedFPOffset() const {
+ return false; // Not used in any currently defined ABI.
+ }
+
+private:
+ DataExtractor Data;
+ sframe::sframe_header Header;
+
+ SFrameParser(DataExtractor Data, sframe::sframe_header Header)
+ : Data(Data), Header(Header) {}
+};
+
+} // end namespace object
+} // end namespace llvm
+
+#endif // LLVM_OBJECT_SFRAME_H
diff --git a/llvm/lib/BinaryFormat/CMakeLists.txt b/llvm/lib/BinaryFormat/CMakeLists.txt
index 38ba2d9e85a06..4b2debb7ae236 100644
--- a/llvm/lib/BinaryFormat/CMakeLists.txt
+++ b/llvm/lib/BinaryFormat/CMakeLists.txt
@@ -11,6 +11,7 @@ add_llvm_component_library(LLVMBinaryFormat
MsgPackDocumentYAML.cpp
MsgPackReader.cpp
MsgPackWriter.cpp
+ SFrame.cpp
Wasm.cpp
XCOFF.cpp
diff --git a/llvm/lib/BinaryFormat/SFrame.cpp b/llvm/lib/BinaryFormat/SFrame.cpp
new file mode 100644
index 0000000000000..439b303bbb9e8
--- /dev/null
+++ b/llvm/lib/BinaryFormat/SFrame.cpp
@@ -0,0 +1,40 @@
+//===-- SFrame.cpp -----------------------------------------------*- C++-*-===//
+//
+// 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/BinaryFormat/SFrame.h"
+#include "llvm/Support/ScopedPrinter.h"
+
+using namespace llvm;
+
+ArrayRef<EnumEntry<uint8_t>> sframe::getVersions() {
+ static constexpr EnumEntry<uint8_t> Versions[] = {
+#define HANDLE_SFRAME_VERSION(CODE, NAME) \
+ {"SFRAME_" #NAME, sframe::SFRAME_##NAME},
+#include "llvm/BinaryFormat/SFrameConstants.def"
+ };
+
+ return ArrayRef(Versions);
+}
+
+ArrayRef<EnumEntry<uint8_t>> sframe::getHeaderFlags() {
+ static constexpr EnumEntry<uint8_t> Flags[] = {
+#define HANDLE_SFRAME_HEADER_FLAG(CODE, NAME) \
+ {"SFRAME_F_" #NAME, sframe::SFRAME_F_##NAME},
+#include "llvm/BinaryFormat/SFrameConstants.def"
+ };
+ return ArrayRef(Flags);
+}
+
+ArrayRef<EnumEntry<uint8_t>> sframe::getABIs() {
+ static constexpr EnumEntry<uint8_t> ABIs[] = {
+#define HANDLE_SFRAME_ABI(CODE, NAME) \
+ {"SFRAME_ABI_" #NAME, sframe::SFRAME_ABI_##NAME},
+#include "llvm/BinaryFormat/SFrameConstants.def"
+ };
+ return ArrayRef(ABIs);
+}
diff --git a/llvm/lib/Object/CMakeLists.txt b/llvm/lib/Object/CMakeLists.txt
index 870169a83174f..0f6d2f7c59a5c 100644
--- a/llvm/lib/Object/CMakeLists.txt
+++ b/llvm/lib/Object/CMakeLists.txt
@@ -25,6 +25,7 @@ add_llvm_component_library(LLVMObject
OffloadBundle.cpp
RecordStreamer.cpp
RelocationResolver.cpp
+ SFrameParser.cpp
SymbolicFile.cpp
SymbolSize.cpp
TapiFile.cpp
diff --git a/llvm/lib/Object/SFrameParser.cpp b/llvm/lib/Object/SFrameParser.cpp
new file mode 100644
index 0000000000000..6f6d7ca2dc740
--- /dev/null
+++ b/llvm/lib/Object/SFrameParser.cpp
@@ -0,0 +1,65 @@
+//===- SFrameParser.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/Object/SFrameParser.h"
+#include "llvm/Object/Error.h"
+
+using namespace llvm;
+using namespace llvm::object;
+using namespace llvm::sframe;
+
+Expected<SFrameParser> SFrameParser::create(ArrayRef<uint8_t> Contents) {
+ sframe_header Header;
+ sframe_preamble &Preamble = Header.sfh_preamble;
+
+ // IsLittleEndian will be swapped if needed, AddressSize is unused.
+ DataExtractor Data(Contents, /*IsLittleEndian=*/true, /*AddressSize=*/0);
+ DataExtractor::Cursor Cursor(0);
+ Preamble.sfp_magic = Data.getU16(Cursor);
+ if (!Cursor)
+ return Cursor.takeError();
+
+ switch (Preamble.sfp_magic) {
+ case SFRAME_MAGIC:
+ break;
+ case byteswap(SFRAME_MAGIC):
+ // Fix endianness.
+ Preamble.sfp_magic = SFRAME_MAGIC;
+ // And make sure everything that comes afterwards is read correctly.
+ Data = DataExtractor(Data.getData(), !Data.isLittleEndian(),
+ Data.getAddressSize());
+ break;
+ default:
+ return make_error<GenericBinaryError>("invalid magic number",
+ object_error::parse_failed);
+ }
+ Preamble.sfp_version = Data.getU8(Cursor);
+ Preamble.sfp_flags = Data.getU8(Cursor);
+ if (!Cursor)
+ return Cursor.takeError();
+
+ if (Preamble.sfp_version != SFRAME_VERSION_2) {
+ return make_error<GenericBinaryError>("invalid/unsupported version number",
+ object_error::parse_failed);
+ }
+
+ Header.sfh_abi_arch = Data.getU8(Cursor);
+ Header.sfh_cfa_fixed_fp_offset = (int8_t)Data.getU8(Cursor);
+ Header.sfh_cfa_fixed_ra_offset = (int8_t)Data.getU8(Cursor);
+ Header.sfh_auxhdr_len = Data.getU8(Cursor);
+ Header.sfh_num_fdes = Data.getU32(Cursor);
+ Header.sfh_num_fres = Data.getU32(Cursor);
+ Header.sfh_fre_len = Data.getU32(Cursor);
+ Header.sfh_fdeoff = Data.getU32(Cursor);
+ Header.sfh_freoff = Data.getU32(Cursor);
+
+ if (!Cursor)
+ return Cursor.takeError();
+
+ return SFrameParser(Data, Header);
+}
diff --git a/llvm/test/tools/llvm-readobj/ELF/sframe-header.test b/llvm/test/tools/llvm-readobj/ELF/sframe-header.test
new file mode 100644
index 0000000000000..6731d7d154a49
--- /dev/null
+++ b/llvm/test/tools/llvm-readobj/ELF/sframe-header.test
@@ -0,0 +1,124 @@
+## Check parsing and dumping of the SFrame header.
+# RUN: yaml2obj --docnum=1 %s -o %t.1
+# RUN: llvm-readobj --sframe=.sframe_1b --sframe=.sframe_bad_magic \
+# RUN: --sframe=.sframe_3b --sframe=.sframe_bad_version --sframe=.sframe_6b \
+# RUN: --sframe=.sframe_header %t.1 2>&1 | FileCheck %s --check-prefix=CASE1
+
+## Check big-endian support and the handling of --sframe argument default.
+# RUN: yaml2obj --docnum=2 %s -o %t.2
+# RUN: llvm-readobj --sframe %t.2 2>&1 | FileCheck %s --check-prefix=CASE2
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_X86_64
+Sections:
+ - Name: .sframe_1b
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC ]
+ ContentArray: [ 0x00 ]
+# CASE1-LABEL: Contents of SFrame section '.sframe_1b':
+# CASE1: warning: {{.*}}: unexpected end of data at offset 0x1 while reading [0x0, 0x2)
+
+ - Name: .sframe_bad_magic
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC ]
+ ContentArray: [ 0xde, 0xad ]
+# CASE1-LABEL: Contents of SFrame section '.sframe_bad_magic':
+# CASE1: warning: {{.*}}: invalid magic number
+
+ - Name: .sframe_3b
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC ]
+ ContentArray: [ 0xe2, 0xde, 0x01 ]
+# CASE1-LABEL: Contents of SFrame section '.sframe_3b':
+# CASE1: warning: {{.*}}: unexpected end of data at offset 0x3 while reading [0x3, 0x4)
+
+ - Name: .sframe_bad_version
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC ]
+ ContentArray: [
+ 0xe2, 0xde, 0x01, 0x00 # Preamble (magic, version, flags)
+ ]
+# CASE1-LABEL: Contents of SFrame section '.sframe_bad_version':
+# CASE1: warning: {{.*}}: invalid/unsupported version number
+
+ - Name: .sframe_6b
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC ]
+ ContentArray: [
+ 0xe2, 0xde, 0x02, 0x00, # Preamble (magic, version, flags)
+ 0x01, 0x02
+ ]
+
+# CASE1-LABEL: Contents of SFrame section '.sframe_6b':
+# CASE1: warning: {{.*}}: unexpected end of data at offset 0x6 while reading [0x6, 0x7)
+
+ - Name: .sframe_header
+ Type: SHT_PROGBITS
+ 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
+ ]
+# CASE1-LABEL: Contents of SFrame section '.sframe_header':
+# CASE1: Header {
+# CASE1-NEXT: Magic: 0xDEE2
+# CASE1-NEXT: Version: SFRAME_VERSION_2 (0x2)
+# CASE1-NEXT: Flags [ (0x0)
+# CASE1-NEXT: ]
+# CASE1-NEXT: ABI: SFRAME_ABI_AMD64_ENDIAN_LITTLE (0x3)
+# CASE1-NEXT: CFA fixed RA offset: 71
+# CASE1-NEXT: CFA fixed FP offset (unused): 66
+# 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: }
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2MSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+Sections:
+ - Name: .sframe
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC ]
+ ContentArray: [
+ 0xde, 0xe2, 0x02, 0x00, # 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, 0x04, # FDE offset
+ 0x00, 0x00, 0x01, 0x00, # FRE offset
+ ]
+# CASE2-LABEL: Contents of SFrame section '.sframe':
+# CASE2: Header {
+# CASE2-NEXT: Magic: 0xDEE2
+# CASE2-NEXT: Version: SFRAME_VERSION_2 (0x2)
+# CASE2-NEXT: Flags [ (0x0)
+# CASE2-NEXT: ]
+# CASE2-NEXT: ABI: SFRAME_ABI_AARCH64_ENDIAN_BIG (0x1)
+# CASE2-NEXT: CFA fixed RA offset (unused): 71
+# CASE2-NEXT: CFA fixed FP offset (unused): 66
+# 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: 4
+# CASE2-NEXT: FRE subsection offset: 256
+# CASE2-NEXT: }
diff --git a/llvm/tools/llvm-readobj/ObjDumper.cpp b/llvm/tools/llvm-readobj/ObjDumper.cpp
index d3c613ee823ba..00317ba9df6c7 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.cpp
+++ b/llvm/tools/llvm-readobj/ObjDumper.cpp
@@ -13,9 +13,11 @@
#include "ObjDumper.h"
#include "llvm-readobj.h"
+#include "llvm/BinaryFormat/SFrame.h"
#include "llvm/Object/Archive.h"
#include "llvm/Object/Decompressor.h"
#include "llvm/Object/ObjectFile.h"
+#include "llvm/Object/SFrameParser.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -230,4 +232,53 @@ void ObjDumper::printSectionsAsHex(const object::ObjectFile &Obj,
}
}
+void ObjDumper::printSectionsAsSFrame(const object::ObjectFile &Obj,
+ ArrayRef<std::string> Sections) {
+ for (object::SectionRef Section :
+ getSectionRefsByNameOrIndex(Obj, Sections)) {
+ StringRef SectionName = unwrapOrError(Obj.getFileName(), Section.getName());
+ W.getOStream() << '\n';
+ W.startLine() << "Contents of SFrame section '" << SectionName << "':\n";
+
+ StringRef SectionContent =
+ unwrapOrError(Obj.getFileName(), Section.getContents());
+ Expected<object::SFrameParser> Parser =
+ object::SFrameParser::create(arrayRefFromStringRef(SectionContent));
+ if (!Parser) {
+ reportWarning(Parser.takeError(), Obj.getFileName());
+ continue;
+ }
+
+ W.indent();
+ W.objectBegin("Header");
+
+ const sframe::sframe_preamble &Preamble = Parser->getPreamble();
+ W.printHex("Magic", Preamble.sfp_magic);
+ W.printEnum("Version", Preamble.sfp_version, sframe::getVersions());
+ W.printFlags("Flags", Preamble.sfp_flags, sframe::getHeaderFlags());
+
+ const sframe::sframe_header &Header = Parser->getHeader();
+ W.printEnum("ABI", Header.sfh_abi_arch, sframe::getABIs());
+
+ W.printNumber(("CFA fixed RA offset" +
+ Twine(Parser->usesFixedRAOffset() ? "" : " (unused)"))
+ .str(),
+ Header.sfh_cf...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specification that can be pointed to in the commit description? It's a bit hard to review a parser without actually knowing what the specification is :)
|
||
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); | ||
|
||
constexpr uint16_t Magic = 0xdee2; | ||
|
||
enum class Version : uint8_t { | ||
V1 = 1, | ||
V2 = 2, | ||
#define HANDLE_SFRAME_VERSION(CODE, NAME) NAME = CODE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: I'm looking forward to C++26 when we can have reflection do this. Only need to wait until about 2036 for it to be available in LLVM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, definitely. I'm actually contemplating removing this and hardcoding everything in c++. All of the enums here have at most three cases, which means that the .def file adds more boilerplate (the preprocessor goo) than it removes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to go with whichever approach you prefer, as long as it's easily maintainable (i.e. isn't likely to fall over/get forgotten if we e.g. introduce a new enum value).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last part is the tricky one. I don't see a way to ensure a new enumerator isn't added without the corresponding stringifier without introducing more boilerplate. I guess I'm going to stick with the .def file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something involving an "end" entry in the enum combined with a static assert verifying the value hasn't increased might do the trick, but it feels a bit suboptimal.
llvm/lib/Object/SFrameParser.cpp
Outdated
return make_error<GenericBinaryError>("invalid magic number", | ||
object_error::parse_failed); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not worth it, if this and the below errors are the only two times you need it, but if you plan on having more errors, I'd consider a small helper to wrap this, i.e. so you can do something like return makeError("invalid magic number");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a createError in Object/Error.h, so I've used that.
llvm/lib/Object/SFrameParser.cpp
Outdated
if (!Preamble) | ||
return Preamble.takeError(); | ||
|
||
if (Preamble->Magic != sframe::Magic) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: here and below, these are single line ifs, so shouldn't have braces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. (technically, they weren't single-line (though they were probably "simple"), but they are now. :) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specification that can be pointed to in the commit description? It's a bit hard to review a parser without actually knowing what the specification is :)
Sorry about that. The specification is here and this is the RFC thread. It's linked to in the first PR, but I'm going to add it to the next couple of PRs at least.
The SFrame section starts with a preamble, which contains which contains the version number, and it is the only fixed part of the format. It is followed by the header, whose layout can depend on the version (but we're only supporting the current version (2)). The sframe sections are not linked by concatenating, so the section always contains only one header/preamble pair.
|
||
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); | ||
|
||
constexpr uint16_t Magic = 0xdee2; | ||
|
||
enum class Version : uint8_t { | ||
V1 = 1, | ||
V2 = 2, | ||
#define HANDLE_SFRAME_VERSION(CODE, NAME) NAME = CODE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, definitely. I'm actually contemplating removing this and hardcoding everything in c++. All of the enums here have at most three cases, which means that the .def file adds more boilerplate (the preprocessor goo) than it removes.
llvm/lib/Object/SFrameParser.cpp
Outdated
if (!Preamble) | ||
return Preamble.takeError(); | ||
|
||
if (Preamble->Magic != sframe::Magic) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. (technically, they weren't single-line (though they were probably "simple"), but they are now. :) )
llvm/lib/Object/SFrameParser.cpp
Outdated
return make_error<GenericBinaryError>("invalid magic number", | ||
object_error::parse_failed); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a createError in Object/Error.h, so I've used that.
Machine: EM_X86_64 | ||
Sections: | ||
- Name: .sframe_1b | ||
Type: SHT_PROGBITS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new section type for SFrame SHT_GNU_SFRAME with value 0x6ffffff4 has been added (https://sourceware.org/git/?p=gnu-gabi.git;a=commit;h=b85391cf2ad97ec209b7cfe84b24c65ad5748576). GNU Binutils 2.45 and later will generate SFrame sections with that section type. Please have SHT_GNU_SFRAME as the section type on the LLVM side too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know. I've created #148803 for that.
For the moment, I'm keeping the PROGBITS here. I'll change that once the other PR lands (the occurrence in this test is not particularly important, it's the generator that matters).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has GNU readelf's --all been updated to dump sframe too, since it does imply --unwind? If so, we should update to match.
I'm not seeing the changes you said you made for things like createError
? Did you push the wrong commit or something?
|
||
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); | ||
|
||
constexpr uint16_t Magic = 0xdee2; | ||
|
||
enum class Version : uint8_t { | ||
V1 = 1, | ||
V2 = 2, | ||
#define HANDLE_SFRAME_VERSION(CODE, NAME) NAME = CODE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to go with whichever approach you prefer, as long as it's easily maintainable (i.e. isn't likely to fall over/get forgotten if we e.g. introduce a new enum value).
constexpr endianness E = ELFT::Endianness; | ||
for (object::SectionRef Section : | ||
getSectionRefsByNameOrIndex(ObjF, Sections)) { | ||
StringRef SectionName = unwrapOrError(FileName, Section.getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unwrapOrError
is generally considered deprecated for new code: we should avoid using it because it causes llvm-readobj to abort and prevent dumping any other requested information. Instead, we typically bail out of the specific bit that is being done and report a warning using reportWarning
. Here, for example, I'd report the warning and simply skip the section.
Same comment goes for other uses of unwrapOrError
below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Done.
W.getOStream() << '\n'; | ||
W.startLine() << "Contents of SFrame section '" << SectionName << "':\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when --elf-output-style=json
is specified with sframe dumping? Is the output valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not. It's a left over from when I was trying to match GNU output format. I've fixed it now.
Any thoughts on testing the json output? Repeating all of the expectations for json output is rather.. repetitive, and it doesn't really ensure the output is valid json. Is there any way to assert that the output parses as json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've tended to only test JSON output specifically where there needs to be a difference between the JSON and LLVM-style logic and then have just done regular FileCheck matching (which is indeed a bit repetitive). I have no idea how well this would work in practice, but one option that has occurred to me, if we just want to make sure it is valid, is to pass the output to python and see if its json
library can read it successfully.
@@ -62,6 +62,8 @@ def memtag : FF<"memtag", "Display memory tagging metadata (modes, Android notes | |||
def needed_libs : FF<"needed-libs", "Display the needed libraries">, Group<grp_elf>; | |||
def notes : FF<"notes", "Display notes">, Group<grp_elf>; | |||
def program_headers : FF<"program-headers", "Display program headers">, Group<grp_elf>; | |||
def sframe_EQ : Joined<["--"], "sframe=">, HelpText<"Display SFrame section <name>">, MetaVarName<"<name>">, Group<grp_elf>; | |||
def sframe: FF<"sframe", "Alias for --sframe=.sframe">, Alias<sframe_EQ>, AliasArgs<[".sframe"]>, Group<grp_elf>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's a section type that is used going forward, perhaps we should not do this as an alias for that name and instead have the "default" behaviour for the option be to dump all sections with the corresponding type? If people want to dump the default sframe section from older objects, they can still manually specify the name then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would make sense, but it seems that the GNU version still uses (even at HEAD) the name based mechanism:
$ binutils/readelf /tmp/a2.o -e | grep sframe
[ 9] .sframe2 GNU_SFRAME 0000000000000000 000000f8
[10] .rela.sframe2 RELA 0000000000000000 000001c0
$ binutils/readelf /tmp/a2.o --sframe
readelf: Warning: Section '.sframe' was not dumped because it does not exist
@ibhagatgnu, any thoughts on this?
I'm also not sure how much we care about compatibility here -- I'd be fine with changing this even if gnu readelf doesn't do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realise there was a GNU version to follow. We should probably stick with that, but if we can add type-based alongside it somehow, that would be good too (and more ELF-like).
# RUN: yaml2obj --docnum=1 %s -o %t.1 | ||
# RUN: llvm-readobj --sframe=.sframe_1b --sframe=.sframe_bad_magic \ | ||
# RUN: --sframe=.sframe_bad_version --sframe=.sframe_6b \ | ||
# RUN: --sframe=.sframe_header %t.1 2>&1 | FileCheck %s --check-prefix=CASE1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably worth having --strict-whitespace
and --match-full-lines
to ensure that all indentation etc is as expected. Same below for the big endian case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
.str(), | ||
Header.CFAFixedRAOffset.value()); | ||
|
||
W.printNumber(("CFA fixed FP offset" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the spec, the FP offset appears before the RA offset field, so is it worth printing it first, so that the printing order matches the structure order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good idea. I'm not sure how I ended up switching those.
This PR adds the SFrameParser class and uses it from llvm-readobj to dump the section contents. Currently, it only supports parsing the SFrame section header. Other parts of the section will be added in follow-up patches. llvm-readobj uses the same sframe flag syntax as GNU readelf, but I have not attempted match the output format of the tool. I'm starting with the "llvm" output format because it's easier to generate and lets us tweak the format to make it useful for testing the generation code. If needed, support for the GNU format could be added by overriding this functionality in the GNU ELF Dumper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. I'm going to do a rebase force-push now that the dependent PR has been merged. I apologise if that loses some the comments. I believe I have addressed all of them. I'll do subsequent updates via merges.
constexpr endianness E = ELFT::Endianness; | ||
for (object::SectionRef Section : | ||
getSectionRefsByNameOrIndex(ObjF, Sections)) { | ||
StringRef SectionName = unwrapOrError(FileName, Section.getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Done.
W.getOStream() << '\n'; | ||
W.startLine() << "Contents of SFrame section '" << SectionName << "':\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not. It's a left over from when I was trying to match GNU output format. I've fixed it now.
Any thoughts on testing the json output? Repeating all of the expectations for json output is rather.. repetitive, and it doesn't really ensure the output is valid json. Is there any way to assert that the output parses as json?
.str(), | ||
Header.CFAFixedRAOffset.value()); | ||
|
||
W.printNumber(("CFA fixed FP offset" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good idea. I'm not sure how I ended up switching those.
@@ -62,6 +62,8 @@ def memtag : FF<"memtag", "Display memory tagging metadata (modes, Android notes | |||
def needed_libs : FF<"needed-libs", "Display the needed libraries">, Group<grp_elf>; | |||
def notes : FF<"notes", "Display notes">, Group<grp_elf>; | |||
def program_headers : FF<"program-headers", "Display program headers">, Group<grp_elf>; | |||
def sframe_EQ : Joined<["--"], "sframe=">, HelpText<"Display SFrame section <name>">, MetaVarName<"<name>">, Group<grp_elf>; | |||
def sframe: FF<"sframe", "Alias for --sframe=.sframe">, Alias<sframe_EQ>, AliasArgs<[".sframe"]>, Group<grp_elf>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would make sense, but it seems that the GNU version still uses (even at HEAD) the name based mechanism:
$ binutils/readelf /tmp/a2.o -e | grep sframe
[ 9] .sframe2 GNU_SFRAME 0000000000000000 000000f8
[10] .rela.sframe2 RELA 0000000000000000 000001c0
$ binutils/readelf /tmp/a2.o --sframe
readelf: Warning: Section '.sframe' was not dumped because it does not exist
@ibhagatgnu, any thoughts on this?
I'm also not sure how much we care about compatibility here -- I'd be fine with changing this even if gnu readelf doesn't do that.
Machine: EM_X86_64 | ||
Sections: | ||
- Name: .sframe_1b | ||
Type: SHT_PROGBITS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know. I've created #148803 for that.
For the moment, I'm keeping the PROGBITS here. I'll change that once the other PR lands (the occurrence in this test is not particularly important, it's the generator that matters).
# RUN: yaml2obj --docnum=1 %s -o %t.1 | ||
# RUN: llvm-readobj --sframe=.sframe_1b --sframe=.sframe_bad_magic \ | ||
# RUN: --sframe=.sframe_bad_version --sframe=.sframe_6b \ | ||
# RUN: --sframe=.sframe_header %t.1 2>&1 | FileCheck %s --check-prefix=CASE1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); | ||
|
||
constexpr uint16_t Magic = 0xdee2; | ||
|
||
enum class Version : uint8_t { | ||
V1 = 1, | ||
V2 = 2, | ||
#define HANDLE_SFRAME_VERSION(CODE, NAME) NAME = CODE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last part is the tricky one. I don't see a way to ensure a new enumerator isn't added without the corresponding stringifier without introducing more boilerplate. I guess I'm going to stick with the .def file.
No, judging by my experiments,
Yeah, I forgot to commit those changes. They're in now. |
No problem, thanks. Will respond inline to a couple of things now, but will do a proper review tomorrow (I've used up as much time as I have for LLVM reviewing today). |
This PR adds the SFrameParser class and uses it from llvm-readobj to
dump the section contents. Currently, it only supports parsing the
SFrame section header. Other parts of the section will be added in
follow-up patches.
llvm-readobj uses the same sframe flag syntax as GNU readelf, but I have
not attempted match the output format of the tool. I'm starting with the
"llvm" output format because it's easier to generate and lets us
tweak the format to make it useful for testing the generation code. If
needed, support for the GNU format could be added by overriding this
functionality in the GNU ELF Dumper.
For more information, see the sframe specification and the related RFC.