[PATCH] D62516: [llvm-readobj/llvm-readelf] - Implement GNU style dumper of the SHT_GNU_verneed section.
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 29 06:11:05 PDT 2019
MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.
================
Comment at: tools/llvm-readobj/ELFDumper.cpp:3436
StringRef SymTabName = unwrapOrError(Obj->getSectionName(SymTab));
OS << " Addr: " << format_hex_no_prefix(Sec->sh_addr, 16)
<< " Offset: " << format_hex(Sec->sh_offset, 8)
----------------
grimar wrote:
> MaskRay wrote:
> > Nit: `" Addr: "` (two spaces before `"Addr:"`)
> >
> > GNU readelf prints `0x` prefix for `.gnu.version_d` and `.gnu.version_r` but omits the prefix for `.gnu.version`... I think the inconsistency may be due to a bug. We may stick with one style :)
> GNU output is:
>
> ```
> Version symbols section '.gnu.version' contains 6 entries:
> Addr: 0000000000000000 Offset: 0x000280 Link: 7 (.dynsym)
> readelf: Warning: Cannot interpret virtual addresses without program headers.
> 000:457f 464c 102 901
> 004: 0 (*local*) 0 (*local*)
>
> Version definition section '.gnu.version_d' contains 3 entries:
> Addr: 0x0000000000000000 Offset: 0x00028c Link: 8 (.dynstr)
> 000000: Rev: 1 Flags: none Index: 2 Cnt: 1 Name: VERSION1
> 0x001c: Rev: 1 Flags: none Index: 3 Cnt: 2 Name: VERSION2
> 0x0038: Parent 1: VERSION1
> readelf: Warning: Invalid vd_next field of 0
>
> Version needs section '.gnu.version_r' contains 2 entries:
> Addr: 0x0000000000000000 Offset: 0x0002cc Link: 8 (.dynstr)
> 000000: Version: 1 File: verneed1.so.0 Cnt: 2
> 0x0010: Name: v1 Flags: none Version: 4
> 0x0020: Name: v2 Flags: none Version: 5
> 0x0030: Version: 1 File: verneed2.so.0 Cnt: 1
> 0x0040: Name: v3 Flags: none Version: 6
>
> ```
>
> There are following inconsistencies can be observed:
> 1) Number of spaces before `Addr`. For `.gnu.version_d` there are 2, for other sections - 1.
> So having a single space is a bit more consistent with GNU (2:1) :)
>
> I would probably leave a single space, because this line is kind of prolog(header),
> it is different from following entries (and hence can be separated).
>
> 2) `0x` prefix, yes. As you mentioned, GNU omits it for `.gnu.version`, but not only that. It also does
> it for the first entries. IMO looks ugly and crazy inconsistent. So I am always printing it.
1 space looks good to me.
I didn't know the GNU output is so bad until you printed it out.. Pick the style that is aesthetically appealing then :)
`000000` -> `0x0000` I believe this is also a GNU readelf bug.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62516/new/
https://reviews.llvm.org/D62516
More information about the llvm-commits
mailing list