[PATCH] D62964: [llvm-objdump] Match GNU objdump on symbol types shown in disassembly output.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-dynamic-symbols.test:63
     Binding: STB_GLOBAL
-  - Name:    both_dyn
     Value:   0x1001
----------------
I think you may have gone overboard with the symbol renaming. These first two are deliberately named so that they clearly clash/don't clash with a corresponding static symbol (only_[static|dyn] means only the static/dynamic symbol respectively is present at that address whilst the both_[static|dyn] means both symbols have that value).


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-dynamic-symbols.test:83
+  - Name:    func_dyn
+    Value:   0x1002
+    Section: .text
----------------
This value is bad, because the test case was supposed to be for just a static symbol at this address. See the symbol name comments.


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-dynamic-symbols.test:88
     Binding: STB_GLOBAL
-  ## The rest of the dynamic symbols won't be used for various reasons.
-  ## FIXME: the first two symbols here should be dumped.
-  ##        See https://bugs.llvm.org/show_bug.cgi?id=41947
-  - Name:    not_func
+  - Name:    zero_sized_func_dyn
     Value:   0x1003
----------------
I don't think you need to rename this symbol from zero_sized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62964