Page MenuHomePhabricator

[llvm-readobj] Add support for decoding FreeBSD ELF notes
Changes PlannedPublic

Authored by arichardson on Tue, Feb 11, 3:20 AM.

Details

Summary

The current support only printed coredump notes, but most binaries also
contain notes.

This change adds names for four FreeBSD-specific notes and pretty-prints
two of them:

  • NT_FREEBSD_ABI_TAG: This note holds a 32-bit (decimal) integer containing the value of the __FreeBSD_version macro, which is defined in crt1.o and will hold a value such as 1300076 for a binary build on a FreeBSD 13 system.
  • NT_FREEBSD_FEATURE_CTL: A 32-bit flag that indicates to the kernel that the binary wants certain bevahiour. Examples include setting NT_FREEBSD_FCTL_ASLR_DISABLE which tells the kernel to disable ASLR.

After this change llvm-readobj also no longer decodes coredump-only FreeBSD
notes in non-coredump files.

Depends on https://reviews.llvm.org/D74367

Diff Detail

Unit TestsFailed

TimeTest
10 msLLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; llvm-go test llvm.org/llvm/bindings/go/llvm
40 msLLVM.tools/llvm-readobj/ELF::note-freebsd.test
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/yaml2obj /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/llvm/test/tools/llvm-readobj/ELF/note-freebsd.test > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/tools/llvm-readobj/ELF/Output/note-freebsd.test.tmp.o

Event Timeline

arichardson created this revision.Tue, Feb 11, 3:20 AM
jhenderson added inline comments.Fri, Feb 14, 1:34 AM
llvm/test/tools/llvm-readobj/ELF/note-freebsd-core.test
1

binaries -> coredump(?)

2

Minor point, but we've started to tend to use -o %t.o instead of shell redirection in newer tests.

3–4

Two spurious lines?

10–13

Could you remove the excessive whitespace, please? I find it makes things more readable. I'd prefer it if all values line up, with the minimal amount of space possible, i.e. something like:

Class:   ELFCLASS64
Data:    ELFDATA2LSB
Type:    ET_CORE
Machine: EM_RISCV
19

Desc is optional - it emits an empty description if unspecified, so you can delete this line (and similar ones below).

20

Could you make a small change to yaml2obj to allow it to take the NT_* string as well as hex numbers, if it's practical? We already do that for most other enum-style fields (e.g. dynamic entries, section types etc), so it should be straightforward enough to copy what has been done before.

34

Is this just arbitrary data? If so, can you make it look more arbitrary by reducing the size and maybe make it some sort of pattern like "aabbccdd00"? If not, please add a comment showing what this should represent.

35

Is Type 3 one of the non-core ones, or just undefined? Please add a comment explaining why "3".

llvm/test/tools/llvm-readobj/ELF/note-freebsd.test
1

binaries -> binary

2

Same comment re. -o, whitespace in the YAML, using enum strings etc also apply in this test.

3

Spurious line?

16–17

You should be able to delete the flag and address values here. They aren't relevant to the test.

20

I think there should be a test case for a core note in this file, showing the behaviour for this case.

33

Delete extra blank line.

llvm/tools/llvm-readobj/ELFDumper.cpp
4874

Seems like we only test a small number of these, but we should test all of them. Could you extend the test accordingly (possibly as a separate change), please?

4887

Ditto - this seems to be untested.

5165

Nit: missing trailing full stop.

5171

I don't see any testing for this and the other corrupt tag code path?

5186–5188

A non-zero Feature flags test would be nice.

6542

Nit: missing trailing full stops for all these comments.

arichardson planned changes to this revision.Fri, Feb 14, 2:25 AM

Thanks for the detailed review! I'll update this patch early next week.
Many of the problems exist because I simply converted the existing assembly test to a YAML one instead of writing a new one.