[Bug dynamic-link/18572] [arm] Lazy TLSDESC relocation has data race

["cvs-commit at gcc dot gnu.org" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=18572

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  659ca267360e1c1f64eea9205bb81cb5e9049908 (commit)
       via  94d2f0af157d8c02e688a315039f8044aedbcc89 (commit)
       via  28e1ddf340e205cd8e1eff58e92957318c6e9966 (commit)
       via  0ca3d1d6d096e222346c74601d50e9013c8bb25d (commit)
       via  2c1d4e5fe4e722e0b747d6bddd7ce3a6b1766c52 (commit)
       via  91c5a366d8d398d2fc4542f961c93058a92ade6f (commit)
       via  b7cf203b5c17dd6d9878537d41e0c7cc3d270a67 (commit)
       via  0cc5b022f817eeaa81735ae58717b5dabae92941 (commit)
      from  43ddff2e364c69847f5f698f6a43f9dde328b76a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=659ca267360e1c1f64eea9205bb81cb5e9049908

commit 659ca267360e1c1f64eea9205bb81cb5e9049908
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Tue Oct 24 17:49:14 2017 +0100

    aarch64: optimize _dl_tlsdesc_dynamic fast path

    Remove some load/store instructions from the dynamic tlsdesc resolver
    fast path.  This gives around 20% faster tls access in dlopened shared
    libraries (assuming glibc ran out of static tls space).

        * sysdeps/aarch64/dl-tlsdesc.S (_dl_tlsdesc_dynamic): Optimize.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=94d2f0af157d8c02e688a315039f8044aedbcc89

commit 94d2f0af157d8c02e688a315039f8044aedbcc89
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Fri Oct 20 17:53:44 2017 +0100

    arm: Remove lazy tlsdesc initialization related code

    Lazy tlsdesc initialization is no longer used in the dynamic linker
    so all related code can be removed.

        * sysdeps/arm/dl-machine.h (elf_machine_runtime_setup): Remove
        DT_TLSDESC_GOT initialization.
        * sysdeps/arm/dl-tlsdesc.S (_dl_tlsdesc_lazy_resolver): Remove.
        (_dl_tlsdesc_resolve_hold): Likewise.
        * sysdeps/aarch64/dl-tlsdesc.h (_dl_tlsdesc_lazy_resolver): Remove.
        (_dl_tlsdesc_resolve_hold): Likewise.
        * sysdeps/aarch64/tlsdesc.c (_dl_tlsdesc_lazy_resolver_fixup): Remove.
        (_dl_tlsdesc_resolve_hold_fixup): Likewise.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=28e1ddf340e205cd8e1eff58e92957318c6e9966

commit 28e1ddf340e205cd8e1eff58e92957318c6e9966
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Fri Oct 20 17:44:18 2017 +0100

    arm: Remove unnecessary volatile qualifier

    There is no reason to treat tlsdesc entries as volatile objects.

        * sysdeps/arm/dl-machine.h (elf_machine_rel): Remove volatile.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=0ca3d1d6d096e222346c74601d50e9013c8bb25d

commit 0ca3d1d6d096e222346c74601d50e9013c8bb25d
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Fri Oct 20 17:35:12 2017 +0100

    [BZ #18572] arm: Disable lazy initialization of tlsdesc entries

    Follow up to
    https://sourceware.org/ml/libc-alpha/2015-11/msg00272.html

    Always do tls descriptor initialization at load time during relocation
    processing (as if DF_BIND_NOW were set for the binary) to avoid barriers
    at every tls access.  This patch mimics bind-now semantics in the lazy
    relocation code of the arm target (elf_machine_lazy_rel).

    Ideally the static linker should be updated too to not emit tlsdesc
    relocs in DT_REL*, so elf_machine_lazy_rel is not called on them at all.

        [BZ #18572]
        * sysdeps/arm/dl-machine.h (elf_machine_lazy_rel): Do symbol binding
        non-lazily for R_ARM_TLS_DESC.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=2c1d4e5fe4e722e0b747d6bddd7ce3a6b1766c52

commit 2c1d4e5fe4e722e0b747d6bddd7ce3a6b1766c52
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Fri Oct 20 17:10:50 2017 +0100

    [BZ #17078] arm: remove prelinker support for R_ARM_TLS_DESC

    This patch reverts

    commit 9c82da17b5794efebe005de2fd22d61a3ea4b58a
    Author: Maciej W. Rozycki <macro@codesourcery.com>
    Date:   2014-07-17 19:22:05 +0100

        [BZ #17078] ARM: R_ARM_TLS_DESC prelinker support

    This only implemented support for the lazy binding case (and thus
    closed the bugzilla ticket prematurely), however tlsdesc on arm is
    not correct with lazy binding because there is a data race between
    the lazy initialization code and tlsdesc resolver functions.

    Lazy initialization of tlsdesc entries will be removed from arm to
    fix the data races and thus this half-finished prelinker support
    is no longer useful.

        [BZ #17078]
        * sysdeps/arm/dl-machine.h (elf_machine_rela): Remove the
        R_ARM_TLS_DESC case.
        (elf_machine_lazy_rel): Remove the prelink check.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=91c5a366d8d398d2fc4542f961c93058a92ade6f

commit 91c5a366d8d398d2fc4542f961c93058a92ade6f
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Wed Sep 27 18:14:21 2017 +0100

    aarch64: Remove barriers from TLS descriptor functions

    Remove ldar synchronization and most lazy TLSDESC initialization
    related code.

        * sysdeps/aarch64/dl-machine.h (elf_machine_runtime_setup): Remove
        DT_TLSDESC_GOT initialization.
        * sysdeps/aarch64/dl-tlsdesc.S (_dl_tlsdesc_return_lazy): Remove.
        (_dl_tlsdesc_resolve_rela): Likewise.
        (_dl_tlsdesc_resolve_hold): Likewise.
        (_dl_tlsdesc_undefweak): Remove ldar.
        (_dl_tlsdesc_dynamic): Likewise.
        * sysdeps/aarch64/dl-tlsdesc.h (_dl_tlsdesc_return_lazy): Remove.
        (_dl_tlsdesc_resolve_rela): Likewise.
        (_dl_tlsdesc_resolve_hold): Likewise.
        * sysdeps/aarch64/tlsdesc.c (_dl_tlsdesc_resolve_rela_fixup): Remove.
        (_dl_tlsdesc_resolve_hold_fixup): Likewise.
        (_dl_tlsdesc_resolve_rela): Likewise.
        (_dl_tlsdesc_resolve_hold): Likewise.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=b7cf203b5c17dd6d9878537d41e0c7cc3d270a67

commit b7cf203b5c17dd6d9878537d41e0c7cc3d270a67
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Wed Sep 27 16:55:14 2017 +0100

    aarch64: Disable lazy symbol binding of TLSDESC

    Always do TLS descriptor initialization at load time during relocation
    processing to avoid barriers at every TLS access. In non-dlopened shared
    libraries the overhead of tls access vs static global access is > 3x
    bigger when lazy initialization is used (_dl_tlsdesc_return_lazy)
    compared to bind-now (_dl_tlsdesc_return) so the barriers dominate tls
    access performance.

    TLSDESC relocs are in DT_JMPREL which are processed at load time using
    elf_machine_lazy_rel which is only supposed to do lightweight
    initialization using the DT_TLSDESC_PLT trampoline (the trampoline code
    jumps to the entry point in DT_TLSDESC_GOT which does the lazy tlsdesc
    initialization at runtime).  This patch changes elf_machine_lazy_rel
    in aarch64 to do the symbol binding and initialization as if DF_BIND_NOW
    was set, so the non-lazy code path of elf/do-rel.h was replicated.

    The static linker could be changed to emit TLSDESC relocs in DT_REL*,
    which are processed non-lazily, but the goal of this patch is to always
    guarantee bind-now semantics, even if the binary was produced with an
    old linker, so the barriers can be dropped in tls descriptor functions.

    After this change the synchronizing ldar instructions can be dropped
    as well as the lazy initialization machinery including the DT_TLSDESC_GOT
    setup.

    I believe this should be done on all targets, including ones where no
    barrier is needed for lazy initialization.  There is very little gain in
    optimizing for large number of symbolic tlsdesc relocations which is an
    extremely uncommon case.  And currently the tlsdesc entries are only
    readonly protected with -z now and some hardennings against writable
    JUMPSLOT relocs don't work for TLSDESC so they are a security hazard.
    (But to fix that the static linker has to be changed.)

        * sysdeps/aarch64/dl-machine.h (elf_machine_lazy_rel): Do symbol
        binding and initialization non-lazily for R_AARCH64_TLSDESC.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=0cc5b022f817eeaa81735ae58717b5dabae92941

commit 0cc5b022f817eeaa81735ae58717b5dabae92941
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Mon Oct 23 12:15:40 2017 +0100

    Mark lazy tlsdesc helper functions unused to avoid warnings

    These static functions are not needed if a target does not do lazy
    tlsdesc initialization.

        * elf/tlsdeschtab.h (_dl_tls_resolve_early_return_p): Mark unused.
        (_dl_tlsdesc_wake_up_held_fixups): Likewise.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                    |   59 ++++++++
 elf/tlsdeschtab.h            |    2 +
 sysdeps/aarch64/dl-machine.h |   23 ++--
 sysdeps/aarch64/dl-tlsdesc.S |  308 +++++++-----------------------------------
 sysdeps/aarch64/dl-tlsdesc.h |    9 --
 sysdeps/aarch64/tlsdesc.c    |  127 +-----------------
 sysdeps/arm/dl-machine.h     |   59 +++------
 sysdeps/arm/dl-tlsdesc.S     |   84 ------------
 sysdeps/arm/dl-tlsdesc.h     |    4 +-
 sysdeps/arm/tlsdesc.c        |  119 +----------------
 10 files changed, 145 insertions(+), 649 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.