[PATCH] D60312: [llvm-readobj] Add GNU style dumper for .gnu.version_d section

[Xing via Phabricator via llvm-commits]

Higuoxing marked an inline comment as done and an inline comment as not done.
Higuoxing added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3417
 
+  const uint8_t *SecStartAddress =
+      reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
----------------
jhenderson wrote:
> Given that much of this code is a copy of the LLVMStyle, that indicates that it should be shared somehow. I don't know if in this file, or in the Object library somewhere makes sense, but I think the only distinction should be how to print the output.
Yes, I agree. I would like to improve this in the future.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3453
+    if (Verdef->vd_cnt > 2)
+      report_fatal_error("more than one predecessor is not expected");
+
----------------
jhenderson wrote:
> Higuoxing wrote:
> > In `GNU readelf`, there's no limit on the number of predecessors. For example, if one symbol has more than one parents, `GNU readelf` will give:
> > 
> > ```
> > Parent 1: VERSION1
> > Parent 2: VERSION0
> > ...
> > ```
> > 
> > Shall we do the same thing in `llvm-readelf` ?
> Is there a good reason to limit it? If not, I don't think we should. However, I get the feeling that this is a different patch, since we should do it in LLVM style too, unless there's a good reason not to.
I had asked @grimar about this in https://reviews.llvm.org/D21552

I think it's good to be robust when dumping some uncommon object files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60312