[PATCH] D21681: [ELF] - Implemented support of default/non-default symbols versions

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/SymbolTable.cpp:188
@@ +187,3 @@
+  size_t I = 2;
+  for (elf::Version &V : Config->SymbolVersions) {
+    if (V.Name == Version) {
----------------
ruiu wrote:
> This is O(n)*O(m) where n is the number of symbols with "@" and m is the number of symbols in version scripts. This is probably too inefficient. Please use a hash table.
I think you're mistaken. Point here that I am iterating over symbol versions, not over all globals for each version.
So it is sure O(n)*O(m), but m is number of versions, not the number of symbols in version script. That should be very fast.

I think there should be little amount of both versions and symbols with @.

================
Comment at: ELF/SymbolTable.cpp:211
@@ -177,6 +210,3 @@
     Sym->ExportDynamic = false;
-    if (Config->VersionScriptGlobalByDefault)
-      Sym->VersionId = VER_NDX_GLOBAL;
-    else
-      Sym->VersionId = VER_NDX_LOCAL;
+    setupVersionAttributes(Sym, Name);
     SymVector.push_back(Sym);
----------------
ruiu wrote:
> I don't see a reason to do this right here in insert(). You can add a new scan* path, no?
scanVersionScript() iterates over globals of version script file, not iterates through all symtab.
We should set Sym->VersionedName flag somewhere. If I move this code to scanVersionScript(),
I`ll need to iterate over all symtab symbols to check the names, I do not think it is acceptable.

Also please look closer on a code from method above:
```
size_t I = 2;
  for (elf::Version &V : Config->SymbolVersions) {
    if (V.Name == Version) {
```
I am iterating over SymbolVersions here and check if versioned name has the same version.
Imagine that name of symbols are "foo at version1" and "foo@@version2".
Appropriate version script can be:
```
version0{  }; 
version1{  } version1; 
version2{ global: foo; } version2;
```

If we have only script, like we have in scanVersionScript(), we can imagine that
foo can also be foo at version0. I do not think we want to bruteforce name before
doing search in symtab, so again we return to the iterating over all symtab then.

So I think doing it here is just a faster and probably fastest way.


http://reviews.llvm.org/D21681