[PATCH] D57691: [yaml2obj][obj2yaml] - Add support for dumping/parsing .dynamic sections.

[George Rimar via Phabricator via llvm-commits]

grimar marked 2 inline comments as done.
grimar added inline comments.


================
Comment at: lib/ObjectYAML/ELFYAML.cpp:677
+#define STRINGIFY(X) (#X)
+#define DYNAMIC_TAG(X, Y) IO.enumCase(Value, STRINGIFY(DT_##X), ELF::DT_##X);
+#include "llvm/BinaryFormat/DynamicTags.def"
----------------
jhenderson wrote:
> I may be missing something, but why is this line different to the equivalent for e.g. the relocations above?
See how relocations are declared. For example in ELFRelocs/RISCV.def:

```
ELF_RELOC(R_RISCV_NONE,               0)
ELF_RELOC(R_RISCV_32,                 1)
...
```

But dynamic tags are refined without prefix `DT_*`:

```
DYNAMIC_TAG(NULL, 0)        // Marks end of dynamic array.
DYNAMIC_TAG(NEEDED, 1)      // String table offset of needed library.
```

ELF.h declares an enum like that:

```
enum {
#define DYNAMIC_TAG(name, value) DT_##name = value,
#include "DynamicTags.def"
#undef DYNAMIC_TAG
};
```
(https://github.com/llvm-mirror/llvm/blob/master/include/llvm/BinaryFormat/ELF.h#L1242)

Do I had to use `ELF::DT_##X`.

The reason I used `STRINGIFY` and `STRINGIFY(DT_##X)` is that I think we should dump
tags as `DT_TAGNAME` and not just `TAGNAME` because:

1) That is consistent with how we dump relocations (we dump them with `R_*` prefix.
2) Dumping just a tag name does not look clear to me. `DT_NEEDED` looks better than `NEEDED` IMO.
`DT_NULL` is more clear than just `NULL` and so on.


================
Comment at: test/tools/llvm-readobj/gnu-hash-symbols.test:77
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x0000000000001000
+    Link:            .dynstr
----------------
jhenderson wrote:
> Not that it really matters, but any particular reason you've moved this to before the Link field?
No. I had to use yaml2obj with the old YAMLs to convert then objects to a new YAML format (to update the tests),
so seems it just the natural order of the fields now (i.e. obj2yaml writes them in this order). I did not try to swap the lines intentionally and/or manually.


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

https://reviews.llvm.org/D57691