Skip to content

[SHT_LLVM_FUNC_MAP][ObjectYaml]Introduce function address map section and emit dynamic instruction count(ObjectYaml part) #124332

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

wlei-llvm
Copy link
Contributor

@wlei-llvm wlei-llvm commented Jan 24, 2025

This stack of PRs introduces the function address map section(SHT_LLVM_FUNC_MAP) to store the function-level analysis data. The initial feature is dynamic instruction count, which is calculated as the sum of all instruction's sample count(extracted from PGO MBFI) within the function and serves as a metric for offline performance analysis. The section is designed to be extendable, allowing for future function-level analysis features to be added.

For more context, this is a follow-up to #119303, which implemented the feature as an extension to PGOAnalysisMap of SHT_LLVM_BB_ADDR_MAP. However, since it only requires function address and doesn't need the basic block info, we are switching to use the new extension for function-level data, which should be better for long-term maintainability.

PR stack:

(This one) #124332 [SHT_LLVM_FUNC_MAP][ObjectYaml]Introduce function address map section and emit dynamic instruction count(ObjectYaml part)
#124333 [SHT_LLVM_FUNC_MAP][llvm-readobj]Introduce function address map section and emit dynamic instruction count(readobj part)
#124334 [SHT_LLVM_FUNC_MAP][CodeGen]Introduce function address map section and emit dynamic instruction count(CodeGen part)

Test Plan: llvm/test/tools/obj2yaml/ELF/func-map.yaml

Created using spr 1.3.6-beta.1
@llvmbot
Copy link
Member

llvmbot commented Jan 24, 2025

@llvm/pr-subscribers-objectyaml

Author: Lei Wang (wlei-llvm)

Changes

Test Plan: llvm/test/tools/obj2yaml/ELF/func-map.yaml


Full diff: https://github.com/llvm/llvm-project/pull/124332.diff

9 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/ELF.h (+1)
  • (modified) llvm/include/llvm/Object/ELFTypes.h (+38)
  • (modified) llvm/include/llvm/ObjectYAML/ELFYAML.h (+25)
  • (modified) llvm/lib/Object/ELF.cpp (+1)
  • (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+30)
  • (modified) llvm/lib/ObjectYAML/ELFYAML.cpp (+22)
  • (added) llvm/test/tools/obj2yaml/ELF/func-map.yaml (+139)
  • (added) llvm/test/tools/yaml2obj/ELF/func-map.yaml (+116)
  • (modified) llvm/tools/obj2yaml/elf2yaml.cpp (+52)
diff --git a/llvm/include/llvm/BinaryFormat/ELF.h b/llvm/include/llvm/BinaryFormat/ELF.h
index 48ae0db80f43ee..46837c402d88b6 100644
--- a/llvm/include/llvm/BinaryFormat/ELF.h
+++ b/llvm/include/llvm/BinaryFormat/ELF.h
@@ -1139,6 +1139,7 @@ enum : unsigned {
   SHT_LLVM_OFFLOADING = 0x6fff4c0b,         // LLVM device offloading data.
   SHT_LLVM_LTO = 0x6fff4c0c,                // .llvm.lto for fat LTO.
   SHT_LLVM_JT_SIZES = 0x6fff4c0d,           // LLVM jump tables sizes.
+  SHT_LLVM_FUNC_MAP = 0x6fff4c0e,           // LLVM function address map.
   // Android's experimental support for SHT_RELR sections.
   // https://android.googlesource.com/platform/bionic/+/b7feec74547f84559a1467aca02708ff61346d2a/libc/include/elf.h#512
   SHT_ANDROID_RELR = 0x6fffff00,   // Relocation entries; only offsets.
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index 87e4dbe4480910..749a3635ebf14b 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -1027,6 +1027,44 @@ struct PGOAnalysisMap {
   }
 };
 
+// Struct representing the FuncMap for one function.
+struct FuncMap {
+
+  // Bitfield of optional features to control the extra information
+  // emitted/encoded in the the section.
+  struct Features {
+    bool DynamicInstCount : 1;
+
+    // Encodes to minimum bit width representation.
+    uint8_t encode() const {
+      return (static_cast<uint8_t>(DynamicInstCount) << 0);
+    }
+
+    // Decodes from minimum bit width representation and validates no
+    // unnecessary bits are used.
+    static Expected<Features> decode(uint8_t Val) {
+      Features Feat{static_cast<bool>(Val & (1 << 0))};
+      if (Feat.encode() != Val)
+        return createStringError(std::error_code(),
+                                 "invalid encoding for FuncMap::Features: 0x%x",
+                                 Val);
+      return Feat;
+    }
+
+    bool operator==(const Features &Other) const {
+      return DynamicInstCount == Other.DynamicInstCount;
+    }
+  };
+
+  uint64_t FunctionAddress = 0;  // Function entry address.
+  uint64_t DynamicInstCount = 0; // Dynamic instruction count for this function
+
+  // Flags to indicate if each feature was enabled in this function
+  Features FeatEnable;
+
+  uint64_t getFunctionAddress() const { return FunctionAddress; }
+};
+
 } // end namespace object.
 } // end namespace llvm.
 
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index dfdfa055d65fa6..9180135683b65c 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -195,6 +195,13 @@ struct PGOAnalysisMapEntry {
   std::optional<std::vector<PGOBBEntry>> PGOBBEntries;
 };
 
+struct FuncMapEntry {
+  uint8_t Version;
+  llvm::yaml::Hex8 Feature;
+  llvm::yaml::Hex64 Address;
+  llvm::yaml::Hex64 DynamicInstCount;
+};
+
 struct StackSizeEntry {
   llvm::yaml::Hex64 Address;
   llvm::yaml::Hex64 Size;
@@ -229,6 +236,7 @@ struct Chunk {
     DependentLibraries,
     CallGraphProfile,
     BBAddrMap,
+    FuncMap,
 
     // Special chunks.
     SpecialChunksStart,
@@ -355,6 +363,18 @@ struct BBAddrMapSection : Section {
   }
 };
 
+struct FuncMapSection : Section {
+  std::optional<std::vector<FuncMapEntry>> Entries;
+
+  FuncMapSection() : Section(ChunkKind::FuncMap) {}
+
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", Entries.has_value()}};
+  };
+
+  static bool classof(const Chunk *S) { return S->Kind == ChunkKind::FuncMap; }
+};
+
 struct StackSizesSection : Section {
   std::optional<std::vector<StackSizeEntry>> Entries;
 
@@ -762,6 +782,7 @@ bool shouldAllocateFileSpace(ArrayRef<ProgramHeader> Phdrs,
 } // end namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::StackSizeEntry)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::FuncMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBRangeEntry)
@@ -929,6 +950,10 @@ template <> struct MappingTraits<ELFYAML::StackSizeEntry> {
   static void mapping(IO &IO, ELFYAML::StackSizeEntry &Rel);
 };
 
+template <> struct MappingTraits<ELFYAML::FuncMapEntry> {
+  static void mapping(IO &IO, ELFYAML::FuncMapEntry &E);
+};
+
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry> {
   static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &E);
 };
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index b6d0699ee4fe08..41c3fb4cc5e406 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -321,6 +321,7 @@ StringRef llvm::object::getELFSectionTypeName(uint32_t Machine, unsigned Type) {
     STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_OFFLOADING);
     STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_LTO);
     STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_JT_SIZES)
+    STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_FUNC_MAP);
     STRINGIFY_ENUM_CASE(ELF, SHT_GNU_ATTRIBUTES);
     STRINGIFY_ENUM_CASE(ELF, SHT_GNU_HASH);
     STRINGIFY_ENUM_CASE(ELF, SHT_GNU_verdef);
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 9ae76a71ede5e0..7316003a90c141 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -287,6 +287,9 @@ template <class ELFT> class ELFState {
   void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::BBAddrMapSection &Section,
                            ContiguousBlobAccumulator &CBA);
+  void writeSectionContent(Elf_Shdr &SHeader,
+                           const ELFYAML::FuncMapSection &Section,
+                           ContiguousBlobAccumulator &CBA);
   void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::HashSection &Section,
                            ContiguousBlobAccumulator &CBA);
@@ -894,6 +897,8 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
       writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::BBAddrMapSection>(Sec)) {
       writeSectionContent(SHeader, *S, CBA);
+    } else if (auto S = dyn_cast<ELFYAML::FuncMapSection>(Sec)) {
+      writeSectionContent(SHeader, *S, CBA);
     } else {
       llvm_unreachable("Unknown section type");
     }
@@ -1537,6 +1542,31 @@ void ELFState<ELFT>::writeSectionContent(
   }
 }
 
+template <class ELFT>
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+                                         const ELFYAML::FuncMapSection &Section,
+                                         ContiguousBlobAccumulator &CBA) {
+  if (!Section.Entries)
+    return;
+
+  for (const auto &[Idx, E] : llvm::enumerate(*Section.Entries)) {
+    // Write version and feature values.
+    if (Section.Type == llvm::ELF::SHT_LLVM_FUNC_MAP) {
+      if (E.Version > 1)
+        WithColor::warning() << "unsupported SHT_LLVM_FUNC_MAP version: "
+                             << static_cast<int>(E.Version)
+                             << "; encoding using the most recent version";
+      CBA.write(E.Version);
+      CBA.write(E.Feature);
+      SHeader.sh_size += 2;
+    }
+    CBA.write<uintX_t>(E.Address, ELFT::Endianness);
+    SHeader.sh_size += sizeof(uintX_t);
+    if (E.DynamicInstCount)
+      SHeader.sh_size += CBA.writeULEB128(E.DynamicInstCount);
+  }
+}
+
 template <class ELFT>
 void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::LinkerOptionsSection &Section,
diff --git a/llvm/lib/ObjectYAML/ELFYAML.cpp b/llvm/lib/ObjectYAML/ELFYAML.cpp
index 539834fc8d4dbf..077fd3aed2d242 100644
--- a/llvm/lib/ObjectYAML/ELFYAML.cpp
+++ b/llvm/lib/ObjectYAML/ELFYAML.cpp
@@ -723,6 +723,7 @@ void ScalarEnumerationTraits<ELFYAML::ELF_SHT>::enumeration(
   ECase(SHT_LLVM_PART_PHDR);
   ECase(SHT_LLVM_BB_ADDR_MAP_V0);
   ECase(SHT_LLVM_BB_ADDR_MAP);
+  ECase(SHT_LLVM_FUNC_MAP);
   ECase(SHT_LLVM_OFFLOADING);
   ECase(SHT_LLVM_LTO);
   ECase(SHT_GNU_ATTRIBUTES);
@@ -1432,6 +1433,12 @@ static void sectionMapping(IO &IO, ELFYAML::BBAddrMapSection &Section) {
   IO.mapOptional("PGOAnalyses", Section.PGOAnalyses);
 }
 
+static void sectionMapping(IO &IO, ELFYAML::FuncMapSection &Section) {
+  commonSectionMapping(IO, Section);
+  IO.mapOptional("Content", Section.Content);
+  IO.mapOptional("Entries", Section.Entries);
+}
+
 static void sectionMapping(IO &IO, ELFYAML::StackSizesSection &Section) {
   commonSectionMapping(IO, Section);
   IO.mapOptional("Entries", Section.Entries);
@@ -1725,6 +1732,12 @@ void MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::mapping(
       Section.reset(new ELFYAML::BBAddrMapSection());
     sectionMapping(IO, *cast<ELFYAML::BBAddrMapSection>(Section.get()));
     break;
+  case ELF::SHT_LLVM_FUNC_MAP:
+    if (!IO.outputting())
+      Section.reset(new ELFYAML::FuncMapSection());
+    sectionMapping(IO, *cast<ELFYAML::FuncMapSection>(Section.get()));
+    break;
+
   default:
     if (!IO.outputting()) {
       StringRef Name;
@@ -1848,6 +1861,15 @@ void MappingTraits<ELFYAML::StackSizeEntry>::mapping(
   IO.mapRequired("Size", E.Size);
 }
 
+void MappingTraits<ELFYAML::FuncMapEntry>::mapping(IO &IO,
+                                                   ELFYAML::FuncMapEntry &E) {
+  assert(IO.getContext() && "The IO context is not initialized");
+  IO.mapRequired("Version", E.Version);
+  IO.mapOptional("Feature", E.Feature, Hex8(0));
+  IO.mapOptional("Address", E.Address, Hex64(0));
+  IO.mapOptional("DynInstCnt", E.DynamicInstCount, Hex64(0));
+}
+
 void MappingTraits<ELFYAML::BBAddrMapEntry>::mapping(
     IO &IO, ELFYAML::BBAddrMapEntry &E) {
   assert(IO.getContext() && "The IO context is not initialized");
diff --git a/llvm/test/tools/obj2yaml/ELF/func-map.yaml b/llvm/test/tools/obj2yaml/ELF/func-map.yaml
new file mode 100644
index 00000000000000..b21b637026f549
--- /dev/null
+++ b/llvm/test/tools/obj2yaml/ELF/func-map.yaml
@@ -0,0 +1,139 @@
+## Check how obj2yaml produces YAML .llvm_func_map descriptions.
+
+## Check that obj2yaml uses the "Entries" tag to describe an .llvm_func_map section.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: obj2yaml %t1 | FileCheck %s --check-prefix=VALID
+
+# VALID:      --- !ELF
+# VALID-NEXT: FileHeader:
+# VALID-NEXT:   Class: ELFCLASS64
+# VALID-NEXT:   Data:  ELFDATA2LSB
+# VALID-NEXT:   Type:  ET_EXEC
+# VALID-NEXT: Sections:
+# VALID-NEXT:   - Name: .llvm_func_map
+# VALID-NEXT:     Type: SHT_LLVM_FUNC_MAP
+# VALID-NEXT:     Entries:
+# VALID-NEXT:       - Version:         1
+# VALID-NEXT:         Feature:         0x1
+## The 'Address' field is omitted when it's zero.
+# VALID-NEXT:         DynInstCnt:      0x10
+# VALID-NEXT:       - Version:         1
+## The 'Feature' field is omitted when it's zero.
+# VALID-NEXT:         Address:         0x1
+# VALID-NEXT:       - Version:         1
+# VALID-NEXT:         Feature:         0x1
+# VALID-NEXT:         Address:         0xFFFFFFFFFFFFFFF1
+# VALID-NEXT:         DynInstCnt:      0xFFFFFFFFFFFFFFF2
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name:   .llvm_func_map
+    Type:   SHT_LLVM_FUNC_MAP
+    ShSize: [[SIZE=<none>]]
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0x0
+        DynInstCnt: 0x10
+      - Version: 1
+        Feature: 0x0
+        Address: 0x1
+      - Version: 1
+        Feature: 0x1
+        Address: 0xFFFFFFFFFFFFFFF1
+        DynInstCnt: 0xFFFFFFFFFFFFFFF2
+
+## Check obj2yaml can dump empty .llvm_func_map sections.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: obj2yaml %t2 | FileCheck %s --check-prefix=EMPTY
+
+# EMPTY:      --- !ELF
+# EMPTY-NEXT: FileHeader:
+# EMPTY-NEXT:   Class: ELFCLASS64
+# EMPTY-NEXT:   Data:  ELFDATA2LSB
+# EMPTY-NEXT:   Type:  ET_EXEC
+# EMPTY-NEXT: Sections:
+# EMPTY-NEXT:   - Name:    .llvm_func_map
+# EMPTY-NEXT:     Type:    SHT_LLVM_FUNC_MAP
+# EMPTY-NOT:      Content:
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name:    .llvm_func_map
+    Type:    SHT_LLVM_FUNC_MAP
+    Content: ""
+
+## Check obj2yaml can dump multiple .llvm_func_map sections.
+
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: obj2yaml %t3 | FileCheck %s --check-prefix=MULTI
+
+# MULTI:      --- !ELF
+# MULTI-NEXT: FileHeader:
+# MULTI-NEXT:   Class: ELFCLASS64
+# MULTI-NEXT:   Data:  ELFDATA2LSB
+# MULTI-NEXT:   Type:  ET_EXEC
+# MULTI-NEXT: Sections:
+# MULTI-NEXT:   - Name: .llvm_func_map
+# MULTI-NEXT:     Type: SHT_LLVM_FUNC_MAP
+# MULTI-NEXT:     Entries:
+# MULTI-NEXT:       - Version: 1
+# MULTI-NEXT:         Feature: 0x1
+# MULTI-NEXT:         Address: 0x2
+# MULTI-NEXT:         DynInstCnt: 0x3
+# MULTI-NEXT:   - Name: '.llvm_func_map (1)'
+# MULTI-NEXT:     Type: SHT_LLVM_FUNC_MAP
+# MULTI-NEXT:     Entries:
+# MULTI-NEXT:       - Version: 1
+# MULTI-NEXT:         Feature: 0x1
+# MULTI-NEXT:         Address: 0xA
+# MULTI-NEXT:         DynInstCnt: 0xB
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name: .llvm_func_map
+    Type: SHT_LLVM_FUNC_MAP
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0x2
+        DynInstCnt: 0x3
+  - Name: '.llvm_func_map (1)'
+    Type:  SHT_LLVM_FUNC_MAP
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0xA
+        DynInstCnt: 0xB
+
+## Check that obj2yaml uses the "Content" tag to describe an .llvm_func_map section
+## when it can't extract the entries, for example, when the section is truncated.
+
+# RUN: yaml2obj --docnum=1 -DSIZE=0x8 %s -o %t4
+# RUN: obj2yaml %t4 | FileCheck %s --check-prefixes=TRUNCATED,INVALID
+
+
+# INVALID:           --- !ELF
+# INVALID-NEXT:      FileHeader:
+# INVALID-NEXT:        Class: ELFCLASS64
+# INVALID-NEXT:        Data:  ELFDATA2LSB
+# INVALID-NEXT:        Type:  ET_EXEC
+# INVALID-NEXT:      Sections:
+# INVALID-NEXT:        - Name:    .llvm_func_map
+# INVALID-NEXT:          Type:    SHT_LLVM_FUNC_MAP
+# BADNUM-NEXT:           Content: {{([[:xdigit:]]+)}}{{$}}
+# TRUNCATED-NEXT:        Content: '{{([[:xdigit:]]{16})}}'{{$}}
diff --git a/llvm/test/tools/yaml2obj/ELF/func-map.yaml b/llvm/test/tools/yaml2obj/ELF/func-map.yaml
new file mode 100644
index 00000000000000..9fee4038b79312
--- /dev/null
+++ b/llvm/test/tools/yaml2obj/ELF/func-map.yaml
@@ -0,0 +1,116 @@
+## Check how yaml2obj produces .llvm_func_map sections.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj --sections --section-data %t1 | FileCheck %s
+
+## Case 1: Specify content.
+# CHECK:      Section {
+# CHECK:        Index: 1
+# CHECK-NEXT:   Name: .llvm_func_map (1)
+# CHECK-NEXT:   Type: SHT_LLVM_FUNC_MAP (0x6FFF4C0E)
+# CHECK-NEXT:   Flags [ (0x0)
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address: 0x0
+# CHECK-NEXT:   Offset: 0x40
+# CHECK-NEXT:   Size: 13
+# CHECK-NEXT:   Link: 0
+# CHECK-NEXT:   Info: 0
+# CHECK-NEXT:   AddressAlignment: 0
+# CHECK-NEXT:   EntrySize: 0
+# CHECK-NEXT:   SectionData (
+# CHECK-NEXT:     0000: 00000000 00000000 01010203 04
+# CHECK-NEXT:   )
+# CHECK-NEXT: }
+
+## Case 2: Empty.
+# CHECK:        Name: .llvm_func_map (1)
+# CHECK:        Size:
+# CHECK-SAME:   {{^ 0$}}
+
+## Case 3: Specify Size only.
+# CHECK:        Name: .llvm_func_map (1)
+# CHECK:        SectionData (
+# CHECK-NEXT:     0000: 00000000 00000000
+# CHECK-NEXT:   )
+
+# Case 4: Specify Entries.
+# CHECK:        Name: .llvm_func_map (1)
+# CHECK:        SectionData (
+# CHECK-NEXT:     0000: 01012222 02000000 000010
+# CHECK-NEXT:   )
+
+
+
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+
+## Test the following cases:
+
+## 1) We can produce an .llvm_func_map section from a description with section
+##    content.
+##  Specify Content.
+  - Name:    '.llvm_func_map (1)'
+    Type:    SHT_LLVM_FUNC_MAP
+    Content: "00000000000000000101020304"
+
+# 2) We can produce an empty .llvm_func_map section from a description
+#    with empty section content.
+  - Name: '.llvm_func_map (2)'
+    Type: SHT_LLVM_FUNC_MAP
+
+## 3) We can produce a zero .llvm_func_map section of a specific size when
+##    we specify the size only.
+  - Name: '.llvm_func_map (3)'
+    Type: SHT_LLVM_FUNC_MAP
+    Size: 8
+
+## 4) We can produce an .llvm_func_map section from a description with
+##    Entries.
+  - Name: '.llvm_func_map (4)'
+    Type: SHT_LLVM_FUNC_MAP
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0x22222
+        DynInstCnt: 0x10
+
+## Check we can't use Entries at the same time as either Content or Size.
+# RUN: not yaml2obj --docnum=2 -DCONTENT="00" %s 2>&1 | FileCheck %s --check-prefix=INVALID
+# RUN: not yaml2obj --docnum=2 -DSIZE="0" %s 2>&1 | FileCheck %s --check-prefix=INVALID
+
+# INVALID: error: "Entries" cannot be used with "Content" or "Size"
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+##  Specify Content and Size
+  - Name:    '.llvm_func_map'
+    Type:    SHT_LLVM_FUNC_MAP
+    Entries: []
+    Content: [[CONTENT=<none>]]
+    Size:    [[SIZE=<none>]]
+
+## Check that yaml2obj generates a warning when we use unsupported versions.
+# RUN: yaml2obj --docnum=3  %s 2>&1 | FileCheck %s --check-prefix=INVALID-VERSION
+
+# INVALID-VERSION: warning: unsupported SHT_LLVM_FUNC_MAP version: 2; encoding using the most recent version
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name: '.llvm_func_map'
+    Type: SHT_LLVM_FUNC_MAP
+    Entries:
+#  Specify unsupported version
+      - Version: 2
diff --git a/llvm/tools/obj2yaml/elf2yaml.cpp b/llvm/tools/obj2yaml/elf2yaml.cpp
index b1c8032ea21929..ad2eb50937c210 100644
--- a/llvm/tools/obj2yaml/elf2yaml.cpp
+++ b/llvm/tools/obj2yaml/elf2yaml.cpp
@@ -97,6 +97,7 @@ class ELFDumper {
   dumpStackSizesSection(const Elf_Shdr *Shdr);
   Expected<ELFYAML::BBAddrMapSection *>
   dumpBBAddrMapSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::FuncMapSection *> dumpFuncMapSection(const Elf_Shdr *Shdr);
   Expected<ELFYAML::RawContentSection *>
   dumpPlaceholderSection(const Elf_Shdr *Shdr);
 
@@ -629,6 +630,8 @@ ELFDumper<ELFT>::dumpSections() {
           [this](const Elf_Shdr *S) { return dumpCallGraphProfileSection(S); };
     case ELF::SHT_LLVM_BB_ADDR_MAP:
       return [this](const Elf_Shdr *S) { return dumpBBAddrMapSection(S); };
+    case ELF::SHT_LLVM_FUNC_MAP:
+      return [this](const Elf_Shdr *S) { return dumpFuncMapSection(S); };
     case ELF::SHT_STRTAB:
     case ELF::SHT_SYMTAB:
     case ELF::SHT_DYNSYM:
@@ -989,6 +992,55 @@ ELFDumper<ELFT>::dumpBBAddrMapSection(const Elf_Shdr *Shdr) {
   return S.release();
 }
 
+template <class ELFT>
+Expected<ELFYAML::FuncMapSection *>
+ELFDumper<ELFT>::dumpFuncMapSection(const Elf_Shdr *Shdr) {
+  auto S = std::make_unique<ELFYAML::FuncMapSection>();
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
+
+  auto ContentOrErr = Obj.getSectionContents(*Shdr);
+  if (!ContentOrErr)
+    return ContentOrErr.takeError();
+
+  ArrayRef<uint8_t> Content = *ContentOrErr;
+  if (Content.empty())
+    return S.release();
+
+  DataExtractor Data(Content, Obj.isLE(), ELFT::Is64Bits ? 8 : 4);
+
+  std::vector<ELFYAML::FuncMapEntry> Entries;
+  DataExtractor::Cursor Cur(0);
+  uint8_t Version = 0;
+  uint8_t Feature = 0;
+  uint64_t Address = 0;
+  while (Cur && Cur.tell() < Content.size()) {
+    if (Shdr->sh_type == ELF::SHT_LLVM_FUNC_MAP) {
+      Version = Data.getU8(Cur);
+      Feature = Data.getU8(Cur);
+    }
+    auto FeatureOrErr = llvm::object::FuncMap::Features::decode(Feature);
+    if (!FeatureOrErr)
+      return FeatureOrErr.takeError();
+
+    Address = Data.getAddress(Cur);
+
+    uint64_t DynamicInstCount =
+        FeatureOrErr->DynamicInstCount ? Data.getULEB128(Cur) : 0;
+    Entries.push_back({Version, Feature, Address, DynamicInstCount});
+  }
+
+  if (!Cur) {
+    // If the section cannot be decoded, we dump it as an array of bytes.
+    consumeError(Cur.takeError());
+    S->Content = yaml::BinaryRef(Content);
+  } else {
+    S->Entries = std::move(Entries);
+  }
+
+  return S.release();
+}
+
 template <class ELFT>
 Expected<ELFYAML::AddrsigSection *>
 ELFDumper<ELFT>::dumpAddrsigSection(const Elf_Shdr *Shdr) {

@llvmbot
Copy link
Member

llvmbot commented Jan 24, 2025

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

Author: Lei Wang (wlei-llvm)

Changes

Test Plan: llvm/test/tools/obj2yaml/ELF/func-map.yaml


Full diff: https://github.com/llvm/llvm-project/pull/124332.diff

9 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/ELF.h (+1)
  • (modified) llvm/include/llvm/Object/ELFTypes.h (+38)
  • (modified) llvm/include/llvm/ObjectYAML/ELFYAML.h (+25)
  • (modified) llvm/lib/Object/ELF.cpp (+1)
  • (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+30)
  • (modified) llvm/lib/ObjectYAML/ELFYAML.cpp (+22)
  • (added) llvm/test/tools/obj2yaml/ELF/func-map.yaml (+139)
  • (added) llvm/test/tools/yaml2obj/ELF/func-map.yaml (+116)
  • (modified) llvm/tools/obj2yaml/elf2yaml.cpp (+52)
diff --git a/llvm/include/llvm/BinaryFormat/ELF.h b/llvm/include/llvm/BinaryFormat/ELF.h
index 48ae0db80f43ee..46837c402d88b6 100644
--- a/llvm/include/llvm/BinaryFormat/ELF.h
+++ b/llvm/include/llvm/BinaryFormat/ELF.h
@@ -1139,6 +1139,7 @@ enum : unsigned {
   SHT_LLVM_OFFLOADING = 0x6fff4c0b,         // LLVM device offloading data.
   SHT_LLVM_LTO = 0x6fff4c0c,                // .llvm.lto for fat LTO.
   SHT_LLVM_JT_SIZES = 0x6fff4c0d,           // LLVM jump tables sizes.
+  SHT_LLVM_FUNC_MAP = 0x6fff4c0e,           // LLVM function address map.
   // Android's experimental support for SHT_RELR sections.
   // https://android.googlesource.com/platform/bionic/+/b7feec74547f84559a1467aca02708ff61346d2a/libc/include/elf.h#512
   SHT_ANDROID_RELR = 0x6fffff00,   // Relocation entries; only offsets.
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index 87e4dbe4480910..749a3635ebf14b 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -1027,6 +1027,44 @@ struct PGOAnalysisMap {
   }
 };
 
+// Struct representing the FuncMap for one function.
+struct FuncMap {
+
+  // Bitfield of optional features to control the extra information
+  // emitted/encoded in the the section.
+  struct Features {
+    bool DynamicInstCount : 1;
+
+    // Encodes to minimum bit width representation.
+    uint8_t encode() const {
+      return (static_cast<uint8_t>(DynamicInstCount) << 0);
+    }
+
+    // Decodes from minimum bit width representation and validates no
+    // unnecessary bits are used.
+    static Expected<Features> decode(uint8_t Val) {
+      Features Feat{static_cast<bool>(Val & (1 << 0))};
+      if (Feat.encode() != Val)
+        return createStringError(std::error_code(),
+                                 "invalid encoding for FuncMap::Features: 0x%x",
+                                 Val);
+      return Feat;
+    }
+
+    bool operator==(const Features &Other) const {
+      return DynamicInstCount == Other.DynamicInstCount;
+    }
+  };
+
+  uint64_t FunctionAddress = 0;  // Function entry address.
+  uint64_t DynamicInstCount = 0; // Dynamic instruction count for this function
+
+  // Flags to indicate if each feature was enabled in this function
+  Features FeatEnable;
+
+  uint64_t getFunctionAddress() const { return FunctionAddress; }
+};
+
 } // end namespace object.
 } // end namespace llvm.
 
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index dfdfa055d65fa6..9180135683b65c 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -195,6 +195,13 @@ struct PGOAnalysisMapEntry {
   std::optional<std::vector<PGOBBEntry>> PGOBBEntries;
 };
 
+struct FuncMapEntry {
+  uint8_t Version;
+  llvm::yaml::Hex8 Feature;
+  llvm::yaml::Hex64 Address;
+  llvm::yaml::Hex64 DynamicInstCount;
+};
+
 struct StackSizeEntry {
   llvm::yaml::Hex64 Address;
   llvm::yaml::Hex64 Size;
@@ -229,6 +236,7 @@ struct Chunk {
     DependentLibraries,
     CallGraphProfile,
     BBAddrMap,
+    FuncMap,
 
     // Special chunks.
     SpecialChunksStart,
@@ -355,6 +363,18 @@ struct BBAddrMapSection : Section {
   }
 };
 
+struct FuncMapSection : Section {
+  std::optional<std::vector<FuncMapEntry>> Entries;
+
+  FuncMapSection() : Section(ChunkKind::FuncMap) {}
+
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", Entries.has_value()}};
+  };
+
+  static bool classof(const Chunk *S) { return S->Kind == ChunkKind::FuncMap; }
+};
+
 struct StackSizesSection : Section {
   std::optional<std::vector<StackSizeEntry>> Entries;
 
@@ -762,6 +782,7 @@ bool shouldAllocateFileSpace(ArrayRef<ProgramHeader> Phdrs,
 } // end namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::StackSizeEntry)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::FuncMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBRangeEntry)
@@ -929,6 +950,10 @@ template <> struct MappingTraits<ELFYAML::StackSizeEntry> {
   static void mapping(IO &IO, ELFYAML::StackSizeEntry &Rel);
 };
 
+template <> struct MappingTraits<ELFYAML::FuncMapEntry> {
+  static void mapping(IO &IO, ELFYAML::FuncMapEntry &E);
+};
+
 template <> struct MappingTraits<ELFYAML::BBAddrMapEntry> {
   static void mapping(IO &IO, ELFYAML::BBAddrMapEntry &E);
 };
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index b6d0699ee4fe08..41c3fb4cc5e406 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -321,6 +321,7 @@ StringRef llvm::object::getELFSectionTypeName(uint32_t Machine, unsigned Type) {
     STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_OFFLOADING);
     STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_LTO);
     STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_JT_SIZES)
+    STRINGIFY_ENUM_CASE(ELF, SHT_LLVM_FUNC_MAP);
     STRINGIFY_ENUM_CASE(ELF, SHT_GNU_ATTRIBUTES);
     STRINGIFY_ENUM_CASE(ELF, SHT_GNU_HASH);
     STRINGIFY_ENUM_CASE(ELF, SHT_GNU_verdef);
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 9ae76a71ede5e0..7316003a90c141 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -287,6 +287,9 @@ template <class ELFT> class ELFState {
   void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::BBAddrMapSection &Section,
                            ContiguousBlobAccumulator &CBA);
+  void writeSectionContent(Elf_Shdr &SHeader,
+                           const ELFYAML::FuncMapSection &Section,
+                           ContiguousBlobAccumulator &CBA);
   void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::HashSection &Section,
                            ContiguousBlobAccumulator &CBA);
@@ -894,6 +897,8 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
       writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::BBAddrMapSection>(Sec)) {
       writeSectionContent(SHeader, *S, CBA);
+    } else if (auto S = dyn_cast<ELFYAML::FuncMapSection>(Sec)) {
+      writeSectionContent(SHeader, *S, CBA);
     } else {
       llvm_unreachable("Unknown section type");
     }
@@ -1537,6 +1542,31 @@ void ELFState<ELFT>::writeSectionContent(
   }
 }
 
+template <class ELFT>
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+                                         const ELFYAML::FuncMapSection &Section,
+                                         ContiguousBlobAccumulator &CBA) {
+  if (!Section.Entries)
+    return;
+
+  for (const auto &[Idx, E] : llvm::enumerate(*Section.Entries)) {
+    // Write version and feature values.
+    if (Section.Type == llvm::ELF::SHT_LLVM_FUNC_MAP) {
+      if (E.Version > 1)
+        WithColor::warning() << "unsupported SHT_LLVM_FUNC_MAP version: "
+                             << static_cast<int>(E.Version)
+                             << "; encoding using the most recent version";
+      CBA.write(E.Version);
+      CBA.write(E.Feature);
+      SHeader.sh_size += 2;
+    }
+    CBA.write<uintX_t>(E.Address, ELFT::Endianness);
+    SHeader.sh_size += sizeof(uintX_t);
+    if (E.DynamicInstCount)
+      SHeader.sh_size += CBA.writeULEB128(E.DynamicInstCount);
+  }
+}
+
 template <class ELFT>
 void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::LinkerOptionsSection &Section,
diff --git a/llvm/lib/ObjectYAML/ELFYAML.cpp b/llvm/lib/ObjectYAML/ELFYAML.cpp
index 539834fc8d4dbf..077fd3aed2d242 100644
--- a/llvm/lib/ObjectYAML/ELFYAML.cpp
+++ b/llvm/lib/ObjectYAML/ELFYAML.cpp
@@ -723,6 +723,7 @@ void ScalarEnumerationTraits<ELFYAML::ELF_SHT>::enumeration(
   ECase(SHT_LLVM_PART_PHDR);
   ECase(SHT_LLVM_BB_ADDR_MAP_V0);
   ECase(SHT_LLVM_BB_ADDR_MAP);
+  ECase(SHT_LLVM_FUNC_MAP);
   ECase(SHT_LLVM_OFFLOADING);
   ECase(SHT_LLVM_LTO);
   ECase(SHT_GNU_ATTRIBUTES);
@@ -1432,6 +1433,12 @@ static void sectionMapping(IO &IO, ELFYAML::BBAddrMapSection &Section) {
   IO.mapOptional("PGOAnalyses", Section.PGOAnalyses);
 }
 
+static void sectionMapping(IO &IO, ELFYAML::FuncMapSection &Section) {
+  commonSectionMapping(IO, Section);
+  IO.mapOptional("Content", Section.Content);
+  IO.mapOptional("Entries", Section.Entries);
+}
+
 static void sectionMapping(IO &IO, ELFYAML::StackSizesSection &Section) {
   commonSectionMapping(IO, Section);
   IO.mapOptional("Entries", Section.Entries);
@@ -1725,6 +1732,12 @@ void MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::mapping(
       Section.reset(new ELFYAML::BBAddrMapSection());
     sectionMapping(IO, *cast<ELFYAML::BBAddrMapSection>(Section.get()));
     break;
+  case ELF::SHT_LLVM_FUNC_MAP:
+    if (!IO.outputting())
+      Section.reset(new ELFYAML::FuncMapSection());
+    sectionMapping(IO, *cast<ELFYAML::FuncMapSection>(Section.get()));
+    break;
+
   default:
     if (!IO.outputting()) {
       StringRef Name;
@@ -1848,6 +1861,15 @@ void MappingTraits<ELFYAML::StackSizeEntry>::mapping(
   IO.mapRequired("Size", E.Size);
 }
 
+void MappingTraits<ELFYAML::FuncMapEntry>::mapping(IO &IO,
+                                                   ELFYAML::FuncMapEntry &E) {
+  assert(IO.getContext() && "The IO context is not initialized");
+  IO.mapRequired("Version", E.Version);
+  IO.mapOptional("Feature", E.Feature, Hex8(0));
+  IO.mapOptional("Address", E.Address, Hex64(0));
+  IO.mapOptional("DynInstCnt", E.DynamicInstCount, Hex64(0));
+}
+
 void MappingTraits<ELFYAML::BBAddrMapEntry>::mapping(
     IO &IO, ELFYAML::BBAddrMapEntry &E) {
   assert(IO.getContext() && "The IO context is not initialized");
diff --git a/llvm/test/tools/obj2yaml/ELF/func-map.yaml b/llvm/test/tools/obj2yaml/ELF/func-map.yaml
new file mode 100644
index 00000000000000..b21b637026f549
--- /dev/null
+++ b/llvm/test/tools/obj2yaml/ELF/func-map.yaml
@@ -0,0 +1,139 @@
+## Check how obj2yaml produces YAML .llvm_func_map descriptions.
+
+## Check that obj2yaml uses the "Entries" tag to describe an .llvm_func_map section.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: obj2yaml %t1 | FileCheck %s --check-prefix=VALID
+
+# VALID:      --- !ELF
+# VALID-NEXT: FileHeader:
+# VALID-NEXT:   Class: ELFCLASS64
+# VALID-NEXT:   Data:  ELFDATA2LSB
+# VALID-NEXT:   Type:  ET_EXEC
+# VALID-NEXT: Sections:
+# VALID-NEXT:   - Name: .llvm_func_map
+# VALID-NEXT:     Type: SHT_LLVM_FUNC_MAP
+# VALID-NEXT:     Entries:
+# VALID-NEXT:       - Version:         1
+# VALID-NEXT:         Feature:         0x1
+## The 'Address' field is omitted when it's zero.
+# VALID-NEXT:         DynInstCnt:      0x10
+# VALID-NEXT:       - Version:         1
+## The 'Feature' field is omitted when it's zero.
+# VALID-NEXT:         Address:         0x1
+# VALID-NEXT:       - Version:         1
+# VALID-NEXT:         Feature:         0x1
+# VALID-NEXT:         Address:         0xFFFFFFFFFFFFFFF1
+# VALID-NEXT:         DynInstCnt:      0xFFFFFFFFFFFFFFF2
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name:   .llvm_func_map
+    Type:   SHT_LLVM_FUNC_MAP
+    ShSize: [[SIZE=<none>]]
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0x0
+        DynInstCnt: 0x10
+      - Version: 1
+        Feature: 0x0
+        Address: 0x1
+      - Version: 1
+        Feature: 0x1
+        Address: 0xFFFFFFFFFFFFFFF1
+        DynInstCnt: 0xFFFFFFFFFFFFFFF2
+
+## Check obj2yaml can dump empty .llvm_func_map sections.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: obj2yaml %t2 | FileCheck %s --check-prefix=EMPTY
+
+# EMPTY:      --- !ELF
+# EMPTY-NEXT: FileHeader:
+# EMPTY-NEXT:   Class: ELFCLASS64
+# EMPTY-NEXT:   Data:  ELFDATA2LSB
+# EMPTY-NEXT:   Type:  ET_EXEC
+# EMPTY-NEXT: Sections:
+# EMPTY-NEXT:   - Name:    .llvm_func_map
+# EMPTY-NEXT:     Type:    SHT_LLVM_FUNC_MAP
+# EMPTY-NOT:      Content:
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name:    .llvm_func_map
+    Type:    SHT_LLVM_FUNC_MAP
+    Content: ""
+
+## Check obj2yaml can dump multiple .llvm_func_map sections.
+
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: obj2yaml %t3 | FileCheck %s --check-prefix=MULTI
+
+# MULTI:      --- !ELF
+# MULTI-NEXT: FileHeader:
+# MULTI-NEXT:   Class: ELFCLASS64
+# MULTI-NEXT:   Data:  ELFDATA2LSB
+# MULTI-NEXT:   Type:  ET_EXEC
+# MULTI-NEXT: Sections:
+# MULTI-NEXT:   - Name: .llvm_func_map
+# MULTI-NEXT:     Type: SHT_LLVM_FUNC_MAP
+# MULTI-NEXT:     Entries:
+# MULTI-NEXT:       - Version: 1
+# MULTI-NEXT:         Feature: 0x1
+# MULTI-NEXT:         Address: 0x2
+# MULTI-NEXT:         DynInstCnt: 0x3
+# MULTI-NEXT:   - Name: '.llvm_func_map (1)'
+# MULTI-NEXT:     Type: SHT_LLVM_FUNC_MAP
+# MULTI-NEXT:     Entries:
+# MULTI-NEXT:       - Version: 1
+# MULTI-NEXT:         Feature: 0x1
+# MULTI-NEXT:         Address: 0xA
+# MULTI-NEXT:         DynInstCnt: 0xB
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name: .llvm_func_map
+    Type: SHT_LLVM_FUNC_MAP
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0x2
+        DynInstCnt: 0x3
+  - Name: '.llvm_func_map (1)'
+    Type:  SHT_LLVM_FUNC_MAP
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0xA
+        DynInstCnt: 0xB
+
+## Check that obj2yaml uses the "Content" tag to describe an .llvm_func_map section
+## when it can't extract the entries, for example, when the section is truncated.
+
+# RUN: yaml2obj --docnum=1 -DSIZE=0x8 %s -o %t4
+# RUN: obj2yaml %t4 | FileCheck %s --check-prefixes=TRUNCATED,INVALID
+
+
+# INVALID:           --- !ELF
+# INVALID-NEXT:      FileHeader:
+# INVALID-NEXT:        Class: ELFCLASS64
+# INVALID-NEXT:        Data:  ELFDATA2LSB
+# INVALID-NEXT:        Type:  ET_EXEC
+# INVALID-NEXT:      Sections:
+# INVALID-NEXT:        - Name:    .llvm_func_map
+# INVALID-NEXT:          Type:    SHT_LLVM_FUNC_MAP
+# BADNUM-NEXT:           Content: {{([[:xdigit:]]+)}}{{$}}
+# TRUNCATED-NEXT:        Content: '{{([[:xdigit:]]{16})}}'{{$}}
diff --git a/llvm/test/tools/yaml2obj/ELF/func-map.yaml b/llvm/test/tools/yaml2obj/ELF/func-map.yaml
new file mode 100644
index 00000000000000..9fee4038b79312
--- /dev/null
+++ b/llvm/test/tools/yaml2obj/ELF/func-map.yaml
@@ -0,0 +1,116 @@
+## Check how yaml2obj produces .llvm_func_map sections.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj --sections --section-data %t1 | FileCheck %s
+
+## Case 1: Specify content.
+# CHECK:      Section {
+# CHECK:        Index: 1
+# CHECK-NEXT:   Name: .llvm_func_map (1)
+# CHECK-NEXT:   Type: SHT_LLVM_FUNC_MAP (0x6FFF4C0E)
+# CHECK-NEXT:   Flags [ (0x0)
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address: 0x0
+# CHECK-NEXT:   Offset: 0x40
+# CHECK-NEXT:   Size: 13
+# CHECK-NEXT:   Link: 0
+# CHECK-NEXT:   Info: 0
+# CHECK-NEXT:   AddressAlignment: 0
+# CHECK-NEXT:   EntrySize: 0
+# CHECK-NEXT:   SectionData (
+# CHECK-NEXT:     0000: 00000000 00000000 01010203 04
+# CHECK-NEXT:   )
+# CHECK-NEXT: }
+
+## Case 2: Empty.
+# CHECK:        Name: .llvm_func_map (1)
+# CHECK:        Size:
+# CHECK-SAME:   {{^ 0$}}
+
+## Case 3: Specify Size only.
+# CHECK:        Name: .llvm_func_map (1)
+# CHECK:        SectionData (
+# CHECK-NEXT:     0000: 00000000 00000000
+# CHECK-NEXT:   )
+
+# Case 4: Specify Entries.
+# CHECK:        Name: .llvm_func_map (1)
+# CHECK:        SectionData (
+# CHECK-NEXT:     0000: 01012222 02000000 000010
+# CHECK-NEXT:   )
+
+
+
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+
+## Test the following cases:
+
+## 1) We can produce an .llvm_func_map section from a description with section
+##    content.
+##  Specify Content.
+  - Name:    '.llvm_func_map (1)'
+    Type:    SHT_LLVM_FUNC_MAP
+    Content: "00000000000000000101020304"
+
+# 2) We can produce an empty .llvm_func_map section from a description
+#    with empty section content.
+  - Name: '.llvm_func_map (2)'
+    Type: SHT_LLVM_FUNC_MAP
+
+## 3) We can produce a zero .llvm_func_map section of a specific size when
+##    we specify the size only.
+  - Name: '.llvm_func_map (3)'
+    Type: SHT_LLVM_FUNC_MAP
+    Size: 8
+
+## 4) We can produce an .llvm_func_map section from a description with
+##    Entries.
+  - Name: '.llvm_func_map (4)'
+    Type: SHT_LLVM_FUNC_MAP
+    Entries:
+      - Version: 1
+        Feature: 0x1
+        Address: 0x22222
+        DynInstCnt: 0x10
+
+## Check we can't use Entries at the same time as either Content or Size.
+# RUN: not yaml2obj --docnum=2 -DCONTENT="00" %s 2>&1 | FileCheck %s --check-prefix=INVALID
+# RUN: not yaml2obj --docnum=2 -DSIZE="0" %s 2>&1 | FileCheck %s --check-prefix=INVALID
+
+# INVALID: error: "Entries" cannot be used with "Content" or "Size"
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+##  Specify Content and Size
+  - Name:    '.llvm_func_map'
+    Type:    SHT_LLVM_FUNC_MAP
+    Entries: []
+    Content: [[CONTENT=<none>]]
+    Size:    [[SIZE=<none>]]
+
+## Check that yaml2obj generates a warning when we use unsupported versions.
+# RUN: yaml2obj --docnum=3  %s 2>&1 | FileCheck %s --check-prefix=INVALID-VERSION
+
+# INVALID-VERSION: warning: unsupported SHT_LLVM_FUNC_MAP version: 2; encoding using the most recent version
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+Sections:
+  - Name: '.llvm_func_map'
+    Type: SHT_LLVM_FUNC_MAP
+    Entries:
+#  Specify unsupported version
+      - Version: 2
diff --git a/llvm/tools/obj2yaml/elf2yaml.cpp b/llvm/tools/obj2yaml/elf2yaml.cpp
index b1c8032ea21929..ad2eb50937c210 100644
--- a/llvm/tools/obj2yaml/elf2yaml.cpp
+++ b/llvm/tools/obj2yaml/elf2yaml.cpp
@@ -97,6 +97,7 @@ class ELFDumper {
   dumpStackSizesSection(const Elf_Shdr *Shdr);
   Expected<ELFYAML::BBAddrMapSection *>
   dumpBBAddrMapSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::FuncMapSection *> dumpFuncMapSection(const Elf_Shdr *Shdr);
   Expected<ELFYAML::RawContentSection *>
   dumpPlaceholderSection(const Elf_Shdr *Shdr);
 
@@ -629,6 +630,8 @@ ELFDumper<ELFT>::dumpSections() {
           [this](const Elf_Shdr *S) { return dumpCallGraphProfileSection(S); };
     case ELF::SHT_LLVM_BB_ADDR_MAP:
       return [this](const Elf_Shdr *S) { return dumpBBAddrMapSection(S); };
+    case ELF::SHT_LLVM_FUNC_MAP:
+      return [this](const Elf_Shdr *S) { return dumpFuncMapSection(S); };
     case ELF::SHT_STRTAB:
     case ELF::SHT_SYMTAB:
     case ELF::SHT_DYNSYM:
@@ -989,6 +992,55 @@ ELFDumper<ELFT>::dumpBBAddrMapSection(const Elf_Shdr *Shdr) {
   return S.release();
 }
 
+template <class ELFT>
+Expected<ELFYAML::FuncMapSection *>
+ELFDumper<ELFT>::dumpFuncMapSection(const Elf_Shdr *Shdr) {
+  auto S = std::make_unique<ELFYAML::FuncMapSection>();
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
+
+  auto ContentOrErr = Obj.getSectionContents(*Shdr);
+  if (!ContentOrErr)
+    return ContentOrErr.takeError();
+
+  ArrayRef<uint8_t> Content = *ContentOrErr;
+  if (Content.empty())
+    return S.release();
+
+  DataExtractor Data(Content, Obj.isLE(), ELFT::Is64Bits ? 8 : 4);
+
+  std::vector<ELFYAML::FuncMapEntry> Entries;
+  DataExtractor::Cursor Cur(0);
+  uint8_t Version = 0;
+  uint8_t Feature = 0;
+  uint64_t Address = 0;
+  while (Cur && Cur.tell() < Content.size()) {
+    if (Shdr->sh_type == ELF::SHT_LLVM_FUNC_MAP) {
+      Version = Data.getU8(Cur);
+      Feature = Data.getU8(Cur);
+    }
+    auto FeatureOrErr = llvm::object::FuncMap::Features::decode(Feature);
+    if (!FeatureOrErr)
+      return FeatureOrErr.takeError();
+
+    Address = Data.getAddress(Cur);
+
+    uint64_t DynamicInstCount =
+        FeatureOrErr->DynamicInstCount ? Data.getULEB128(Cur) : 0;
+    Entries.push_back({Version, Feature, Address, DynamicInstCount});
+  }
+
+  if (!Cur) {
+    // If the section cannot be decoded, we dump it as an array of bytes.
+    consumeError(Cur.takeError());
+    S->Content = yaml::BinaryRef(Content);
+  } else {
+    S->Entries = std::move(Entries);
+  }
+
+  return S.release();
+}
+
 template <class ELFT>
 Expected<ELFYAML::AddrsigSection *>
 ELFDumper<ELFT>::dumpAddrsigSection(const Elf_Shdr *Shdr) {

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've got one main comment (posted inline) about the need for the "Features".

Should this section format be documented somewhere?

};

uint64_t FunctionAddress = 0; // Function entry address.
uint64_t DynamicInstCount = 0; // Dynamic instruction count for this function
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: don't forget to add "." at the end of each comment. Applies here and below.

struct FuncMap {

// Bitfield of optional features to control the extra information
// emitted/encoded in the the section.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// emitted/encoded in the the section.
// emitted/encoded in the section.

uint64_t DynamicInstCount = 0; // Dynamic instruction count for this function

// Flags to indicate if each feature was enabled in this function
Features FeatEnable;
Copy link
Collaborator

Choose a reason for hiding this comment

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

EnabledFeatures or FeaturesEnabled seem better names to me.


// Bitfield of optional features to control the extra information
// emitted/encoded in the the section.
struct Features {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need this feature bitfield at all if there's only one feature and a version for the section? It feels redundant to me.

If the section needs extending e.g. to add more features, you'll presumably need to update the version number anyway, in which case you can add optional feature toggles as needed at that point.

Otherwise, this just feels like unnecessary complexity and wasted space in the encoding.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. Let's add this Features bitfield once it's really needed. We can keep everything simple for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I will remove the feature stuffs in those PRs.

uint8_t Version;
llvm::yaml::Hex8 Feature;
llvm::yaml::Hex64 Address;
llvm::yaml::Hex64 DynamicInstCount;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a plain integer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to std::optional<uint64_t>, thanks!

@@ -1139,6 +1139,7 @@ enum : unsigned {
SHT_LLVM_OFFLOADING = 0x6fff4c0b, // LLVM device offloading data.
SHT_LLVM_LTO = 0x6fff4c0c, // .llvm.lto for fat LTO.
SHT_LLVM_JT_SIZES = 0x6fff4c0d, // LLVM jump tables sizes.
SHT_LLVM_FUNC_MAP = 0x6fff4c0e, // LLVM function address map.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@wlei-llvm wlei-llvm left a comment

Choose a reason for hiding this comment

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

Thank you for all the reviews and suggestions!

Should this section format be documented somewhere?

Please also update https://llvm.org/docs/Extensions.html.

Ah, I added the doc in other PR(#124334), OK, I will move that into this PR.


// Bitfield of optional features to control the extra information
// emitted/encoded in the the section.
struct Features {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I will remove the feature stuffs in those PRs.

uint8_t Version;
llvm::yaml::Hex8 Feature;
llvm::yaml::Hex64 Address;
llvm::yaml::Hex64 DynamicInstCount;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to std::optional<uint64_t>, thanks!

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
@@ -535,6 +535,27 @@ Example of BBAddrMap with PGO data:
.uleb128 1000 # BB_3 basic block frequency (only when enabled)
.uleb128 0 # BB_3 successors count (only enabled with branch probabilities)

``SHT_LLVM_FUNC_MAP`` Section (function address map)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Pretty sure the underline is supposed to match the title length.

Comment on lines 540 to 541
This section stores the mapping from the binary address of function to its
related metadata features. It is used to emit function-level analysis data and
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This section stores the mapping from the binary address of function to its
related metadata features. It is used to emit function-level analysis data and
This section stores the mapping from the binary address of functions to their
related metadata features. It is used to emit function-level analysis data and

Comment on lines 556 to 557
.quad .Lfunc_begin1 # function address
.uleb128 1000 # dynamic instruction count
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably want 2+ functions in the example.

I'm not convinced we want to use ULEBs in this section. Using them means the section entries have variable width, which in turn means the only way of finding information for a specific function in the map is to read the whole map, rather than just the function addresses. Of course, it's a space versus speed trade-off, so it depends on how this section will likely work in the future.

can be enabled through ``--func-map`` option. The fields are encoded in the
following format:

#. A version number byte used for backward compatibility.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not convinced we want a version byte for every single entry. That feels wasteful when there could be thousands of functions. However, we also can't just have a single version byte at the start of the section, because then the section becomes ambiguous when concatenated together by the linker from two different objects.

Would a better idea be to have a header, consisting of a size (or count of function entries in this table) and a version number (for now), followed by all the function address/count entries? A section might consist of one or more of these header + address/count blocks, to allow for this concatenation. The end of a block (and therefore start of the next block) is identified via the size/count member of the header.

Another idea, if you adopt the header + body approach, is to split the entries into separate function list/data, i.e. you'd have something like the following:

funcaddr1
funcaddr2
funcaddr3
data-for-func1
data-for-func2
data-for-func3

This would be useful for reducing the amount of data that needs to be read to find out information for a specific function. However, it can only be used if the data is fixed size for all entries within a block (i.e. no ULEBs), because accessing the data requires finding the right function and then using its index in the function address list to jump to the right block of data.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jh7370 Does the header approach require adding custom merging support in the linker?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, it's the same approach as taken by many DWARF sections. Pull the common stuff into a table header and then put the data in the table body, with a size in the header to indicate how much data there is (an alternative approach would be some kind of end marker in the data, but that has issues under some conditions, depending on the values used).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds a great idea, I will give a try! Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. So we will get one section per module with multiple function entries and one header. Then the linker will simply concatenate these sections. So we may end up with multiple headers. Is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is my revised suggestion as an alternative. This may not be ideal, because it impedes size reductions through gc-sections/COMDAT deduplication etc as it leaves dead entries in the data.

Whether you adopt either of these approaches or stick with the original design really needs to be a decision that you as clients of the functionality make. Keep in mind that having more data will make it slower to read and write the data. Functionality like gc-sections can help improve this, at a cost about what the section format might look like.

@jh7370 Sorry for late reply and thank you for the detailed clarification, that was super helpful!
I've now done the single section approach you suggested, it does work to emit the good data!(IIUC, the first suggestion might rely on features that aren’t ready yet)
Then to understand the tradeoffs, I ran some experiments to compare the original design(duplicated version filed) vs the one single section design. I ran them on one of our top services(big size binary, contains 1M+ functions), I noticed one significant diff in finial binary's section size.

  • Original design: 25MB.
  • Single section design: 69MB.

It's 2~3X more size, which I think that's due to the dead entries(missing gc-sections). For other overheads, I think that's not a significant factor for our system. For build time, as for our major services, the build time could take 30mins+ time, the extra linking time for the section is too small to measure. And the disk/network overhead is fine for the small intermediate elf obj size increase. But for the finial binary size, given we could extend more data, that means for each additional data, it would cost 2 ~ 3X more(dead entry) size, which I feel could be a problem for long run. Given this, I'm leaning towards the original design. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No problem. It's important to get these things right! On the plus side, as long as the first part of an entry is the version byte, if we come up with a better format in the future, all we have to do is bump the version number and ensure the header or first entry has that version byte first again, i.e. the header approach (should you decide to switch in the future for whatever reason) is interchangeable with the version-per-entry approach, assuming you have the correct version byte, since the version-per-entry approach is effectively "header per entry" where the version field is the sole component of the header.

Another consideration: is it important to be able to traverse this structure quickly to find the appropriate entry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. It's important to get these things right! On the plus side, as long as the first part of an entry is the version byte, if we come up with a better format in the future, all we have to do is bump the version number and ensure the header or first entry has that version byte first again, i.e. the header approach (should you decide to switch in the future for whatever reason) is interchangeable with the version-per-entry approach, assuming you have the correct version byte, since the version-per-entry approach is effectively "header per entry" where the version field is the sole component of the header.

Got it!

Another consideration: is it important to be able to traverse this structure quickly to find the appropriate entry?

Is it related to your earlier comment about "using a fixed size instead of the ULEBs"? That sounds good. As we could extend more data in future, it should be beneficial to quickly look up the entry. I will change to use a fixed size so that we can skip parsing the unused data.

Copy link
Collaborator

Choose a reason for hiding this comment

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

With fixed-sized entries, you can then use a binary search algorithm to search the section for a specific address, assuming that the entries are in address order. I think this will be guaranteed by the SHF_LINK_ORDER flag. You might want to set the seciton's sh_entsize appropriately too.

Some related thoughts:

  1. If the function addresses are ordered, you can use a binary search algorithm to find the specific one you care about, without needing to read any extra data at all (just the ones that get picked in the search) but only if the entry sizes are fixed.
  2. I imagine that a future version of the structure might change the size of entries. In this case, you could end up with two objects with function maps of different versions and then different sh_entsize values. I've forgotten how linkers handle this. My hope is that in such a situation they set the sh_entsize to 0 for the combined section, but you'd need to check. A value of 0 would then mean that "fast traversal via binary search without reading the full structure first" isn't possible (in that case, you'd need to read each entry sequentially, although you could skip the data that isn't actually important after checking the version number to determine the size).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With fixed-sized entries, you can then use a binary search algorithm to search the section for a specific address, assuming that the entries are in address order. I think this will be guaranteed by the SHF_LINK_ORDER flag. You might want to set the seciton's sh_entsize appropriately too.

Some related thoughts:

  1. If the function addresses are ordered, you can use a binary search algorithm to find the specific one you care about, without needing to read any extra data at all (just the ones that get picked in the search) but only if the entry sizes are fixed.

Appreciate the suggestion! Updated to set the entry size.

  1. I imagine that a future version of the structure might change the size of entries. In this case, you could end up with two objects with function maps of different versions and then different sh_entsize values. I've forgotten how linkers handle this. My hope is that in such a situation they set the sh_entsize to 0 for the combined section, but you'd need to check. A value of 0 would then mean that "fast traversal via binary search without reading the full structure first" isn't possible (in that case, you'd need to read each entry sequentially, although you could skip the data that isn't actually important after checking the version number to determine the size).

I verified that on a local test, right, the linker set sh_entsize to 0 if the entry size is not fixed size(combine different entry sizes from two versions)

llvm::yaml::Hex64 Address;
llvm::yaml::Hex64 DynamicInstCount;
uint64_t DynamicInstCount;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I'm not mistaken, not using Hex64 means you can't use hex encoding for this value. I feel like that's a mistake, personally.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. This is just a count. So there is no point in using the hex encoding in Yaml.

@jh7370 jh7370 requested a review from MaskRay January 30, 2025 08:05
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
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 need to look through the tests some more, but I've run out of time for today.

@@ -1550,18 +1550,22 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
return;

for (const auto &[Idx, E] : llvm::enumerate(*Section.Entries)) {
unsigned Size = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The size setting code in this loop could be dramatically simplified for now, now that we're using fixed size entries. Outside the loop, you could just do something like:

SHeader.sh_entsize = 9 + sizeof(uintX_t);
SHeader.sh_size = Section.Entries->size() * SHeader.sh_entsize;

CBA.write(E.Version);
Size += 1;
}
CBA.write<uintX_t>(E.Address, ELFT::Endianness);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but shouldn't this be using the Elf_Addr type, since it's representing an address? They might amount to the same thing, but it conveys the meaning better. (NB: I haven't tested it, so this might not work as desired)

Suggested change
CBA.write<uintX_t>(E.Address, ELFT::Endianness);
CBA.write<ELFT::Elf_Addr>(E.Address, ELFT::Endianness);

}
CBA.write<uintX_t>(E.Address, ELFT::Endianness);
Size += sizeof(uintX_t);
CBA.write<uint64_t>(E.DynamicInstCount, ELFT::Endianness);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if the DynamicInstCount should be architecture-dependent type, so that on 32-bit architectures, it only takes up 4 bytes? There's probably an argument actually that it should always just be 4 bytes, since you'd be hard pressed to need more than what 4 bytes can represent in instruction counts.


## Check that obj2yaml uses the "Entries" tag to describe an .llvm_func_map section.

# RUN: yaml2obj --docnum=1 %s -o %t1
Copy link
Collaborator

Choose a reason for hiding this comment

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

You probably should have a 32-bit version of this test case too, since you're writing an address, which has a different size on 32-bit architectures.

# RUN: yaml2obj --docnum=1 -DSIZE=0x8 %s -o %t4
# RUN: obj2yaml %t4 | FileCheck %s --check-prefixes=TRUNCATED,INVALID


Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: double blank line

# INVALID-NEXT: - Name: .llvm_func_map
# INVALID-NEXT: Type: SHT_LLVM_FUNC_MAP
# INVALID-NEXT: EntSize: 0x11
# TRUNCATED-NEXT: Content: '{{([[:xdigit:]]{16})}}'{{$}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd be tempted to explicitly check the expected 8 bytes here, to show that it's not producing garbage here.

Comment on lines +42 to +45




Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a lot of blank lines here...

if (Shdr->sh_type == ELF::SHT_LLVM_FUNC_MAP) {
Version = Data.getU8(Cur);
if (Cur && Version > 1)
return createStringError(errc::invalid_argument,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test case?

uint8_t Version = 0;
uint64_t Address = 0;
while (Cur && Cur.tell() < Content.size()) {
if (Shdr->sh_type == ELF::SHT_LLVM_FUNC_MAP) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of this check? When could it be false if you're inside this function?

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.

4 participants