[PATCH] D26754: [ELF] - Separate locals list from versions.

[Rafael Espíndola via llvm-commits]

LGTM.

I would suggest removing

  if (VerStr.empty())
    setError("locals list for anonymous version is not supported");

in a followup patch as the should now be easy.

Cheers,
Rafael



On 16 November 2016 at 13:32, George Rimar <grimar at accesssoftek.com> wrote:
> grimar created this revision.
> grimar added reviewers: ruiu, rafael.
> grimar added subscribers: llvm-commits, grimar, evgeny777.
>
> This change separates all versioned locals to be a separate list in config,
> that was suggested by Rafael and simplifies the logic a bit.
>
>
> https://reviews.llvm.org/D26754
>
> Files:
>   ELF/Config.h
>   ELF/LinkerScript.cpp
>   ELF/SymbolTable.cpp
>
>
> Index: ELF/SymbolTable.cpp
> ===================================================================
> --- ELF/SymbolTable.cpp
> +++ ELF/SymbolTable.cpp
> @@ -715,24 +715,21 @@
>
>    // First, we assign versions to exact matching symbols,
>    // i.e. version definitions not containing any glob meta-characters.
> -  for (VersionDefinition &V : Config->VersionDefinitions) {
> +  for (SymbolVersion Sym : Config->VersionScriptLocals)
> +    assignVersion(Sym, VER_NDX_LOCAL, "local");
> +  for (VersionDefinition &V : Config->VersionDefinitions)
>      for (SymbolVersion Sym : V.Globals)
>        assignVersion(Sym, V.Id, V.Name);
> -    for (SymbolVersion Sym : V.Locals)
> -      assignVersion(Sym, VER_NDX_LOCAL, "local");
> -  }
>
>    // Next, we assign versions to fuzzy matching symbols,
>    // i.e. version definitions containing glob meta-characters.
>    // Note that because the last match takes precedence over previous matches,
>    // we iterate over the definitions in the reverse order.
> -  for (size_t I = Config->VersionDefinitions.size() - 1; I != (size_t)-1; --I) {
> -    VersionDefinition &V = Config->VersionDefinitions[I];
> -    for (SymbolVersion &Ver : V.Locals)
> -      assignWildcardVersion(Ver, VER_NDX_LOCAL);
> -    for (SymbolVersion &Ver : V.Globals)
> -      assignWildcardVersion(Ver, V.Id);
> -  }
> +  for (SymbolVersion &Ver : Config->VersionScriptLocals)
> +    assignWildcardVersion(Ver, VER_NDX_LOCAL);
> +  for (size_t I = Config->VersionDefinitions.size() - 1; I != (size_t)-1; --I)
> +    for (SymbolVersion &Ver : Config->VersionDefinitions[I].Globals)
> +      assignWildcardVersion(Ver, Config->VersionDefinitions[I].Id);
>  }
>
>  template class elf::SymbolTable<ELF32LE>;
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -1844,7 +1844,7 @@
>    if (VerStr.empty())
>      setError("locals list for anonymous version is not supported");
>
> -  readSymbols(Config->VersionDefinitions.back().Locals);
> +  readSymbols(Config->VersionScriptLocals);
>  }
>
>  void ScriptParser::readVersionExtern(std::vector<SymbolVersion> *V) {
> Index: ELF/Config.h
> ===================================================================
> --- ELF/Config.h
> +++ ELF/Config.h
> @@ -62,7 +62,6 @@
>    llvm::StringRef Name;
>    size_t Id;
>    std::vector<SymbolVersion> Globals;
> -  std::vector<SymbolVersion> Locals;
>    size_t NameOff; // Offset in string table.
>  };
>
> @@ -92,6 +91,7 @@
>    std::vector<llvm::StringRef> SearchPaths;
>    std::vector<llvm::StringRef> Undefined;
>    std::vector<SymbolVersion> VersionScriptGlobals;
> +  std::vector<SymbolVersion> VersionScriptLocals;
>    std::vector<uint8_t> BuildIdVector;
>    bool AllowMultipleDefinition;
>    bool AsNeeded = false;
>
>