From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x530.google.com (mail-pg1-x530.google.com [IPv6:2607:f8b0:4864:20::530]) by sourceware.org (Postfix) with ESMTPS id 27A243858C83 for ; Fri, 22 Apr 2022 18:47:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 27A243858C83 Received: by mail-pg1-x530.google.com with SMTP id z21so373012pgj.1 for ; Fri, 22 Apr 2022 11:47:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2elTlZw0LDc/xq+aQ8WquYB1TAxbioLJCN/QkxWyyvM=; b=g5hzkOrOdFLOrEfgNgSgpXKmExjnJOmZQV3LHlPQQPi2q2+KGhgkC8GFx+KT3PH6/T 6YLcxNyD9h7WttbC3HJE/M94RN5ybXRbPY+6ZNRtOdcJdNIfouq3kqCiD5sC8s/vcl5h yaSaUvIgTkTVgqIjy/G2kK/jo8ERZL0aMQHA/FZPdzrtckfZaPJrt7nBXyV/9H6QFHSU 00nnUxV8dKhqIk0zkrxSDh4zfyKf+G2ndFGfPLLO4TVxHmlf5MvX0YN3hWbgw7m0535f afWuBR1YbFJgrQWRN9WoniEUw2JGlVbkB3B1fLOu+EBUpXlEjMayaODEYkfz0aJoO3vf utrQ== X-Gm-Message-State: AOAM532ymr53zy5biNOH/7+WYf+uWYEAze+UtrPwlPATUh7OiMZTGWLS W8B6R0gp3Wu5scI160HvxaNqw8o9L+bc1Tk4gGQ= X-Google-Smtp-Source: ABdhPJzcV9Dgs0OBsJI5xRf36V6rTNub8KJEkdWRl4pGt0D5cWq60bPT79zujsx9arIvi3OSq11DIP7TVoq9zAQh1Co= X-Received: by 2002:a63:af06:0:b0:378:3582:a49f with SMTP id w6-20020a63af06000000b003783582a49fmr5058930pge.125.1650653263899; Fri, 22 Apr 2022 11:47:43 -0700 (PDT) MIME-Version: 1.0 References: <20220414232129.1886210-1-hjl.tools@gmail.com> <20220414232129.1886210-4-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Fri, 22 Apr 2022 11:47:07 -0700 Message-ID: Subject: Re: [PATCH v10 3/7] elf: Support DT_RELR relative relocation format [BZ #27924] To: Adhemerval Zanella Cc: GNU C Library , Fangrui Song Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3024.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Apr 2022 18:47:48 -0000 On Wed, Apr 20, 2022 at 6:44 AM Adhemerval Zanella wrote: > > > > On 14/04/2022 20:21, H.J. Lu wrote: > > From: Fangrui Song > > > > 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. > > 6. Move ELF_DYNAMIC_DO_RELR before ELF_DYNAMIC_DO_REL. > > Patch looks good, only a small nit below. > > Reviewed-by: Adhemerval Zanella > > > --- > > configure | 37 ++++++++++++++++++++++++ > > configure.ac | 5 ++++ > > elf/Makefile | 25 ++++++++++++++++ > > elf/dynamic-link.h | 34 ++++++++++++++++++++++ > > elf/get-dynamic-info.h | 3 ++ > > elf/tst-relr-pie.c | 1 + > > elf/tst-relr.c | 65 ++++++++++++++++++++++++++++++++++++++++++ > > 7 files changed, 170 insertions(+) > > create mode 100644 elf/tst-relr-pie.c > > create mode 100644 elf/tst-relr.c > > > > diff --git a/configure b/configure > > index d2f413d05d..c0c2246597 100755 > > --- a/configure > > +++ b/configure > > @@ -6076,6 +6076,43 @@ $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 < > +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" > > + > > { $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 b6a747dece..66cad71431 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -1336,6 +1336,11 @@ 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]) > > + > > LIBC_LINKER_FEATURE([--no-dynamic-linker], > > [-Wl,--no-dynamic-linker], > > [libc_cv_no_dynamic_linker=yes], > > Ok. > > > diff --git a/elf/Makefile b/elf/Makefile > > index ee8778c3a2..3f3e711dd2 100644 > > --- a/elf/Makefile > > +++ b/elf/Makefile > > @@ -541,6 +541,25 @@ tests-special += \ > > # tests-special > > endif > > endif > > +ifeq ($(have-dt-relr),yes) > > +tests += \ > > + tst-relr \ > > +# tests > > +ifeq ($(have-fpie),yes) > > +tests += \ > > + tst-relr-pie \ > > +# tests > > +tests-pie += \ > > + tst-relr-pie \ > > +# tests-pie > > +tests-special += \ > > + $(objpfx)check-tst-relr-pie.out \ > > +# tests-special > > +endif > > +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) > > @@ -2743,3 +2762,9 @@ $(objpfx)check-abi-version-libc.out: $(common-objpfx)libc.so > > | sed -ne '/.gnu.version_d/, /.gnu.version_r/ p' \ > > | grep GLIBC_ABI_DT_RELR > $@; \ > > $(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) > > Ok. > > > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h > > index 25dd7ca4f2..7325e0d9fc 100644 > > --- a/elf/dynamic-link.h > > +++ b/elf/dynamic-link.h > > @@ -146,12 +146,46 @@ 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]) \ > > Maybe check against NULL? Fixed in v11. > > + 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 int i = 0; (entry >>= 1) != 0; i++) \ > > + if ((entry & 1) != 0) \ > > + where[i] += l_addr; \ > > + where += CHAR_BIT * sizeof(ElfW(Relr)) - 1; \ > > + } \ > > + } \ > > + } while (0); > > + > > Ok. > > > /* 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)); \ > > + if (((map) != &GL(dl_rtld_map) || DO_RTLD_BOOTSTRAP)) \ > > + ELF_DYNAMIC_DO_RELR (map); \ > > ELF_DYNAMIC_DO_REL ((map), (scope), edr_lazy, skip_ifunc); \ > > ELF_DYNAMIC_DO_RELA ((map), (scope), edr_lazy, skip_ifunc); \ > > } while (0) > > Ok. > > > diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h > > index 6c2ccd6db4..6c2a3a12b1 100644 > > --- a/elf/get-dynamic-info.h > > +++ b/elf/get-dynamic-info.h > > @@ -89,6 +89,7 @@ elf_get_dynamic_info (struct link_map *l, bool bootstrap, > > # 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)); > > @@ -113,6 +114,8 @@ elf_get_dynamic_info (struct link_map *l, bool bootstrap, > > 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))); > > if (bootstrap || static_pie_bootstrap) > > { > > assert (info[DT_RUNPATH] == NULL); > > Ok. > > > diff --git a/elf/tst-relr-pie.c b/elf/tst-relr-pie.c > > new file mode 100644 > > index 0000000000..7df0cdbfd6 > > --- /dev/null > > +++ b/elf/tst-relr-pie.c > > @@ -0,0 +1 @@ > > +#include "tst-relr.c" > > diff --git a/elf/tst-relr.c b/elf/tst-relr.c > > new file mode 100644 > > index 0000000000..c634ce0d21 > > --- /dev/null > > +++ b/elf/tst-relr.c > > @@ -0,0 +1,65 @@ > > +/* Basic tests for DT_RELR. > > + Copyright (C) 2022 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library; if not, see > > + . */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +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) > > +{ > > + 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 defined __PIE__ || defined __pie__ || defined PIE || defined pie > > + TEST_VERIFY (has_relr); > > +#else > > + TEST_VERIFY (!has_relr); > > +#endif > > + } > > + > > + for (int i = 0; i < array_length (arr); i++) > > + TEST_VERIFY ((arr[i] == 0 && val[i] == 0) > > + || (arr[i] == &o && val[i] == 1) > > + || (arr[i] == &x && val[i] == 2)); > > + > > + return 0; > > +} > > + > > +#include > > Ok. -- H.J.