[PATCH] D74367: [llvm-readelf] Print raw ELF note contents if we can't parse it

Fri Feb 5 00:58:34 PST 2021

jhenderson added inline comments.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-note-size.test:5-6
 # RUN: llvm-readobj --elf-output-style LLVM --notes %t | FileCheck %s --check-prefix=LLVM
+# GNU binutils does not print the raw description data when encountering an
+# invalid NT_GNU_ABI_TAG, but in LLVM style we print it.

----------------
`##` for comments.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/note-amd.s:29
 // GNU-EMPTY:
+// GNU-NEXT: Displaying notes found in: .note.invalid
+// GNU-NEXT:   Owner                Data size 	Description
----------------
Maybe `.note.unknown` rather than `invalid`? Invalid to me suggests a recognised type, but broken data.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s:90
+        Desc: '123456'
+      # TODO: https://bugs.llvm.org/show_bug.cgi?id=49034
+      # - Name: AMDGPU
----------------
Unless you're planning on fixing this immediately, it would be worth mentioning this test in the bug, or the TODO might get missed.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/note-amdgpu.test:106
     Content:     07000000E600000020000000414D44475055000082AE616D646873612E6B65726E656C73918AB92E67726F75705F7365676D656E745F66697865645F73697A6502B62E6B65726E6172675F7365676D656E745F616C69676E04B52E6B65726E6172675F7365676D656E745F73697A6501B82E6D61785F666C61745F776F726B67726F75705F73697A6508A52E6E616D65A3666F6FBB2E707269766174655F7365676D656E745F66697865645F73697A6503AB2E736770725F636F756E7406A72E73796D626F6CA3666F6FAB2E766770725F636F756E7407AF2E7761766566726F6E745F73697A6505AE616D646873612E76657273696F6E9201000000
+  - Name:        .note.bar
+    Type:        SHT_NOTE
----------------
As above: maybe .note.unknown for clarity?

================
Comment at: llvm/test/tools/llvm-readobj/ELF/note-generic.s:4

-// RUN: llvm-readobj --notes %t.o | FileCheck %s --check-prefix=LLVM
-// RUN: llvm-readelf --notes %t.o | FileCheck %s --check-prefix=GNU
+// RUN: x86_64-elf-readelf --notes %t.o
+// RUN: llvm-readelf --notes %t.o
----------------
This looks wrong?

Also, I'm not sure why this test is changing as part of this change?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74367/new/

https://reviews.llvm.org/D74367