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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 18, 2025
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
701efbe
[llvm][yaml2obj] Modify section header overriding timing
cabbaken Mar 12, 2025
f079240
Distinguish ShOffset and Offset in program-header-size-offset.yaml, r…
cabbaken Mar 17, 2025
9260c30
Modify malformed-pt-dynamic.test to fix program header offset
cabbaken Mar 17, 2025
db32881
Optimize code style
cabbaken Mar 17, 2025
2b7f229
Add the check for the order of Offset and ShOffset
Jul 1, 2025
f943ab1
Fix tests about the change of `ShOffset` overriding timing
Jul 3, 2025
7908868
Fix comment format of program-header.yaml
Jul 11, 2025
4d88fa5
Remove unrelated comment
cabbaken Sep 9, 2025
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
22 changes: 18 additions & 4 deletions llvm/lib/ObjectYAML/ELFEmitter.cpp
View file
Open in desktop
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<Elf_Shdr *, 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({&SHeader, *Sec});
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({&SHeader, *Sec});
}
}

template <class ELFT>
void ELFState<ELFT>::overrideSectionHeaders(std::vector<Elf_Shdr> &SHeaders) {
for (std::pair<Elf_Shdr *, ELFYAML::Section> &HeaderAndSec :
SectionHeadersOverrideHelper)
overrideFields<ELFT>(&HeaderAndSec.second, *HeaderAndSec.first);
}

template <class ELFT>
void ELFState<ELFT>::assignSectionAddress(Elf_Shdr &SHeader,
ELFYAML::Section *YAMLSec) {
Expand Down Expand Up @@ -2090,6 +2099,11 @@ 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. This needs to happen after program
// header layout happens, because otherwise the layout will use the new
// values.
State.overrideSectionHeaders(SHeaders);

bool ReachedLimit = CBA.getOffset() > MaxSize;
if (Error E = CBA.takeLimitError()) {
// We report a custom error message instead below.
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/tools/llvm-readobj/ELF/malformed-pt-dynamic.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ Sections:
Type: SHT_DYNAMIC
Address: 0x1000
Offset: 0x1000
ShOffset: [[OFFSET=<none>]]
Entries:
- Tag: DT_NULL
Value: 0
Expand All @@ -142,5 +141,6 @@ Sections:
ProgramHeaders:
- Type: PT_DYNAMIC
FileSize: [[FILESIZE=<none>]]
Offset: [[OFFSET=<none>]]
FirstSec: .dynamic
LastSec: .dynamic
22 changes: 12 additions & 10 deletions llvm/test/tools/yaml2obj/ELF/program-header-size-offset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,25 +44,25 @@ Sections:
- Name: .text
Type: SHT_PROGBITS
Size: 4
ShOffset: 0x1000
Offset: 0x1000
AddressAlign: 0x1000
- Name: .rodata
Type: SHT_PROGBITS
Size: 4
ShOffset: 0x2000
Offset: 0x2000
AddressAlign: 0x1000
- Name: .data
Type: SHT_PROGBITS
ShOffset: 0x2004
Offset: 0x2004
Size: 4
- Name: .nobits1
Type: SHT_NOBITS
ShOffset: 0x2008
Offset: 0x2008
Size: 1
- Name: .nobits2
Type: SHT_NOBITS
# Intentionally set to 0x2009 though the previous section is SHT_NOBITS.
ShOffset: 0x2009
Offset: 0x2009
Size: 1
ProgramHeaders:
# Program header with no sections.
Expand Down Expand Up @@ -180,15 +180,16 @@ ProgramHeaders:

# INVALID-OFFSET: yaml2obj: error: 'Offset' for segment with index 1 must be less than or equal to the minimum file offset of all included sections (0x78)

## Document that the "Offset" value is checked after the section offset is overriden using "ShOffset".
## Document that the "Offset" value should be checked before the section offset is overriden using "ShOffset".
## And the offset of the first section in a segment should not greater than the offset of the segment.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some comments:

  • We use the term "Document" in these yaml2obj tests to indicate that we aren't convinced the behaviour is what we actually want, but it is the current behaviour. In this case, I think the behaviour (with your changes) IS what you want, so I'd use "Show" instead.
  • I think the first part of this comment needs to explain what check is being performed.
  • In good English grammar, you don't normally start a sentence with "And". If your test has multiple points that it covers, I'd just make it all one sentence, i.e. "Show that the "Offset" value ... and that the offset of ..." or use the term "Also", i.e. "Also, show that the offset of ..."
  • You're missing a "be" between "should not" and "greater than" in the second sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am sorry for my poor English. I will fix the comment later...

# RUN: yaml2obj --docnum=4 %s -o %t5
# RUN: llvm-readelf %t5 --sections --program-headers | FileCheck %s --check-prefix=SHOFFSET

# SHOFFSET: [Nr] Name Type Address Off
# SHOFFSET: [ 1] .foo PROGBITS 0000000000000000 ffffffff
# SHOFFSET: [ 1] .foo PROGBITS 0000000000000000 0000ff

# SHOFFSET: Type Offset
# SHOFFSET-NEXT: LOAD 0xffffff00
# SHOFFSET-NEXT: LOAD 0x000078

--- !ELF
FileHeader:
Expand All @@ -202,9 +203,10 @@ Sections:
Size: 0x1
## Note: the real .foo offset is much less than 0xFFFFFFFF or
## 0xFFFFFF00, but no error is reported.
ShOffset: 0xFFFFFFFF
ShOffset: 0xFF
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 make this substantially bigger. 0xFF might well be within the bounds of the file, whereas something like 0xFFFFFF wouldn't be. You could also be explicit about the real section offset, by adding an Offset field, to show the difference, but that's not a strict requirement of the test.

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, you are right, I will fix it later.

ProgramHeaders:
- Type: PT_LOAD
Offset: 0xFFFFFF00
Offset: 0x78
FirstSec: .foo
LastSec: .foo

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: you've got too many blank lines at EOF. The file should end with exactly one "\n".

Loading