Re: [PATCH] i386: Use comdat instead of .gnu.linkonce for i386 setup pic register (BZ #20543)

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Feb 28, 2020 at 6:25 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> GCC has moved from using .gnu.linkonce for i386 setup pic register with
> minimum current version (as for binutils minimum binutils that support
> comdat).
>
> Trying to pinpoint when binutils has added comdat support for i686, it
> seems it was around 2004 [1].  I also checking with some ancient
> binutils older than 2.16 I see:
>
> test.o: In function `__x86.get_pc_thunk.bx':
> test.o(.text.__x86.get_pc_thunk.bx+0x0): multiple definition of `__x86.get_pc_thunk.bx'
> /usr/lib/gcc/x86_64-linux-gnu/5/../../../i386-linux-gnu/crti.o(.gnu.linkonce.t.__x86.get_pc_thunk.bx+0x0): first defined here
>
> Which seems that such version can not handle either comdat at all or
> a mix of linkonce and comdat.  For binutils 2.16.1 I am getting a
> different issue trying to link a binary with and more recent
> ctri.o (unrecognized relocation (0x2b) in section `.init', which is
> R_386_GOT32X and old binutils won't generate it anyway).
>
> So I think that either unlikely someone will use an older binutils than
> the one used to glibc and even this scenario may fail with some issue
> as the R_386_GOT32X.  Also, 2.16.1 is quite old and not really supported
> (glibc itself required 2.25).
>
> Checked on i686-linux-gnu.
>
> [1] https://gcc.gnu.org/ml/gcc/2004-05/msg00030.html
> ---
>  sysdeps/i386/sysdep.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/i386/sysdep.h b/sysdeps/i386/sysdep.h
> index b4bcd8fb6c..6094af8fec 100644
> --- a/sysdeps/i386/sysdep.h
> +++ b/sysdeps/i386/sysdep.h
> @@ -61,7 +61,7 @@ lose: SYSCALL_PIC_SETUP                                                             \
>
>  # define SETUP_PIC_REG(reg) \
>    .ifndef GET_PC_THUNK(reg);                                                 \
> -  .section .gnu.linkonce.t.GET_PC_THUNK(reg),"ax",@progbits;                 \
> +  .section .text.GET_PC_THUNK(reg),"axG",@progbits,GET_PC_THUNK(reg),comdat;  \
>    .globl GET_PC_THUNK(reg);                                                  \
>    .hidden GET_PC_THUNK(reg);                                                 \
>    .p2align 4;                                                                \
> @@ -97,7 +97,8 @@ GET_PC_THUNK(reg):                                                          \
>
>  # define SETUP_PIC_REG_STR(reg)                                                \
>    ".ifndef " GET_PC_THUNK_STR (reg) "\n"                               \
> -  ".section .gnu.linkonce.t." GET_PC_THUNK_STR (reg) ",\"ax\",@progbits\n" \
> +  ".section .text." GET_PC_THUNK_STR (reg) ",\"axG\",@progbits,"       \
> +    GET_PC_THUNK_STR (reg) ",comdat\n"                                 \
>    ".globl " GET_PC_THUNK_STR (reg) "\n"                                        \
>    ".hidden " GET_PC_THUNK_STR (reg) "\n"                               \
>    ".p2align 4\n"                                                       \
> --
> 2.17.1
>

LGTM.

Thanks.

-- 
H.J.