diff mbox series

[v2] elf: Support DT_RELR relative relocation format [BZ #27924]

Message ID 20211017005020.2645717-1-maskray@google.com
State Superseded
Headers
Series [v2] elf: Support DT_RELR relative relocation format [BZ #27924] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Fangrui Song Oct. 17, 2021, 12:50 a.m. UTC 
  PIE and shared objects usually have many relative relocations. In
2017/2018, SHT_RELR/DT_RELR was proposed on
https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
usually takes 3% or smaller space than R_*_RELATIVE relocations. The
virtual memory size of a mostly statically linked PIE is typically 5~10%
smaller.

---

Notes I will not include in the submitted commit:

Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr

"pre-standard": even Solaris folks are happy with the refined generic-abi
proposal. Cary Coutant will apply the change
https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html

This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
available to all ports. I don't think the current glibc implementation
supports ia64 in an ELFCLASS32 container. That said, the style I used is
works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
64-bit.

* Chrome OS folks have carried a local patch since 2018 (latest version:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
  I.e. this feature has been battle tested.
* Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
* The Linux kernel has supported CONFIG_RELR since 2019-08
  (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
* A musl patch (by me) exists but is not applied:
  https://www.openwall.com/lists/musl/2019/03/06/3
* rtld-elf from FreeBSD 14 will support DT_RELR.

I believe upstream glibc should support DT_RELR to benefit all Linux
distributions. I filed some feature requests to get their attention:

* Gentoo: https://bugs.gentoo.org/818376
* Arch Linux: https://bugs.archlinux.org/task/72433
* Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
* Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699

As of linker support (to the best of my knowledge):

* LLD support DT_RELR.
* https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
  has a gold patch.
* GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923

I wish that GNU ld and gold maintainers can implement the feature as well :)

Tested on aarch64 and x86_64.

Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
* Fix style, simplify code
* Improve test
---
 configure              | 31 +++++++++++++++++++++++++++++++
 configure.ac           |  4 ++++
 elf/Makefile           |  4 ++++
 elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
 elf/elf.h              | 13 +++++++++++--
 elf/get-dynamic-info.h |  3 +++
 elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
 7 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-relr.c

Comments

H.J. Lu Oct. 18, 2021, 2:42 p.m. UTC | #1 
On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
<binutils@sourceware.org> wrote:
>
> PIE and shared objects usually have many relative relocations. In
> 2017/2018, SHT_RELR/DT_RELR was proposed on
> https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> virtual memory size of a mostly statically linked PIE is typically 5~10%
> smaller.
>
> ---
>
> Notes I will not include in the submitted commit:
>
> Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
>
> "pre-standard": even Solaris folks are happy with the refined generic-abi
> proposal. Cary Coutant will apply the change
> https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
>
> This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
> available to all ports. I don't think the current glibc implementation
> supports ia64 in an ELFCLASS32 container. That said, the style I used is
> works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
> 64-bit.
>
> * Chrome OS folks have carried a local patch since 2018 (latest version:
>   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
>   I.e. this feature has been battle tested.
> * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
> * The Linux kernel has supported CONFIG_RELR since 2019-08
>   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
> * A musl patch (by me) exists but is not applied:
>   https://www.openwall.com/lists/musl/2019/03/06/3
> * rtld-elf from FreeBSD 14 will support DT_RELR.
>
> I believe upstream glibc should support DT_RELR to benefit all Linux
> distributions. I filed some feature requests to get their attention:
>
> * Gentoo: https://bugs.gentoo.org/818376
> * Arch Linux: https://bugs.archlinux.org/task/72433
> * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
> * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
>
> As of linker support (to the best of my knowledge):
>
> * LLD support DT_RELR.
> * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
>   has a gold patch.
> * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
>
> I wish that GNU ld and gold maintainers can implement the feature as well :)
>
> Tested on aarch64 and x86_64.
>
> Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
> * Fix style, simplify code
> * Improve test
> ---
>  configure              | 31 +++++++++++++++++++++++++++++++
>  configure.ac           |  4 ++++
>  elf/Makefile           |  4 ++++
>  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
>  elf/elf.h              | 13 +++++++++++--
>  elf/get-dynamic-info.h |  3 +++
>  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
>  7 files changed, 108 insertions(+), 2 deletions(-)
>  create mode 100644 elf/tst-relr.c
>
> diff --git a/configure b/configure
> index 3227e434d3..fdab6a97ef 100755
> --- a/configure
> +++ b/configure
> @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
>  config_vars="$config_vars
>  have-depaudit = $libc_cv_depaudit"
>
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
> +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
> +libc_linker_feature=no
> +if test x"$gnu_ld" = x"yes"; then
> +  cat > conftest.c <<EOF
> +int _start (void) { return 42; }
> +EOF
> +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
> +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
> +                   -fPIC -shared -o conftest.so conftest.c
> +                   1>&5'
> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> +  (eval $ac_try) 2>&5
> +  ac_status=$?
> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> +  test $ac_status = 0; }; }
> +  then
> +    libc_linker_feature=yes
> +  fi
> +  rm -f conftest*
> +fi
> +if test $libc_linker_feature = yes; then
> +  libc_cv_relr=yes
> +else
> +  libc_cv_relr=no
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
> +$as_echo "$libc_linker_feature" >&6; }
> +config_vars="$config_vars
> +have-relr = $libc_cv_relr"
> +
>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
>  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
>  libc_linker_feature=no
> diff --git a/configure.ac b/configure.ac
> index 00f49f09f7..96110f9d7d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
>                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
>  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
>
> +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
> +                   [libc_cv_relr=yes], [libc_cv_relr=no])
> +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
> +
>  LIBC_LINKER_FEATURE([--no-dynamic-linker],
>                     [-Wl,--no-dynamic-linker],
>                     [libc_cv_no_dynamic_linker=yes],
> diff --git a/elf/Makefile b/elf/Makefile
> index bf45d8ee24..2c4cdfac68 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
>                  $(objpfx)tst-audit16-cmp.out
>  endif
>  endif
> +ifeq ($(have-relr),yes)
> +tests += tst-relr
> +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> +endif
>  endif

Is DT_RELR only generated for PIE? If yes, you need to add it
to tests-pie and compile it as PIE.

[hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
-Wl,--pack-dyn-relocs=relr -fuse-ld=lld
[hjl@gnu-cfl-2 tmp]$ ./a.out
Segmentation fault (core dumped)
[hjl@gnu-cfl-2 tmp]$

Given that the current lld implementation generates broken
binaries for existing glibc without any warning at run-time,
we should use a different linker command line option to
implement it properly so that the new binary will fail to
run on glibc without DT_RELR support at run-time.
Fangrui Song Oct. 18, 2021, 4:16 p.m. UTC | #2 
On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
> <binutils@sourceware.org> wrote:
> >
> > PIE and shared objects usually have many relative relocations. In
> > 2017/2018, SHT_RELR/DT_RELR was proposed on
> > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> > virtual memory size of a mostly statically linked PIE is typically 5~10%
> > smaller.
> >
> > ---
> >
> > Notes I will not include in the submitted commit:
> >
> > Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
> >
> > "pre-standard": even Solaris folks are happy with the refined generic-abi
> > proposal. Cary Coutant will apply the change
> > https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
> >
> > This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
> > available to all ports. I don't think the current glibc implementation
> > supports ia64 in an ELFCLASS32 container. That said, the style I used is
> > works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
> > 64-bit.
> >
> > * Chrome OS folks have carried a local patch since 2018 (latest version:
> >   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
> >   I.e. this feature has been battle tested.
> > * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
> > * The Linux kernel has supported CONFIG_RELR since 2019-08
> >   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
> > * A musl patch (by me) exists but is not applied:
> >   https://www.openwall.com/lists/musl/2019/03/06/3
> > * rtld-elf from FreeBSD 14 will support DT_RELR.
> >
> > I believe upstream glibc should support DT_RELR to benefit all Linux
> > distributions. I filed some feature requests to get their attention:
> >
> > * Gentoo: https://bugs.gentoo.org/818376
> > * Arch Linux: https://bugs.archlinux.org/task/72433
> > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
> > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
> >
> > As of linker support (to the best of my knowledge):
> >
> > * LLD support DT_RELR.
> > * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
> >   has a gold patch.
> > * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
> >
> > I wish that GNU ld and gold maintainers can implement the feature as well :)
> >
> > Tested on aarch64 and x86_64.
> >
> > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
> > * Fix style, simplify code
> > * Improve test
> > ---
> >  configure              | 31 +++++++++++++++++++++++++++++++
> >  configure.ac           |  4 ++++
> >  elf/Makefile           |  4 ++++
> >  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
> >  elf/elf.h              | 13 +++++++++++--
> >  elf/get-dynamic-info.h |  3 +++
> >  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
> >  7 files changed, 108 insertions(+), 2 deletions(-)
> >  create mode 100644 elf/tst-relr.c
> >
> > diff --git a/configure b/configure
> > index 3227e434d3..fdab6a97ef 100755
> > --- a/configure
> > +++ b/configure
> > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
> >  config_vars="$config_vars
> >  have-depaudit = $libc_cv_depaudit"
> >
> > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
> > +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
> > +libc_linker_feature=no
> > +if test x"$gnu_ld" = x"yes"; then
> > +  cat > conftest.c <<EOF
> > +int _start (void) { return 42; }
> > +EOF
> > +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
> > +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
> > +                   -fPIC -shared -o conftest.so conftest.c
> > +                   1>&5'
> > +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> > +  (eval $ac_try) 2>&5
> > +  ac_status=$?
> > +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> > +  test $ac_status = 0; }; }
> > +  then
> > +    libc_linker_feature=yes
> > +  fi
> > +  rm -f conftest*
> > +fi
> > +if test $libc_linker_feature = yes; then
> > +  libc_cv_relr=yes
> > +else
> > +  libc_cv_relr=no
> > +fi
> > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
> > +$as_echo "$libc_linker_feature" >&6; }
> > +config_vars="$config_vars
> > +have-relr = $libc_cv_relr"
> > +
> >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
> >  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
> >  libc_linker_feature=no
> > diff --git a/configure.ac b/configure.ac
> > index 00f49f09f7..96110f9d7d 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
> >                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
> >  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
> >
> > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
> > +                   [libc_cv_relr=yes], [libc_cv_relr=no])
> > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
> > +
> >  LIBC_LINKER_FEATURE([--no-dynamic-linker],
> >                     [-Wl,--no-dynamic-linker],
> >                     [libc_cv_no_dynamic_linker=yes],
> > diff --git a/elf/Makefile b/elf/Makefile
> > index bf45d8ee24..2c4cdfac68 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> >                  $(objpfx)tst-audit16-cmp.out
> >  endif
> >  endif
> > +ifeq ($(have-relr),yes)
> > +tests += tst-relr
> > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> > +endif
> >  endif
>
> Is DT_RELR only generated for PIE? If yes, you need to add it
> to tests-pie and compile it as PIE.

PIE and shared objects. PDE doesn't need relative relocations.
It is useful to ensure that -Wl,--pack-dyn-relocs=relr doesn't cause
breakage to PDE.

> [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
> -Wl,--pack-dyn-relocs=relr -fuse-ld=lld
> [hjl@gnu-cfl-2 tmp]$ ./a.out
> Segmentation fault (core dumped)
> [hjl@gnu-cfl-2 tmp]$
>
> Given that the current lld implementation generates broken
> binaries for existing glibc without any warning at run-time,
> we should use a different linker command line option to
> implement it properly so that the new binary will fail to
> run on glibc without DT_RELR support at run-time.

I don't think so. LLD's design is to be machine agnostic and NOT make
decisions which would vary on different machines.
--pack-dyn-relocs=relr is not the default. The user tells LLD to use
DT_RELR and the user is responsible for making sure target ld.so
supports DT_RELR.
LLD is often used as a cross linker. The host ld.so doesn't support
LLD doesn't mean that LLD should disable the format.
For example, it is totally fine to link a FreeBSD executable on a Linux machine.
H.J. Lu Oct. 18, 2021, 5:28 p.m. UTC | #3 
On Mon, Oct 18, 2021 at 9:17 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
> > <binutils@sourceware.org> wrote:
> > >
> > > PIE and shared objects usually have many relative relocations. In
> > > 2017/2018, SHT_RELR/DT_RELR was proposed on
> > > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> > > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> > > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> > > virtual memory size of a mostly statically linked PIE is typically 5~10%
> > > smaller.
> > >
> > > ---
> > >
> > > Notes I will not include in the submitted commit:
> > >
> > > Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
> > >
> > > "pre-standard": even Solaris folks are happy with the refined generic-abi
> > > proposal. Cary Coutant will apply the change
> > > https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
> > >
> > > This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
> > > available to all ports. I don't think the current glibc implementation
> > > supports ia64 in an ELFCLASS32 container. That said, the style I used is
> > > works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
> > > 64-bit.
> > >
> > > * Chrome OS folks have carried a local patch since 2018 (latest version:
> > >   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
> > >   I.e. this feature has been battle tested.
> > > * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
> > > * The Linux kernel has supported CONFIG_RELR since 2019-08
> > >   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
> > > * A musl patch (by me) exists but is not applied:
> > >   https://www.openwall.com/lists/musl/2019/03/06/3
> > > * rtld-elf from FreeBSD 14 will support DT_RELR.
> > >
> > > I believe upstream glibc should support DT_RELR to benefit all Linux
> > > distributions. I filed some feature requests to get their attention:
> > >
> > > * Gentoo: https://bugs.gentoo.org/818376
> > > * Arch Linux: https://bugs.archlinux.org/task/72433
> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
> > >
> > > As of linker support (to the best of my knowledge):
> > >
> > > * LLD support DT_RELR.
> > > * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
> > >   has a gold patch.
> > > * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
> > >
> > > I wish that GNU ld and gold maintainers can implement the feature as well :)
> > >
> > > Tested on aarch64 and x86_64.
> > >
> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
> > > * Fix style, simplify code
> > > * Improve test
> > > ---
> > >  configure              | 31 +++++++++++++++++++++++++++++++
> > >  configure.ac           |  4 ++++
> > >  elf/Makefile           |  4 ++++
> > >  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
> > >  elf/elf.h              | 13 +++++++++++--
> > >  elf/get-dynamic-info.h |  3 +++
> > >  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
> > >  7 files changed, 108 insertions(+), 2 deletions(-)
> > >  create mode 100644 elf/tst-relr.c
> > >
> > > diff --git a/configure b/configure
> > > index 3227e434d3..fdab6a97ef 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
> > >  config_vars="$config_vars
> > >  have-depaudit = $libc_cv_depaudit"
> > >
> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
> > > +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
> > > +libc_linker_feature=no
> > > +if test x"$gnu_ld" = x"yes"; then
> > > +  cat > conftest.c <<EOF
> > > +int _start (void) { return 42; }
> > > +EOF
> > > +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
> > > +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
> > > +                   -fPIC -shared -o conftest.so conftest.c
> > > +                   1>&5'
> > > +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> > > +  (eval $ac_try) 2>&5
> > > +  ac_status=$?
> > > +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> > > +  test $ac_status = 0; }; }
> > > +  then
> > > +    libc_linker_feature=yes
> > > +  fi
> > > +  rm -f conftest*
> > > +fi
> > > +if test $libc_linker_feature = yes; then
> > > +  libc_cv_relr=yes
> > > +else
> > > +  libc_cv_relr=no
> > > +fi
> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
> > > +$as_echo "$libc_linker_feature" >&6; }
> > > +config_vars="$config_vars
> > > +have-relr = $libc_cv_relr"
> > > +
> > >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
> > >  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
> > >  libc_linker_feature=no
> > > diff --git a/configure.ac b/configure.ac
> > > index 00f49f09f7..96110f9d7d 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
> > >                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
> > >  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
> > >
> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
> > > +                   [libc_cv_relr=yes], [libc_cv_relr=no])
> > > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
> > > +
> > >  LIBC_LINKER_FEATURE([--no-dynamic-linker],
> > >                     [-Wl,--no-dynamic-linker],
> > >                     [libc_cv_no_dynamic_linker=yes],
> > > diff --git a/elf/Makefile b/elf/Makefile
> > > index bf45d8ee24..2c4cdfac68 100644
> > > --- a/elf/Makefile
> > > +++ b/elf/Makefile
> > > @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> > >                  $(objpfx)tst-audit16-cmp.out
> > >  endif
> > >  endif
> > > +ifeq ($(have-relr),yes)
> > > +tests += tst-relr
> > > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> > > +endif
> > >  endif
> >
> > Is DT_RELR only generated for PIE? If yes, you need to add it
> > to tests-pie and compile it as PIE.
>
> PIE and shared objects. PDE doesn't need relative relocations.
> It is useful to ensure that -Wl,--pack-dyn-relocs=relr doesn't cause
> breakage to PDE.

Then you need to add a PIE test to verify that

1. It has DT_RELR.
2. It runs correctly.

> > [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
> > -Wl,--pack-dyn-relocs=relr -fuse-ld=lld
> > [hjl@gnu-cfl-2 tmp]$ ./a.out
> > Segmentation fault (core dumped)
> > [hjl@gnu-cfl-2 tmp]$
> >
> > Given that the current lld implementation generates broken
> > binaries for existing glibc without any warning at run-time,
> > we should use a different linker command line option to
> > implement it properly so that the new binary will fail to
> > run on glibc without DT_RELR support at run-time.
>
> I don't think so. LLD's design is to be machine agnostic and NOT make
> decisions which would vary on different machines.
> --pack-dyn-relocs=relr is not the default. The user tells LLD to use
> DT_RELR and the user is responsible for making sure target ld.so
> supports DT_RELR.
> LLD is often used as a cross linker. The host ld.so doesn't support
> LLD doesn't mean that LLD should disable the format.
> For example, it is totally fine to link a FreeBSD executable on a Linux machine.

For Linux targets, we need the glibc version dependency for
DT_RELR support at run-time.
Fangrui Song Oct. 18, 2021, 6:15 p.m. UTC | #4 
On 2021-10-18, H.J. Lu wrote:
>On Mon, Oct 18, 2021 at 9:17 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>>
>> On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>> >
>> > On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
>> > <binutils@sourceware.org> wrote:
>> > >
>> > > PIE and shared objects usually have many relative relocations. In
>> > > 2017/2018, SHT_RELR/DT_RELR was proposed on
>> > > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
>> > > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
>> > > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
>> > > virtual memory size of a mostly statically linked PIE is typically 5~10%
>> > > smaller.
>> > >
>> > > ---
>> > >
>> > > Notes I will not include in the submitted commit:
>> > >
>> > > Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
>> > >
>> > > "pre-standard": even Solaris folks are happy with the refined generic-abi
>> > > proposal. Cary Coutant will apply the change
>> > > https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
>> > >
>> > > This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
>> > > available to all ports. I don't think the current glibc implementation
>> > > supports ia64 in an ELFCLASS32 container. That said, the style I used is
>> > > works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
>> > > 64-bit.
>> > >
>> > > * Chrome OS folks have carried a local patch since 2018 (latest version:
>> > >   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
>> > >   I.e. this feature has been battle tested.
>> > > * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
>> > > * The Linux kernel has supported CONFIG_RELR since 2019-08
>> > >   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
>> > > * A musl patch (by me) exists but is not applied:
>> > >   https://www.openwall.com/lists/musl/2019/03/06/3
>> > > * rtld-elf from FreeBSD 14 will support DT_RELR.
>> > >
>> > > I believe upstream glibc should support DT_RELR to benefit all Linux
>> > > distributions. I filed some feature requests to get their attention:
>> > >
>> > > * Gentoo: https://bugs.gentoo.org/818376
>> > > * Arch Linux: https://bugs.archlinux.org/task/72433
>> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
>> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
>> > >
>> > > As of linker support (to the best of my knowledge):
>> > >
>> > > * LLD support DT_RELR.
>> > > * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
>> > >   has a gold patch.
>> > > * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
>> > >
>> > > I wish that GNU ld and gold maintainers can implement the feature as well :)
>> > >
>> > > Tested on aarch64 and x86_64.
>> > >
>> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
>> > > * Fix style, simplify code
>> > > * Improve test
>> > > ---
>> > >  configure              | 31 +++++++++++++++++++++++++++++++
>> > >  configure.ac           |  4 ++++
>> > >  elf/Makefile           |  4 ++++
>> > >  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
>> > >  elf/elf.h              | 13 +++++++++++--
>> > >  elf/get-dynamic-info.h |  3 +++
>> > >  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
>> > >  7 files changed, 108 insertions(+), 2 deletions(-)
>> > >  create mode 100644 elf/tst-relr.c
>> > >
>> > > diff --git a/configure b/configure
>> > > index 3227e434d3..fdab6a97ef 100755
>> > > --- a/configure
>> > > +++ b/configure
>> > > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
>> > >  config_vars="$config_vars
>> > >  have-depaudit = $libc_cv_depaudit"
>> > >
>> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
>> > > +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
>> > > +libc_linker_feature=no
>> > > +if test x"$gnu_ld" = x"yes"; then
>> > > +  cat > conftest.c <<EOF
>> > > +int _start (void) { return 42; }
>> > > +EOF
>> > > +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
>> > > +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
>> > > +                   -fPIC -shared -o conftest.so conftest.c
>> > > +                   1>&5'
>> > > +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>> > > +  (eval $ac_try) 2>&5
>> > > +  ac_status=$?
>> > > +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>> > > +  test $ac_status = 0; }; }
>> > > +  then
>> > > +    libc_linker_feature=yes
>> > > +  fi
>> > > +  rm -f conftest*
>> > > +fi
>> > > +if test $libc_linker_feature = yes; then
>> > > +  libc_cv_relr=yes
>> > > +else
>> > > +  libc_cv_relr=no
>> > > +fi
>> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
>> > > +$as_echo "$libc_linker_feature" >&6; }
>> > > +config_vars="$config_vars
>> > > +have-relr = $libc_cv_relr"
>> > > +
>> > >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
>> > >  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
>> > >  libc_linker_feature=no
>> > > diff --git a/configure.ac b/configure.ac
>> > > index 00f49f09f7..96110f9d7d 100644
>> > > --- a/configure.ac
>> > > +++ b/configure.ac
>> > > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
>> > >                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
>> > >  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
>> > >
>> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
>> > > +                   [libc_cv_relr=yes], [libc_cv_relr=no])
>> > > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
>> > > +
>> > >  LIBC_LINKER_FEATURE([--no-dynamic-linker],
>> > >                     [-Wl,--no-dynamic-linker],
>> > >                     [libc_cv_no_dynamic_linker=yes],
>> > > diff --git a/elf/Makefile b/elf/Makefile
>> > > index bf45d8ee24..2c4cdfac68 100644
>> > > --- a/elf/Makefile
>> > > +++ b/elf/Makefile
>> > > @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
>> > >                  $(objpfx)tst-audit16-cmp.out
>> > >  endif
>> > >  endif
>> > > +ifeq ($(have-relr),yes)
>> > > +tests += tst-relr
>> > > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
>> > > +endif
>> > >  endif
>> >
>> > Is DT_RELR only generated for PIE? If yes, you need to add it
>> > to tests-pie and compile it as PIE.
>>
>> PIE and shared objects. PDE doesn't need relative relocations.
>> It is useful to ensure that -Wl,--pack-dyn-relocs=relr doesn't cause
>> breakage to PDE.
>
>Then you need to add a PIE test to verify that
>
>1. It has DT_RELR.
>2. It runs correctly.

--- a/elf/Makefile
+++ b/elf/Makefile
@@ -246,4 +246,8 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
  endif
  endif
+ifeq ($(have-relr),yes)
+tests += tst-relr
+LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
+endif
  endif
  tests += $(tests-execstack-$(have-z-execstack))

I added this chunk to my test (https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr):

   ElfW(Dyn) *d = _DYNAMIC;
   if (d)
     {
       bool has_relr = false;
       for (; d->d_tag != DT_NULL; d++)
        if (d->d_tag == DT_RELR)
          has_relr = true;
       if (!has_relr)
        {
          fprintf (stderr, "no DT_RELR\n");
          return 1;
        }
     }

>> > [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
>> > -Wl,--pack-dyn-relocs=relr -fuse-ld=lld
>> > [hjl@gnu-cfl-2 tmp]$ ./a.out
>> > Segmentation fault (core dumped)
>> > [hjl@gnu-cfl-2 tmp]$
>> >
>> > Given that the current lld implementation generates broken
>> > binaries for existing glibc without any warning at run-time,
>> > we should use a different linker command line option to
>> > implement it properly so that the new binary will fail to
>> > run on glibc without DT_RELR support at run-time.
>>
>> I don't think so. LLD's design is to be machine agnostic and NOT make
>> decisions which would vary on different machines.
>> --pack-dyn-relocs=relr is not the default. The user tells LLD to use
>> DT_RELR and the user is responsible for making sure target ld.so
>> supports DT_RELR.
>> LLD is often used as a cross linker. The host ld.so doesn't support
>> LLD doesn't mean that LLD should disable the format.
>> For example, it is totally fine to link a FreeBSD executable on a Linux machine.
>
>For Linux targets, we need the glibc version dependency for
>DT_RELR support at run-time.

I know that having an executable crash right away (SIGSEGV, if ld.so doesn't
resolve relative relocations) without a diagnostic is arguably poor user experience,
and you might come from this angle and therefore suggest a version check.

However, this tight libc coupling is not LLD's usual design decision.
LLD doesn't want to understand what libc it needs to work with.

The user experience problem is related to the ELF spirit that we don't want to
unnecessarily error for unknown sections/tags/etc. In this case, the evolvement
of a foundamental feature (relocation) has to be incompatible with older loaders.
That compatibility check could be implemented in many ways, with their own pros and cons.
For example, allocating a range of dynamic tags saying that unknown tags in this range
are hard errors. However, such a foundamental evolution is rare. I tend to think we just
accept the status quo and do nothing on it. --pack-dyn-relocs=relr users, at least
from the time being, understand that they need new loaders for the relatively new feature.

The problem can be moot when users use --pack-dyn-relocs=relr with new glibc.
The linked binary will get a high version requirement and running with older glibc
will be detected right away. I just think it's not linker's responsibility to synthesize
a symbol targeting a specific glibc version.

-z relr=glibc targeting glibc and -z relr=none targeting non-glibc? That'd just
be unneeded additional complexity.
If GNU ld finally decides to proceed with the -z relr=* road, I can consider
adding a -z relr compatibility option to LLD as well, but I may not synthesize a symbol.
It's complexity with relatively little benefit IMHO.
H.J. Lu Oct. 18, 2021, 6:27 p.m. UTC | #5 
On Mon, Oct 18, 2021 at 11:15 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>
>
> On 2021-10-18, H.J. Lu wrote:
> >On Mon, Oct 18, 2021 at 9:17 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >>
> >> On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >> >
> >> > On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
> >> > <binutils@sourceware.org> wrote:
> >> > >
> >> > > PIE and shared objects usually have many relative relocations. In
> >> > > 2017/2018, SHT_RELR/DT_RELR was proposed on
> >> > > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> >> > > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> >> > > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> >> > > virtual memory size of a mostly statically linked PIE is typically 5~10%
> >> > > smaller.
> >> > >
> >> > > ---
> >> > >
> >> > > Notes I will not include in the submitted commit:
> >> > >
> >> > > Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
> >> > >
> >> > > "pre-standard": even Solaris folks are happy with the refined generic-abi
> >> > > proposal. Cary Coutant will apply the change
> >> > > https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
> >> > >
> >> > > This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
> >> > > available to all ports. I don't think the current glibc implementation
> >> > > supports ia64 in an ELFCLASS32 container. That said, the style I used is
> >> > > works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
> >> > > 64-bit.
> >> > >
> >> > > * Chrome OS folks have carried a local patch since 2018 (latest version:
> >> > >   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
> >> > >   I.e. this feature has been battle tested.
> >> > > * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
> >> > > * The Linux kernel has supported CONFIG_RELR since 2019-08
> >> > >   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
> >> > > * A musl patch (by me) exists but is not applied:
> >> > >   https://www.openwall.com/lists/musl/2019/03/06/3
> >> > > * rtld-elf from FreeBSD 14 will support DT_RELR.
> >> > >
> >> > > I believe upstream glibc should support DT_RELR to benefit all Linux
> >> > > distributions. I filed some feature requests to get their attention:
> >> > >
> >> > > * Gentoo: https://bugs.gentoo.org/818376
> >> > > * Arch Linux: https://bugs.archlinux.org/task/72433
> >> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
> >> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
> >> > >
> >> > > As of linker support (to the best of my knowledge):
> >> > >
> >> > > * LLD support DT_RELR.
> >> > > * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
> >> > >   has a gold patch.
> >> > > * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
> >> > >
> >> > > I wish that GNU ld and gold maintainers can implement the feature as well :)
> >> > >
> >> > > Tested on aarch64 and x86_64.
> >> > >
> >> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
> >> > > * Fix style, simplify code
> >> > > * Improve test
> >> > > ---
> >> > >  configure              | 31 +++++++++++++++++++++++++++++++
> >> > >  configure.ac           |  4 ++++
> >> > >  elf/Makefile           |  4 ++++
> >> > >  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
> >> > >  elf/elf.h              | 13 +++++++++++--
> >> > >  elf/get-dynamic-info.h |  3 +++
> >> > >  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
> >> > >  7 files changed, 108 insertions(+), 2 deletions(-)
> >> > >  create mode 100644 elf/tst-relr.c
> >> > >
> >> > > diff --git a/configure b/configure
> >> > > index 3227e434d3..fdab6a97ef 100755
> >> > > --- a/configure
> >> > > +++ b/configure
> >> > > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
> >> > >  config_vars="$config_vars
> >> > >  have-depaudit = $libc_cv_depaudit"
> >> > >
> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
> >> > > +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
> >> > > +libc_linker_feature=no
> >> > > +if test x"$gnu_ld" = x"yes"; then
> >> > > +  cat > conftest.c <<EOF
> >> > > +int _start (void) { return 42; }
> >> > > +EOF
> >> > > +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
> >> > > +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
> >> > > +                   -fPIC -shared -o conftest.so conftest.c
> >> > > +                   1>&5'
> >> > > +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> >> > > +  (eval $ac_try) 2>&5
> >> > > +  ac_status=$?
> >> > > +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> >> > > +  test $ac_status = 0; }; }
> >> > > +  then
> >> > > +    libc_linker_feature=yes
> >> > > +  fi
> >> > > +  rm -f conftest*
> >> > > +fi
> >> > > +if test $libc_linker_feature = yes; then
> >> > > +  libc_cv_relr=yes
> >> > > +else
> >> > > +  libc_cv_relr=no
> >> > > +fi
> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
> >> > > +$as_echo "$libc_linker_feature" >&6; }
> >> > > +config_vars="$config_vars
> >> > > +have-relr = $libc_cv_relr"
> >> > > +
> >> > >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
> >> > >  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
> >> > >  libc_linker_feature=no
> >> > > diff --git a/configure.ac b/configure.ac
> >> > > index 00f49f09f7..96110f9d7d 100644
> >> > > --- a/configure.ac
> >> > > +++ b/configure.ac
> >> > > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
> >> > >                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
> >> > >  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
> >> > >
> >> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
> >> > > +                   [libc_cv_relr=yes], [libc_cv_relr=no])
> >> > > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
> >> > > +
> >> > >  LIBC_LINKER_FEATURE([--no-dynamic-linker],
> >> > >                     [-Wl,--no-dynamic-linker],
> >> > >                     [libc_cv_no_dynamic_linker=yes],
> >> > > diff --git a/elf/Makefile b/elf/Makefile
> >> > > index bf45d8ee24..2c4cdfac68 100644
> >> > > --- a/elf/Makefile
> >> > > +++ b/elf/Makefile
> >> > > @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> >> > >                  $(objpfx)tst-audit16-cmp.out
> >> > >  endif
> >> > >  endif
> >> > > +ifeq ($(have-relr),yes)
> >> > > +tests += tst-relr
> >> > > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> >> > > +endif
> >> > >  endif
> >> >
> >> > Is DT_RELR only generated for PIE? If yes, you need to add it
> >> > to tests-pie and compile it as PIE.
> >>
> >> PIE and shared objects. PDE doesn't need relative relocations.
> >> It is useful to ensure that -Wl,--pack-dyn-relocs=relr doesn't cause
> >> breakage to PDE.
> >
> >Then you need to add a PIE test to verify that
> >
> >1. It has DT_RELR.
> >2. It runs correctly.
>
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -246,4 +246,8 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
>   endif
>   endif
> +ifeq ($(have-relr),yes)
> +tests += tst-relr
> +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> +endif
>   endif
>   tests += $(tests-execstack-$(have-z-execstack))
>
> I added this chunk to my test (https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr):
>
>    ElfW(Dyn) *d = _DYNAMIC;
>    if (d)
>      {
>        bool has_relr = false;
>        for (; d->d_tag != DT_NULL; d++)
>         if (d->d_tag == DT_RELR)
>           has_relr = true;
>        if (!has_relr)
>         {
>           fprintf (stderr, "no DT_RELR\n");
>           return 1;
>         }
>      }

We need 2 tests from the same source:

1. The DT_RELR linker option doesn't break PDE.
2.  The DT_RELR linker option works for PIE.

You only have one test.

> >> > [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
> >> > -Wl,--pack-dyn-relocs=relr -fuse-ld=lld
> >> > [hjl@gnu-cfl-2 tmp]$ ./a.out
> >> > Segmentation fault (core dumped)
> >> > [hjl@gnu-cfl-2 tmp]$
> >> >
> >> > Given that the current lld implementation generates broken
> >> > binaries for existing glibc without any warning at run-time,
> >> > we should use a different linker command line option to
> >> > implement it properly so that the new binary will fail to
> >> > run on glibc without DT_RELR support at run-time.
> >>
> >> I don't think so. LLD's design is to be machine agnostic and NOT make
> >> decisions which would vary on different machines.
> >> --pack-dyn-relocs=relr is not the default. The user tells LLD to use
> >> DT_RELR and the user is responsible for making sure target ld.so
> >> supports DT_RELR.
> >> LLD is often used as a cross linker. The host ld.so doesn't support
> >> LLD doesn't mean that LLD should disable the format.
> >> For example, it is totally fine to link a FreeBSD executable on a Linux machine.
> >
> >For Linux targets, we need the glibc version dependency for
> >DT_RELR support at run-time.
>
> I know that having an executable crash right away (SIGSEGV, if ld.so doesn't
> resolve relative relocations) without a diagnostic is arguably poor user experience,
> and you might come from this angle and therefore suggest a version check.

Correct.  We need to resolve this before DT_RELR can be accepted.

> However, this tight libc coupling is not LLD's usual design decision.
> LLD doesn't want to understand what libc it needs to work with.
>
> The user experience problem is related to the ELF spirit that we don't want to
> unnecessarily error for unknown sections/tags/etc. In this case, the evolvement
> of a foundamental feature (relocation) has to be incompatible with older loaders.
> That compatibility check could be implemented in many ways, with their own pros and cons.
> For example, allocating a range of dynamic tags saying that unknown tags in this range
> are hard errors. However, such a foundamental evolution is rare. I tend to think we just
> accept the status quo and do nothing on it. --pack-dyn-relocs=relr users, at least
> from the time being, understand that they need new loaders for the relatively new feature.

It may be OK for some use cases.   I don't think it is acceptable for all use
cases which glibc cares about.

>
> The problem can be moot when users use --pack-dyn-relocs=relr with new glibc.
> The linked binary will get a high version requirement and running with older glibc
> will be detected right away. I just think it's not linker's responsibility to synthesize
> a symbol targeting a specific glibc version.
>
> -z relr=glibc targeting glibc and -z relr=none targeting non-glibc? That'd just
> be unneeded additional complexity.
> If GNU ld finally decides to proceed with the -z relr=* road, I can consider
> adding a -z relr compatibility option to LLD as well, but I may not synthesize a symbol.
> It's complexity with relatively little benefit IMHO.
Fangrui Song Oct. 18, 2021, 7:19 p.m. UTC | #6 
On 2021-10-18, H.J. Lu wrote:
>On Mon, Oct 18, 2021 at 11:15 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>>
>>
>> On 2021-10-18, H.J. Lu wrote:
>> >On Mon, Oct 18, 2021 at 9:17 AM Fāng-ruì Sòng <maskray@google.com> wrote:
>> >>
>> >> On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>> >> >
>> >> > On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
>> >> > <binutils@sourceware.org> wrote:
>> >> > >
>> >> > > PIE and shared objects usually have many relative relocations. In
>> >> > > 2017/2018, SHT_RELR/DT_RELR was proposed on
>> >> > > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
>> >> > > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
>> >> > > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
>> >> > > virtual memory size of a mostly statically linked PIE is typically 5~10%
>> >> > > smaller.
>> >> > >
>> >> > > ---
>> >> > >
>> >> > > Notes I will not include in the submitted commit:
>> >> > >
>> >> > > Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
>> >> > >
>> >> > > "pre-standard": even Solaris folks are happy with the refined generic-abi
>> >> > > proposal. Cary Coutant will apply the change
>> >> > > https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
>> >> > >
>> >> > > This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
>> >> > > available to all ports. I don't think the current glibc implementation
>> >> > > supports ia64 in an ELFCLASS32 container. That said, the style I used is
>> >> > > works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
>> >> > > 64-bit.
>> >> > >
>> >> > > * Chrome OS folks have carried a local patch since 2018 (latest version:
>> >> > >   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
>> >> > >   I.e. this feature has been battle tested.
>> >> > > * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
>> >> > > * The Linux kernel has supported CONFIG_RELR since 2019-08
>> >> > >   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
>> >> > > * A musl patch (by me) exists but is not applied:
>> >> > >   https://www.openwall.com/lists/musl/2019/03/06/3
>> >> > > * rtld-elf from FreeBSD 14 will support DT_RELR.
>> >> > >
>> >> > > I believe upstream glibc should support DT_RELR to benefit all Linux
>> >> > > distributions. I filed some feature requests to get their attention:
>> >> > >
>> >> > > * Gentoo: https://bugs.gentoo.org/818376
>> >> > > * Arch Linux: https://bugs.archlinux.org/task/72433
>> >> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
>> >> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
>> >> > >
>> >> > > As of linker support (to the best of my knowledge):
>> >> > >
>> >> > > * LLD support DT_RELR.
>> >> > > * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
>> >> > >   has a gold patch.
>> >> > > * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
>> >> > >
>> >> > > I wish that GNU ld and gold maintainers can implement the feature as well :)
>> >> > >
>> >> > > Tested on aarch64 and x86_64.
>> >> > >
>> >> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
>> >> > > * Fix style, simplify code
>> >> > > * Improve test
>> >> > > ---
>> >> > >  configure              | 31 +++++++++++++++++++++++++++++++
>> >> > >  configure.ac           |  4 ++++
>> >> > >  elf/Makefile           |  4 ++++
>> >> > >  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
>> >> > >  elf/elf.h              | 13 +++++++++++--
>> >> > >  elf/get-dynamic-info.h |  3 +++
>> >> > >  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
>> >> > >  7 files changed, 108 insertions(+), 2 deletions(-)
>> >> > >  create mode 100644 elf/tst-relr.c
>> >> > >
>> >> > > diff --git a/configure b/configure
>> >> > > index 3227e434d3..fdab6a97ef 100755
>> >> > > --- a/configure
>> >> > > +++ b/configure
>> >> > > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
>> >> > >  config_vars="$config_vars
>> >> > >  have-depaudit = $libc_cv_depaudit"
>> >> > >
>> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
>> >> > > +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
>> >> > > +libc_linker_feature=no
>> >> > > +if test x"$gnu_ld" = x"yes"; then
>> >> > > +  cat > conftest.c <<EOF
>> >> > > +int _start (void) { return 42; }
>> >> > > +EOF
>> >> > > +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
>> >> > > +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
>> >> > > +                   -fPIC -shared -o conftest.so conftest.c
>> >> > > +                   1>&5'
>> >> > > +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>> >> > > +  (eval $ac_try) 2>&5
>> >> > > +  ac_status=$?
>> >> > > +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>> >> > > +  test $ac_status = 0; }; }
>> >> > > +  then
>> >> > > +    libc_linker_feature=yes
>> >> > > +  fi
>> >> > > +  rm -f conftest*
>> >> > > +fi
>> >> > > +if test $libc_linker_feature = yes; then
>> >> > > +  libc_cv_relr=yes
>> >> > > +else
>> >> > > +  libc_cv_relr=no
>> >> > > +fi
>> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
>> >> > > +$as_echo "$libc_linker_feature" >&6; }
>> >> > > +config_vars="$config_vars
>> >> > > +have-relr = $libc_cv_relr"
>> >> > > +
>> >> > >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
>> >> > >  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
>> >> > >  libc_linker_feature=no
>> >> > > diff --git a/configure.ac b/configure.ac
>> >> > > index 00f49f09f7..96110f9d7d 100644
>> >> > > --- a/configure.ac
>> >> > > +++ b/configure.ac
>> >> > > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
>> >> > >                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
>> >> > >  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
>> >> > >
>> >> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
>> >> > > +                   [libc_cv_relr=yes], [libc_cv_relr=no])
>> >> > > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
>> >> > > +
>> >> > >  LIBC_LINKER_FEATURE([--no-dynamic-linker],
>> >> > >                     [-Wl,--no-dynamic-linker],
>> >> > >                     [libc_cv_no_dynamic_linker=yes],
>> >> > > diff --git a/elf/Makefile b/elf/Makefile
>> >> > > index bf45d8ee24..2c4cdfac68 100644
>> >> > > --- a/elf/Makefile
>> >> > > +++ b/elf/Makefile
>> >> > > @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
>> >> > >                  $(objpfx)tst-audit16-cmp.out
>> >> > >  endif
>> >> > >  endif
>> >> > > +ifeq ($(have-relr),yes)
>> >> > > +tests += tst-relr
>> >> > > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
>> >> > > +endif
>> >> > >  endif
>> >> >
>> >> > Is DT_RELR only generated for PIE? If yes, you need to add it
>> >> > to tests-pie and compile it as PIE.
>> >>
>> >> PIE and shared objects. PDE doesn't need relative relocations.
>> >> It is useful to ensure that -Wl,--pack-dyn-relocs=relr doesn't cause
>> >> breakage to PDE.
>> >
>> >Then you need to add a PIE test to verify that
>> >
>> >1. It has DT_RELR.
>> >2. It runs correctly.
>>
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -246,4 +246,8 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
>>   endif
>>   endif
>> +ifeq ($(have-relr),yes)
>> +tests += tst-relr
>> +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
>> +endif
>>   endif
>>   tests += $(tests-execstack-$(have-z-execstack))
>>
>> I added this chunk to my test (https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr):
>>
>>    ElfW(Dyn) *d = _DYNAMIC;
>>    if (d)
>>      {
>>        bool has_relr = false;
>>        for (; d->d_tag != DT_NULL; d++)
>>         if (d->d_tag == DT_RELR)
>>           has_relr = true;
>>        if (!has_relr)
>>         {
>>           fprintf (stderr, "no DT_RELR\n");
>>           return 1;
>>         }
>>      }
>
>We need 2 tests from the same source:
>
>1. The DT_RELR linker option doesn't break PDE.
>2.  The DT_RELR linker option works for PIE.
>
>You only have one test.

Thanks for the feedback.
https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
has switched to

--- a/elf/Makefile
+++ b/elf/Makefile
@@ -246,4 +246,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
  endif
  endif
+ifeq ($(have-relr),yes)
+tests += tst-relr tst-relr-no-pie
+tests-no-pie += tst-relr-no-pie
+LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
+LDFLAGS-tst-relr-no-pie += -Wl,--pack-dyn-relocs=relr
+endif
  endif
  tests += $(tests-execstack-$(have-z-execstack))

>> >> > [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
>> >> > -Wl,--pack-dyn-relocs=relr -fuse-ld=lld
>> >> > [hjl@gnu-cfl-2 tmp]$ ./a.out
>> >> > Segmentation fault (core dumped)
>> >> > [hjl@gnu-cfl-2 tmp]$
>> >> >
>> >> > Given that the current lld implementation generates broken
>> >> > binaries for existing glibc without any warning at run-time,
>> >> > we should use a different linker command line option to
>> >> > implement it properly so that the new binary will fail to
>> >> > run on glibc without DT_RELR support at run-time.
>> >>
>> >> I don't think so. LLD's design is to be machine agnostic and NOT make
>> >> decisions which would vary on different machines.
>> >> --pack-dyn-relocs=relr is not the default. The user tells LLD to use
>> >> DT_RELR and the user is responsible for making sure target ld.so
>> >> supports DT_RELR.
>> >> LLD is often used as a cross linker. The host ld.so doesn't support
>> >> LLD doesn't mean that LLD should disable the format.
>> >> For example, it is totally fine to link a FreeBSD executable on a Linux machine.
>> >
>> >For Linux targets, we need the glibc version dependency for
>> >DT_RELR support at run-time.
>>
>> I know that having an executable crash right away (SIGSEGV, if ld.so doesn't
>> resolve relative relocations) without a diagnostic is arguably poor user experience,
>> and you might come from this angle and therefore suggest a version check.
>
>Correct.  We need to resolve this before DT_RELR can be accepted.

Introducing DT_RELR to provide an alternative way encoding relative relocations
is like introducing DT_INIT_ARRAY/DT_PREINIT_ARRAY to replace legacy .ctors/.dtors ,
but more peaceful: it is a pure linker decision, not mixed compiler codegen and linker decision.

In some sense, the problem is not too different from linker creating a WX
PT_LOAD segment and the system is configured with rejecting WX. The user
provides input and options to make the linker do so. When something is reasonably within
the linker's error checking scope, an error checker can be added. But some use cases
have valid use of WX PT_LOAD (--omagic), why is the linker in the business of rejecting
valid usage?

This is not a glibc problem. We could add an error now in the absence of DT_RELR support.
But that is moot when the support is coming.

Actually, the problem does not need to be solved on linker or glibc' side.
The compiler driver can do it. If a Linux distro decides to add -Wl,--pack-dyn-relocs=relr,
they do it. The choice is made by them and they are responsible if they want better user experience
(usually can be satisfied by upgrating glibc to have a new versioned symbol).

When GCC finds such an option universally (or sufficiently) beneficial and decides
to add a configure time to make it the default, it can check whether the host supports it.
That's the usual way a new thing is added.

>> However, this tight libc coupling is not LLD's usual design decision.
>> LLD doesn't want to understand what libc it needs to work with.
>>
>> The user experience problem is related to the ELF spirit that we don't want to
>> unnecessarily error for unknown sections/tags/etc. In this case, the evolvement
>> of a foundamental feature (relocation) has to be incompatible with older loaders.
>> That compatibility check could be implemented in many ways, with their own pros and cons.
>> For example, allocating a range of dynamic tags saying that unknown tags in this range
>> are hard errors. However, such a foundamental evolution is rare. I tend to think we just
>> accept the status quo and do nothing on it. --pack-dyn-relocs=relr users, at least
>> from the time being, understand that they need new loaders for the relatively new feature.
>
>It may be OK for some use cases.   I don't think it is acceptable for all use
>cases which glibc cares about.

See above.

>> The problem can be moot when users use --pack-dyn-relocs=relr with new glibc.
>> The linked binary will get a high version requirement and running with older glibc
>> will be detected right away. I just think it's not linker's responsibility to synthesize
>> a symbol targeting a specific glibc version.
>>
>> -z relr=glibc targeting glibc and -z relr=none targeting non-glibc? That'd just
>> be unneeded additional complexity.
>> If GNU ld finally decides to proceed with the -z relr=* road, I can consider
>> adding a -z relr compatibility option to LLD as well, but I may not synthesize a symbol.
>> It's complexity with relatively little benefit IMHO.
>
>
>
>-- 
>H.J.
Joseph Myers Oct. 18, 2021, 7:21 p.m. UTC | #7 
On Mon, 18 Oct 2021, H.J. Lu via Binutils wrote:

> For Linux targets, we need the glibc version dependency for
> DT_RELR support at run-time.

What we've supposedly previously done with new ELF features is use 
EI_ABIVERSION (see libc-abis in glibc).

That requires the static linker to compute the right ABI version based on 
which of the relevant features is used by the generated binary (note that 
the ordering of different features, and thus the numbers assigned to them 
as well as which of them counts as the newest feature to be stored in 
EI_ABIVERSION, is *not* the same across glibc architectures).  I'm not 
sure the binutils side of things was ever implemented on most 
architectures, or that the checks in glibc get properly applied to all 
relevant ELF objects (dynamic linker, main program, loaded shared 
libraries), but in principle the mechanism exists and avoids the linker 
needing to know about specific glibc version numbers itself (at the 
expense of needing to know the per-architecture version numbers and 
ordering for different ELF features).
H.J. Lu Oct. 18, 2021, 7:44 p.m. UTC | #8 
On Mon, Oct 18, 2021 at 12:19 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>
> On 2021-10-18, H.J. Lu wrote:
> >On Mon, Oct 18, 2021 at 11:15 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >>
> >>
> >> On 2021-10-18, H.J. Lu wrote:
> >> >On Mon, Oct 18, 2021 at 9:17 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> >> >>
> >> >> On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >> >> >
> >> >> > On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
> >> >> > <binutils@sourceware.org> wrote:
> >> >> > >
> >> >> > > PIE and shared objects usually have many relative relocations. In
> >> >> > > 2017/2018, SHT_RELR/DT_RELR was proposed on
> >> >> > > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> >> >> > > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> >> >> > > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> >> >> > > virtual memory size of a mostly statically linked PIE is typically 5~10%
> >> >> > > smaller.
> >> >> > >
> >> >> > > ---
> >> >> > >
> >> >> > > Notes I will not include in the submitted commit:
> >> >> > >
> >> >> > > Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
> >> >> > >
> >> >> > > "pre-standard": even Solaris folks are happy with the refined generic-abi
> >> >> > > proposal. Cary Coutant will apply the change
> >> >> > > https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
> >> >> > >
> >> >> > > This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
> >> >> > > available to all ports. I don't think the current glibc implementation
> >> >> > > supports ia64 in an ELFCLASS32 container. That said, the style I used is
> >> >> > > works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
> >> >> > > 64-bit.
> >> >> > >
> >> >> > > * Chrome OS folks have carried a local patch since 2018 (latest version:
> >> >> > >   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
> >> >> > >   I.e. this feature has been battle tested.
> >> >> > > * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
> >> >> > > * The Linux kernel has supported CONFIG_RELR since 2019-08
> >> >> > >   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
> >> >> > > * A musl patch (by me) exists but is not applied:
> >> >> > >   https://www.openwall.com/lists/musl/2019/03/06/3
> >> >> > > * rtld-elf from FreeBSD 14 will support DT_RELR.
> >> >> > >
> >> >> > > I believe upstream glibc should support DT_RELR to benefit all Linux
> >> >> > > distributions. I filed some feature requests to get their attention:
> >> >> > >
> >> >> > > * Gentoo: https://bugs.gentoo.org/818376
> >> >> > > * Arch Linux: https://bugs.archlinux.org/task/72433
> >> >> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
> >> >> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
> >> >> > >
> >> >> > > As of linker support (to the best of my knowledge):
> >> >> > >
> >> >> > > * LLD support DT_RELR.
> >> >> > > * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
> >> >> > >   has a gold patch.
> >> >> > > * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
> >> >> > >
> >> >> > > I wish that GNU ld and gold maintainers can implement the feature as well :)
> >> >> > >
> >> >> > > Tested on aarch64 and x86_64.
> >> >> > >
> >> >> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
> >> >> > > * Fix style, simplify code
> >> >> > > * Improve test
> >> >> > > ---
> >> >> > >  configure              | 31 +++++++++++++++++++++++++++++++
> >> >> > >  configure.ac           |  4 ++++
> >> >> > >  elf/Makefile           |  4 ++++
> >> >> > >  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
> >> >> > >  elf/elf.h              | 13 +++++++++++--
> >> >> > >  elf/get-dynamic-info.h |  3 +++
> >> >> > >  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
> >> >> > >  7 files changed, 108 insertions(+), 2 deletions(-)
> >> >> > >  create mode 100644 elf/tst-relr.c
> >> >> > >
> >> >> > > diff --git a/configure b/configure
> >> >> > > index 3227e434d3..fdab6a97ef 100755
> >> >> > > --- a/configure
> >> >> > > +++ b/configure
> >> >> > > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
> >> >> > >  config_vars="$config_vars
> >> >> > >  have-depaudit = $libc_cv_depaudit"
> >> >> > >
> >> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
> >> >> > > +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
> >> >> > > +libc_linker_feature=no
> >> >> > > +if test x"$gnu_ld" = x"yes"; then
> >> >> > > +  cat > conftest.c <<EOF
> >> >> > > +int _start (void) { return 42; }
> >> >> > > +EOF
> >> >> > > +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
> >> >> > > +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
> >> >> > > +                   -fPIC -shared -o conftest.so conftest.c
> >> >> > > +                   1>&5'
> >> >> > > +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> >> >> > > +  (eval $ac_try) 2>&5
> >> >> > > +  ac_status=$?
> >> >> > > +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> >> >> > > +  test $ac_status = 0; }; }
> >> >> > > +  then
> >> >> > > +    libc_linker_feature=yes
> >> >> > > +  fi
> >> >> > > +  rm -f conftest*
> >> >> > > +fi
> >> >> > > +if test $libc_linker_feature = yes; then
> >> >> > > +  libc_cv_relr=yes
> >> >> > > +else
> >> >> > > +  libc_cv_relr=no
> >> >> > > +fi
> >> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
> >> >> > > +$as_echo "$libc_linker_feature" >&6; }
> >> >> > > +config_vars="$config_vars
> >> >> > > +have-relr = $libc_cv_relr"
> >> >> > > +
> >> >> > >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
> >> >> > >  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
> >> >> > >  libc_linker_feature=no
> >> >> > > diff --git a/configure.ac b/configure.ac
> >> >> > > index 00f49f09f7..96110f9d7d 100644
> >> >> > > --- a/configure.ac
> >> >> > > +++ b/configure.ac
> >> >> > > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
> >> >> > >                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
> >> >> > >  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
> >> >> > >
> >> >> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
> >> >> > > +                   [libc_cv_relr=yes], [libc_cv_relr=no])
> >> >> > > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
> >> >> > > +
> >> >> > >  LIBC_LINKER_FEATURE([--no-dynamic-linker],
> >> >> > >                     [-Wl,--no-dynamic-linker],
> >> >> > >                     [libc_cv_no_dynamic_linker=yes],
> >> >> > > diff --git a/elf/Makefile b/elf/Makefile
> >> >> > > index bf45d8ee24..2c4cdfac68 100644
> >> >> > > --- a/elf/Makefile
> >> >> > > +++ b/elf/Makefile
> >> >> > > @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> >> >> > >                  $(objpfx)tst-audit16-cmp.out
> >> >> > >  endif
> >> >> > >  endif
> >> >> > > +ifeq ($(have-relr),yes)
> >> >> > > +tests += tst-relr
> >> >> > > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> >> >> > > +endif
> >> >> > >  endif
> >> >> >
> >> >> > Is DT_RELR only generated for PIE? If yes, you need to add it
> >> >> > to tests-pie and compile it as PIE.
> >> >>
> >> >> PIE and shared objects. PDE doesn't need relative relocations.
> >> >> It is useful to ensure that -Wl,--pack-dyn-relocs=relr doesn't cause
> >> >> breakage to PDE.
> >> >
> >> >Then you need to add a PIE test to verify that
> >> >
> >> >1. It has DT_RELR.
> >> >2. It runs correctly.
> >>
> >> --- a/elf/Makefile
> >> +++ b/elf/Makefile
> >> @@ -246,4 +246,8 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> >>   endif
> >>   endif
> >> +ifeq ($(have-relr),yes)
> >> +tests += tst-relr
> >> +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> >> +endif
> >>   endif
> >>   tests += $(tests-execstack-$(have-z-execstack))
> >>
> >> I added this chunk to my test (https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr):
> >>
> >>    ElfW(Dyn) *d = _DYNAMIC;
> >>    if (d)
> >>      {
> >>        bool has_relr = false;
> >>        for (; d->d_tag != DT_NULL; d++)
> >>         if (d->d_tag == DT_RELR)
> >>           has_relr = true;
> >>        if (!has_relr)
> >>         {
> >>           fprintf (stderr, "no DT_RELR\n");
> >>           return 1;
> >>         }
> >>      }
> >
> >We need 2 tests from the same source:
> >
> >1. The DT_RELR linker option doesn't break PDE.
> >2.  The DT_RELR linker option works for PIE.
> >
> >You only have one test.
>
> Thanks for the feedback.
> https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
> has switched to
>
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -246,4 +246,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
>   endif
>   endif
> +ifeq ($(have-relr),yes)
> +tests += tst-relr tst-relr-no-pie

You need one test with explicit PIE and one with explicit non-PIE
since not all compilers default to PIE.

> +tests-no-pie += tst-relr-no-pie
> +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> +LDFLAGS-tst-relr-no-pie += -Wl,--pack-dyn-relocs=relr
> +endif
>   endif
>   tests += $(tests-execstack-$(have-z-execstack))
> >> >> > [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
> >> >> > -Wl,--pack-dyn-relocs=relr -fuse-ld=lld
> >> >> > [hjl@gnu-cfl-2 tmp]$ ./a.out
> >> >> > Segmentation fault (core dumped)
> >> >> > [hjl@gnu-cfl-2 tmp]$
> >> >> >
> >> >> > Given that the current lld implementation generates broken
> >> >> > binaries for existing glibc without any warning at run-time,
> >> >> > we should use a different linker command line option to
> >> >> > implement it properly so that the new binary will fail to
> >> >> > run on glibc without DT_RELR support at run-time.
> >> >>
> >> >> I don't think so. LLD's design is to be machine agnostic and NOT make
> >> >> decisions which would vary on different machines.
> >> >> --pack-dyn-relocs=relr is not the default. The user tells LLD to use
> >> >> DT_RELR and the user is responsible for making sure target ld.so
> >> >> supports DT_RELR.
> >> >> LLD is often used as a cross linker. The host ld.so doesn't support
> >> >> LLD doesn't mean that LLD should disable the format.
> >> >> For example, it is totally fine to link a FreeBSD executable on a Linux machine.
> >> >
> >> >For Linux targets, we need the glibc version dependency for
> >> >DT_RELR support at run-time.
> >>
> >> I know that having an executable crash right away (SIGSEGV, if ld.so doesn't
> >> resolve relative relocations) without a diagnostic is arguably poor user experience,
> >> and you might come from this angle and therefore suggest a version check.
> >
> >Correct.  We need to resolve this before DT_RELR can be accepted.
>
> Introducing DT_RELR to provide an alternative way encoding relative relocations
> is like introducing DT_INIT_ARRAY/DT_PREINIT_ARRAY to replace legacy .ctors/.dtors ,
> but more peaceful: it is a pure linker decision, not mixed compiler codegen and linker decision.
>
> In some sense, the problem is not too different from linker creating a WX
> PT_LOAD segment and the system is configured with rejecting WX. The user
> provides input and options to make the linker do so. When something is reasonably within
> the linker's error checking scope, an error checker can be added. But some use cases
> have valid use of WX PT_LOAD (--omagic), why is the linker in the business of rejecting
> valid usage?
>
> This is not a glibc problem. We could add an error now in the absence of DT_RELR support.
> But that is moot when the support is coming.
>

The problem is running the DT_RELR binary on the old ld.so, which
can't be changed.

H.J.
H.J. Lu Oct. 18, 2021, 7:45 p.m. UTC | #9 
On Mon, Oct 18, 2021 at 12:22 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 18 Oct 2021, H.J. Lu via Binutils wrote:
>
> > For Linux targets, we need the glibc version dependency for
> > DT_RELR support at run-time.
>
> What we've supposedly previously done with new ELF features is use
> EI_ABIVERSION (see libc-abis in glibc).
>
> That requires the static linker to compute the right ABI version based on
> which of the relevant features is used by the generated binary (note that
> the ordering of different features, and thus the numbers assigned to them
> as well as which of them counts as the newest feature to be stored in
> EI_ABIVERSION, is *not* the same across glibc architectures).  I'm not
> sure the binutils side of things was ever implemented on most
> architectures, or that the checks in glibc get properly applied to all
> relevant ELF objects (dynamic linker, main program, loaded shared
> libraries), but in principle the mechanism exists and avoids the linker
> needing to know about specific glibc version numbers itself (at the
> expense of needing to know the per-architecture version numbers and
> ordering for different ELF features).
>

We may use it if the existing ld.so implements the check.
Fangrui Song Oct. 18, 2021, 8:11 p.m. UTC | #10 
On Mon, Oct 18, 2021 at 12:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Oct 18, 2021 at 12:19 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> >
> > On 2021-10-18, H.J. Lu wrote:
> > >On Mon, Oct 18, 2021 at 11:15 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >>
> > >>
> > >> On 2021-10-18, H.J. Lu wrote:
> > >> >On Mon, Oct 18, 2021 at 9:17 AM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >> >>
> > >> >> On Mon, Oct 18, 2021 at 7:42 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >> >> >
> > >> >> > On Sat, Oct 16, 2021 at 5:50 PM Fangrui Song via Binutils
> > >> >> > <binutils@sourceware.org> wrote:
> > >> >> > >
> > >> >> > > PIE and shared objects usually have many relative relocations. In
> > >> >> > > 2017/2018, SHT_RELR/DT_RELR was proposed on
> > >> >> > > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> > >> >> > > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> > >> >> > > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> > >> >> > > virtual memory size of a mostly statically linked PIE is typically 5~10%
> > >> >> > > smaller.
> > >> >> > >
> > >> >> > > ---
> > >> >> > >
> > >> >> > > Notes I will not include in the submitted commit:
> > >> >> > >
> > >> >> > > Available on https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
> > >> >> > >
> > >> >> > > "pre-standard": even Solaris folks are happy with the refined generic-abi
> > >> >> > > proposal. Cary Coutant will apply the change
> > >> >> > > https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html
> > >> >> > >
> > >> >> > > This patch is simpler than Chrome OS's glibc patch and makes ELF_DYNAMIC_DO_RELR
> > >> >> > > available to all ports. I don't think the current glibc implementation
> > >> >> > > supports ia64 in an ELFCLASS32 container. That said, the style I used is
> > >> >> > > works with an ELFCLASS32 container for 64-bit machine if ElfW(Addr) is
> > >> >> > > 64-bit.
> > >> >> > >
> > >> >> > > * Chrome OS folks have carried a local patch since 2018 (latest version:
> > >> >> > >   https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.32).
> > >> >> > >   I.e. this feature has been battle tested.
> > >> >> > > * Android bionic supports 2018 and switched to DT_RELR==36 in 2020.
> > >> >> > > * The Linux kernel has supported CONFIG_RELR since 2019-08
> > >> >> > >   (https://git.kernel.org/linus/5cf896fb6be3effd9aea455b22213e27be8bdb1d).
> > >> >> > > * A musl patch (by me) exists but is not applied:
> > >> >> > >   https://www.openwall.com/lists/musl/2019/03/06/3
> > >> >> > > * rtld-elf from FreeBSD 14 will support DT_RELR.
> > >> >> > >
> > >> >> > > I believe upstream glibc should support DT_RELR to benefit all Linux
> > >> >> > > distributions. I filed some feature requests to get their attention:
> > >> >> > >
> > >> >> > > * Gentoo: https://bugs.gentoo.org/818376
> > >> >> > > * Arch Linux: https://bugs.archlinux.org/task/72433
> > >> >> > > * Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996598
> > >> >> > > * Fedora https://bugzilla.redhat.com/show_bug.cgi?id=2014699
> > >> >> > >
> > >> >> > > As of linker support (to the best of my knowledge):
> > >> >> > >
> > >> >> > > * LLD support DT_RELR.
> > >> >> > > * https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-devel/binutils/files/
> > >> >> > >   has a gold patch.
> > >> >> > > * GNU ld feature request https://sourceware.org/bugzilla/show_bug.cgi?id=27923
> > >> >> > >
> > >> >> > > I wish that GNU ld and gold maintainers can implement the feature as well :)
> > >> >> > >
> > >> >> > > Tested on aarch64 and x86_64.
> > >> >> > >
> > >> >> > > Changes from v1 (https://sourceware.org/pipermail/libc-alpha/2021-October/131768.html)
> > >> >> > > * Fix style, simplify code
> > >> >> > > * Improve test
> > >> >> > > ---
> > >> >> > >  configure              | 31 +++++++++++++++++++++++++++++++
> > >> >> > >  configure.ac           |  4 ++++
> > >> >> > >  elf/Makefile           |  4 ++++
> > >> >> > >  elf/dynamic-link.h     | 28 ++++++++++++++++++++++++++++
> > >> >> > >  elf/elf.h              | 13 +++++++++++--
> > >> >> > >  elf/get-dynamic-info.h |  3 +++
> > >> >> > >  elf/tst-relr.c         | 27 +++++++++++++++++++++++++++
> > >> >> > >  7 files changed, 108 insertions(+), 2 deletions(-)
> > >> >> > >  create mode 100644 elf/tst-relr.c
> > >> >> > >
> > >> >> > > diff --git a/configure b/configure
> > >> >> > > index 3227e434d3..fdab6a97ef 100755
> > >> >> > > --- a/configure
> > >> >> > > +++ b/configure
> > >> >> > > @@ -6067,6 +6067,37 @@ $as_echo "$libc_linker_feature" >&6; }
> > >> >> > >  config_vars="$config_vars
> > >> >> > >  have-depaudit = $libc_cv_depaudit"
> > >> >> > >
> > >> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
> > >> >> > > +$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
> > >> >> > > +libc_linker_feature=no
> > >> >> > > +if test x"$gnu_ld" = x"yes"; then
> > >> >> > > +  cat > conftest.c <<EOF
> > >> >> > > +int _start (void) { return 42; }
> > >> >> > > +EOF
> > >> >> > > +  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
> > >> >> > > +                   -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
> > >> >> > > +                   -fPIC -shared -o conftest.so conftest.c
> > >> >> > > +                   1>&5'
> > >> >> > > +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> > >> >> > > +  (eval $ac_try) 2>&5
> > >> >> > > +  ac_status=$?
> > >> >> > > +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> > >> >> > > +  test $ac_status = 0; }; }
> > >> >> > > +  then
> > >> >> > > +    libc_linker_feature=yes
> > >> >> > > +  fi
> > >> >> > > +  rm -f conftest*
> > >> >> > > +fi
> > >> >> > > +if test $libc_linker_feature = yes; then
> > >> >> > > +  libc_cv_relr=yes
> > >> >> > > +else
> > >> >> > > +  libc_cv_relr=no
> > >> >> > > +fi
> > >> >> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
> > >> >> > > +$as_echo "$libc_linker_feature" >&6; }
> > >> >> > > +config_vars="$config_vars
> > >> >> > > +have-relr = $libc_cv_relr"
> > >> >> > > +
> > >> >> > >  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
> > >> >> > >  $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
> > >> >> > >  libc_linker_feature=no
> > >> >> > > diff --git a/configure.ac b/configure.ac
> > >> >> > > index 00f49f09f7..96110f9d7d 100644
> > >> >> > > --- a/configure.ac
> > >> >> > > +++ b/configure.ac
> > >> >> > > @@ -1354,6 +1354,10 @@ LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
> > >> >> > >                     [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
> > >> >> > >  LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
> > >> >> > >
> > >> >> > > +LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
> > >> >> > > +                   [libc_cv_relr=yes], [libc_cv_relr=no])
> > >> >> > > +LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
> > >> >> > > +
> > >> >> > >  LIBC_LINKER_FEATURE([--no-dynamic-linker],
> > >> >> > >                     [-Wl,--no-dynamic-linker],
> > >> >> > >                     [libc_cv_no_dynamic_linker=yes],
> > >> >> > > diff --git a/elf/Makefile b/elf/Makefile
> > >> >> > > index bf45d8ee24..2c4cdfac68 100644
> > >> >> > > --- a/elf/Makefile
> > >> >> > > +++ b/elf/Makefile
> > >> >> > > @@ -245,6 +245,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> > >> >> > >                  $(objpfx)tst-audit16-cmp.out
> > >> >> > >  endif
> > >> >> > >  endif
> > >> >> > > +ifeq ($(have-relr),yes)
> > >> >> > > +tests += tst-relr
> > >> >> > > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> > >> >> > > +endif
> > >> >> > >  endif
> > >> >> >
> > >> >> > Is DT_RELR only generated for PIE? If yes, you need to add it
> > >> >> > to tests-pie and compile it as PIE.
> > >> >>
> > >> >> PIE and shared objects. PDE doesn't need relative relocations.
> > >> >> It is useful to ensure that -Wl,--pack-dyn-relocs=relr doesn't cause
> > >> >> breakage to PDE.
> > >> >
> > >> >Then you need to add a PIE test to verify that
> > >> >
> > >> >1. It has DT_RELR.
> > >> >2. It runs correctly.
> > >>
> > >> --- a/elf/Makefile
> > >> +++ b/elf/Makefile
> > >> @@ -246,4 +246,8 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> > >>   endif
> > >>   endif
> > >> +ifeq ($(have-relr),yes)
> > >> +tests += tst-relr
> > >> +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> > >> +endif
> > >>   endif
> > >>   tests += $(tests-execstack-$(have-z-execstack))
> > >>
> > >> I added this chunk to my test (https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr):
> > >>
> > >>    ElfW(Dyn) *d = _DYNAMIC;
> > >>    if (d)
> > >>      {
> > >>        bool has_relr = false;
> > >>        for (; d->d_tag != DT_NULL; d++)
> > >>         if (d->d_tag == DT_RELR)
> > >>           has_relr = true;
> > >>        if (!has_relr)
> > >>         {
> > >>           fprintf (stderr, "no DT_RELR\n");
> > >>           return 1;
> > >>         }
> > >>      }
> > >
> > >We need 2 tests from the same source:
> > >
> > >1. The DT_RELR linker option doesn't break PDE.
> > >2.  The DT_RELR linker option works for PIE.
> > >
> > >You only have one test.
> >
> > Thanks for the feedback.
> > https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/maskray/relr
> > has switched to
> >
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -246,4 +246,10 @@ tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
> >   endif
> >   endif
> > +ifeq ($(have-relr),yes)
> > +tests += tst-relr tst-relr-no-pie
>
> You need one test with explicit PIE and one with explicit non-PIE
> since not all compilers default to PIE.

OK. I added:

tests-pie += tst-relr

> > +tests-no-pie += tst-relr-no-pie
> > +LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
> > +LDFLAGS-tst-relr-no-pie += -Wl,--pack-dyn-relocs=relr
> > +endif
> >   endif
> >   tests += $(tests-execstack-$(have-z-execstack))
> > >> >> > [hjl@gnu-cfl-2 tmp]$ gcc -pie -fPIE -O2 tst-relr.c
> > >> >> > -Wl,--pack-dyn-relocs=relr -fuse-ld=lld
> > >> >> > [hjl@gnu-cfl-2 tmp]$ ./a.out
> > >> >> > Segmentation fault (core dumped)
> > >> >> > [hjl@gnu-cfl-2 tmp]$
> > >> >> >
> > >> >> > Given that the current lld implementation generates broken
> > >> >> > binaries for existing glibc without any warning at run-time,
> > >> >> > we should use a different linker command line option to
> > >> >> > implement it properly so that the new binary will fail to
> > >> >> > run on glibc without DT_RELR support at run-time.
> > >> >>
> > >> >> I don't think so. LLD's design is to be machine agnostic and NOT make
> > >> >> decisions which would vary on different machines.
> > >> >> --pack-dyn-relocs=relr is not the default. The user tells LLD to use
> > >> >> DT_RELR and the user is responsible for making sure target ld.so
> > >> >> supports DT_RELR.
> > >> >> LLD is often used as a cross linker. The host ld.so doesn't support
> > >> >> LLD doesn't mean that LLD should disable the format.
> > >> >> For example, it is totally fine to link a FreeBSD executable on a Linux machine.
> > >> >
> > >> >For Linux targets, we need the glibc version dependency for
> > >> >DT_RELR support at run-time.
> > >>
> > >> I know that having an executable crash right away (SIGSEGV, if ld.so doesn't
> > >> resolve relative relocations) without a diagnostic is arguably poor user experience,
> > >> and you might come from this angle and therefore suggest a version check.
> > >
> > >Correct.  We need to resolve this before DT_RELR can be accepted.
> >
> > Introducing DT_RELR to provide an alternative way encoding relative relocations
> > is like introducing DT_INIT_ARRAY/DT_PREINIT_ARRAY to replace legacy .ctors/.dtors ,
> > but more peaceful: it is a pure linker decision, not mixed compiler codegen and linker decision.
> >
> > In some sense, the problem is not too different from linker creating a WX
> > PT_LOAD segment and the system is configured with rejecting WX. The user
> > provides input and options to make the linker do so. When something is reasonably within
> > the linker's error checking scope, an error checker can be added. But some use cases
> > have valid use of WX PT_LOAD (--omagic), why is the linker in the business of rejecting
> > valid usage?
> >
> > This is not a glibc problem. We could add an error now in the absence of DT_RELR support.
> > But that is moot when the support is coming.
> >
>
> The problem is running the DT_RELR binary on the old ld.so, which
> can't be changed.
>
> H.J.

I know. I have mentioned that the situation is not different from many
previous situations where
a compatibility check may not catch everything.
(When ET_DYN executable was first introduced, did it need to bump
e_version or add something to e_flags?
When DT_INIT_ARRAY was added?
...
)

I did highlight that even in the absence of the check, the harm is minimal:
--pack-dyn-relocs=relr with new glibc will introduce new version
requirement, which will be naturally caught by older ld.so.
There are a ton non-default linker options which can create
non-runnable binaries.

In the end, I consider that the discussion is useful but has derailed
from glibc getting the feature.
Changes are made to new glibc, not old glibc. We cannot time travel to
the past and change glibc to reject new features today.
It is still valuable and we may need it again when GCC/distributions
want to make relr the default for some targets.
Joseph Myers Oct. 18, 2021, 9:10 p.m. UTC | #11 
On Mon, 18 Oct 2021, Fāng-ruì Sòng via Libc-alpha wrote:

> I know. I have mentioned that the situation is not different from many 
> previous situations where a compatibility check may not catch 
> everything.

That also applies to many changes to glibc functions.

We use symbol versioning when adding new symbols, or when changing a 
symbol in a way incompatible with existing binaries making valid use of 
glibc APIs.  We rarely use it when adding a new feature to an existing 
symbol (for example, a new flag for a function that takes a flags 
argument, or providing stricter guarantees about a return value that 
wasn't fully specified before), even though in fact binaries using the new 
feature would not work correctly when run with older glibc versions.

I don't think a new ELF feature is that different from new features added 
to existing functions.
H.J. Lu Oct. 18, 2021, 10:27 p.m. UTC | #12 
On Mon, Oct 18, 2021 at 2:10 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 18 Oct 2021, Fāng-ruì Sòng via Libc-alpha wrote:
>
> > I know. I have mentioned that the situation is not different from many
> > previous situations where a compatibility check may not catch
> > everything.
>
> That also applies to many changes to glibc functions.
>
> We use symbol versioning when adding new symbols, or when changing a
> symbol in a way incompatible with existing binaries making valid use of
> glibc APIs.  We rarely use it when adding a new feature to an existing
> symbol (for example, a new flag for a function that takes a flags
> argument, or providing stricter guarantees about a return value that
> wasn't fully specified before), even though in fact binaries using the new
> feature would not work correctly when run with older glibc versions.
>
> I don't think a new ELF feature is that different from new features added
> to existing functions.

If a binary requires a new glibc to run, running it with the old glibc will
lead to an ld.so diagnostic message.   DT_RELR shouldn't be different.
Joseph Myers Oct. 18, 2021, 10:30 p.m. UTC | #13 
On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:

> If a binary requires a new glibc to run, running it with the old glibc will
> lead to an ld.so diagnostic message.   DT_RELR shouldn't be different.

My point is that in many cases (for example, a binary using a new flag as 
an argument to a function taking a flags argument) it *won't* lead to an 
ld.so diagnostic message, just to the relevant function call misbehaving 
at runtime.
H.J. Lu Oct. 18, 2021, 10:42 p.m. UTC | #14 
On Mon, Oct 18, 2021 at 3:30 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:
>
> > If a binary requires a new glibc to run, running it with the old glibc will
> > lead to an ld.so diagnostic message.   DT_RELR shouldn't be different.
>
> My point is that in many cases (for example, a binary using a new flag as
> an argument to a function taking a flags argument) it *won't* lead to an
> ld.so diagnostic message, just to the relevant function call misbehaving
> at runtime.

Do you have such a testcase?  It sounds like a glibc bug to me.
Joseph Myers Oct. 18, 2021, 11 p.m. UTC | #15 
On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:

> On Mon, Oct 18, 2021 at 3:30 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:
> >
> > > If a binary requires a new glibc to run, running it with the old glibc will
> > > lead to an ld.so diagnostic message.   DT_RELR shouldn't be different.
> >
> > My point is that in many cases (for example, a binary using a new flag as
> > an argument to a function taking a flags argument) it *won't* lead to an
> > ld.so diagnostic message, just to the relevant function call misbehaving
> > at runtime.
> 
> Do you have such a testcase?  It sounds like a glibc bug to me.

We could take my recently submitted patch 
<https://sourceware.org/pipermail/libc-alpha/2021-October/131905.html> for 
printf %b and %B as an example of the sort of thing we haven't considered 
to need new symbol versions in the past.  If someone builds a program 
using any of about 70 printf-like functions (three times as many on 
powerpc64le because of long double variants) with a %b or %B format, it 
won't work correctly with older glibc versions, but the symbol versions 
won't prevent it from running on older glibc versions.  We haven't 
considered new printf/scanf/strftime/strfmon formats to need new symbol 
versions before (and if we did, there would be a lot of such versions in 
the printf case because there are a lot of printf-like functions).  (The 
strtol-like and scanf-like functions will need new symbol versions for the 
binary input changes because they will also affect the handling of base 2 
/ base 0 input starting with 0b, i.e. change semantics of code that was 
already valid with previous glibc and standard versions.  But there are 
fewer such functions.)
H.J. Lu Oct. 18, 2021, 11:36 p.m. UTC | #16 
On Mon, Oct 18, 2021 at 4:00 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:
>
> > On Mon, Oct 18, 2021 at 3:30 PM Joseph Myers <joseph@codesourcery.com> wrote:
> > >
> > > On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:
> > >
> > > > If a binary requires a new glibc to run, running it with the old glibc will
> > > > lead to an ld.so diagnostic message.   DT_RELR shouldn't be different.
> > >
> > > My point is that in many cases (for example, a binary using a new flag as
> > > an argument to a function taking a flags argument) it *won't* lead to an
> > > ld.so diagnostic message, just to the relevant function call misbehaving
> > > at runtime.
> >
> > Do you have such a testcase?  It sounds like a glibc bug to me.
>
> We could take my recently submitted patch
> <https://sourceware.org/pipermail/libc-alpha/2021-October/131905.html> for
> printf %b and %B as an example of the sort of thing we haven't considered
> to need new symbol versions in the past.  If someone builds a program
> using any of about 70 printf-like functions (three times as many on
> powerpc64le because of long double variants) with a %b or %B format, it
> won't work correctly with older glibc versions, but the symbol versions
> won't prevent it from running on older glibc versions.  We haven't
> considered new printf/scanf/strftime/strfmon formats to need new symbol
> versions before (and if we did, there would be a lot of such versions in
> the printf case because there are a lot of printf-like functions).  (The
> strtol-like and scanf-like functions will need new symbol versions for the
> binary input changes because they will also affect the handling of base 2
> / base 0 input starting with 0b, i.e. change semantics of code that was
> already valid with previous glibc and standard versions.  But there are
> fewer such functions.)
>

Ideally when the new specifiers are used, it should require the new
glibc version.  Is there a way to do it?
Joseph Myers Oct. 18, 2021, 11:44 p.m. UTC | #17 
On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:

> Ideally when the new specifiers are used, it should require the new
> glibc version.  Is there a way to do it?

I don't think there's any sensible way to do it.  You'd need new symbol 
versions (aliased to the old ones, not actual separate entry points with 
different addresses) for all the 70 or so printf-like functions (twice 
that number when two long double variants are supported, three times that 
number when three long double variants are supported).  That's not 
something we've done before for new printf/scanf/strftime/strfmon formats.

The scanf changes will get new __isoc23_* entry points because of API 
differences with scanf %i (but someone could still e.g. use scanf %b when 
building for strict C11, and so get a binary using the old entry point 
with a new format that doesn't work with older glibc - there would only be 
the 32 / 44 / 56 new entry points, not new symbol versions for old entry 
points).
H.J. Lu Oct. 19, 2021, 1:05 a.m. UTC | #18 
On Mon, Oct 18, 2021 at 4:44 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 18 Oct 2021, H.J. Lu via Libc-alpha wrote:
>
> > Ideally when the new specifiers are used, it should require the new
> > glibc version.  Is there a way to do it?
>
> I don't think there's any sensible way to do it.  You'd need new symbol
> versions (aliased to the old ones, not actual separate entry points with
> different addresses) for all the 70 or so printf-like functions (twice
> that number when two long double variants are supported, three times that
> number when three long double variants are supported).  That's not
> something we've done before for new printf/scanf/strftime/strfmon formats.
>
> The scanf changes will get new __isoc23_* entry points because of API
> differences with scanf %i (but someone could still e.g. use scanf %b when
> building for strict C11, and so get a binary using the old entry point
> with a new format that doesn't work with older glibc - there would only be
> the 32 / 44 / 56 new entry points, not new symbol versions for old entry
> points).
>

On glibc 2.34 machine:

[hjl@gnu-snb-1 tmp]$ cat x.c
#include <stdio.h>

int
main ()
{
  printf ("hello\n");
  return 0;
}
[hjl@gnu-snb-1 tmp]$ cat v.S
.section .data.retain,"awR",%progbits
.dc.a GLIBC_2.34
[hjl@gnu-snb-1 tmp]$ gcc v.S x.c
[hjl@gnu-snb-1 tmp]$ ./a.out
hello
[hjl@gnu-snb-1 tmp]$

Copy it to glibc 2.33 machine:

[hjl@gnu-cfl-1 tmp]$ ./a.out
./a.out: /lib64/libc.so.6: version `GLIBC_2.34' not found (required by ./a.out)
[hjl@gnu-cfl-1 tmp]$

Does glibc built by lld have glibc versions in the dynamic symbol table?
If not, lld can't be used to generate glibc.
Carlos O'Donell Oct. 29, 2021, 6:21 p.m. UTC | #19 
On 10/16/21 20:50, Fangrui Song via Binutils wrote:
> PIE and shared objects usually have many relative relocations. In
> 2017/2018, SHT_RELR/DT_RELR was proposed on
> https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> virtual memory size of a mostly statically linked PIE is typically 5~10%
> smaller.

We've been going over this patch on the weekly Monday patch queue review.

I took a note to point out that one of the blockers here is that it is difficult
to immediately test this work because it requires a working glibc build using
ldd (which has support for DT_RELR).

What is the status of the lld support patches for glibc?

If we made progress on the lld support then we'd be able to more easily review
a testable configuration and keep the review going forward.
H.J. Lu Oct. 29, 2021, 6:36 p.m. UTC | #20 
On Fri, Oct 29, 2021 at 11:22 AM Carlos O'Donell via Binutils
<binutils@sourceware.org> wrote:
>
> On 10/16/21 20:50, Fangrui Song via Binutils wrote:
> > PIE and shared objects usually have many relative relocations. In
> > 2017/2018, SHT_RELR/DT_RELR was proposed on
> > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> > virtual memory size of a mostly statically linked PIE is typically 5~10%
> > smaller.
>
> We've been going over this patch on the weekly Monday patch queue review.
>
> I took a note to point out that one of the blockers here is that it is difficult
> to immediately test this work because it requires a working glibc build using
> ldd (which has support for DT_RELR).
>
> What is the status of the lld support patches for glibc?
>
> If we made progress on the lld support then we'd be able to more easily review
> a testable configuration and keep the review going forward.
>

I raised the mysterious crash issue:

https://groups.google.com/g/generic-abi/c/bX460iggiKg

We need an updated proposal without mysterious crashes.  One option is
that the linker should bump EI_ABIVERSION when generating DT_RELR.
After this issue is resolved, I can look into the bfd linker support.
Fangrui Song Oct. 29, 2021, 6:38 p.m. UTC | #21 
On Fri, Oct 29, 2021 at 11:21 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 10/16/21 20:50, Fangrui Song via Binutils wrote:
> > PIE and shared objects usually have many relative relocations. In
> > 2017/2018, SHT_RELR/DT_RELR was proposed on
> > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
> > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
> > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
> > virtual memory size of a mostly statically linked PIE is typically 5~10%
> > smaller.
>
> We've been going over this patch on the weekly Monday patch queue review.
>
> I took a note to point out that one of the blockers here is that it is difficult
> to immediately test this work because it requires a working glibc build using
> ldd (which has support for DT_RELR).

No:) There may be a misunderstanding.

To test the feature: a DT_RELR executable is needed.
Currently only ld.lld --pack-dyn-relocs=relr supports generating
DT_RELR binaries,
but glibc itself does not need to be linked with ld.lld.

If the patch is accepted, GNU ld built glibc will support DT_RELR
programs linked by ld.lld.
ld.lld has supported --pack-dyn-relocs=relr since 2017-12.
ld.lld --pack-dyn-relocs=relr (-pie|-shared) produced output works on
Android, Fuchsia, FreeBSD (https://reviews.freebsd.org/D32524)

You can see that the code here is fully generic, not tightly to a
specific linker.

For my purpose and to Linux distro's interest, I'd definitely want to
see that the glibc patch is accepted even in the lack of GNU ld
support.

> What is the status of the lld support patches for glibc?

aarch64 and x86-64 work well for me.
Seems that Adhemerval may be interested in adding a lld configuration
to scripts/build-many-glibcs.py

> If we made progress on the lld support then we'd be able to more easily review
> a testable configuration and keep the review going forward.
>
> --
> Cheers,
> Carlos.
>
Fangrui Song Oct. 29, 2021, 7:15 p.m. UTC | #22 
On 2021-10-29, H.J. Lu wrote:
>On Fri, Oct 29, 2021 at 11:22 AM Carlos O'Donell via Binutils
><binutils@sourceware.org> wrote:
>>
>> On 10/16/21 20:50, Fangrui Song via Binutils wrote:
>> > PIE and shared objects usually have many relative relocations. In
>> > 2017/2018, SHT_RELR/DT_RELR was proposed on
>> > https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
>> > ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
>> > usually takes 3% or smaller space than R_*_RELATIVE relocations. The
>> > virtual memory size of a mostly statically linked PIE is typically 5~10%
>> > smaller.
>>
>> We've been going over this patch on the weekly Monday patch queue review.
>>
>> I took a note to point out that one of the blockers here is that it is difficult
>> to immediately test this work because it requires a working glibc build using
>> ldd (which has support for DT_RELR).
>>
>> What is the status of the lld support patches for glibc?
>>
>> If we made progress on the lld support then we'd be able to more easily review
>> a testable configuration and keep the review going forward.
>>
>
>I raised the mysterious crash issue:
>
>https://groups.google.com/g/generic-abi/c/bX460iggiKg
>
>We need an updated proposal without mysterious crashes.  One option is
>that the linker should bump EI_ABIVERSION when generating DT_RELR.

The generic ABI says :  "EI_ABIVERSION: ... The interpretation of this
version number is dependent on the ABI identified by the EI_OSABI
field."

Operating systems decide their EI_ABIVERSION.  Having a linker option
not bumping the ABI version can benefit some operating systems. We know
that FreeBSD (ELFOSABI_FREEBSD)/Fuchsia/ChromeOS don't find it necessary
(or don't want) to bump the ABI version.

See https://groups.google.com/g/generic-abi/c/vdG_G4l3N-Y for Solaris'
development model. They simply don't support (new object, old system) (I
believe that means a diagnostic is good but not required). "it's been
pretty drama free over many decades of SunOS."

---

So many folks on binutils/glibc are more interested on Linux and let's
discuss Linux:

Even within ELFOSABI_GNU, different architectures may have different
EI_ABIVERSION values.
I know that mips may use EI_ABIVERSION==1 for
(e_eflags & (EF_MIPS_PIC | EF_MIPS_CPIC)) == EF_MIPS_CPIC
position-dependent executables.


If you think a linker option like -z relr=glibc or --pack-dyn-relocs=relr-glibc
is useful to rule out (DT_RELR object, glibc not supporting DT_RELR)
loudly, you may choose the `_dl_have_relr` in .dynsym scheme
(https://sourceware.org/pipermail/binutils/2021-October/118347.html).

I will probably just make such an option an alias for
--pack-dyn-relocs=relr because LLD has (1) some users who don't need this
checking (2) --pack-dyn-relocs=relr semantic change would be unnecessary
(3) I don't want them to migrate away from --pack-dyn-relocs=relr just
because glibc has a different development model.

>After this issue is resolved, I can look into the bfd linker support.

Will the above said, I grealy appreciate that you can take a stab on the
GNU ld support. This will benefit so many Linux distributions.

I learned that my /usr/bin/perf_* can be 20% smaller if switching to
DT_RELR :) Perhaps 5~10% is more typical for other executables.
Carlos O'Donell Oct. 29, 2021, 7:35 p.m. UTC | #23 
On 10/29/21 14:38, Fāng-ruì Sòng wrote:
> On Fri, Oct 29, 2021 at 11:21 AM Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> On 10/16/21 20:50, Fangrui Song via Binutils wrote:
>>> PIE and shared objects usually have many relative relocations. In
>>> 2017/2018, SHT_RELR/DT_RELR was proposed on
>>> https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
>>> ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
>>> usually takes 3% or smaller space than R_*_RELATIVE relocations. The
>>> virtual memory size of a mostly statically linked PIE is typically 5~10%
>>> smaller.
>>
>> We've been going over this patch on the weekly Monday patch queue review.
>>
>> I took a note to point out that one of the blockers here is that it is difficult
>> to immediately test this work because it requires a working glibc build using
>> ldd (which has support for DT_RELR).
> 
> No:) There may be a misunderstanding.
> 
> To test the feature: a DT_RELR executable is needed.
> Currently only ld.lld --pack-dyn-relocs=relr supports generating
> DT_RELR binaries,
> but glibc itself does not need to be linked with ld.lld.

This is true.

However, running the entire testsuite with ld.lld and DT_RELR gives wider coverage
and improves confidence in the feature.

> If the patch is accepted, GNU ld built glibc will support DT_RELR
> programs linked by ld.lld.

I agree, but in that scenario, lacking the ability to link the entire testsuite
with ld.lld reduces testing coverage for the feature.

>> What is the status of the lld support patches for glibc?
> 
> aarch64 and x86-64 work well for me.

... but they aren't yet committed?

Do we have a patchwork series for them?

https://patchwork.sourceware.org/project/glibc/list/?series=4199

> Seems that Adhemerval may be interested in adding a lld configuration
> to scripts/build-many-glibcs.py

checking version of ld... 12.0.1, bad
...
configure: error: 
*** These critical programs are missing or too old: LLD
*** Check the INSTALL file for required versions.

What's the fix for this?
 
>> If we made progress on the lld support then we'd be able to more easily review
>> a testable configuration and keep the review going forward.
>>
>> --
>> Cheers,
>> Carlos.
>>
>
Adhemerval Zanella Netto Oct. 29, 2021, 7:53 p.m. UTC | #24 
On 29/10/2021 16:35, Carlos O'Donell via Binutils wrote:
> On 10/29/21 14:38, Fāng-ruì Sòng wrote:
>> On Fri, Oct 29, 2021 at 11:21 AM Carlos O'Donell <carlos@redhat.com> wrote:
>>>
>>> On 10/16/21 20:50, Fangrui Song via Binutils wrote:
>>>> PIE and shared objects usually have many relative relocations. In
>>>> 2017/2018, SHT_RELR/DT_RELR was proposed on
>>>> https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
>>>> ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
>>>> usually takes 3% or smaller space than R_*_RELATIVE relocations. The
>>>> virtual memory size of a mostly statically linked PIE is typically 5~10%
>>>> smaller.
>>>
>>> We've been going over this patch on the weekly Monday patch queue review.
>>>
>>> I took a note to point out that one of the blockers here is that it is difficult
>>> to immediately test this work because it requires a working glibc build using
>>> ldd (which has support for DT_RELR).
>>
>> No:) There may be a misunderstanding.
>>
>> To test the feature: a DT_RELR executable is needed.
>> Currently only ld.lld --pack-dyn-relocs=relr supports generating
>> DT_RELR binaries,
>> but glibc itself does not need to be linked with ld.lld.
> 
> This is true.
> 
> However, running the entire testsuite with ld.lld and DT_RELR gives wider coverage
> and improves confidence in the feature.
> 
>> If the patch is accepted, GNU ld built glibc will support DT_RELR
>> programs linked by ld.lld.
> 
> I agree, but in that scenario, lacking the ability to link the entire testsuite
> with ld.lld reduces testing coverage for the feature.
> 
>>> What is the status of the lld support patches for glibc?
>>
>> aarch64 and x86-64 work well for me.
> 
> ... but they aren't yet committed?
> 
> Do we have a patchwork series for them?
> 
> https://patchwork.sourceware.org/project/glibc/list/?series=4199
> 
>> Seems that Adhemerval may be interested in adding a lld configuration
>> to scripts/build-many-glibcs.py
> 
> checking version of ld... 12.0.1, bad
> ...
> configure: error: 
> *** These critical programs are missing or too old: LLD
> *** Check the INSTALL file for required versions.
> 
> What's the fix for this?


I summarized the previous status of glibc plus lld on [1].  In a short,
only x86_64, i686, and aarch64 works without testcase regressions (i686
does show one regression, elf/ifuncmain6pie).  I am working on arm,
I could get it build and having the testcase without only 11 regression
(all of them related to ifunc). I plan to send the patches soon.

The powerpc builds but either loader or libc fails to run.  The riscv
also builds with an extra patch to set -mno-relax (to avoid R_RISCV_ALIGN
generation since lld does not support it), but I didn't not actually
test if works as expected.

The other supported architectures, sparc64 and mips, seems to be broken
and it would require much more work.

[1] https://sourceware.org/pipermail/libc-alpha/2021-October/132315.html
Fangrui Song Nov. 1, 2021, 4:50 a.m. UTC | #25 
On 2021-10-29, Adhemerval Zanella wrote:
>
>
>On 29/10/2021 16:35, Carlos O'Donell via Binutils wrote:
>> On 10/29/21 14:38, Fāng-ruì Sòng wrote:
>>> On Fri, Oct 29, 2021 at 11:21 AM Carlos O'Donell <carlos@redhat.com> wrote:
>>>>
>>>> On 10/16/21 20:50, Fangrui Song via Binutils wrote:
>>>>> PIE and shared objects usually have many relative relocations. In
>>>>> 2017/2018, SHT_RELR/DT_RELR was proposed on
>>>>> https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
>>>>> ("Proposal for a new section type SHT_RELR") and is a pre-standard. RELR
>>>>> usually takes 3% or smaller space than R_*_RELATIVE relocations. The
>>>>> virtual memory size of a mostly statically linked PIE is typically 5~10%
>>>>> smaller.
>>>>
>>>> We've been going over this patch on the weekly Monday patch queue review.
>>>>
>>>> I took a note to point out that one of the blockers here is that it is difficult
>>>> to immediately test this work because it requires a working glibc build using
>>>> ldd (which has support for DT_RELR).
>>>
>>> No:) There may be a misunderstanding.
>>>
>>> To test the feature: a DT_RELR executable is needed.
>>> Currently only ld.lld --pack-dyn-relocs=relr supports generating
>>> DT_RELR binaries,
>>> but glibc itself does not need to be linked with ld.lld.
>>
>> This is true.
>>
>> However, running the entire testsuite with ld.lld and DT_RELR gives wider coverage
>> and improves confidence in the feature.
>>
>>> If the patch is accepted, GNU ld built glibc will support DT_RELR
>>> programs linked by ld.lld.
>>
>> I agree, but in that scenario, lacking the ability to link the entire testsuite
>> with ld.lld reduces testing coverage for the feature.

I use a hacked /usr/local/bin/ld to link tst- test- objects with --pack-dyn-relocs=relr:

% cat /usr/local/bin/ld
#!/bin/zsh
output=
prev_arg=
for arg in "$@"; do
   [[ $prev_arg == -o ]] && output=$arg
   prev_arg=$arg
done

if [[ $output =~ 'tst-|test-' ]]; then
   exec ~/Stable/bin/ld.lld --pack-dyn-relocs=relr "$@"
else
   exec ~/Stable/bin/ld.lld "$@"
fi
% ~/Stable/bin/ld.lld --version
LLD 14.0.0 (compatible with GNU linkers)
% readelf -WS elf/tst-absolute-sym | grep relr.dyn
   [11] .relr.dyn         00000013: <unknown> 0000000000000f90 000f90 000020 08   A  0   0  8


Most tests pass, except the 7 nptl tests which spawn a gdb process and do some complex stuff with it:

% comm -3 <(grep '^FAIL' tests.sum | sort) <(grep '^FAIL' ../lld0/tests.sum | sort)
FAIL: nptl/test-condattr-printers
FAIL: nptl/test-cond-printers
FAIL: nptl/test-mutexattr-printers
FAIL: nptl/test-mutex-printers
FAIL: nptl/test-rwlockattr-printers
FAIL: nptl/test-rwlock-printers
FAIL: nptl/tst-pthread-gdb-attach-static

% cat nptl/test-condattr-printers.out
Error: Response does not match the expected pattern.
Command: print *condvar
Expected pattern: pthread_cond_t
Response:  Cannot access memory at address 0xffffd788
(gdb)



The 4000+ PASSes give me confidence that the DT_RELR does the right thing.



I read the generic-abi thread in 2019 but just noted that a comment from Ali Bahrami deserved my re-read
https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/0PMCJ0hjBAAJ

"My free advice (worth what you paid for it) is to roll out the support, and
  then wait a bit before turning on the use widely, so that the support is in
  place before it is needed, and to not complicate things with a way to
  catch time travelers. The window of time where this can be a problem
  is finite, and once you're past it, you'll be glad to have a simpler system."

I hope that I have captured the status well enough in https://maskray.me/blog/2021-10-31-relative-relocations-and-relr#glibc :)

>>>> What is the status of the lld support patches for glibc?
>>>
>>> aarch64 and x86-64 work well for me.
>>
>> ... but they aren't yet committed?
>>
>> Do we have a patchwork series for them?
>>
>> https://patchwork.sourceware.org/project/glibc/list/?series=4199
>>
>>> Seems that Adhemerval may be interested in adding a lld configuration
>>> to scripts/build-many-glibcs.py
>>
>> checking version of ld... 12.0.1, bad
>> ...
>> configure: error:
>> *** These critical programs are missing or too old: LLD
>> *** Check the INSTALL file for required versions.
>>
>> What's the fix for this?

13.0.0 is needed :)

>I summarized the previous status of glibc plus lld on [1].  In a short,
>only x86_64, i686, and aarch64 works without testcase regressions (i686
>does show one regression, elf/ifuncmain6pie).  I am working on arm,
>I could get it build and having the testcase without only 11 regression
>(all of them related to ifunc). I plan to send the patches soon.
>
>The powerpc builds but either loader or libc fails to run.  The riscv
>also builds with an extra patch to set -mno-relax (to avoid R_RISCV_ALIGN
>generation since lld does not support it), but I didn't not actually
>test if works as expected.
>
>The other supported architectures, sparc64 and mips, seems to be broken
>and it would require much more work.
>
>[1] https://sourceware.org/pipermail/libc-alpha/2021-October/132315.html
diff mbox series

Patch

diff --git a/configure b/configure
index 3227e434d3..fdab6a97ef 100755
--- a/configure
+++ b/configure
@@ -6067,6 +6067,37 @@  $as_echo "$libc_linker_feature" >&6; }
 config_vars="$config_vars
 have-depaudit = $libc_cv_depaudit"
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --pack-dyn-relocs=relr" >&5
+$as_echo_n "checking for linker that supports --pack-dyn-relocs=relr... " >&6; }
+libc_linker_feature=no
+if test x"$gnu_ld" = x"yes"; then
+  cat > conftest.c <<EOF
+int _start (void) { return 42; }
+EOF
+  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
+		    -Wl,--pack-dyn-relocs=relr -nostdlib -nostartfiles
+		    -fPIC -shared -o conftest.so conftest.c
+		    1>&5'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; }
+  then
+    libc_linker_feature=yes
+  fi
+  rm -f conftest*
+fi
+if test $libc_linker_feature = yes; then
+  libc_cv_relr=yes
+else
+  libc_cv_relr=no
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_linker_feature" >&5
+$as_echo "$libc_linker_feature" >&6; }
+config_vars="$config_vars
+have-relr = $libc_cv_relr"
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker that supports --no-dynamic-linker" >&5
 $as_echo_n "checking for linker that supports --no-dynamic-linker... " >&6; }
 libc_linker_feature=no
diff --git a/configure.ac b/configure.ac
index 00f49f09f7..96110f9d7d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1354,6 +1354,10 @@  LIBC_LINKER_FEATURE([--depaudit], [-Wl,--depaudit,x],
 		    [libc_cv_depaudit=yes], [libc_cv_depaudit=no])
 LIBC_CONFIG_VAR([have-depaudit], [$libc_cv_depaudit])
 
+LIBC_LINKER_FEATURE([--pack-dyn-relocs=relr], [-Wl,--pack-dyn-relocs=relr],
+		    [libc_cv_relr=yes], [libc_cv_relr=no])
+LIBC_CONFIG_VAR([have-relr], [$libc_cv_relr])
+
 LIBC_LINKER_FEATURE([--no-dynamic-linker],
 		    [-Wl,--no-dynamic-linker],
 		    [libc_cv_no_dynamic_linker=yes],
diff --git a/elf/Makefile b/elf/Makefile
index bf45d8ee24..2c4cdfac68 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -245,6 +245,10 @@  tests-special += $(objpfx)tst-audit14-cmp.out $(objpfx)tst-audit15-cmp.out \
 		 $(objpfx)tst-audit16-cmp.out
 endif
 endif
+ifeq ($(have-relr),yes)
+tests += tst-relr
+LDFLAGS-tst-relr += -Wl,--pack-dyn-relocs=relr
+endif
 endif
 tests += $(tests-execstack-$(have-z-execstack))
 ifeq ($(run-built-tests),yes)
diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
index ac4cc70dea..fafd42f918 100644
--- a/elf/dynamic-link.h
+++ b/elf/dynamic-link.h
@@ -160,6 +160,33 @@  elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
 #  define ELF_DYNAMIC_DO_RELA(map, scope, lazy, skip_ifunc) /* Nothing to do.  */
 # endif
 
+# define ELF_DYNAMIC_DO_RELR(map)					      \
+  do {									      \
+    ElfW(Addr) l_addr = (map)->l_addr, *where = 0;			      \
+    const ElfW(Relr) *r, *end;						      \
+    if (!(map)->l_info[DT_RELR])					      \
+      break;								      \
+    r = (const ElfW(Relr) *)D_PTR((map), l_info[DT_RELR]);		      \
+    end = (const ElfW(Relr) *)((const char *)r +			      \
+                               (map)->l_info[DT_RELRSZ]->d_un.d_val);	      \
+    for (; r < end; r++)						      \
+      {									      \
+	ElfW(Relr) entry = *r;						      \
+	if ((entry & 1) == 0)						      \
+	  {								      \
+	    where = (ElfW(Addr) *)(l_addr + entry);			      \
+	    *where++ += l_addr;						      \
+	  }								      \
+	else 								      \
+	  {								      \
+	    for (long i = 0; (entry >>= 1) != 0; i++)			      \
+	      if ((entry & 1) != 0)					      \
+		where[i] += l_addr;					      \
+	    where += CHAR_BIT * sizeof(ElfW(Relr)) - 1;			      \
+	  }								      \
+      }									      \
+  } while (0);
+
 /* This can't just be an inline function because GCC is too dumb
    to inline functions containing inlines themselves.  */
 # define ELF_DYNAMIC_RELOCATE(map, scope, lazy, consider_profile, skip_ifunc) \
@@ -168,6 +195,7 @@  elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
 					      (consider_profile));	      \
     ELF_DYNAMIC_DO_REL ((map), (scope), edr_lazy, skip_ifunc);		      \
     ELF_DYNAMIC_DO_RELA ((map), (scope), edr_lazy, skip_ifunc);		      \
+    ELF_DYNAMIC_DO_RELR ((map));					      \
   } while (0)
 
 #endif
diff --git a/elf/elf.h b/elf/elf.h
index 50f87baceb..6dc1ee0ac0 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -443,7 +443,8 @@  typedef struct
 #define SHT_PREINIT_ARRAY 16		/* Array of pre-constructors */
 #define SHT_GROUP	  17		/* Section group */
 #define SHT_SYMTAB_SHNDX  18		/* Extended section indices */
-#define	SHT_NUM		  19		/* Number of defined types.  */
+#define SHT_RELR	  19            /* RELR relative relocations */
+#define	SHT_NUM		  20		/* Number of defined types.  */
 #define SHT_LOOS	  0x60000000	/* Start OS-specific.  */
 #define SHT_GNU_ATTRIBUTES 0x6ffffff5	/* Object attributes.  */
 #define SHT_GNU_HASH	  0x6ffffff6	/* GNU-style hash table.  */
@@ -662,6 +663,11 @@  typedef struct
   Elf64_Sxword	r_addend;		/* Addend */
 } Elf64_Rela;
 
+/* RELR relocation table entry */
+
+typedef Elf32_Word	Elf32_Relr;
+typedef Elf64_Xword	Elf64_Relr;
+
 /* How to extract and insert information held in the r_info field.  */
 
 #define ELF32_R_SYM(val)		((val) >> 8)
@@ -887,7 +893,10 @@  typedef struct
 #define DT_PREINIT_ARRAY 32		/* Array with addresses of preinit fct*/
 #define DT_PREINIT_ARRAYSZ 33		/* size in bytes of DT_PREINIT_ARRAY */
 #define DT_SYMTAB_SHNDX	34		/* Address of SYMTAB_SHNDX section */
-#define	DT_NUM		35		/* Number used */
+#define DT_RELRSZ	35		/* Total size of RELR relative relocations */
+#define DT_RELR		36		/* Address of RELR relative relocations */
+#define DT_RELRENT	37		/* Size of one RELR relative relocaction */
+#define	DT_NUM		38		/* Number used */
 #define DT_LOOS		0x6000000d	/* Start of OS-specific */
 #define DT_HIOS		0x6ffff000	/* End of OS-specific */
 #define DT_LOPROC	0x70000000	/* Start of processor-specific */
diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
index 1ac0663d1f..ce0151c2b4 100644
--- a/elf/get-dynamic-info.h
+++ b/elf/get-dynamic-info.h
@@ -88,6 +88,7 @@  elf_get_dynamic_info (struct link_map *l)
 # if ! ELF_MACHINE_NO_REL
       ADJUST_DYN_INFO (DT_REL);
 # endif
+      ADJUST_DYN_INFO (DT_RELR);
       ADJUST_DYN_INFO (DT_JMPREL);
       ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
       ADJUST_DYN_INFO (ADDRIDX (DT_GNU_HASH));
@@ -112,6 +113,8 @@  elf_get_dynamic_info (struct link_map *l)
   if (info[DT_REL] != NULL)
     assert (info[DT_RELENT]->d_un.d_val == sizeof (ElfW(Rel)));
 #endif
+  if (info[DT_RELR] != NULL)
+    assert (info[DT_RELRENT]->d_un.d_val == sizeof (ElfW(Relr)));
 #ifdef STATIC_PIE_BOOTSTRAP
   assert (info[DT_RUNPATH] == NULL);
   assert (info[DT_RPATH] == NULL);
diff --git a/elf/tst-relr.c b/elf/tst-relr.c
new file mode 100644
index 0000000000..79736a4a8a
--- /dev/null
+++ b/elf/tst-relr.c
@@ -0,0 +1,27 @@ 
+static int o, x;
+
+#define ELEMS O O O O O O O O X X X X X X X O O X O O X X X E X E E O X O E
+#define E 0,
+
+#define O &o,
+#define X &x,
+void *arr[] = { ELEMS };
+#undef O
+#undef X
+
+#define O 1,
+#define X 2,
+static char val[] = { ELEMS };
+
+static int
+do_test (void)
+{
+  for (int i = 0; i < sizeof (arr) / sizeof (arr[0]); i++)
+    if (!((arr[i] == 0 && val[i] == 0) ||
+	  (arr[i] == &o && val[i] == 1) ||
+	  (arr[i] == &x && val[i] == 2)))
+      return 1;
+  return 0;
+}
+
+#include <support/test-driver.c>