[PATCH] D35207: [ELF] - Give a symbol version extracted from name a priority over version set by script.

[Rafael Avila de Espindola via llvm-commits]

LGTM


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

> grimar updated this revision to Diff 105984.
> grimar added a comment.
>
> - Addressed review comments.
>
>
> https://reviews.llvm.org/D35207
>
> Files:
>   ELF/SymbolTable.cpp
>   test/ELF/version-script-symver2.s
>
>
> Index: test/ELF/version-script-symver2.s
> ===================================================================
> --- test/ELF/version-script-symver2.s
> +++ test/ELF/version-script-symver2.s
> @@ -0,0 +1,28 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
> +# RUN: echo "VER1 { global: foo; local: *; }; VER2 { global: foo; }; VER3 { global: foo; };" > %t.map
> +# RUN: ld.lld -shared %t.o --version-script %t.map -o %t.so --fatal-warnings
> +# RUN: llvm-readobj -V %t.so | FileCheck %s
> +
> +# CHECK:      Symbols [
> +# CHECK-NEXT:   Symbol {
> +# CHECK-NEXT:     Version: 0
> +# CHECK-NEXT:     Name: @
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Symbol {
> +# CHECK-NEXT:     Version: 3
> +# CHECK-NEXT:     Name: foo@@VER2
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Symbol {
> +# CHECK-NEXT:     Version: 2
> +# CHECK-NEXT:     Name: foo at VER1
> +# CHECK-NEXT:   }
> +# CHECK-NEXT: ]
> +
> +.global bar
> +bar:
> +.symver bar, foo at VER1
> +
> +.global zed
> +zed:
> +.symver zed, foo@@VER2
> Index: ELF/SymbolTable.cpp
> ===================================================================
> --- ELF/SymbolTable.cpp
> +++ ELF/SymbolTable.cpp
> @@ -696,6 +696,11 @@
>  
>    // Assign the version.
>    for (SymbolBody *B : Syms) {
> +    // Symbols that have version baked in name will be proccessed later, see
> +    // parseSymbolVersion(). Version from name has priority over version script.
> +    if (B->getName().find('@') != StringRef::npos)
> +      continue;
> +
>      Symbol *Sym = B->symbol();
>      if (Sym->InVersionScript)
>        warn("duplicate symbol '" + Ver.Name + "' in version script");
> @@ -722,17 +727,9 @@
>  // 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 and update their names to exclude version suffix.
> -  for (Symbol *Sym : SymVector)
> -    Sym->body()->parseSymbolVersion();
> -
>    // Handle edge cases first.
>    handleAnonymousVersion();
>  
> -  if (Config->VersionDefinitions.empty())
> -    return;
>  
>    // Now we have version definitions, so we need to set version ids to symbols.
>    // Each version definition has a glob pattern, and all symbols that match
> @@ -751,6 +748,12 @@
>    for (VersionDefinition &V : llvm::reverse(Config->VersionDefinitions))
>      for (SymbolVersion &Ver : V.Globals)
>        assignWildcardVersion(Ver, V.Id);
> +
> +  // Symbol themselves might know their versions because symbols
> +  // can contain versions in the form of <name>@<version>.
> +  // Let them parse and update their names to exclude version suffix.
> +  for (Symbol *Sym : SymVector)
> +      Sym->body()->parseSymbolVersion();
>  }
>  
>  template class elf::SymbolTable<ELF32LE>;
>
>
> Index: test/ELF/version-script-symver2.s
> ===================================================================
> --- test/ELF/version-script-symver2.s
> +++ test/ELF/version-script-symver2.s
> @@ -0,0 +1,28 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
> +# RUN: echo "VER1 { global: foo; local: *; }; VER2 { global: foo; }; VER3 { global: foo; };" > %t.map
> +# RUN: ld.lld -shared %t.o --version-script %t.map -o %t.so --fatal-warnings
> +# RUN: llvm-readobj -V %t.so | FileCheck %s
> +
> +# CHECK:      Symbols [
> +# CHECK-NEXT:   Symbol {
> +# CHECK-NEXT:     Version: 0
> +# CHECK-NEXT:     Name: @
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Symbol {
> +# CHECK-NEXT:     Version: 3
> +# CHECK-NEXT:     Name: foo@@VER2
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Symbol {
> +# CHECK-NEXT:     Version: 2
> +# CHECK-NEXT:     Name: foo at VER1
> +# CHECK-NEXT:   }
> +# CHECK-NEXT: ]
> +
> +.global bar
> +bar:
> +.symver bar, foo at VER1
> +
> +.global zed
> +zed:
> +.symver zed, foo@@VER2
> Index: ELF/SymbolTable.cpp
> ===================================================================
> --- ELF/SymbolTable.cpp
> +++ ELF/SymbolTable.cpp
> @@ -696,6 +696,11 @@
>  
>    // Assign the version.
>    for (SymbolBody *B : Syms) {
> +    // Symbols that have version baked in name will be proccessed later, see
> +    // parseSymbolVersion(). Version from name has priority over version script.
> +    if (B->getName().find('@') != StringRef::npos)
> +      continue;
> +
>      Symbol *Sym = B->symbol();
>      if (Sym->InVersionScript)
>        warn("duplicate symbol '" + Ver.Name + "' in version script");
> @@ -722,17 +727,9 @@
>  // 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 and update their names to exclude version suffix.
> -  for (Symbol *Sym : SymVector)
> -    Sym->body()->parseSymbolVersion();
> -
>    // Handle edge cases first.
>    handleAnonymousVersion();
>  
> -  if (Config->VersionDefinitions.empty())
> -    return;
>  
>    // Now we have version definitions, so we need to set version ids to symbols.
>    // Each version definition has a glob pattern, and all symbols that match
> @@ -751,6 +748,12 @@
>    for (VersionDefinition &V : llvm::reverse(Config->VersionDefinitions))
>      for (SymbolVersion &Ver : V.Globals)
>        assignWildcardVersion(Ver, V.Id);
> +
> +  // Symbol themselves might know their versions because symbols
> +  // can contain versions in the form of <name>@<version>.
> +  // Let them parse and update their names to exclude version suffix.
> +  for (Symbol *Sym : SymVector)
> +      Sym->body()->parseSymbolVersion();
>  }
>  
>  template class elf::SymbolTable<ELF32LE>;