[ELF] - Add Id field to Version struct.

[grimar]

Summary

That helps to avoid expressions like I + 2 in code
that assigns version number to symbols.

Change was suggested by Rui Ueyama.

[grimar]

• Fixed comment.

[• rafael]

LGTM

[ruiu]

LGTM

ELF/Config.h

45

I'd move this after Name so that Name and Id (arguments passed by user) are next to each other.

ELF/SymbolListFile.cpp

88

s/by/for/

[grimar]

After rebasing I had to modify that patch. I had to introduce createSymbolVersion() method,
that creates new version entry in one place. So could this change be reviewed before I commit please ?

[• rafael]

Lgtm, but define it as

Lld::createSymbolVersion

Cheers,
Rafael

[ruiu]

LGTM

ELF/SymbolListFile.cpp

85

I'd name this defineSymbolVersion.

ELF/SymbolTable.cpp

33

This should be in a .h file.

[grimar]

In D22086#480283, @rafael wrote:

Lgtm, but define it as

Lld::createSymbolVersion

Cheers,
Rafael

Ok, after moving to .h it is under lld::elf namespace.

ELF/Config.h

 
 enum class BuildIdKind { None, Fnv1, Md5, Sha1, Hexstring };
 
 enum class UnresolvedPolicy { NoUndef, Error, Warn, Ignore };
 
 // This struct contains symbols version definition that
 // can be found in version script if it is used for link.
 struct Version {
-  Version(llvm::StringRef Name) : Name(Name) {}
+  Version(llvm::StringRef Name, size_t Id) : Name(Name), Id(Id) {}
   llvm::StringRef Name;
   std::vector<llvm::StringRef> Globals;
   size_t NameOff; // Offset in string table.
+  size_t Id;
 };
 
 // This struct contains the global configuration for the linker.
 // Most fields are direct mapping from the command line options
 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.
 struct Configuration {
   Symbol *EntrySym = nullptr;

ELF/SymbolListFile.cpp

public:
   void run();
 
 private:
   void parseVersion(StringRef Version);
   void parseLocal();
   void parseVersionSymbols(StringRef Version);
 };
 
 void VersionScriptParser::parseVersion(StringRef Version) {
   expect("{");
-  Config->SymbolVersions.push_back(elf::Version(Version));
+  // Identifiers start at 2 because 0 and 1 are reserved
+  // by VER_NDX_LOCAL and VER_NDX_GLOBAL constants.
+  Config->SymbolVersions.push_back(
+      elf::Version(Version, Config->SymbolVersions.size() + 2));
   if (peek() == "global:") {
     next();
     parseVersionSymbols(Version);
   }
   if (peek() == "local:")
     parseLocal();
   else if (peek() != "}")
     parseVersionSymbols(Version);

ELF/SymbolTable.cpp

 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::ELF;
 
 using namespace lld;
 using namespace lld::elf;
 
 // All input object files must be for the same architecture
 // (e.g. it does not make sense to link x86 object files with
 // MIPS object files.) This function checks for that error.
 template <class ELFT> static bool isCompatible(InputFile *F) {
   if (!isa<ELFFileBase<ELFT>>(F) && !isa<BitcodeFile>(F))
     return true;
   if (F->EKind == Config->EKind && F->EMachine == Config->EMachine)
     return true;
   StringRef A = F->getName();
static uint16_t getVersionId(Symbol *Sym, StringRef Name) {
   // If symbol name contains '@' or '@@' we can assign its version id right
   // here. '@@' means the default version. It is usually the most recent one.
   // VERSYM_HIDDEN flag should be set for all non-default versions.
   StringRef Version = Name.drop_front(VersionBegin + 1);
   bool Default = Version.startswith("@");
   if (Default)
     Version = Version.drop_front();
 
-  size_t I = 2;
-  for (elf::Version &V : Config->SymbolVersions) {
+  for (elf::Version &V : Config->SymbolVersions)
     if (V.Name == Version)
-      return Default ? I : (I | VERSYM_HIDDEN);
-    ++I;
-  }
+      return Default ? V.Id : (V.Id | VERSYM_HIDDEN);
+
   error("symbol " + Name + " has undefined version " + Version);
   return 0;
 }
 
 // Find an existing symbol or create and insert a new one.
 template <class ELFT>
 std::pair<Symbol *, bool> SymbolTable<ELFT>::insert(StringRef Name) {
   unsigned NumSyms = SymVector.size();
for (StringRef Name : V.Globals) {
           error("version script assignment of " + V.Name + " to symbol " +
                 Name + " failed: symbol not defined");
         continue;
       }
 
       if (B->symbol()->VersionId != VER_NDX_GLOBAL &&
           B->symbol()->VersionId != VER_NDX_LOCAL)
         warning("duplicate symbol " + Name + " in version script");
-      B->symbol()->VersionId = I + 2;
+      B->symbol()->VersionId = V.Id;
     }
   }
 
   for (size_t I = Config->SymbolVersions.size() - 1; I != (size_t)-1; --I) {
     Version &V = Config->SymbolVersions[I];
     for (StringRef Name : V.Globals) {
       if (!hasWildcard(Name))
         continue;
 
       for (SymbolBody *B : findAll(Name))
         if (B->symbol()->VersionId == VER_NDX_GLOBAL ||
             B->symbol()->VersionId == VER_NDX_LOCAL)
-          B->symbol()->VersionId = I + 2;
+          B->symbol()->VersionId = V.Id;
     }
   }
 }
 
 // Print the module names which define the notified
 // symbols provided through -y or --trace-symbol option.
 template <class ELFT> void SymbolTable<ELFT>::traceDefined() {
   for (const auto &Symbol : Config->TraceSymbol)