This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj] Allow printing of the watermark note section proposed in D66426
AbandonedPublic

Authored by chrisjackson on Nov 15 2019, 8:22 AM.

Details

Summary

This revision separates the changes to readobj that enable the watermark note section to be dumped. See D66426.

readobj can also compute the watermark hash. This computed value can then be checked against the value dumped from the proposed .note.llvm.watermark section.

Diff Detail

Event Timeline

chrisjackson created this revision.Nov 15 2019, 8:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2019, 8:22 AM
chrisjackson edited the summary of this revision. (Show Details)Nov 15 2019, 9:26 AM
aganea added a subscriber: aganea.Nov 21 2019, 5:27 AM

Added --compute-watermark, additional tests, use of library from update to D66426.

Removed library test accidentally added in last update.

chrisjackson edited the summary of this revision. (Show Details)Dec 5 2019, 11:31 AM

I've not looked at the code here yet. I'll do that once there's a consensus on D66426.

llvm/test/tools/llvm-readobj/elf-watermark.test
1

Assembly tests should have a '.s' extension. Also, since this is using X86_64 assembly, it needs a REQUIRES: x86. Finally, please rebase this change (there have been significant changes to the test directory structure recently).

6

LLD can't be used in an llvm-readobj test I'm afraid (it is not required to build check-llvm). You'll need a different way to setup this test somehow, or possibly this should be moved into the LLD testing if no sensible way can be come up with. One option might be to pre-calculate the watermark for a given set of content and use yaml2obj to generate the file with that watermark in it.

7

-check-prefix -> --check-prefix (also below)

10

Minor nit: add extra spacing to make the value being checked line up with the following lines.

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

This test's name is note-llvm.test, but the test is specifically about the watermark note?

3–4

This part feels like it's more generically "notes can be dumped after --strip-sections has been used". Why is the watermark here special?

6

General consensus is to use -o rather than shell redirection in new tests.

Also --docnum.

41

Nit: too many blank lines

43

Same as above.

76

Same as above.

The comment related to this section should be moved to here.

114–115

The spacing looks all off here. Either remove excessive spacing so that it all lines up, or add --strict-whitespace to show that you're testing the latter.

This revision will soon be abandoned along with D66426, unless there are any further comments.

chrisjackson abandoned this revision.Feb 19 2020, 1:39 AM