[PATCH] D42486: [ELF][MIPS] Ignore incorrect version definition index for _gp_disp symbol

[Rafael Avila de Espindola via llvm-commits]

LGTM

Thanks,
Rafael

Simon Atanasyan via Phabricator <reviews at reviews.llvm.org> writes:

> atanasyan updated this revision to Diff 131402.
> atanasyan added a comment.
>
> - Added FIXME tag to the comment
>
>
> Repository:
>   rLLD LLVM Linker
>
> https://reviews.llvm.org/D42486
>
> Files:
>   ELF/InputFiles.cpp
>   test/ELF/Inputs/mips-gp-dips-corrupt-ver.s
>   test/ELF/Inputs/mips-gp-dips-corrupt-ver.so
>   test/ELF/mips-gp-disp-ver.s
>
>
> Index: test/ELF/mips-gp-disp-ver.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/mips-gp-disp-ver.s
> @@ -0,0 +1,13 @@
> +# MIPS BFD linker puts _gp_disp symbol into DSO files and assigns zero
> +# version definition index to it. This value means 'unversioned local symbol'
> +# while _gp_disp is a section global symbol. We have to handle this bug
> +# in the LLD because BFD linker is used for building MIPS toolchain
> +# libraries. This test checks such handling.
> +
> +# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux %s -o %t.o
> +# RUN: ld.lld %t.o %S/Inputs/mips-gp-dips-corrupt-ver.so
> +
> +  .global __start
> +  .text
> +__start:
> +  lw     $t0, %got(foo)($gp)
> Index: test/ELF/Inputs/mips-gp-dips-corrupt-ver.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/Inputs/mips-gp-dips-corrupt-ver.s
> @@ -0,0 +1,14 @@
> +# Source file for mips-gp-dips-corrupt-ver.so
> +#
> +# % cat gpdisp.ver
> +# LLD_1.0.0 { global: foo; };
> +#
> +# % as mips-gp-dips-corrupt-ver.s -o mips-gp-dips-corrupt-ver.o
> +# % ld -shared -o mips-gp-dips-corrupt-ver.so \
> +#      --version-script gpdisp.ver mips-gp-dips-corrupt-ver.o
> +
> +  .global foo
> +  .text
> +foo:
> +  lui    $t0, %hi(_gp_disp)
> +  addi   $t0, $t0, %lo(_gp_disp)
> Index: ELF/InputFiles.cpp
> ===================================================================
> --- ELF/InputFiles.cpp
> +++ ELF/InputFiles.cpp
> @@ -856,6 +856,14 @@
>        continue;
>      }
>  
> +    if (Config->EMachine == EM_MIPS) {
> +      // FIXME: MIPS BFD linker puts _gp_disp symbol into DSO files
> +      // and incorrectly assigns VER_NDX_LOCAL to this section global
> +      // symbol. Here is a workaround for this bug.
> +      if (Versym && VersymIndex == VER_NDX_LOCAL && Name == "_gp_disp")
> +        continue;
> +    }
> +
>      const Elf_Verdef *Ver = nullptr;
>      if (VersymIndex != VER_NDX_GLOBAL) {
>        if (VersymIndex >= Verdefs.size() || VersymIndex == VER_NDX_LOCAL) {
>
>
> Index: test/ELF/mips-gp-disp-ver.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/mips-gp-disp-ver.s
> @@ -0,0 +1,13 @@
> +# MIPS BFD linker puts _gp_disp symbol into DSO files and assigns zero
> +# version definition index to it. This value means 'unversioned local symbol'
> +# while _gp_disp is a section global symbol. We have to handle this bug
> +# in the LLD because BFD linker is used for building MIPS toolchain
> +# libraries. This test checks such handling.
> +
> +# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux %s -o %t.o
> +# RUN: ld.lld %t.o %S/Inputs/mips-gp-dips-corrupt-ver.so
> +
> +  .global __start
> +  .text
> +__start:
> +  lw     $t0, %got(foo)($gp)
> Index: test/ELF/Inputs/mips-gp-dips-corrupt-ver.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/Inputs/mips-gp-dips-corrupt-ver.s
> @@ -0,0 +1,14 @@
> +# Source file for mips-gp-dips-corrupt-ver.so
> +#
> +# % cat gpdisp.ver
> +# LLD_1.0.0 { global: foo; };
> +#
> +# % as mips-gp-dips-corrupt-ver.s -o mips-gp-dips-corrupt-ver.o
> +# % ld -shared -o mips-gp-dips-corrupt-ver.so \
> +#      --version-script gpdisp.ver mips-gp-dips-corrupt-ver.o
> +
> +  .global foo
> +  .text
> +foo:
> +  lui    $t0, %hi(_gp_disp)
> +  addi   $t0, $t0, %lo(_gp_disp)
> Index: ELF/InputFiles.cpp
> ===================================================================
> --- ELF/InputFiles.cpp
> +++ ELF/InputFiles.cpp
> @@ -856,6 +856,14 @@
>        continue;
>      }
>  
> +    if (Config->EMachine == EM_MIPS) {
> +      // FIXME: MIPS BFD linker puts _gp_disp symbol into DSO files
> +      // and incorrectly assigns VER_NDX_LOCAL to this section global
> +      // symbol. Here is a workaround for this bug.
> +      if (Versym && VersymIndex == VER_NDX_LOCAL && Name == "_gp_disp")
> +        continue;
> +    }
> +
>      const Elf_Verdef *Ver = nullptr;
>      if (VersymIndex != VER_NDX_GLOBAL) {
>        if (VersymIndex >= Verdefs.size() || VersymIndex == VER_NDX_LOCAL) {