[PATCH] D56569: [ObjectYAML][yaml2obj][ELF] Add basic support for dynamic entries

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: llvm/include/llvm/ObjectYAML/ELFYAML.h:94
+  ELF_DT Tag;
+  Optional<uint64_t> Val;
+  Optional<llvm::yaml::Hex64> Ptr;
----------------
amontanez wrote:
> ruiu wrote:
> > jhenderson wrote:
> > > Somewhat moot if you follow @ruiu's suggestion (which I support), but this should be an int64_t.
> > Hmm, why is this of type `StringRef`? Isn't this `uint64_t`?
> Making `Value` a StringRef gives more flexibility for how the value of a dynamic entry can be populated. I've listed a few examples below that illustrate how this is currently used.
> 
> Using a section name rather than manually entering the address of a section:
> ```
> Tag: DT_STRTAB
> Value: .dynstr
> ```
> 
> 
> Using a string that will be added to .dynstr:
> ```
> Tag: DT_SONAME
> Value: libsomething.so
> ```
> 
> Or, the simple case of using a numeric value:
> ```
> Tag: DT_STRSZ
> Value:  0xa0
> ```
> 
  Tag: DT_STRTAB
  Value: .dynstr

What if two or more sections have the same name ".dynstr"? In ELF, at least in spirit, section names are not significant.

I feel that in order to discuss the final file format of the text-based ELF in YAML, code review is not the best place. Also, I think that we shouldn't discuss one YAML tag at a time. I'd rather want to take a look at the final form of the text-based ELF file so that I can get a big picture. Do you have such example? It doesn't have to be a formal document or anything, but just an example or two of a text-based ELF file should suffice.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56569