Skip to content

Reland "[ObjectYAML][ELF] Take alignment into account when generating notes" #118434

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 6 commits into from
Dec 5, 2024
Merged

Reland "[ObjectYAML][ELF] Take alignment into account when generating notes" #118434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
10ee40a
Reland "[ObjectYAML][ELF] Take alignment into account when generating…
igorkudrin Dec 3, 2024
ca33510
fixup: 'SHeader.sh_addralign' -> 'Section.AddressAlign'
igorkudrin Dec 3, 2024
5df8895
fixup: Add a test for 32-bit ELF
igorkudrin Dec 3, 2024
f2abc4e
fixup: use `-D ELFCLASS=` and remove the duplicate YAML document
igorkudrin Dec 4, 2024
cfc807d
fixup: add a test with "ShAddrAlign" and "AddressAlign" having differ…
igorkudrin Dec 5, 2024
16b29f5
fixup: add a test with "AddressAlign: 0"
igorkudrin Dec 5, 2024
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: 18 additions & 2 deletions llvm/lib/ObjectYAML/ELFEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1799,6 +1799,21 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
if (!Section.Notes)
return;

unsigned Align;
switch (Section.AddressAlign) {
case 0:
case 4:
Align = 4;
break;
case 8:
Align = 8;
break;
default:
reportError(Section.Name + ": invalid alignment for a note section: 0x" +
Twine::utohexstr(Section.AddressAlign));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Possibly not worth it, but I wonder if a test is in order to highlight which alignment is actually used (i.e. Section.AddressAlign, not SHeader.sh_addralign)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think a test can check this because Sec->AddressAlign is assigned to SHeader.sh_addralign in ELFState<ELFT>::initSectionHeaders() just a few lines before writeSectionContent() is called.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I posted this comment in the wrong place by accident, so it might be confusing things a little. I actually was referring to the switch a bit above, not specifically the error message. In other words, showing that if ShAddrAlign: 4 is specified but AddressAlign: 8 is specified, the 8 is used for padding purposes. In other words, catching the problem I pointed out earlier and that got changed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, overrideFields() is called after writeSectionContent(), so even if there are different values in ShAddrAlign and AddressAlign properties, writeSectionContent() will still see the same value in Section.AddressAlign and SHeader.sh_addralign, taken from AddressAlign. Nevertheless, I've added a new test for this scenario. It even shows that you can generate a note section with an invalid alignment without triggering an error, as long as an expected alignment is specified in the AddressAlign property.

return;
}

uint64_t Offset = CBA.tell();
for (const ELFYAML::NoteEntry &NE : *Section.Notes) {
// Write name size.
Expand All @@ -1820,14 +1835,15 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
if (!NE.Name.empty()) {
CBA.write(NE.Name.data(), NE.Name.size());
CBA.write('\0');
CBA.padToAlignment(4);
}

// Write description and padding.
if (NE.Desc.binary_size() != 0) {
CBA.padToAlignment(Align);
CBA.writeAsBinary(NE.Desc);
CBA.padToAlignment(4);
}

CBA.padToAlignment(Align);
}

SHeader.sh_size = CBA.tell() - Offset;
Expand Down
181 changes: 181 additions & 0 deletions llvm/test/tools/yaml2obj/ELF/note-section.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,184 @@ Sections:
- Name: ABC
Desc: '123456'
Type: NT_VERSION

## Check that an incorrect alignment is reported.

# RUN: not yaml2obj --docnum=16 %s 2>&1 | FileCheck %s --check-prefix=ERR_ALIGN1
# ERR_ALIGN1: error: .note.foo: invalid alignment for a note section: 0x1

--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Sections:
- Name: .note.foo
Type: SHT_NOTE
AddressAlign: 1
Notes:
- Type: 0x1

## Check that note entries and their "Desc" fields are aligned according to the
## specified section alignment.

# RUN: yaml2obj --docnum=17 -D ELFCLASS=64 %s -o - | \
# RUN: llvm-readobj --sections --section-data --notes - | \
# RUN: FileCheck %s --check-prefix=TEST17

# RUN: yaml2obj --docnum=17 -D ELFCLASS=32 %s -o - | \
# RUN: llvm-readobj --sections --section-data --notes - | \
# RUN: FileCheck %s --check-prefix=TEST17

# TEST17: Name: .note.foo4
# TEST17: SectionData (
# TEST17-NEXT: 0000: 05000000 02000000 01000000 41424344 |............ABCD|
# TEST17-NEXT: 0010: 00000000 01020000 00000000 03000000 |................|
# TEST17-NEXT: 0020: 02000000 03040500 04000000 00000000 |................|
# TEST17-NEXT: 0030: 03000000 474E5500 |....GNU.|
# TEST17-NEXT: )
# TEST17: Name: .note.foo8
# TEST17: SectionData (
# TEST17-NEXT: 0000: 05000000 02000000 01000000 41424344 |............ABCD|
# TEST17-NEXT: 0010: 00000000 00000000 01020000 00000000 |................|
# TEST17-NEXT: 0020: 00000000 03000000 02000000 00000000 |................|
# TEST17-NEXT: 0030: 03040500 00000000 04000000 00000000 |................|
# TEST17-NEXT: 0040: 03000000 474E5500 |....GNU.|
# TEST17-NEXT: )
# TEST17: NoteSections [
# TEST17-NEXT: NoteSection {
# TEST17-NEXT: Name: .note.foo4
# TEST17-NEXT: Offset:
# TEST17-NEXT: Size:
# TEST17-NEXT: Notes [
# TEST17-NEXT: {
# TEST17-NEXT: Owner: ABCD
# TEST17-NEXT: Data size: 0x2
# TEST17-NEXT: Type: NT_VERSION (version)
# TEST17-NEXT: Description data (
# TEST17-NEXT: 0000: 0102 |..|
# TEST17-NEXT: )
# TEST17-NEXT: }
# TEST17-NEXT: {
# TEST17-NEXT: Owner:
# TEST17-NEXT: Data size: 0x3
# TEST17-NEXT: Type: NT_ARCH (architecture)
# TEST17-NEXT: Description data (
# TEST17-NEXT: 0000: 030405 |...|
# TEST17-NEXT: )
# TEST17-NEXT: }
# TEST17-NEXT: {
# TEST17-NEXT: Owner: GNU
# TEST17-NEXT: Data size: 0x0
# TEST17-NEXT: Type: NT_GNU_BUILD_ID (unique build ID bitstring)
# TEST17-NEXT: Build ID:
# TEST17-NEXT: }
# TEST17-NEXT: ]
# TEST17-NEXT: }
# TEST17-NEXT: NoteSection {
# TEST17-NEXT: Name: .note.foo8
# TEST17-NEXT: Offset:
# TEST17-NEXT: Size:
# TEST17-NEXT: Notes [
# TEST17-NEXT: {
# TEST17-NEXT: Owner: ABCD
# TEST17-NEXT: Data size: 0x2
# TEST17-NEXT: Type: NT_VERSION (version)
# TEST17-NEXT: Description data (
# TEST17-NEXT: 0000: 0102 |..|
# TEST17-NEXT: )
# TEST17-NEXT: }
# TEST17-NEXT: {
# TEST17-NEXT: Owner:
# TEST17-NEXT: Data size: 0x3
# TEST17-NEXT: Type: NT_ARCH (architecture)
# TEST17-NEXT: Description data (
# TEST17-NEXT: 0000: 030405 |...|
# TEST17-NEXT: )
# TEST17-NEXT: }
# TEST17-NEXT: {
# TEST17-NEXT: Owner: GNU
# TEST17-NEXT: Data size: 0x0
# TEST17-NEXT: Type: NT_GNU_BUILD_ID (unique build ID bitstring)
# TEST17-NEXT: Build ID:
# TEST17-NEXT: }
# TEST17-NEXT: ]
# TEST17-NEXT: }
# TEST17-NEXT: ]

--- !ELF
FileHeader:
Class: ELFCLASS[[ELFCLASS]]
Data: ELFDATA2LSB
Type: ET_EXEC
Sections:
- Name: .note.foo4
Type: SHT_NOTE
AddressAlign: 4
Notes:
- Name: ABCD
Type: NT_VERSION
Desc: 0102
- Type: NT_ARCH
Desc: 030405
- Name: GNU
Type: NT_GNU_BUILD_ID
- Name: .note.foo8
Type: SHT_NOTE
AddressAlign: 8
Notes:
- Name: ABCD
Type: NT_VERSION
Desc: 0102
- Type: NT_ARCH
Desc: 030405
- Name: GNU
Type: NT_GNU_BUILD_ID

## Check that the alignment for note entries is taken from the "AddressAlign"
## field even if "ShAddrAlign" is also specified; an unexpected value in the
## "ShAddrAlign" property does not trigger an incorrect alignment error.

# RUN: yaml2obj --docnum=18 -D ADDRALIGN=0 -D SHADDRALIGN=8 %s -o - | \
# RUN: llvm-readobj --sections --section-data --notes - | \
# RUN: FileCheck %s --check-prefixes=TEST18,TEST18_4

# RUN: yaml2obj --docnum=18 -D ADDRALIGN=4 -D SHADDRALIGN=3 %s -o - | \
# RUN: llvm-readobj --sections --section-data --notes - | \
# RUN: FileCheck %s --check-prefixes=TEST18,TEST18_4

# RUN: yaml2obj --docnum=18 -D ADDRALIGN=8 -D SHADDRALIGN=4 %s -o - | \
# RUN: llvm-readobj --sections --section-data --notes - | \
# RUN: FileCheck %s --check-prefixes=TEST18,TEST18_8

# TEST18: Name: .note
# TEST18: SectionData (
# TEST18_4-NEXT: 0000: 05000000 02000000 01000000 41424344 |............ABCD|
# TEST18_4-NEXT: 0010: 00000000 01020000 00000000 03000000 |................|
# TEST18_4-NEXT: 0020: 02000000 03040500 04000000 00000000 |................|
# TEST18_4-NEXT: 0030: 03000000 474E5500 |....GNU.|
# TEST18_8-NEXT: 0000: 05000000 02000000 01000000 41424344 |............ABCD|
# TEST18_8-NEXT: 0010: 00000000 00000000 01020000 00000000 |................|
# TEST18_8-NEXT: 0020: 00000000 03000000 02000000 00000000 |................|
# TEST18_8-NEXT: 0030: 03040500 00000000 04000000 00000000 |................|
# TEST18_8-NEXT: 0040: 03000000 474E5500 |....GNU.|
# TEST18-NEXT: )

--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Sections:
- Name: .note
Type: SHT_NOTE
AddressAlign: [[ADDRALIGN]]
ShAddrAlign: [[SHADDRALIGN]]
Notes:
- Name: ABCD
Type: NT_VERSION
Desc: 0102
- Type: NT_ARCH
Desc: 030405
- Name: GNU
Type: NT_GNU_BUILD_ID
Loading