[llvm-readobj] Display sections that do not belong to a segment in the section-mapping

[mattd]

Summary

The following patch adds the "None" line to the section to segment mapping dump.
That line lists the sections that do not belong to any segment.
I realize that this change differs from GNU readelf which does not display the latter information.

I'd rather not add this "feature" under a command line option. I think that might introduce confusion, since users would have to
make an additional decision as to if they want to see all of the section-to-segment map or just a subset of it.

Another option is to only print the "None" line if the --section-mapping option is passed; however,
that might also introduce some confusion, because the section-to-segment map would be different between`--program-headers`
and the --section-mapping output. While the difference is just the "None" line, it seems that if we choose to display
the segment-to-section mapping, then we should always display the whole map including the sections
that do not belong to segments.

Section to Segment mapping:
  Segment Sections...
   00
   01     .interp
   02     .interp .note.ABI-tag .gnu.hash 
   03     .init_array .fini_array .dynamic 
   04     .dynamic
   05     .note.ABI-tag
   06     .eh_frame_hdr
   07
   08     .init_array .fini_array .dynamic .got
   None   .comment .symtab .strtab .shstrtab <--- THIS LINE

[jhenderson]

I'm happy with this, but since I was the one who suggested this offline, and I'm aware that there might be some reluctance to accept it from elsewhere, I'd like some other people's comments on it.

llvm/tools/llvm-readobj/ELFDumper.cpp

3345 ↗

(On Diff #185081)

I'd rename this variable to something else, either Sections, like above or NoSegmentSections or similar. The reason is to a void any potential confusion with Optional::None. The latter name is also slightly more self-documenting.

[grimar]

In D57700#1384747, @jhenderson wrote:

I'm happy with this, but since I was the one who suggested this offline, and I'm aware that there might be some reluctance to accept it from elsewhere, I'd like some other people's comments on it.

My main concern is: is it ok we change the GNU style output?

Actually, I am confused and wonder why we have LLVM and GNU styles first of all.
I would understand if we would have GNU style that would try to be 1:1 to GNU and we would have LLVM style that would be an extension/restyling.

This patch changes GNU style of printSectionMapping. It makes me think we want LLVM style implementation of printSectionMapping instead maybe?

[jhenderson]

In D57700#1384760, @grimar wrote:

In D57700#1384747, @jhenderson wrote:

I'm happy with this, but since I was the one who suggested this offline, and I'm aware that there might be some reluctance to accept it from elsewhere, I'd like some other people's comments on it.

My main concern is: is it ok we change the GNU style output?

Actually, I am confused and wonder why we have LLVM and GNU styles first of all.
I would understand if we would have GNU style that would try to be 1:1 to GNU and we would have LLVM style that would be an extension/restyling.

Taken from D14128, which was the review to introduce the new style:

Hello Davide,
The present format is good enough to convey the information. It is however not sufficient or sometimes difficult for present users of GNU tools (which are quite ubiquitous) to make a
switch as it is complete redesign of the post processing tools they may have based on.

-Hemant

It's not immediately clear from the original introduction of GNU style whether complete output identity was the original goal, or just similarity. Similarly, it's not clear from my previously attempt to gather information as to users expectations. I've subscribed @khemant in case he's got any further comments, since he made the original change. My reading of rL260430 which commits it is that it is intended to be like GNU readelf, but doesn't necessarily have to be identical. I'm open to further discussion on this (and I actually have a BoF on this subject at the next Euro LLVM, but that's still a couple of months away).

This patch changes GNU style of printSectionMapping. It makes me think we want LLVM style implementation of printSectionMapping instead maybe?

Certainly at some point we should add this to LLVM style, but I don't think it means we can't have it for GNU style too necessarily.

[mattd]

In D57700#1384786, @jhenderson wrote:

Hello Davide,
The present format is good enough to convey the information. It is however not sufficient or sometimes difficult for present users of GNU tools (which are quite ubiquitous) to make a
switch as it is complete redesign of the post processing tools they may have based on.

-Hemant

It's not immediately clear from the original introduction of GNU style whether complete output identity was the original goal, or just similarity. Similarly, it's not clear from my previously attempt to gather information as to users expectations. I've subscribed @khemant in case he's got any further comments, since he made the original change. My reading of rL260430 which commits it is that it is intended to be like GNU readelf, but doesn't necessarily have to be identical. I'm open to further discussion on this (and I actually have a BoF on this subject at the next Euro LLVM, but that's still a couple of months away).

@jhenderson Thanks for tracking down that comment. I interpreted that comment as you described. Personally, I think a one line addition to the existing GNU style output is a reasonable change that will not detract users who might be expecting GNU-styled output. The change provides the user with the familiar GNU style, and provides a one line bonus (it provides a complete table (all of the sections are accounted for)). The main "issue" I see is that anyone diffing llvm-readelf output against the equivalent GNU output will see a line difference (llvm-readelf has the Notes, GNU readelf does not). If users diff output between llvm-readelf and GNU readelf, they would see other differences as well, primarily the number of digits used to represent some of the values in the --program-headers output.

[rupprecht]

In D57700#1385476, @mattd wrote:

In D57700#1384786, @jhenderson wrote:

Hello Davide,
The present format is good enough to convey the information. It is however not sufficient or sometimes difficult for present users of GNU tools (which are quite ubiquitous) to make a
switch as it is complete redesign of the post processing tools they may have based on.

-Hemant

It's not immediately clear from the original introduction of GNU style whether complete output identity was the original goal, or just similarity. Similarly, it's not clear from my previously attempt to gather information as to users expectations. I've subscribed @khemant in case he's got any further comments, since he made the original change. My reading of rL260430 which commits it is that it is intended to be like GNU readelf, but doesn't necessarily have to be identical. I'm open to further discussion on this (and I actually have a BoF on this subject at the next Euro LLVM, but that's still a couple of months away).

@jhenderson Thanks for tracking down that comment. I interpreted that comment as you described. Personally, I think a one line addition to the existing GNU style output is a reasonable change that will not detract users who might be expecting GNU-styled output. The change provides the user with the familiar GNU style, and provides a one line bonus (it provides a complete table (all of the sections are accounted for)). The main "issue" I see is that anyone diffing llvm-readelf output against the equivalent GNU output will see a line difference (llvm-readelf has the Notes, GNU readelf does not). If users diff output between llvm-readelf and GNU readelf, they would see other differences as well, primarily the number of digits used to represent some of the values in the --program-headers output.

I have the same interpretation. llvm-readelf should be possible to use as a replacement for readelf, but it may not be a drop-in replacement -- modifications to build scripts may be necessary to work around formatting differences. Also, I could come up with a long list of existing differences...

diff <(readelf -W -a llvm-readelf) <(llvm-readelf -a llvm-readelf) | wc -l
158116

lld/trunk/test/ELF/linkerscript/segment-none.s

 # RUN:           .text : {*(.text .text*)} :text \
 # RUN:           .foo : {*(.foo)} :NONE \
 # RUN:       }" > %t.script
 # RUN: ld.lld -o %t --script %t.script %t.o
 # RUN: llvm-readelf -S -l %t | FileCheck --check-prefix=DEFINED %s
 
 # CHECK: Section to Segment mapping:
 # CHECK-NEXT: Segment Sections...
-# CHECK-NOT: .foo
+# CHECK: None {{.*}}.foo
 
 # DEFINED: Section to Segment mapping:
 # DEFINED-NEXT: Segment Sections...
 # DEFINED-NEXT:  00     .text
 # DEFINED-NEXT:  01     .foo
 
 .global _start
 _start:
  nop
 
 .section .foo,"a"
 foo:
  .long 0

llvm/trunk/test/tools/llvm-readobj/gnu-phdrs.test

 ELF32-NEXT:    02     .interp .note.ABI-tag .note.gnu.build-id .hash .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rel.dyn .rel.plt .init .plt .text .fini .rodata .eh_frame_hdr .eh_frame
 ELF32-NEXT:    03     .tdata .ctors .dtors .jcr .dynamic .got .got.plt .data .bss
 ELF32-NEXT:    04     .dynamic
 ELF32-NEXT:    05     .note.ABI-tag .note.gnu.build-id
 ELF32-NEXT:    06     .tdata .tbss
 ELF32-NEXT:    07     .eh_frame_hdr
 ELF32-NEXT:    08
 ELF32-NEXT:    09     .tdata .ctors .dtors .jcr .dynamic .got
+ELF32-NEXT:    None   .comment .shstrtab .symtab .strtab
 
 ELF64-PHDRS: Elf file type is EXEC (Executable file)
 ELF64-PHDRS-NEXT: Entry point 0x400610
 ELF64-PHDRS-NEXT: There are 10 program headers, starting at offset 64
 
 ELF64-PHDRS: Program Headers:
 ELF64-PHDRS-NEXT:   Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
 ELF64-PHDRS-NEXT:   PHDR           0x000040 0x0000000000400040 0x0000000000400040 0x000230 0x000230 R E 0x8

 ELF64-MAPPING-NEXT:    02     .interp .note.ABI-tag .note.gnu.build-id .hash .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .text .fini .rodata .eh_frame_hdr .eh_frame
 ELF64-MAPPING-NEXT:    03     .tdata .init_array .fini_array .jcr .dynamic .got .got.plt .data .bss
 ELF64-MAPPING-NEXT:    04     .dynamic
 ELF64-MAPPING-NEXT:    05     .note.ABI-tag .note.gnu.build-id
 ELF64-MAPPING-NEXT:    06     .tdata .tbss
 ELF64-MAPPING-NEXT:    07     .eh_frame_hdr
 ELF64-MAPPING-NEXT:    08
 ELF64-MAPPING-NEXT:    09     .tdata .init_array .fini_array .jcr .dynamic .got
+ELF64-MAPPING-NEXT:    None   .comment .shstrtab .symtab .strtab
 
 ELF64-ONEMAPPING: Section to Segment mapping:
 ELF64-ONEMAPPING-NOT: Section to Segment mapping:

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

 #include "ARMEHABIPrinter.h"
 #include "DwarfCFIEHPrinter.h"
 #include "Error.h"
 #include "ObjDumper.h"
 #include "StackMapPrinter.h"
 #include "llvm-readobj.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
for (const auto &Phdr : unwrapOrError(Obj->program_headers())) {
     }
     OS << "\n";
   }
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printSectionMapping(const ELFO *Obj) {
   OS << "\n Section to Segment mapping:\n  Segment Sections...\n";
+  DenseSet<const Elf_Shdr *> BelongsToSegment;
   int Phnum = 0;
   for (const Elf_Phdr &Phdr : unwrapOrError(Obj->program_headers())) {
     std::string Sections;
     OS << format("   %2.2d     ", Phnum++);
     for (const Elf_Shdr &Sec : unwrapOrError(Obj->sections())) {
       // Check if each section is in a segment and then print mapping.
       // readelf additionally makes sure it does not print zero sized sections
       // at end of segments and for PT_DYNAMIC both start and end of section
       // .tbss must only be shown in PT_TLS section.
       bool TbssInNonTLS = (Sec.sh_type == ELF::SHT_NOBITS) &&
                           ((Sec.sh_flags & ELF::SHF_TLS) != 0) &&
                           Phdr.p_type != ELF::PT_TLS;
       if (!TbssInNonTLS && checkTLSSections(Phdr, Sec) &&
           checkoffsets(Phdr, Sec) && checkVMA(Phdr, Sec) &&
-          checkPTDynamic(Phdr, Sec) && (Sec.sh_type != ELF::SHT_NULL))
+          checkPTDynamic(Phdr, Sec) && (Sec.sh_type != ELF::SHT_NULL)) {
         Sections += unwrapOrError(Obj->getSectionName(&Sec)).str() + " ";
+        BelongsToSegment.insert(&Sec);
+      }
     }
     OS << Sections << "\n";
     OS.flush();
   }
+
+  // Display sections that do not belong to a segment.
+  std::string Sections;
+  for (const Elf_Shdr &Sec : unwrapOrError(Obj->sections())) {
+    if (BelongsToSegment.find(&Sec) == BelongsToSegment.end())
+      Sections += unwrapOrError(Obj->getSectionName(&Sec)).str() + ' ';
+  }
+  if (!Sections.empty()) {
+    OS << "   None  " << Sections << '\n';
+    OS.flush();
+  }
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printDynamicRelocation(const ELFO *Obj, Elf_Rela R,
                                             bool IsRela) {
   unsigned Bias = ELFT::Is64Bits ? 8 : 0;
   // First two fields are bit width dependent. The rest of them are after are
   // fixed width.