[PATCH] D21018: [ELF/WIP] - Basic versioned symbols support implemented.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: ELF/OutputSections.cpp:1431
@@ +1430,3 @@
+template <class ELFT> void VersionDefinitionSection<ELFT>::createDefs() {
+  FileDefNameOff = Out<ELFT>::DynStrTab->addString(Config->OutputFile);
+  for (Version &V : Config->SymbolVersions)
----------------
I think this should be the soname, not the output file name.

================
Comment at: ELF/OutputSections.cpp:1462
@@ +1461,3 @@
+    Verdef->vd_cnt = 1;
+    Verdef->vd_hash = IsBase ? hashSysv(Config->OutputFile)
+                             : hashSysv(Config->SymbolVersions[I - 1].Name);
----------------
Same here

================
Comment at: ELF/OutputSections.cpp:1499
@@ -1427,5 +1498,3 @@
        Out<ELFT>::DynSymTab->getSymbols()) {
-    if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(P.first))
-      OutVersym->vs_index = SS->VersionId;
-    else
-      OutVersym->vs_index = VER_NDX_GLOBAL;
+    SymbolBody *Body = P.first;
+    OutVersym->vs_index = Body->symbol()->VersionId;
----------------
Fold this variable into its use.

================
Comment at: ELF/OutputSections.h:256
@@ -235,2 +255,3 @@
 // table entry. An Elf_Versym is just a 16-bit integer that refers to a version
-// identifier defined in the .gnu.version_r section.
+// identifier defined in the .gnu.version_d section. The values 0 and 1 are
+// reserved. All other values are used for versions in the own object or in any
----------------
Either .gnu.version_r or .gnu.version_d.

================
Comment at: ELF/Symbols.cpp:269
@@ -268,3 +268,3 @@
     return false;
-  return (ExportDynamic && VersionScriptGlobal) || body()->isShared() ||
-         (body()->isUndefined() && Config->Shared);
+  return (ExportDynamic && (VersionScriptGlobal || body()->symbol()->VerDef)) ||
+         body()->isShared() || (body()->isUndefined() && Config->Shared);
----------------
This is the only place where we are using the `VerDef` and `VersionScriptGlobal` fields. Can you replace `(VersionScriptGlobal || body()->symbol()->VerDef)`  here with `VersionId != VER_NDX_LOCAL` and remove both of those fields?

(Also, `body()->symbol()->` is unnecessary here, as it just takes you back to the `Symbol` you started with.)


http://reviews.llvm.org/D21018