Page MenuHomePhabricator

[llvm-readelf] Match GNU readelf more more closely when dumping notes
Needs ReviewPublic

Authored by arichardson on Tue, Feb 11, 2:03 AM.

Details

Summary

GNU readelf prints "Unknown note type: (0x00000003)", whereas we were
omitting the colon. Additonally for unknown notes GNU readelf prints
" Description data:" whereas we were using a lowercase d and missing
one space of indentation.

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

Event Timeline

arichardson created this revision.Tue, Feb 11, 2:03 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson added inline comments.Tue, Feb 11, 2:41 AM
llvm/tools/llvm-readobj/ELFDumper.cpp
4582–4585

Would you mind refactoring this block into a separate function, to avoid all the code duplication? Perhaps called "getUnknownNoteTypeName" or something like that.

rupprecht added inline comments.Tue, Feb 11, 2:20 PM
llvm/tools/llvm-readobj/ELFDumper.cpp
5054

I'm not able to reproduce this part. Using the example binary generated by llvm/test/tools/llvm-readobj/ELF/note-freebsd.s:

$ readelf --version
GNU readelf (GNU Binutils) 2.34.50.20200211
...
$ readelf -n /tmp/freebsd.o

Displaying notes found in: .note.foo
  Owner                Data size        Description
  FreeBSD              0x00000000       NT_THRMISC (thrmisc structure)
  FreeBSD              0x00000000       NT_PROCSTAT_PROC (proc data)

Displaying notes found in: .note.bar
  Owner                Data size        Description
  FreeBSD              0x00000000       NT_PROCSTAT_FILES (files data)

Displaying notes found in: .note.baz
  Owner                Data size        Description
  FreeBSD              0x0000001c       Unknown note type: (0x00000003)
   description data: 4c 6f 72 65 6d 20 69 70 73 75 6d 20 64 6f 6c 6f 72 20 73 69 74 20 61 6d 65 74 00 00

Are you comparing against readelf sources that have a local patch to capitalize description?

arichardson marked an inline comment as done.Tue, Feb 11, 3:07 PM
arichardson added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
5054

Interesting, I am using GNU readelf (GNU Binutils) 2.32.51. Will check if it's changed upstream since.

arichardson marked an inline comment as done.Tue, Feb 11, 3:13 PM
arichardson added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
5054

Looking at the binutils sources, it seems like they use the upper-case variant (with four spaces) for GNU notes, but the lowercase one (with three spaces) for unknown ones.

I can revert this part of the change to remain exactly compatible with GNU readelf?

MaskRay added inline comments.Tue, Feb 11, 4:35 PM
llvm/tools/llvm-readobj/ELFDumper.cpp
5054

If you have a suggestion for GNU readelf output, it is recommended sending an email to https://sourceware.org/ml/binutils/2020-02/ to get some feedback...

jhenderson added inline comments.Wed, Feb 12, 1:40 AM
llvm/tools/llvm-readobj/ELFDumper.cpp
5054

+1 to feeding back to GNU on this one. It seems like a bug to me that their output is inconsistent, and I don't think we should try to match it in that case.

rupprecht added inline comments.Wed, Feb 12, 8:33 AM
llvm/tools/llvm-readobj/ELFDumper.cpp
5054

I can revert this part of the change to remain exactly compatible with GNU readelf?

I'm 100% ok with this change if this part is reverted (also w/ James' suggestion about refactoring that one method).

I'm actually wondering if we should accept a difference here too -- this feels like it falls in the bucket of the GNU tool having a bug that we don't want to emulate. OTOH, I'm *sure* based on experience there is some tool out there parsing readelf | grep 'description data' that would be broken...

nhaehnle removed a subscriber: nhaehnle.Sun, Feb 16, 9:20 AM