Skip to content

[llvm][yaml2obj] Modify section header overriding timing #130942

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 4 commits into
base: main
Choose a base branch
from
Open
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
20 changes: 16 additions & 4 deletions llvm/lib/ObjectYAML/ELFEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ template <class ELFT> class ELFState {
NameToIdxMap DynSymN2I;
ELFYAML::Object &Doc;

std::vector<std::pair<unsigned, ELFYAML::Section>>
SectionHeadersOverrideHelper;

StringSet<> ExcludedSectionHeaders;

uint64_t LocationCounter = 0;
Expand All @@ -226,6 +229,7 @@ template <class ELFT> class ELFState {
StringRef SecName, ELFYAML::Section *YAMLSec);
void initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
ContiguousBlobAccumulator &CBA);
void overrideSectionHeaders(std::vector<Elf_Shdr> &SHeaders);
void initSymtabSectionHeader(Elf_Shdr &SHeader, SymtabType STType,
ContiguousBlobAccumulator &CBA,
ELFYAML::Section *YAMLSec);
Expand Down Expand Up @@ -845,7 +849,7 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
}

LocationCounter += SHeader.sh_size;
overrideFields<ELFT>(Sec, SHeader);
SectionHeadersOverrideHelper.push_back({SN2I.get(Sec->Name), *Sec});
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 struggling to follow why you can't just store Sec and SHeader in the vector (SHeader as a pointer, obviously)?

continue;
}

Expand Down Expand Up @@ -899,12 +903,17 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
}

LocationCounter += SHeader.sh_size;

// Override section fields if requested.
overrideFields<ELFT>(Sec, SHeader);
SectionHeadersOverrideHelper.push_back({SN2I.get(Sec->Name), *Sec});
}
}

template <class ELFT>
void ELFState<ELFT>::overrideSectionHeaders(std::vector<Elf_Shdr> &SHeaders) {
for (std::pair<unsigned, ELFYAML::Section> &IndexAndSec :
SectionHeadersOverrideHelper)
overrideFields<ELFT>(&IndexAndSec.second, SHeaders[IndexAndSec.first]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would just be simpler to update the overrideFields function to take the vector of pairs and iterate it over it inside the function, rather than have this additional function. If you want to rename it to overrideSectionHeaders for additional clarity, I'm not opposed to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

initImplicitHeader() will also use overrideFields(), so I defined a new function to call overrideFields().

}

template <class ELFT>
void ELFState<ELFT>::assignSectionAddress(Elf_Shdr &SHeader,
ELFYAML::Section *YAMLSec) {
Expand Down Expand Up @@ -2090,6 +2099,9 @@ bool ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc,
// Now we can decide segment offsets.
State.setProgramHeaderLayout(PHeaders, SHeaders);

// Override section fields if requested.
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
// Override section fields if requested.
// Override section fields, if requested. This needs to happen after program header layout happens, because otherwise the layout will use the new values.

(and reflow to column width rules of course)

State.overrideSectionHeaders(SHeaders);

bool ReachedLimit = CBA.getOffset() > MaxSize;
if (Error E = CBA.takeLimitError()) {
// We report a custom error message instead below.
Expand Down
Loading