[PATCH] D75588: [llvm-objdump] --syms: print "%016" PRIx64 instead of "%08" PRIx64 for 64-bit objects

Wed Mar 4 02:20:28 PST 2020

grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

LGTM with ELF32 and ELF64.

================
Comment at: llvm/test/tools/llvm-objdump/symbol-table-elf.test:4
+# RUN: llvm-objdump -t %t.32 | cmp - %t.32.txt
+# RUN: FileCheck --check-prefix=32 %s --input-file=%t.32.txt

----------------
Lets use ELF32 and ELF64 as it is more common?

================
Comment at: llvm/test/tools/llvm-objdump/symbol-table-elf.test:19
+# 64-NEXT: 0000000000001004 g     F .text     0000000000000000 foo
+# 64-NEXT: 0000000000001008 g     O .text     0000000000000000 bar

----------------
I guess it is fine for this patch.

But FTR, it seems this test might be improved. Probably 64 bit values also should be used
to demonstrate we can dump them. And testing of symbol sizes would also probably be not harmful.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75588