[PATCH] D59877: [llvm-readobj] Add GNU style dumper for .gnu.version section

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/elf-hidden-versym.test:32-41
+      - Version:         1
+        File:            somefile
+        Entries:
+          - Name:            hiddensym
+            Hash:            1234
+            Flags:           0
+            Other:           3
----------------
Could you add less padding between the Name and other fields, and the values for those fields, please? Ditto in the other new test.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3413
+          VersionName = "(*invalid*)";
+        OS << left_justify(VersionName, 13);
+      }
----------------
Higuoxing wrote:
> jhenderson wrote:
> > What happens if the version name is longer than this? (Is that even possible?)
> Yes, it's possible. But `GNU readelf` does not do any improvement here, shall we?
I think it's okay to leave for now, but you might want to consider improving it in a future version, by using a column width that varies depending on name widths.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3375
+  // readelf prints 4 entries per line.
+  for (uint32_t VersymRow = 0; VersymRow < Entries; VersymRow += 4) {
+    OS << "  " << format_hex_no_prefix(VersymRow, 3) << ":";
----------------
Entries is now a uin64_t, so VersymRow and VersymIndex below should be too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59877