[llvm-readobj] Implement GNU-style output for dynamic table

[atanasyan]

Summary

GNU readelf tool prints slightly different dynamic table "header" and surrounds dynamic tag names by brackets. This patch implements the same formatting for GNU-style output of the llvm-readobj.

LLVM

DynamicSection [ (13 entries)
  Tag        Type                 Name/Value
  0x00000006 SYMTAB               0x168
  ...
]

GNU

Dynamic section at offset 0x1d0 contains 13 entries:
  Tag        Type                 Name/Value
  0x00000006 (SYMTAB)             0x168
  ...

[MaskRay]

I think the header row is apparently misaligned. Not sure we want to misalign ourselves...

% ~/projects/binutils-gdb/Debug/binutils/readelf -d =cat
                                          
Dynamic section at offset 0x7df8 contains 26 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000c (INIT)               0x15a0

Regarding the extra parentheses, I am not sure if we really want to make llvm-readelf/llvm-readobj different just to become more readelf-like, but I'd like to hear what others say. I'll not oppose if we decide to become a better readelf replacement.

llvm/test/tools/llvm-readobj/elf-dynamic-tags-machine-specific.test

123 ↗

(On Diff #200748)

(MIPS_RLD_TEXT_RESOLVE_ADDR)0x8

This missing space separator definitely looks weird. What does GNU readelf do?

[jhenderson]

Not looked at the implementation in detail yet, but to address @MaskRay's comment, we should definitely add the extra parentheses in llvm-readelf output. We could always add it in llvm-readobj too, but I'm less bothered by that. As close to byte-to-byte identical output as possible was requested for GNU style output by multiple individuals at the recent LLVM Binutils BoF and round table.

I wonder if it's worth adding --strict-whitespace to one or more of the tests, to show that the formatting is correct? What do you think?

llvm/tools/llvm-readobj/ELFDumper.cpp

2482 ↗

(On Diff #200748)

Could this just be a while loop?

[atanasyan]

In D62256#1512194, @MaskRay wrote:

I think the header row is apparently misaligned. Not sure we want to misalign ourselves...

I do not emulate this misalignment. Header row aligned properly in both 32/64-bit cases.

Regarding the extra parentheses, I am not sure if we really want to make llvm-readelf/llvm-readobj different just to become more readelf-like

As to me, the parentheses do not make sense. If everybody agree that we do not need to emulate GNU readelf formatting in all details, I will be happy to drop these parentheses.

llvm/test/tools/llvm-readobj/elf-dynamic-tags-machine-specific.test

123 ↗

(On Diff #200748)

GNU readelf adds space between fields. I will fix that.

[atanasyan]

• Add tests with the strict-whitespace flag to check headers alignment.
• Replace for by while.
• Add space between too long dynamic table tag name and it's value.

[rupprecht]

FYI there's another patch that changes .dynamic output, including the readelf output -- D62179. You may need to update that test if it gets committed first.

I do agree with the others that parens is better for compatibility. But I think it's fine to deviate with the other whitespace differences; breaking compatibility makes it easier to read the table. LGTM, but please wait for James/Ray to approve.

btw, I noticed other missing differences unrelated to this patch -- filed https://bugs.llvm.org/show_bug.cgi?id=41987.

llvm/tools/llvm-readobj/ELFDumper.cpp

3348–3350 ↗

(On Diff #200781)

I know this (and the LLVMStyle below) is just copied from the old code... but I don't think there needs to be any << between strings, they can be implicitly concatenated (or joined into a single string, I don't know why it needs to be so narrow)

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

3334 ↗

(On Diff #200781)

Are there any cases where we don't want the dumper to provide a rectified table? I don't think so. In that case, could the rectify code move into the dynamic_table() method?

[atanasyan]

• Move code cuts dynamic table after the first DT_NULL entry into the dynamic_table() method.
• Print dynamic table header as a single string without multiple << operators.

[jhenderson]

LGTM.

[atanasyan]

Thanks for review.

test/ELF/ppc64-dynamic-relocations.s

 
 // There should be 2 reserved doublewords before the first entry. The dynamic
 // linker will fill those in with the address of the resolver entry point and
 // the dynamic object identifier.
 // DIS: Idx Name       Size      VMA              Type
 // DIS:     .plt       00000018  0000000010030000 BSS
 
 // DT_PLTGOT should point to the start of the .plt section.
-// DT: 0x0000000000000003 PLTGOT               0x10030000
+// DT: 0x0000000000000003 (PLTGOT)             0x10030000
 
     .text
     .abiversion 2
     .globl  _start
     .p2align        4
     .type   _start,@function
 _start:
 .Lfunc_begin0: