[PATCH] D39582: Add ELF dynamic symbol support to yaml2obj/obj2yaml

[Sean Silva via Phabricator via llvm-commits]

silvas accepted this revision.
silvas added inline comments.
This revision is now accepted and ready to land.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:162-163
   unsigned getDotShStrTabSecNo() const { return Doc.Sections.size() + 3; }
-  unsigned getSectionCount() const { return Doc.Sections.size() + 4; }
+  unsigned getDotDynSymSecNo() const { return Doc.Sections.size() + 4; }
+  unsigned getDotDynStrSecNo() const { return Doc.Sections.size() + 5; }
+  unsigned getSectionCount() const {
----------------
kastiglione wrote:
> kastiglione wrote:
> > kastiglione wrote:
> > > jakehehrlich wrote:
> > > > .dyn* sections are going to be allocated and so we need to be able place them next to certain other allocated sections so that we can't put them in program headers. It won't be sufficient to decide for the user, at least not this way. One way might be to require that the user add a .dynsym section if they want to use DynamicSymbols and then using the index of that section if they do. This will require that you compute the size of the .dynsym and .dynstr sections early enough for the standard layout algorithm to work.
> > > What about making the `.dynsym` section optional? If it's there, that's the order that's used, and if it no `.dynsym` section is specified, then this default ordering is used? The same could be applied to these other sections (`.symtab` etc) if desired.
> > @silvas Hi Sean, do you have an opinion on how you would like to see section ordering handled, particularly as it relates to dynamic symbols?
> @jakehehrlich I have a diff (D39749) to allow the input yaml to control section order. If folks are happy with that, I will rebase this change on top of that, which will allow the `.dynsym` sections to be ordered as specified in the input.
Sorry for the very late reply. Don't know why it only just now showed up in my inbox.

I really like the approach you've taken in D39749. LGTM.


https://reviews.llvm.org/D39582