Page MenuHomePhabricator

[llvm-readelf] Print raw ELF note contents if we can't parse it
Needs ReviewPublic

Authored by arichardson on Mon, Feb 10, 3:55 PM.

Details

Summary

Currently, if the note name is known, but the value isn't we don't print
the contents.

Diff Detail

Unit TestsFailed

TimeTest
20 msLLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; llvm-go test llvm.org/llvm/bindings/go/llvm

Event Timeline

arichardson created this revision.Mon, Feb 10, 3:55 PM
Herald added a project: Restricted Project. · View Herald Transcript
MaskRay added a comment.EditedMon, Feb 10, 4:01 PM

Since this extends llvm-readelf output, can you also send an email to binutils@sourceware.org and check what output they want for GNU readelf? For llvm-readobj output, we are not bound to GNU decisions. Though, if the GNU output is not bad, aiming for consistency can usually make our implementation simpler.

arichardson retitled this revision from [UPSTREAM][llvm-readelf] Print raw ELF note contents if we can't parse it to [llvm-readelf] Print raw ELF note contents if we can't parse it.Mon, Feb 10, 4:06 PM

Is there a test case that shows we don't print the raw data when the decoding is known?

arichardson marked an inline comment as done.Tue, Feb 11, 2:41 AM

Is there a test case that shows we don't print the raw data when the decoding is known?

Not yet, but I've added a test case that would previously not print the contents for a GNU note of an unknown type.

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

It seems like GNU readelf does not print the contents in this case.

Add a test showing the change in behaviour for GNU notes with unknown types

I assume that you've verified against GNU readelf all the behaviour that you're changing here?

llvm/test/tools/llvm-readobj/ELF/gnu-note-size.test
1

It's unclear to me what this test is testing, so I can't really say whether this change is good. Would you mind adding a comment at the top of the test, either in this patch or a different one, briefly explaining its purpose, please? I imagine you might need to dig into the old commit messages for this file to find out.

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

Nit: missing trailing full stop.