[PATCH] D68636: [llvm-readobj] - Refine the LLVM-style output to be consistent.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:5598
                                                 const Elf_Shdr *Sec) {
-  DictScope SS(W, "Version symbols");
+  ListScope SS(W, "VersionSymbols");
   if (!Sec)
----------------
grimar wrote:
> MaskRay wrote:
> > jhenderson wrote:
> > > grimar wrote:
> > > > jhenderson wrote:
> > > > > Ditto, though I'm wondering here why the VersionSymbols data includes stuff to do with its section header? If it didn't have that stuff, it would be a list.
> > > > > though I'm wondering here why the VersionSymbols data includes stuff to do with its section header?
> > > > 
> > > > I do not know. The same information is printed under "Sections [" tag anyways, so it is not useful probably:
> > > > 
> > > > ```
> > > >   Section {
> > > >     Index: 3
> > > >     Name: .gnu.version (30)
> > > >     Type: SHT_GNU_versym (0x6FFFFFFF)
> > > >     Flags [ (0x0)
> > > >     ]
> > > >     Address: 0x0
> > > >     Offset: 0xB4
> > > >     Size: 2
> > > >     Link: 0
> > > >     Info: 0
> > > >     AddressAlignment: 0
> > > >     EntrySize: 2
> > > >   }
> > > > ```
> > > > 
> > > > Should we remove "Section Name"/"Address"/"Offset"/"Link" and make it to be a list?
> > > I'd be inclined to do that personally, but it should be a separate change.
> > The Linux Standard Base calls this "Symbol Version Table" but this is named "VersionSymbols" here... What do you think if we just use the regular section type name "SHT_GNU_versym"? It may improve discoverability as well.
> >  What do you think if we just use the regular section type name "SHT_GNU_versym"? It may improve discoverability as well.
> 
> I.e. this is an opposite direction to what this patch does:
> 
> ```
> SHT_GNU_verdef { -> VersionDefinitions [
> SHT_GNU_verneed { -> VersionRequirements [
> ```
> 
> It will be only sections for which we use type names. Should we?
> I.e. this is an opposite direction to what this patch does:

Yes. .gnu.version is currently not consistent with .gnu.version_r and .gnu.version_d, and I know this patch tries to make them consistent.

I am not clear which direction we should go. I have a very weak preference for SHT_GNU_versym.

The naming does not seem very consistent here. While LSB names .gnu.version_r "version requirements", binutils-gdb elf.h names it "version needs section".


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

https://reviews.llvm.org/D68636