Re: [PATCH v5 1/5] elf: Support DT_RELR relative relocation format [BZ #27924]

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

On Thu, Mar 10, 2022 at 10:48 AM Fangrui Song <maskray@google.com> wrote:
>
>
> On 2022-03-02, H.J. Lu wrote:
> >From: Fangrui Song <maskray@google.com>
> >
> >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
> >
> >Changes from the original patch:
> >
> >1. Check the linker option, -z pack-relative-relocs, which add a
> >GLIBC_ABI_DT_RELR symbol version dependency on the shared C library if
> >it provides a GLIBC_2.XX symbol version.
> >2. Change make variale to have-dt-relr.
> >3. Rename tst-relr-no-pie to tst-relr-pie for --disable-default-pie.
> >4. Use TEST_VERIFY in tst-relr.c.
> >5. Add the check-tst-relr-pie.out test to check for linker generated
> >libc.so version dependency on GLIBC_ABI_DT_RELR.
> >---
> > configure              | 42 +++++++++++++++++++++++++++
> > configure.ac           | 10 +++++++
> > elf/Makefile           | 14 +++++++++
> > elf/dynamic-link.h     | 34 ++++++++++++++++++++++
> > elf/elf.h              | 13 +++++++--
> > elf/get-dynamic-info.h |  3 ++
> > elf/tst-relr-pie.c     |  1 +
> > elf/tst-relr.c         | 64 ++++++++++++++++++++++++++++++++++++++++++
> > 8 files changed, 179 insertions(+), 2 deletions(-)
> > create mode 100644 elf/tst-relr-pie.c
> > create mode 100644 elf/tst-relr.c
> >
> >diff --git a/configure b/configure
> >index 8e5bee775a..9156e29fe9 100755
> >--- a/configure
> >+++ b/configure
> >@@ -6115,6 +6115,48 @@ $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 -z pack-relative-relocs" >&5
> >+$as_echo_n "checking for linker that supports -z pack-relative-relocs... " >&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,-z,pack-relative-relocs -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
> >+    if ${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp -Wl,-z,pack-relative-relocs -nostdlib \
> >+      -nostartfiles -fPIC -shared -o conftest.so conftest.c 2>&1 \
> >+      | grep "warning: -z pack-relative-relocs ignored" > /dev/null 2>&1; then
> >+      true
> >+    else
> >+      libc_linker_feature=yes
> >+    fi
> >+  fi
> >+  rm -f conftest*
> >+fi
> >+if test $libc_linker_feature = yes; then
> >+  libc_cv_dt_relr=yes
> >+else
> >+  libc_cv_dt_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-dt-relr = $libc_cv_dt_relr"
> >+if test "$libc_cv_dt_relr" = yes; then
> >+  have_dt_relr=1
> >+else
> >+  have_dt_relr=0
> >+fi
> >+
> > { $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 87f67d25ec..5c09871fee 100644
> >--- a/configure.ac
> >+++ b/configure.ac
> >@@ -1367,6 +1367,16 @@ 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([-z pack-relative-relocs],
> >+                  [-Wl,-z,pack-relative-relocs],
> >+                  [libc_cv_dt_relr=yes], [libc_cv_dt_relr=no])
> >+LIBC_CONFIG_VAR([have-dt-relr], [$libc_cv_dt_relr])
> >+if test "$libc_cv_dt_relr" = yes; then
> >+  have_dt_relr=1
> >+else
> >+  have_dt_relr=0
> >+fi
> >+
> > 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 c96924e9c2..fd462ba315 100644
> >--- a/elf/Makefile
> >+++ b/elf/Makefile
> >@@ -541,6 +541,14 @@ tests-special += \
> >   # tests-special
> > endif
> > endif
> >+ifeq ($(have-dt-relr),yes)
> >+tests += tst-relr tst-relr-pie
> >+tests-pie += tst-relr-pie
> >+tests-special += $(objpfx)check-tst-relr-pie.out
> >+CFLAGS-tst-relr-pie.c += $(pie-ccflag)
> >+LDFLAGS-tst-relr += -Wl,-z,pack-relative-relocs
> >+LDFLAGS-tst-relr-pie += -Wl,-z,pack-relative-relocs
> >+endif
> > endif
> >
> > ifeq ($(run-built-tests),yes)
> >@@ -2725,3 +2733,9 @@ $(objpfx)tst-p_align3: $(objpfx)tst-p_alignmod3.so
> > $(objpfx)tst-p_align3.out: tst-p_align3.sh $(objpfx)tst-p_align3
> >       $(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
> >       $(evaluate-test)
> >+
> >+$(objpfx)check-tst-relr-pie.out: $(objpfx)tst-relr-pie
> >+      LC_ALL=C $(OBJDUMP) -p $< \
> >+              | sed -ne '/required from libc.so/,$$ p' \
> >+              | grep GLIBC_ABI_DT_RELR > $@; \
> >+      $(evaluate-test)
> >diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> >index 25dd7ca4f2..5318079bac 100644
> >--- a/elf/dynamic-link.h
> >+++ b/elf/dynamic-link.h
> >@@ -146,14 +146,48 @@ 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.  */
> >+# ifdef RTLD_BOOTSTRAP
> >+#  define DO_RTLD_BOOTSTRAP 1
> >+# else
> >+#  define DO_RTLD_BOOTSTRAP 0
> >+# endif
> > # define ELF_DYNAMIC_RELOCATE(map, scope, lazy, consider_profile, skip_ifunc) \
> >   do {                                                                              \
> >     int edr_lazy = elf_machine_runtime_setup ((map), (scope), (lazy),       \
> >                                             (consider_profile));            \
> >     ELF_DYNAMIC_DO_REL ((map), (scope), edr_lazy, skip_ifunc);                      \
> >     ELF_DYNAMIC_DO_RELA ((map), (scope), edr_lazy, skip_ifunc);                     \
> >+    if (((map) != &GL(dl_rtld_map) || DO_RTLD_BOOTSTRAP))                   \
> >+      ELF_DYNAMIC_DO_RELR (map);                                            \
> >   } while (0)
> >
> > #endif
>
> As mentioned on a binutils thread, ELF_DYNAMIC_DO_RELR needs to be
> reordered before DL_REL/DO_RELA to help powerpc get the support in the future
> https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=elf/dynamic-link.h;h=169745f9a409aec7995f7418736ae64bfeebc9d6;hp=25dd7ca4f2eca3c97fdd1649c15e1b5106e70dc1;hb=1c6cc29baf9a4c7129ab2e94b0d4022bfa4f3299;hpb=edc696a73a7cb07b1aa68792a845a98d036ee7eb

Fixed in the v6 patch.

Thanks.

-- 
H.J.