[PATCH] D59186: [llvm-readobj] Separate `Symbol Version` dumpers into `LLVM style` and `GNU style`

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/elf-versioninfo.test:119
+GNU-VERNEED: Dumper for .gnu.version_r is not implemented
\ No newline at end of file

----------------
Nit: no newline at EOF.


================
Comment at: test/tools/yaml2obj/verneed-section.yaml:74
+    - Name:            f1
\ No newline at end of file

----------------
Nit: missing new line at EOF here.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:4495
+        Dumper->getFullSymbolName(&Sym, StrTable, true /* IsDynamic */);
+    W.printNumber("Version", Versym->vs_index & VERSYM_VERSION);
+    W.printString("Name", FullSymbolName);
----------------
Higuoxing wrote:
> The version of a symbol in `.gnu.version` should be calculated using `Versym->vs_index & VERSYM_VERSION (0x7fff)`. Before this patch, we use `*VersymBuf` as its version. I think it's not a right way, though it does the right thing in most time.
I wasn't able to find any documentation for this outside of code, but I see that it is already used elsewhere in this file in a similar manner, and I found other examples of it in code elsewhere, so this looks fine to me.

I'm not quite sure what you mean by:
> I think it's not a right way, though it does the right thing in most time.
Which way don't you think is right? What conditions does it do the right thin most of the time?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59186