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
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions llvm/include/llvm/BinaryFormat/ELF.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.

// 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.
Expand Down
38 changes: 38 additions & 0 deletions llvm/include/llvm/Object/ELFTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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.

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.

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
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.


// 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.


uint64_t getFunctionAddress() const { return FunctionAddress; }
};

} // end namespace object.
} // end namespace llvm.

Expand Down
25 changes: 25 additions & 0 deletions llvm/include/llvm/ObjectYAML/ELFYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
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!

};

struct StackSizeEntry {
llvm::yaml::Hex64 Address;
llvm::yaml::Hex64 Size;
Expand Down Expand Up @@ -229,6 +236,7 @@ struct Chunk {
DependentLibraries,
CallGraphProfile,
BBAddrMap,
FuncMap,

// Special chunks.
SpecialChunksStart,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
};
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Object/ELF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 30 additions & 0 deletions llvm/lib/ObjectYAML/ELFEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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);
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);

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,
Expand Down
22 changes: 22 additions & 0 deletions llvm/lib/ObjectYAML/ELFYAML.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
139 changes: 139 additions & 0 deletions llvm/test/tools/obj2yaml/ELF/func-map.yaml
Original file line number Diff line number Diff line change
@@ -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
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: 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


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: --- !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})}}'{{$}}
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.

Loading
Loading