[PATCH] D21552: [ELF] - Teach llvm-readobj to print dependencies of SHT_GNU_verdef and refactor dumping method.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: test/tools/llvm-readobj/elf-versioninfo.test:58
@@ +57,3 @@
+CHECK-NEXT:     Name: blah
+CHECK-NEXT:     Dependencies {
+CHECK-NEXT:     }
----------------
ruiu wrote:
> Is it a common practice for llvm-dump* tools to print out an empty set `{}` if an element is missing? An alternative way is to not print out `Dependencies {}` if a version definition has no dependencies.
I think it is common, for example we often have relocations empty in testcases:

```
Relocations [
]
```

I am going to change this in according to my answers to Rafael and Davide.

================
Comment at: tools/llvm-readobj/ELFDumper.cpp:570
@@ +569,3 @@
+
+    DictScope Dep(W, "Dependencies");
+    const uint8_t *PAux = P + VD->vd_aux + VD->getAux()->vda_next;
----------------
rafael wrote:
> The "spec" used the name "predecessor versions". Maybe this should be "Predecessors". I assume that if you have
> 
> foo {
> } bar;
> 
> bar {
> } zed;
> 
> it will chain them?
> 
> If that is the case, maybe it would be better to print just the first predecessor?
> 
> In any case, I agree with Davide, we need a test :-)
No, it will not chain. Foo will have [foo, bar], where bar is dependency. Bar will have [bar, zed].
About terminology, I used sun docs: https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-61174/index.html, it says "version dependencies".

And like I wrote in my answer to Davide, I was unable to get more than one "dependency", so looks "predecessor" word is more correct here.
So I plan to use it and print predecessor without a loop.

================
Comment at: tools/llvm-readobj/ELFDumper.cpp:571
@@ +570,3 @@
+    DictScope Dep(W, "Dependencies");
+    const uint8_t *PAux = P + VD->vd_aux + VD->getAux()->vda_next;
+    for (unsigned J = 0; J < (unsigned)VD->vd_cnt - 1; ++J) {
----------------
davide wrote:
> How many dependencies can you have?
> IIRC you can have only one immediate parent/dependency so it's probably easier/more reasonable to only print that, no?
> If you can have multiple dependencies, instead, can you please add a test exercising that behaviour?
I did not found how to get more that one dependency here. I also think that it can be only one now. But spec says: "At least one version dependency must exist. Additional version dependencies can be present, the number being indicated by the vn_cnt value.".
So initially I implemented it in a more generic way, basing on what it says.
I am ok to print only one since it looks to be the only case and it is more "predecessor" than "dependency".


http://reviews.llvm.org/D21552