[PATCH] D33680: [ELF] - Resolve references properly when using .symver directive

[Rafael Avila de Espindola via llvm-commits]

LGTM

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 105086.
> grimar added a comment.
>
> - Reopening reverted patch with fix for FreeBSD failture testcase provided by Ed Maste.
>
>
> https://reviews.llvm.org/D33680
>
> Files:
>   ELF/SymbolTable.cpp
>   ELF/Symbols.cpp
>   test/ELF/version-script-symver.s
>
>
> Index: test/ELF/version-script-symver.s
> ===================================================================
> --- test/ELF/version-script-symver.s
> +++ test/ELF/version-script-symver.s
> @@ -0,0 +1,9 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
> +# RUN: ld.lld %t.o -o %t
> +
> +.global _start
> +.global bar
> +.symver _start, bar@@VERSION
> +_start:
> +  jmp bar
> Index: ELF/Symbols.cpp
> ===================================================================
> --- ELF/Symbols.cpp
> +++ ELF/Symbols.cpp
> @@ -272,7 +272,12 @@
>    }
>  
>    // It is an error if the specified version is not defined.
> -  error(toString(File) + ": symbol " + S + " has undefined version " + Verstr);
> +  // Usually version script is not provided when linking executable,
> +  // but we may still want to override a versioned symbol from DSO,
> +  // so we do not report error in this case.
> +  if (Config->Shared)
> +    error(toString(File) + ": symbol " + S + " has undefined version " +
> +          Verstr);
>  }
>  
>  Defined::Defined(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther,
> Index: ELF/SymbolTable.cpp
> ===================================================================
> --- ELF/SymbolTable.cpp
> +++ ELF/SymbolTable.cpp
> @@ -712,15 +712,31 @@
>        B->symbol()->VersionId = VersionId;
>  }
>  
> +static bool isDefaultVersion(SymbolBody *B) {
> +  return B->isInCurrentDSO() && B->getName().find("@@") != StringRef::npos;
> +}
> +
>  // This function processes version scripts by updating VersionId
>  // member of symbols.
>  template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
>    // Symbol themselves might know their versions because symbols
>    // can contain versions in the form of <name>@<version>.
> -  // Let them parse their names.
> -  if (!Config->VersionDefinitions.empty())
> -    for (Symbol *Sym : SymVector)
> -      Sym->body()->parseSymbolVersion();
> +  // Let them parse and update their names to exclude version suffix.
> +  for (Symbol *Sym : SymVector) {
> +    SymbolBody *Body = Sym->body();
> +    bool IsDefault = isDefaultVersion(Body);
> +    Body->parseSymbolVersion();
> +
> +    if (!IsDefault)
> +      continue;
> +
> +    // <name>@@<version> means the symbol is the default version. If that's the
> +    // case, the symbol is not used only to resolve <name> of version <version>
> +    // but also undefined unversioned symbols with name <name>.
> +    SymbolBody *S = find(Body->getName());
> +    if (S && S->isUndefined())
> +      S->copy(Body);
> +  }
>  
>    // Handle edge cases first.
>    handleAnonymousVersion();
>
>
> Index: test/ELF/version-script-symver.s
> ===================================================================
> --- test/ELF/version-script-symver.s
> +++ test/ELF/version-script-symver.s
> @@ -0,0 +1,9 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
> +# RUN: ld.lld %t.o -o %t
> +
> +.global _start
> +.global bar
> +.symver _start, bar@@VERSION
> +_start:
> +  jmp bar
> Index: ELF/Symbols.cpp
> ===================================================================
> --- ELF/Symbols.cpp
> +++ ELF/Symbols.cpp
> @@ -272,7 +272,12 @@
>    }
>  
>    // It is an error if the specified version is not defined.
> -  error(toString(File) + ": symbol " + S + " has undefined version " + Verstr);
> +  // Usually version script is not provided when linking executable,
> +  // but we may still want to override a versioned symbol from DSO,
> +  // so we do not report error in this case.
> +  if (Config->Shared)
> +    error(toString(File) + ": symbol " + S + " has undefined version " +
> +          Verstr);
>  }
>  
>  Defined::Defined(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther,
> Index: ELF/SymbolTable.cpp
> ===================================================================
> --- ELF/SymbolTable.cpp
> +++ ELF/SymbolTable.cpp
> @@ -712,15 +712,31 @@
>        B->symbol()->VersionId = VersionId;
>  }
>  
> +static bool isDefaultVersion(SymbolBody *B) {
> +  return B->isInCurrentDSO() && B->getName().find("@@") != StringRef::npos;
> +}
> +
>  // This function processes version scripts by updating VersionId
>  // member of symbols.
>  template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
>    // Symbol themselves might know their versions because symbols
>    // can contain versions in the form of <name>@<version>.
> -  // Let them parse their names.
> -  if (!Config->VersionDefinitions.empty())
> -    for (Symbol *Sym : SymVector)
> -      Sym->body()->parseSymbolVersion();
> +  // Let them parse and update their names to exclude version suffix.
> +  for (Symbol *Sym : SymVector) {
> +    SymbolBody *Body = Sym->body();
> +    bool IsDefault = isDefaultVersion(Body);
> +    Body->parseSymbolVersion();
> +
> +    if (!IsDefault)
> +      continue;
> +
> +    // <name>@@<version> means the symbol is the default version. If that's the
> +    // case, the symbol is not used only to resolve <name> of version <version>
> +    // but also undefined unversioned symbols with name <name>.
> +    SymbolBody *S = find(Body->getName());
> +    if (S && S->isUndefined())
> +      S->copy(Body);
> +  }
>  
>    // Handle edge cases first.
>    handleAnonymousVersion();