[PATCH] D22086: [ELF] - Add Id field to Version struct.

[Rafael Espíndola via llvm-commits]

Lgtm, but define it as

Lld::createSymbolVersion

Cheers,
Rafael

On Monday, 11 July 2016, George Rimar <grimar at accesssoftek.com> wrote:

> grimar updated this revision to Diff 63484.
> grimar added a comment.
>
> 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 ?
>
>
> http://reviews.llvm.org/D22086
>
> Files:
>   ELF/Config.h
>   ELF/SymbolListFile.cpp
>   ELF/SymbolTable.cpp
>
> Index: ELF/SymbolTable.cpp
> ===================================================================
> --- ELF/SymbolTable.cpp
> +++ ELF/SymbolTable.cpp
> @@ -30,6 +30,8 @@
>  using namespace lld;
>  using namespace lld::elf;
>
> +size_t createSymbolVersion(StringRef Version);
> +
>  // 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.
> @@ -180,20 +182,18 @@
>    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);
> +
>
>    // If we are not building shared and version script
>    // is not specified, then it is not a error, it is
>    // in common not to use script for linking executables.
>    // In this case we just create new version.
>    if (!Config->Shared && !Config->HasVersionScript) {
> -    Config->SymbolVersions.push_back(elf::Version(Version));
> -    return Default ? I : (I | VERSYM_HIDDEN);
> +    size_t Id = createSymbolVersion(Version);
> +    return Default ? Id : (Id | VERSYM_HIDDEN);
>    }
>
>    error("symbol " + Name + " has undefined version " + Version);
> @@ -608,7 +608,7 @@
>        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;
>      }
>    }
>
> @@ -621,7 +621,7 @@
>        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;
>      }
>    }
>  }
> Index: ELF/SymbolListFile.cpp
> ===================================================================
> --- ELF/SymbolListFile.cpp
> +++ ELF/SymbolListFile.cpp
> @@ -82,9 +82,17 @@
>    void parseVersionSymbols(StringRef Version);
>  };
>
> +size_t createSymbolVersion(StringRef Version) {
> +  // Identifiers start at 2 because 0 and 1 are reserved
> +  // for VER_NDX_LOCAL and VER_NDX_GLOBAL constants.
> +  size_t VersionId = Config->SymbolVersions.size() + 2;
> +  Config->SymbolVersions.push_back(elf::Version(Version, VersionId));
> +  return VersionId;
> +}
> +
>  void VersionScriptParser::parseVersion(StringRef Version) {
>    expect("{");
> -  Config->SymbolVersions.push_back(elf::Version(Version));
> +  createSymbolVersion(Version);
>    if (peek() == "global:") {
>      next();
>      parseVersionSymbols(Version);
> Index: ELF/Config.h
> ===================================================================
> --- ELF/Config.h
> +++ ELF/Config.h
> @@ -38,8 +38,9 @@
>  // 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;
> +  size_t Id;
>    std::vector<llvm::StringRef> Globals;
>    size_t NameOff; // Offset in string table.
>  };
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160711/7cc91712/attachment.html>