Page MenuHomePhabricator

[ELF] Support for PT_GNU_PROPERTY in program headers and tools
ClosedPublic

Authored by peter.smith on Dec 3 2019, 5:51 AM.

Details

Summary

The PT_GNU_PROPERTY is generated by a linker to describe the .note.gnu.property section. The Linux kernel uses this program header to locate the .note.gnu.property section. If there is a program property that requires the kernel to perform some action before loading and ELF file such as AArch64 BTI or intel CET then the program won't load unless the PT_GNU_PROPERTY program header is present. This patch adds the definition and support in llvm-readobj, llvm-objdump to print it out. Support is also added to obj2yaml so that we can write a test for it. This patch is a pre-requisite for adding support for PT_GNU_PROPERTY generation in LLD.

The definition of PT_GNU_PROPERTY can be found in https://github.com/hjl-tools/linux-abi/wiki/linux-abi-draft.pdf

Diff Detail

Event Timeline

peter.smith created this revision.Dec 3 2019, 5:51 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar added a comment.Dec 3 2019, 6:31 AM

The context is missing in the diff uploaded. A few comments from me are inlined.

llvm/test/tools/llvm-objdump/elf-pt-gnu-property.test
2

Seems most of the comments from elf-pt-gnu-property.test below can also be applied for this test case.

llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test
2

Missing a full stop.

5

It is probably a bit more common to use "GNU" and "LLVM" prefixes in llvm-readobj/llvm-readelf tests.

19

Please align properly (see test\tools\llvm-readobj\program-headers.test for example).

26

Please remove the excessive identations here and in other tests.
e.g:

Class:   ELFCLASS64
Data:    ELFDATA2LSB
Type:    ET_EXEC
Machine: EM_AARCH64
35

This needs a comment about what is exactly encoded here.

42

You do not need the last line with dots.

Thanks for the comments. I've updated the diff with context, apologies it has been some time since I last posted a review and I forgot. I've also applied the suggested changes to the test.

jhenderson added inline comments.Dec 3 2019, 7:59 AM
llvm/test/tools/llvm-objdump/elf-pt-gnu-property.test
4

We've started using yaml2obj %s -o %t to avoid an unnecessary shell redirection. Probably best here too.

8

Does this output match GNU objdump's by the way?

18

If I'm not mistaken, this section is completely irrelevant for the test. The only thing that matters is that there is a segment with type PT_GNU_PROPERTY.

31

The alignment is irrelevant to the interpretation of the program header type. Delete it to keep the test simpler.

llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test
4

-o

8

You can simplify this to just something like {{ }}GNU_PROPERTY{{ }}. The rest of the data is irrelevant. In fact, I wonder if we should just add an equivalent test to "elf-section-types.test" for program headers. You'll see in that test we do the bare minimum to create a section of each section type to show that llvm-readelf/llvm-readobj can display the various section types.

12–19

You don't need the rest of these fields.

28

See the other test: the section is irrelevant and can be deleted.

peter.smith marked 17 inline comments as done.

Thanks for the comments. I've paired the tests back to only print the header type. The GNU readelf string type matches, it looks like GNU objdump is missing the string, I've followed the same convention it uses for PT_GNU_RELRO, which is just RELRO.

llvm/test/tools/llvm-objdump/elf-pt-gnu-property.test
8

Looking at the binutils source, there doesn't seem to be an entry in objdump for PT_GNU_PROPERTY, although there is one for readelf. Going by the way PT_GNU_RELRO is printed out in objdump "RELRO", I think PROPERTY is closer to what I'd expect objdump to do.

In the description, we can briefly mention that PT_GNU_PROPERTY was introduced because old linkers just "concatenate all input .note.gnu.property sections into one output .note.gnu.property section without merging them, we may see one or more NT_GNU_PROPERTY_TYPE_0 notes in PT_NOTE segment, which are invalid.".

llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test
2

... as GNU_PROPERTY is not appropriate because the LLVM style dumps the segment type as PT_GNU_PROPERTY.

We probably don't even need a comment because the test is simple enough.

jhenderson accepted this revision.Dec 4 2019, 1:25 AM

LGTM, with @MaskRay's comment addressed (I don't mind if the comment is deleted, or kept, but it should at least be fixed).

On the LLD side, I'm assuming that LLD needs to emit it because the kernel in question doesn't parse the PT_NOTE segment for this section? It seems a bit superfluous otherwise...

This revision is now accepted and ready to land.Dec 4 2019, 1:25 AM

LGTM, with @MaskRay's comment addressed (I don't mind if the comment is deleted, or kept, but it should at least be fixed).

On the LLD side, I'm assuming that LLD needs to emit it because the kernel in question doesn't parse the PT_NOTE segment for this section? It seems a bit superfluous otherwise...

Thanks I'll remove the comments. Yes the linux kernel does not parse the PT_NOTE section. This is also true for intel CET, https://bugzilla.redhat.com/show_bug.cgi?id=1721606

This revision was automatically updated to reflect the committed changes.