[PATCH] D19464: ELF: Create .gnu.version and .gnu.version_r sections when linking against versioned DSOs.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/InputFiles.cpp:438
@@ +437,3 @@
+  // Mapping from version identifiers to Elf_Verdef entries.
+  std::vector<const Elf_Verdef *> Verdefs(1);
+
----------------
Does this hide this->Verdefs?

================
Comment at: ELF/InputFiles.cpp:440
@@ +439,3 @@
+
+  // We only need to process symbol versions for this DSO if it has both a
+  // versym and a verdef section, which indicates that the DSO contains symbol
----------------
Can you make this new code a new function that returns a std::vector<const Elf_Verdef *>?

================
Comment at: ELF/InputFiles.cpp:458
@@ +457,3 @@
+    // from the start of the .gnu.version_d section.
+    const unsigned char *Verdef = this->ELFObj.base() + VerdefSec->sh_offset;
+    for (unsigned I = 0; I != VerdefCount; ++I) {
----------------
unsigned char -> uint8_t

================
Comment at: ELF/OutputSections.cpp:1500
@@ -1493,2 +1499,3 @@
 
 template <class ELFT>
+VersionTableSection<ELFT>::VersionTableSection()
----------------
Please add a link to https://www.akkadia.org/drepper/symbol-versioning. I'd probably write a brief description of the .gnu.version* section formats.

================
Comment at: ELF/OutputSections.cpp:1517
@@ +1516,3 @@
+    else
+      OutVersym->vs_index = 1;
+    ++OutVersym;
----------------
We should set 0, no?

================
Comment at: ELF/OutputSections.cpp:1528
@@ +1527,3 @@
+void VersionNeedSection<ELFT>::addSymbol(SharedSymbol<ELFT> *SS) {
+  if (!SS->Verdef) {
+    SS->VersionId = 1;
----------------
Add a comment saying that this is for a non-versioned symbol and 1 is default value for that.

================
Comment at: ELF/OutputSections.cpp:1549
@@ +1548,3 @@
+
+  for (std::pair<SharedFile<ELFT> *, size_t> &P : Needed) {
+    // Create the Elf_Verneed.
----------------
OK, so this loop basically copies .GNU.version_d sections from source DSOs. I'd appreciate if you write that in comment.


http://reviews.llvm.org/D19464