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

Changes from 1 commit
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
31 changes: 31 additions & 0 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 @@ -179,3 +179,34 @@ ProgramHeaders:
# RUN: FileCheck %s --check-prefix=INVALID-OFFSET

# 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 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 0000ff

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

--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Sections:
- Name: .foo
Type: SHT_PROGBITS
Flags: [ SHF_ALLOC ]
Size: 0x1
## Note: the real .foo offset is much less than 0xFFFFFFFF or
## 0xFFFFFF00, but no error is reported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are the values in this comment referring to? I'd expect there to be one value and it to match the ShOffset value.

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 just forgot to remove the comment.
The origin test seems to be used to show that yaml2obj use ShOffset instead of Offset to place segments.
Now the purpose of this test is to show the offset of segments is decided by Offset, and the output of yaml2obj with this yaml is different now.
So the comment here is unnecessary now.

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