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

[arichardson]

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.

[jhenderson]

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]

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]

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]

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]

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]

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]

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...

[arichardson]

I don't currently have time to work on this. Hopefully later this year.

lld/test/ELF/partition-notes.s

 // RUN: llvm-readelf --all %t1 | FileCheck --check-prefixes=CHECK,PART1 %s
 
 // CHECK: Program Headers:
 // CHECK: NOTE 0x{{0*}}[[NOTE_OFFSET:[^ ]*]]
 
 // CHECK: Displaying notes found at file offset 0x{{0*}}[[NOTE_OFFSET]]
 // CHECK-NEXT: Owner
 // CHECK-NEXT: foo                   0x00000004 NT_VERSION (version)
-// CHECK-NEXT:  description data: 62 61 72 00
+// CHECK-NEXT:   Description data: 62 61 72 00
 // CHECK-NEXT: Displaying notes
 // CHECK-NEXT: Owner
 // CHECK-NEXT: GNU                   0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring)
 // CHECK-NEXT: Build ID: 08b93eab87177a2356d1b0d1148339463f98dac2
 
 .section .llvm_sympart,"",@llvm_sympart
 .asciz "part1"
 .quad p1

llvm/test/CodeGen/AMDGPU/elf-notes.ll

 ; OSABI-UNK-ELF-NOT: Unknown note type
 
 ; OSABI-HSA: .hsa_code_object_version
 ; OSABI-HSA: .hsa_code_object_isa
 ; OSABI-HSA: .amd_amdgpu_isa "amdgcn-amd-amdhsa--gfx802"
 ; OSABI-HSA: .amd_amdgpu_hsa_metadata
 ; OSABI-HSA-NOT: .amd_amdgpu_pal_metadata
 
-; OSABI-HSA-ELF: Unknown note type (0x00000001)
-; OSABI-HSA-ELF: Unknown note type (0x00000003)
+; OSABI-HSA-ELF: Unknown note type: (0x00000001)
+; OSABI-HSA-ELF: Unknown note type: (0x00000003)
 ; OSABI-HSA-ELF: NT_AMD_AMDGPU_ISA (ISA Version)
 ; OSABI-HSA-ELF: ISA Version:
 ; OSABI-HSA-ELF: amdgcn-amd-amdhsa--gfx802
 ; OSABI-HSA-ELF: NT_AMD_AMDGPU_HSA_METADATA (HSA Metadata)
 ; OSABI-HSA-ELF: HSA Metadata:
 ; OSABI-HSA-ELF: ---
 ; OSABI-HSA-ELF: Version: [ 1, 0 ]
 ; OSABI-HSA-ELF: Kernels:

 ; OSABI-HSA-ELF-NOT: NT_AMD_AMDGPU_PAL_METADATA (PAL Metadata)
 
 ; OSABI-PAL-NOT: .hsa_code_object_version
 ; OSABI-PAL: .hsa_code_object_isa
 ; OSABI-PAL: .amd_amdgpu_isa "amdgcn-amd-amdpal--gfx802"
 ; OSABI-PAL-NOT: .amd_amdgpu_hsa_metadata
 ; OSABI-PAL: .amd_amdgpu_pal_metadata
 
-; OSABI-PAL-ELF: Unknown note type (0x00000003)
+; OSABI-PAL-ELF: Unknown note type: (0x00000003)
 ; OSABI-PAL-ELF: NT_AMD_AMDGPU_ISA (ISA Version)
 ; OSABI-PAL-ELF: ISA Version:
 ; OSABI-PAL-ELF: amdgcn-amd-amdpal--gfx802
 ; OSABI-PAL-ELF-NOT: NT_AMD_AMDGPU_HSA_METADATA (HSA Metadata)
 ; OSABI-PAL-ELF: NT_AMD_AMDGPU_PAL_METADATA (PAL Metadata)
 ; TODO: readobj can no longer dump PAL metadata pending resolution of D52821
 ; OSABI-PAL-ELF-XXX: PAL Metadata:
 ; TODO: Following check line fails on mips:

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

 // REQUIRES: x86-registered-target
 // RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t.o
 
 // RUN: llvm-readobj --notes %t.o | FileCheck %s --check-prefix=LLVM
 // RUN: llvm-readelf --notes %t.o | FileCheck %s --check-prefix=GNU
 
 // GNU:      Displaying notes found
 // GNU-NEXT:   Owner                Data size        Description
 // GNU-NEXT:   FreeBSD              0x00000000       NT_THRMISC (thrmisc structure)
 // GNU-NEXT:   FreeBSD              0x00000000       NT_PROCSTAT_PROC (proc data)
 // GNU-NEXT: Displaying notes found
 // GNU-NEXT:   Owner                Data size        Description
 // GNU-NEXT:   FreeBSD              0x00000000       NT_PROCSTAT_FILES (files data)
 // GNU-NEXT: Displaying notes found
 // GNU-NEXT:   Owner                Data size        Description
-// GNU-NEXT:   FreeBSD              0x0000001c       Unknown note type (0x00000003)
-// GNU-NEXT:    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
+// GNU-NEXT:   FreeBSD              0x0000001c       Unknown note type: (0x00000003)
+// GNU-NEXT:     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
 
 // LLVM:      Notes [
 // LLVM-NEXT:   NoteSection {
 // LLVM-NEXT:     Offset:
 // LLVM-NEXT:     Size:
 // LLVM-NEXT:     Note {
 // LLVM-NEXT:       Owner: FreeBSD
 // LLVM-NEXT:       Data size: 0x0

 // LLVM-NEXT:     }
 // LLVM-NEXT:   }
 // LLVM-NEXT:   NoteSection {
 // LLVM-NEXT:     Offset: 0x7C
 // LLVM-NEXT:     Size: 0x30
 // LLVM-NEXT:     Note {
 // LLVM-NEXT:       Owner: FreeBSD
 // LLVM-NEXT:       Data size: 0x1C
-// LLVM-NEXT:       Type: Unknown note type (0x00000003)
+// LLVM-NEXT:       Type: Unknown note type: (0x00000003)
 // LLVM-NEXT:       Description data (
 // LLVM-NEXT:         0000: 4C6F7265 6D206970 73756D20 646F6C6F  |Lorem ipsum dolo|
 // LLVM-NEXT:         0010: 72207369 7420616D 65740000           |r sit amet..|
 // LLVM-NEXT:       )
 // LLVM-NEXT:     }
 // LLVM-NEXT:   }
 // LLVM-NEXT: ]
 

llvm/test/tools/llvm-readobj/ELF/note-unknown.s

 // REQUIRES: x86-registered-target
 // RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t.o
 
-// RUN: llvm-readobj --notes %t.o | FileCheck %s --check-prefix=LLVM
-// RUN: llvm-readelf --notes %t.o | FileCheck %s --check-prefix=GNU
+// RUN: llvm-readobj --notes %t.o | FileCheck %s --check-prefix=LLVM  --strict-whitespace
+// RUN: llvm-readelf --notes %t.o | FileCheck %s --check-prefix=GNU --strict-whitespace
 
 // GNU:      Displaying notes found at file offset 0x00000040 with length 0x00000010:
-// GNU-NEXT:   Owner                 Data size       Description
-// GNU-NEXT:   XYZ                  0x00000000       Unknown note type: (0x00000003)
-// GNU-NEXT: Displaying notes found at file offset 0x00000050 with length 0x0000002c:
-// GNU-NEXT:   Owner                 Data size       Description
-// GNU-NEXT:   XYZ                  0x0000001c       Unknown note type: (0x00000003)
-// GNU-NEXT:     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
+// GNU-NEXT:  Owner                Data size  Description
+// GNU-NEXT:  XYZ                  0x00000000 Unknown note type: (0x00000003)
+// GNU-NEXT:Displaying notes found at file offset 0x00000050 with length 0x0000002c:
+// GNU-NEXT:  Owner                Data size  Description
+// GNU-NEXT:  XYZ                  0x0000001c Unknown note type: (0x00000003)
+// GNU-NEXT:    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
 
 // LLVM:      Notes [
 // LLVM-NEXT:   NoteSection {
 // LLVM-NEXT:     Offset: 0x40
 // LLVM-NEXT:     Size: 0x10
 // LLVM-NEXT:     Note {
 // LLVM-NEXT:       Owner: XYZ
 // LLVM-NEXT:       Data size: 0x0

llvm/tools/llvm-readobj/ELFDumper.cpp

const char *Name;
       {ELF::NT_GNU_GOLD_VERSION, "NT_GNU_GOLD_VERSION (gold version)"},
       {ELF::NT_GNU_PROPERTY_TYPE_0, "NT_GNU_PROPERTY_TYPE_0 (property note)"},
   };
 
   for (const auto &Note : Notes)
     if (Note.ID == NT)
       return std::string(Note.Name);
 
   std::string string;
   raw_string_ostream OS(string);
-  OS << format("Unknown note type (0x%08x)", NT);
+  OS << format("Unknown note type: (0x%08x)", NT);
   return OS.str();
 }
 
 static std::string getFreeBSDNoteTypeName(const uint32_t NT) {
   static const struct {
     uint32_t ID;
     const char *Name;
   } Notes[] = {
       {ELF::NT_FREEBSD_THRMISC, "NT_THRMISC (thrmisc structure)"},
static std::string getFreeBSDNoteTypeName(const uint32_t NT) {
   };
 
   for (const auto &Note : Notes)
     if (Note.ID == NT)
       return std::string(Note.Name);
 
   std::string string;
   raw_string_ostream OS(string);
-  OS << format("Unknown note type (0x%08x)", NT);
+  OS << format("Unknown note type: (0x%08x)", NT);
   return OS.str();
 }
 
 static std::string getAMDNoteTypeName(const uint32_t NT) {
   static const struct {
     uint32_t ID;
     const char *Name;
   } Notes[] = {{ELF::NT_AMD_AMDGPU_HSA_METADATA,
                 "NT_AMD_AMDGPU_HSA_METADATA (HSA Metadata)"},
                {ELF::NT_AMD_AMDGPU_ISA, "NT_AMD_AMDGPU_ISA (ISA Version)"},
                {ELF::NT_AMD_AMDGPU_PAL_METADATA,
                 "NT_AMD_AMDGPU_PAL_METADATA (PAL Metadata)"}};
 
   for (const auto &Note : Notes)
     if (Note.ID == NT)
       return std::string(Note.Name);
 
   std::string string;
   raw_string_ostream OS(string);
-  OS << format("Unknown note type (0x%08x)", NT);
+  OS << format("Unknown note type: (0x%08x)", NT);
   return OS.str();
 }
 
 static std::string getAMDGPUNoteTypeName(const uint32_t NT) {
   if (NT == ELF::NT_AMDGPU_METADATA)
     return std::string("NT_AMDGPU_METADATA (AMDGPU Metadata)");
 
   std::string string;
   raw_string_ostream OS(string);
-  OS << format("Unknown note type (0x%08x)", NT);
+  OS << format("Unknown note type: (0x%08x)", NT);
   return OS.str();
 }
 
 template <typename ELFT>
 static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
                                   ArrayRef<uint8_t> Data) {
   std::string str;
   raw_string_ostream OS(str);
if (Name == "GNU") {
                                     sizeof(Elf_Addr));
         Expected<CoreNote> Note = readCoreNote(DescExtractor);
         if (Note)
           printCoreNote<ELFT>(OS, *Note);
         else
           reportWarning(Note.takeError(), this->FileName);
       }
     } else if (!Descriptor.empty()) {
-      OS << "   description data:";
+      OS << "    Description data:";
       for (uint8_t B : Descriptor)
         OS << " " << format("%02x", B);
       OS << '\n';
     }
   };
 
   ArrayRef<Elf_Shdr> Sections = unwrapOrError(this->FileName, Obj->sections());
   if (Obj->getHeader()->e_type != ELF::ET_CORE && !Sections.empty()) {