public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] Support DT_RELR relative relocation format
@ 2022-03-10 20:03 H.J. Lu
  2022-03-10 20:03 ` [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924] H.J. Lu
                   ` (5 more replies)
  0 siblings, 6 replies; 27+ messages in thread
From: H.J. Lu @ 2022-03-10 20:03 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song, Joseph Myers

Changes in v6:

1. Move ELF_DYNAMIC_DO_RELR before ELF_DYNAMIC_DO_REL.

Changes in v5:

1. Update NEWS entry with the linker option, -z pack-relative-relocs.
2. Remove elf/libc-abi-version.exp and use $(READELF) to check
GLIBC_ABI_DT_RELR.

Changes in v4:

1. Always enable GLIBC_ABI_DT_RELR check.
2. Use $(OBJDUMP) instead of $(NM) for GLIBC_ABI_DT_RELR check. 

Changes in v3:

1. Don't define SUPPORT_DT_RELR.
2. Enable DT_RELR in glibc shared libraries and position independent
executables (PIE) automatically if linker supports -z pack-relative-relocs.

Changes in v2:

1. Enable DT_RELR for all targets.
2. Issue an error if there is a DT_RELR entry without GLIBC_ABI_DT_RELR
dependency nor GLIBC_PRIVATE definition.

---
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.

Binutils 2.38 supports DT_RELR on x86 with the -z report-relative-reloc
option.  When DT_RELR is enabled, ld adds a GLIBC_ABI_DT_RELR symbol
version dependency on libc.so to outputs.  Issue an error if there is a
DT_RELR entry without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE
definition.

DT_RELR is enabled in glibc shared libraries and position independent
executables (PIE) automatically if linker supports -z pack-relative-relocs.

The DT_RELR usage in glibc can be disabled with --disable-default-dt-relr.

Tested with binutils 2.38 on i686, x86-64 and x32.

Fangrui Song (1):
  elf: Support DT_RELR relative relocation format [BZ #27924]

H.J. Lu (4):
  elf: Properly handle zero DT_RELA/DT_REL values
  Add GLIBC_ABI_DT_RELR for DT_RELR support
  Add --disable-default-dt-relr
  NEWS: Mention DT_RELR support

 INSTALL                |  6 ++++
 Makeconfig             | 19 +++++++++++++
 Makerules              |  2 ++
 NEWS                   |  3 +-
 configure              | 60 +++++++++++++++++++++++++++++++++++++++
 configure.ac           | 23 +++++++++++++++
 elf/Makefile           | 30 ++++++++++++++++++--
 elf/Versions           |  5 ++++
 elf/dl-version.c       | 33 ++++++++++++++++++++--
 elf/dynamic-link.h     | 40 +++++++++++++++++++++++++-
 elf/elf.h              | 13 +++++++--
 elf/get-dynamic-info.h | 19 +++++++++++--
 elf/tst-relr-pie.c     |  1 +
 elf/tst-relr.c         | 64 ++++++++++++++++++++++++++++++++++++++++++
 include/link.h         |  6 ++++
 manual/install.texi    |  5 ++++
 scripts/abilist.awk    |  2 ++
 scripts/versions.awk   |  7 ++++-
 18 files changed, 325 insertions(+), 13 deletions(-)
 create mode 100644 elf/tst-relr-pie.c
 create mode 100644 elf/tst-relr.c

-- 
2.35.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924]
  2022-03-10 20:03 [PATCH v6 0/5] Support DT_RELR relative relocation format H.J. Lu
@ 2022-03-10 20:03 ` H.J. Lu
  2022-03-29 16:34   ` Adhemerval Zanella
  2022-03-10 20:03 ` [PATCH v6 2/5] elf: Properly handle zero DT_RELA/DT_REL values H.J. Lu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-10 20:03 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song, Joseph Myers

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.
6. Move ELF_DYNAMIC_DO_RELR before ELF_DYNAMIC_DO_REL.
---
 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..d04c457e55 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])					      \
+      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));	      \
+    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)
diff --git a/elf/elf.h b/elf/elf.h
index 0735f6b579..0195029188 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 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);
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..56addd2ad4
--- /dev/null
+++ b/elf/tst-relr.c
@@ -0,0 +1,64 @@
+/* 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <link.h>
+#include <stdbool.h>
+#include <support/check.h>
+
+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 < sizeof (arr) / sizeof (arr[0]); i++)
+    TEST_VERIFY ((arr[i] == 0 && val[i] == 0)
+		 || (arr[i] == &o && val[i] == 1)
+		 || (arr[i] == &x && val[i] == 2));
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.35.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 2/5] elf: Properly handle zero DT_RELA/DT_REL values
  2022-03-10 20:03 [PATCH v6 0/5] Support DT_RELR relative relocation format H.J. Lu
  2022-03-10 20:03 ` [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924] H.J. Lu
@ 2022-03-10 20:03 ` H.J. Lu
  2022-03-29 16:38   ` Adhemerval Zanella
  2022-03-10 20:03 ` [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support H.J. Lu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-10 20:03 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song, Joseph Myers

With DT_RELR, there may be no relocations in DT_RELA/DT_REL and their
entry values are zero.  Don't relocate DT_RELA/DT_REL and update the
combined relocation start address if their entry values are zero.
---
 elf/dynamic-link.h     |  6 +++++-
 elf/get-dynamic-info.h | 18 ++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
index d04c457e55..252f407a12 100644
--- a/elf/dynamic-link.h
+++ b/elf/dynamic-link.h
@@ -84,7 +84,9 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
 	     __typeof (((ElfW(Dyn) *) 0)->d_un.d_val) nrelative; int lazy; }  \
       ranges[2] = { { 0, 0, 0, 0 }, { 0, 0, 0, 0 } };			      \
 									      \
-    if ((map)->l_info[DT_##RELOC])					      \
+    /* With DT_RELR, DT_RELA/DT_REL can have zero value.  */		      \
+    if ((map)->l_info[DT_##RELOC]					      \
+	&& (map)->l_info[DT_##RELOC]->d_un.d_ptr != 0)			      \
       {									      \
 	ranges[0].start = D_PTR ((map), l_info[DT_##RELOC]);		      \
 	ranges[0].size = (map)->l_info[DT_##RELOC##SZ]->d_un.d_val;	      \
@@ -98,6 +100,8 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
 	ElfW(Addr) start = D_PTR ((map), l_info[DT_JMPREL]);		      \
 	ElfW(Addr) size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val;	      \
 									      \
+	if (ranges[0].start == 0)					      \
+	  ranges[0].start = start;					      \
 	if (ranges[0].start + ranges[0].size == (start + size))		      \
 	  ranges[0].size -= size;					      \
 	if (!(do_lazy)							      \
diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
index 6c2a3a12b1..f4b957684b 100644
--- a/elf/get-dynamic-info.h
+++ b/elf/get-dynamic-info.h
@@ -83,16 +83,26 @@ elf_get_dynamic_info (struct link_map *l, bool bootstrap,
       ADJUST_DYN_INFO (DT_PLTGOT);
       ADJUST_DYN_INFO (DT_STRTAB);
       ADJUST_DYN_INFO (DT_SYMTAB);
+      ADJUST_DYN_INFO (DT_RELR);
+      ADJUST_DYN_INFO (DT_JMPREL);
+      ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
+      ADJUST_DYN_INFO (ADDRIDX (DT_GNU_HASH));
+# undef ADJUST_DYN_INFO
+
+      /* DT_RELA/DT_REL are mandatory.  But they may have zero value if
+	 there is DT_RELR.  Don't relocate them if they are zero.  */
+# define ADJUST_DYN_INFO(tag) \
+      do								      \
+	if (info[tag] != NULL && info[tag]->d_un.d_ptr != 0)		      \
+         info[tag]->d_un.d_ptr += l_addr;				      \
+      while (0)
+
 # if ! ELF_MACHINE_NO_RELA
       ADJUST_DYN_INFO (DT_RELA);
 # endif
 # 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));
 # undef ADJUST_DYN_INFO
     }
   if (info[DT_PLTREL] != NULL)
-- 
2.35.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-10 20:03 [PATCH v6 0/5] Support DT_RELR relative relocation format H.J. Lu
  2022-03-10 20:03 ` [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924] H.J. Lu
  2022-03-10 20:03 ` [PATCH v6 2/5] elf: Properly handle zero DT_RELA/DT_REL values H.J. Lu
@ 2022-03-10 20:03 ` H.J. Lu
  2022-03-29 16:52   ` Adhemerval Zanella
  2022-03-10 20:03 ` [PATCH v6 4/5] Add --disable-default-dt-relr H.J. Lu
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-10 20:03 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song, Joseph Myers

The EI_ABIVERSION field of the ELF header in executables and shared
libraries can be bumped to indicate the minimum ABI requirement on the
dynamic linker.  However, EI_ABIVERSION in executables isn't checked by
the Linux kernel ELF loader nor the existing dynamic linker.  Executables
will crash mysteriously if the dynamic linker doesn't support the ABI
features required by the EI_ABIVERSION field.  The dynamic linker should
be changed to check EI_ABIVERSION in executables.

Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support so
that the existing dynamic linkers will issue an error on executables with
GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR entry
without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.

Support __placeholder_only_for_empty_version_map as the placeholder symbol
used only for empty version map to generate GLIBC_ABI_DT_RELR without any
symbols.
---
 elf/Makefile         | 12 ++++++++++--
 elf/Versions         |  5 +++++
 elf/dl-version.c     | 33 +++++++++++++++++++++++++++++++--
 include/link.h       |  6 ++++++
 scripts/abilist.awk  |  2 ++
 scripts/versions.awk |  7 ++++++-
 6 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/elf/Makefile b/elf/Makefile
index fd462ba315..d3118b9777 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1113,8 +1113,10 @@ $(eval $(call include_dsosort_tests,dso-sort-tests-1.def))
 $(eval $(call include_dsosort_tests,dso-sort-tests-2.def))
 endif
 
-check-abi: $(objpfx)check-abi-ld.out
-tests-special += $(objpfx)check-abi-ld.out
+check-abi: $(objpfx)check-abi-ld.out \
+	   $(objpfx)check-abi-version-libc.out
+tests-special += $(objpfx)check-abi-ld.out \
+	   $(objpfx)check-abi-version-libc.out
 update-abi: update-abi-ld
 update-all-abi: update-all-abi-ld
 
@@ -2739,3 +2741,9 @@ $(objpfx)check-tst-relr-pie.out: $(objpfx)tst-relr-pie
 		| sed -ne '/required from libc.so/,$$ p' \
 		| grep GLIBC_ABI_DT_RELR > $@; \
 	$(evaluate-test)
+
+$(objpfx)check-abi-version-libc.out: $(common-objpfx)libc.so
+	LC_ALL=C $(READELF) -V -W $< \
+		| sed -ne '/.gnu.version_d/, /.gnu.version_r/ p' \
+		| grep GLIBC_ABI_DT_RELR > $@; \
+	$(evaluate-test)
diff --git a/elf/Versions b/elf/Versions
index 8bed855d8c..a9ff278de7 100644
--- a/elf/Versions
+++ b/elf/Versions
@@ -23,6 +23,11 @@ libc {
   GLIBC_2.35 {
     _dl_find_object;
   }
+  GLIBC_ABI_DT_RELR {
+    # This symbol is used only for empty version map and will be removed
+    # by scripts/versions.awk.
+    __placeholder_only_for_empty_version_map;
+  }
   GLIBC_PRIVATE {
     # functions used in other libraries
     __libc_early_init;
diff --git a/elf/dl-version.c b/elf/dl-version.c
index b47bd91727..720ec596a5 100644
--- a/elf/dl-version.c
+++ b/elf/dl-version.c
@@ -214,12 +214,20 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
 	      while (1)
 		{
 		  /* Match the symbol.  */
+		  const char *string = strtab + aux->vna_name;
 		  result |= match_symbol (DSO_FILENAME (map->l_name),
 					  map->l_ns, aux->vna_hash,
-					  strtab + aux->vna_name,
-					  needed->l_real, verbose,
+					  string, needed->l_real, verbose,
 					  aux->vna_flags & VER_FLG_WEAK);
 
+		  if (map->l_abi_version == lav_none
+		      /* 0xfd0e42: _dl_elf_hash ("GLIBC_ABI_DT_RELR").  */
+		      && aux->vna_hash == 0xfd0e42
+		      && __glibc_likely (strcmp (string,
+						 "GLIBC_ABI_DT_RELR")
+					 == 0))
+		    map->l_abi_version = lav_dt_relr_ref;
+
 		  /* Compare the version index.  */
 		  if ((unsigned int) (aux->vna_other & 0x7fff) > ndx_high)
 		    ndx_high = aux->vna_other & 0x7fff;
@@ -253,6 +261,16 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
       ent = (ElfW(Verdef) *) (map->l_addr + def->d_un.d_ptr);
       while (1)
 	{
+	  /* 0x0963cf85: _dl_elf_hash ("GLIBC_PRIVATE").  */
+	  if (ent->vd_hash == 0x0963cf85)
+	    {
+	      ElfW(Verdaux) *aux = (ElfW(Verdaux) *) ((char *) ent
+						      + ent->vd_aux);
+	      if (__glibc_likely (strcmp ("GLIBC_PRIVATE",
+					  strtab + aux->vda_name) == 0))
+		map->l_abi_version = lav_private_def;
+	    }
+
 	  if ((unsigned int) (ent->vd_ndx & 0x7fff) > ndx_high)
 	    ndx_high = ent->vd_ndx & 0x7fff;
 
@@ -352,6 +370,17 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
 	}
     }
 
+  /* Issue an error if there is a DT_RELR entry without GLIBC_ABI_DT_RELR
+     dependency nor GLIBC_PRIVATE definition.  */
+  if (map->l_info[DT_RELR] != NULL
+      && __glibc_unlikely (map->l_abi_version == lav_none))
+    {
+      _dl_exception_create
+	(&exception, DSO_FILENAME (map->l_name),
+	 N_("DT_RELR without GLIBC_ABI_DT_RELR dependency"));
+      goto call_error;
+    }
+
   return result;
 }
 
diff --git a/include/link.h b/include/link.h
index 03db14c7b0..8ec5e35cf2 100644
--- a/include/link.h
+++ b/include/link.h
@@ -177,6 +177,12 @@ struct link_map
 	lt_library,		/* Library needed by main executable.  */
 	lt_loaded		/* Extra run-time loaded shared object.  */
       } l_type:2;
+    enum			/* ABI dependency of this object.  */
+      {
+	lav_none,		/* No ABI dependency.  */
+	lav_dt_relr_ref,	/* Need GLIBC_ABI_DT_RELR.  */
+	lav_private_def		/* Define GLIBC_PRIVATE.  */
+      } l_abi_version:2;
     unsigned int l_relocated:1;	/* Nonzero if object's relocations done.  */
     unsigned int l_init_called:1; /* Nonzero if DT_INIT function called.  */
     unsigned int l_global:1;	/* Nonzero if object in _dl_global_scope.  */
diff --git a/scripts/abilist.awk b/scripts/abilist.awk
index 24a34ccbed..6cc7af6ac8 100644
--- a/scripts/abilist.awk
+++ b/scripts/abilist.awk
@@ -55,6 +55,8 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
   # caused STV_HIDDEN symbols to appear in .dynsym, though that is useless.
   if (NF > 7 && $7 == ".hidden") next;
 
+  if (version ~ /^GLIBC_ABI_/ && !include_abi_version) next;
+
   if (version == "GLIBC_PRIVATE" && !include_private) next;
 
   desc = "";
diff --git a/scripts/versions.awk b/scripts/versions.awk
index 357ad1355e..d70b07bd1a 100644
--- a/scripts/versions.awk
+++ b/scripts/versions.awk
@@ -185,8 +185,13 @@ END {
 	closeversion(oldver, veryoldver);
 	veryoldver = oldver;
       }
-      printf("%s {\n  global:\n", $2) > outfile;
       oldver = $2;
+      # Skip the placeholder symbol used only for empty version map.
+      if ($3 == "__placeholder_only_for_empty_version_map;") {
+	printf("%s {\n", $2) > outfile;
+	continue;
+      }
+      printf("%s {\n  global:\n", $2) > outfile;
     }
     printf("   ") > outfile;
     for (n = 3; n <= NF; ++n) {
-- 
2.35.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 4/5] Add --disable-default-dt-relr
  2022-03-10 20:03 [PATCH v6 0/5] Support DT_RELR relative relocation format H.J. Lu
                   ` (2 preceding siblings ...)
  2022-03-10 20:03 ` [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support H.J. Lu
@ 2022-03-10 20:03 ` H.J. Lu
  2022-03-29 17:19   ` Adhemerval Zanella
  2022-03-10 20:03 ` [PATCH v6 5/5] NEWS: Mention DT_RELR support H.J. Lu
  2022-03-10 20:09 ` [PATCH v6 0/5] Support DT_RELR relative relocation format Fangrui Song
  5 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-10 20:03 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song, Joseph Myers

Enable DT_RELR in glibc shared libraries and position independent
executables (PIE) automatically if linker supports -z pack-relative-relocs.

Also add a new configuration option, --disable-default-dt-relr, to
avoid DT_RELR usage in glibc shared libraries and PIEs.
---
 INSTALL             |  6 ++++++
 Makeconfig          | 19 +++++++++++++++++++
 Makerules           |  2 ++
 configure           | 18 ++++++++++++++++++
 configure.ac        | 13 +++++++++++++
 elf/Makefile        |  4 +++-
 manual/install.texi |  5 +++++
 7 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/INSTALL b/INSTALL
index 63c022d6b9..4a6506f11f 100644
--- a/INSTALL
+++ b/INSTALL
@@ -133,6 +133,12 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
      used with the GCC option, -static-pie, which is available with GCC
      8 or above, to create static PIE.
 
+'--disable-default-dt-relr'
+     Don't enable DT_RELR in glibc shared libraries and position
+     independent executables (PIE). By default, DT_RELR is enabled in
+     glibc shared libraries and position independent executables on
+     targets that support it.
+
 '--enable-cet'
 '--enable-cet=permissive'
      Enable Intel Control-flow Enforcement Technology (CET) support.
diff --git a/Makeconfig b/Makeconfig
index 47db08d6ae..70c0acc065 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -358,6 +358,23 @@ else
 real-static-start-installed-name = $(static-start-installed-name)
 endif
 
+# Linker option to enable and disable DT-RELR.
+ifeq ($(have-dt-relr),yes)
+dt-relr-ldflag = -Wl,-z,pack-relative-relocs
+no-dt-relr-ldflag = -Wl,-z,nopack-relative-relocs
+else
+dt-relr-ldflag =
+no-dt-relr-ldflag =
+endif
+
+# Default linker option for DT-RELR.
+ifeq (yes,$(build-dt-relr-default))
+default-rt-relr-ldflag = $(dt-relr-ldflag)
+else
+default-rt-relr-ldflag = $(no-dt-relr-ldflag)
+endif
+LDFLAGS-rtld += $(default-rt-relr-ldflag)
+
 ifeq (yesyes,$(build-shared)$(have-z-combreloc))
 combreloc-LDFLAGS = -Wl,-z,combreloc
 LDFLAGS.so += $(combreloc-LDFLAGS)
@@ -419,6 +436,7 @@ link-extra-libs-tests = $(libsupport)
 # Command for linking PIE programs with the C library.
 ifndef +link-pie
 +link-pie-before-inputs = $(if $($(@F)-no-pie),$(no-pie-ldflag),-pie) \
+	     $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
 	     -Wl,-O1 -nostdlib -nostartfiles \
 	     $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
 	     $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
@@ -451,6 +469,7 @@ endif
 ifndef +link-static
 +link-static-before-inputs = -nostdlib -nostartfiles -static \
 	      $(if $($(@F)-no-pie),$(no-pie-ldflag),$(static-pie-ldflag)) \
+	      $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
 	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F))  \
 	      $(firstword $(CRT-$(@F)) $(csu-objpfx)$(real-static-start-installed-name)) \
 	      $(+preinit) $(+prectorT)
diff --git a/Makerules b/Makerules
index 428464f092..7c1da551bf 100644
--- a/Makerules
+++ b/Makerules
@@ -536,6 +536,7 @@ lib%.so: lib%_pic.a $(+preinit) $(+postinit) $(link-libc-deps)
 define build-shlib-helper
 $(LINK.o) -shared -static-libgcc -Wl,-O1 $(sysdep-LDFLAGS) \
 	  $(if $($(@F)-no-z-defs)$(no-z-defs),,-Wl,-z,defs) $(rtld-LDFLAGS) \
+	  $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
 	  $(extra-B-$(@F:lib%.so=%).so) -B$(csu-objpfx) \
 	  $(extra-B-$(@F:lib%.so=%).so) $(load-map-file) \
 	  -Wl,-soname=lib$(libprefix)$(@F:lib%.so=%).so$($(@F)-version) \
@@ -595,6 +596,7 @@ endef
 define build-module-helper
 $(LINK.o) -shared -static-libgcc $(sysdep-LDFLAGS) $(rtld-LDFLAGS) \
 	  $(if $($(@F)-no-z-defs)$(no-z-defs),,-Wl,-z,defs) \
+	  $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
 	  -B$(csu-objpfx) $(load-map-file) \
 	  $(LDFLAGS.so) $(LDFLAGS-$(@F:%.so=%).so) \
 	  $(link-test-modules-rpath-link) \
diff --git a/configure b/configure
index 9156e29fe9..ee4137a977 100755
--- a/configure
+++ b/configure
@@ -768,6 +768,7 @@ enable_sanity_checks
 enable_shared
 enable_profile
 enable_default_pie
+enable_default_dt_relr
 enable_timezone_tools
 enable_hardcoded_path_in_tests
 enable_hidden_plt
@@ -1425,6 +1426,7 @@ Optional Features:
   --enable-profile        build profiled library [default=no]
   --disable-default-pie   Do not build glibc programs and the testsuite as PIE
                           [default=no]
+  --disable-dt-relr       Do not enable DT_RELR in glibc[default=no]
   --disable-timezone-tools
                           do not install timezone tools [default=install]
   --enable-hardcoded-path-in-tests
@@ -3441,6 +3443,13 @@ else
   default_pie=yes
 fi
 
+# Check whether --enable-default-dt-relr was given.
+if test "${enable_default_dt_relr+set}" = set; then :
+  enableval=$enable_default_dt_relr; default_dt_relr=$enableval
+else
+  default_dt_relr=yes
+fi
+
 # Check whether --enable-timezone-tools was given.
 if test "${enable_timezone_tools+set}" = set; then :
   enableval=$enable_timezone_tools; enable_timezone_tools=$enableval
@@ -7136,6 +7145,15 @@ fi
 config_vars="$config_vars
 enable-static-pie = $libc_cv_static_pie"
 
+# Disable build-dt-relr-default if linker does not support it or glibc is
+# configured with --disable-default-dt-relr.
+build_dt_relr_default=$default_dt_relr
+if test "x$build_dt_relr_default" != xno; then
+  build_dt_relr_default=$libc_cv_dt_relr
+fi
+config_vars="$config_vars
+build-dt-relr-default = $build_dt_relr_default"
+
 # Set the `multidir' variable by grabbing the variable from the compiler.
 # We do it once and save the result in a generated makefile.
 libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
diff --git a/configure.ac b/configure.ac
index 5c09871fee..680b036ffa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -197,6 +197,11 @@ AC_ARG_ENABLE([default-pie],
 			     [Do not build glibc programs and the testsuite as PIE @<:@default=no@:>@]),
 	      [default_pie=$enableval],
 	      [default_pie=yes])
+AC_ARG_ENABLE([default-dt-relr],
+	      AS_HELP_STRING([--disable-dt-relr],
+			     [Do not enable DT_RELR in glibc@<:@default=no@:>@]),
+	      [default_dt_relr=$enableval],
+	      [default_dt_relr=yes])
 AC_ARG_ENABLE([timezone-tools],
 	      AS_HELP_STRING([--disable-timezone-tools],
 			     [do not install timezone tools @<:@default=install@:>@]),
@@ -1914,6 +1919,14 @@ if test "$libc_cv_static_pie" = "yes"; then
 fi
 LIBC_CONFIG_VAR([enable-static-pie], [$libc_cv_static_pie])
 
+# Disable build-dt-relr-default if linker does not support it or glibc is
+# configured with --disable-default-dt-relr.
+build_dt_relr_default=$default_dt_relr
+if test "x$build_dt_relr_default" != xno; then
+  build_dt_relr_default=$libc_cv_dt_relr
+fi
+LIBC_CONFIG_VAR([build-dt-relr-default], [$build_dt_relr_default])
+
 # Set the `multidir' variable by grabbing the variable from the compiler.
 # We do it once and save the result in a generated makefile.
 libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
diff --git a/elf/Makefile b/elf/Makefile
index d3118b9777..c4384536ba 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1567,6 +1567,7 @@ $(objpfx)nodlopen2.out: $(objpfx)nodlopenmod2.so
 
 $(objpfx)filtmod1.so: $(objpfx)filtmod1.os $(objpfx)filtmod2.so
 	$(LINK.o) -shared -o $@ -B$(csu-objpfx) $(LDFLAGS.so) \
+		  $(default-rt-relr-ldflag) \
 		  -L$(subst :, -L,$(rpath-link)) \
 		  -Wl,-rpath-link=$(rpath-link) \
 		  $< -Wl,-F,$(objpfx)filtmod2.so
@@ -2366,7 +2367,7 @@ $(objpfx)tst-big-note: $(objpfx)tst-big-note-lib.so
 # artificial, large note in tst-big-note-lib.o and invalidate the
 # test.
 $(objpfx)tst-big-note-lib.so: $(objpfx)tst-big-note-lib.o
-	$(LINK.o) -shared -o $@ $(LDFLAGS.so) $<
+	$(LINK.o) -shared -o $@ $(LDFLAGS.so) $(default-rt-relr-ldflag) $<
 
 $(objpfx)tst-unwind-ctor: $(objpfx)tst-unwind-ctor-lib.so
 
@@ -2672,6 +2673,7 @@ $(objpfx)tst-ro-dynamic: $(objpfx)tst-ro-dynamic-mod.so
 $(objpfx)tst-ro-dynamic-mod.so: $(objpfx)tst-ro-dynamic-mod.os \
   tst-ro-dynamic-mod.map
 	$(LINK.o) -nostdlib -nostartfiles -shared -o $@ \
+		$(default-rt-relr-ldflag) \
 		-Wl,--script=tst-ro-dynamic-mod.map \
 		$(objpfx)tst-ro-dynamic-mod.os
 
diff --git a/manual/install.texi b/manual/install.texi
index 29c52f2927..04ea996561 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -161,6 +161,11 @@ and architecture support it, static executables are built as static PIE and the
 resulting glibc can be used with the GCC option, -static-pie, which is
 available with GCC 8 or above, to create static PIE.
 
+@item --disable-default-dt-relr
+Don't enable DT_RELR in glibc shared libraries and position independent
+executables (PIE).  By default, DT_RELR is enabled in glibc shared
+libraries and position independent executables on targets that support it.
+
 @item --enable-cet
 @itemx --enable-cet=permissive
 Enable Intel Control-flow Enforcement Technology (CET) support.  When
-- 
2.35.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 5/5] NEWS: Mention DT_RELR support
  2022-03-10 20:03 [PATCH v6 0/5] Support DT_RELR relative relocation format H.J. Lu
                   ` (3 preceding siblings ...)
  2022-03-10 20:03 ` [PATCH v6 4/5] Add --disable-default-dt-relr H.J. Lu
@ 2022-03-10 20:03 ` H.J. Lu
  2022-03-29 17:25   ` Adhemerval Zanella
  2022-03-10 20:09 ` [PATCH v6 0/5] Support DT_RELR relative relocation format Fangrui Song
  5 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-10 20:03 UTC (permalink / raw)
  To: libc-alpha; +Cc: Fangrui Song, Joseph Myers

---
 NEWS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 626eeabf5d..5d64b794f1 100644
--- a/NEWS
+++ b/NEWS
@@ -9,7 +9,8 @@ Version 2.36
 
 Major new features:
 
-  [Add new features here]
+* Support DT_RELR relative relocation format generated with the linker
+  option, -z pack-relative-relocs.
 
 Deprecated and removed features, and other changes affecting compatibility:
 
-- 
2.35.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 0/5] Support DT_RELR relative relocation format
  2022-03-10 20:03 [PATCH v6 0/5] Support DT_RELR relative relocation format H.J. Lu
                   ` (4 preceding siblings ...)
  2022-03-10 20:03 ` [PATCH v6 5/5] NEWS: Mention DT_RELR support H.J. Lu
@ 2022-03-10 20:09 ` Fangrui Song
  5 siblings, 0 replies; 27+ messages in thread
From: Fangrui Song @ 2022-03-10 20:09 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha, Joseph Myers

On 2022-03-10, H.J. Lu wrote:
>Changes in v6:
>
>1. Move ELF_DYNAMIC_DO_RELR before ELF_DYNAMIC_DO_REL.
>
>Changes in v5:
>
>1. Update NEWS entry with the linker option, -z pack-relative-relocs.
>2. Remove elf/libc-abi-version.exp and use $(READELF) to check
>GLIBC_ABI_DT_RELR.
>
>Changes in v4:
>
>1. Always enable GLIBC_ABI_DT_RELR check.
>2. Use $(OBJDUMP) instead of $(NM) for GLIBC_ABI_DT_RELR check.
>
>Changes in v3:
>
>1. Don't define SUPPORT_DT_RELR.
>2. Enable DT_RELR in glibc shared libraries and position independent
>executables (PIE) automatically if linker supports -z pack-relative-relocs.
>
>Changes in v2:
>
>1. Enable DT_RELR for all targets.
>2. Issue an error if there is a DT_RELR entry without GLIBC_ABI_DT_RELR
>dependency nor GLIBC_PRIVATE definition.
>
>---
>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.
>
>Binutils 2.38 supports DT_RELR on x86 with the -z report-relative-reloc
>option.  When DT_RELR is enabled, ld adds a GLIBC_ABI_DT_RELR symbol
>version dependency on libc.so to outputs.  Issue an error if there is a
>DT_RELR entry without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE
>definition.
>
>DT_RELR is enabled in glibc shared libraries and position independent
>executables (PIE) automatically if linker supports -z pack-relative-relocs.
>
>The DT_RELR usage in glibc can be disabled with --disable-default-dt-relr.
>
>Tested with binutils 2.38 on i686, x86-64 and x32.
>
>Fangrui Song (1):
>  elf: Support DT_RELR relative relocation format [BZ #27924]
>
>H.J. Lu (4):
>  elf: Properly handle zero DT_RELA/DT_REL values
>  Add GLIBC_ABI_DT_RELR for DT_RELR support
>  Add --disable-default-dt-relr
>  NEWS: Mention DT_RELR support
>
> INSTALL                |  6 ++++
> Makeconfig             | 19 +++++++++++++
> Makerules              |  2 ++
> NEWS                   |  3 +-
> configure              | 60 +++++++++++++++++++++++++++++++++++++++
> configure.ac           | 23 +++++++++++++++
> elf/Makefile           | 30 ++++++++++++++++++--
> elf/Versions           |  5 ++++
> elf/dl-version.c       | 33 ++++++++++++++++++++--
> elf/dynamic-link.h     | 40 +++++++++++++++++++++++++-
> elf/elf.h              | 13 +++++++--
> elf/get-dynamic-info.h | 19 +++++++++++--
> elf/tst-relr-pie.c     |  1 +
> elf/tst-relr.c         | 64 ++++++++++++++++++++++++++++++++++++++++++
> include/link.h         |  6 ++++
> manual/install.texi    |  5 ++++
> scripts/abilist.awk    |  2 ++
> scripts/versions.awk   |  7 ++++-
> 18 files changed, 325 insertions(+), 13 deletions(-)
> create mode 100644 elf/tst-relr-pie.c
> create mode 100644 elf/tst-relr.c
>
>-- 
>2.35.1
>

Thanks. Every change looks good to me now.
Other folks can check my extensive comments on
previous iterations on the patch series
https://sourceware.org/pipermail/libc-alpha/2022-March/

Reviewed-by: Fangrui Song <maskray@google.com>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924]
  2022-03-10 20:03 ` [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924] H.J. Lu
@ 2022-03-29 16:34   ` Adhemerval Zanella
  2022-03-29 22:34     ` H.J. Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-29 16:34 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Joseph Myers



On 10/03/2022 17:03, H.J. Lu via Libc-alpha 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.

I really don't think ia64 ELFCLASS32 should be blocker for this change.

> 
> * 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:

Agreed.

> 
> * 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 ok, some comments below.

> ---
>  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
> +

I think you don't need to setup have_dt_relr, it does not accomplish anything here
(LIBC_CONFIG_VAR already sets the required have-dt-relr on config.make).

>  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

Please use one line per test:

 tests += \
   tst-relr \
   tst-relr-pie \
   # tests
 tests-pie +=
   tst-relr-pie \
   # tests-pie
 tests-special += \
   $(objpfx)check-tst-relr-pie.out \
   # tests-special

Also, the tst-rel-pie should only be added if $(have-fpie):

  ifeq ($(have-fpie),yes)
  tests-pie += \
    tst-relr-pie \
    #tests-pie
  endf

> +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)

Ok.

> diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> index 25dd7ca4f2..d04c457e55 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])					      \
> +      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));	      \
> +    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/elf.h b/elf/elf.h
> index 0735f6b579..0195029188 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;
>  

Ok.

> +/* 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 */

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

Ok.

> new file mode 100644
> index 0000000000..56addd2ad4
> --- /dev/null
> +++ b/elf/tst-relr.c
> @@ -0,0 +1,64 @@
> +/* 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <link.h>
> +#include <stdbool.h>
> +#include <support/check.h>
> +
> +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 < sizeof (arr) / sizeof (arr[0]); i++)

Use array_length here.

> +    TEST_VERIFY ((arr[i] == 0 && val[i] == 0)
> +		 || (arr[i] == &o && val[i] == 1)
> +		 || (arr[i] == &x && val[i] == 2));
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

This fails without the rest of the patchset applied:

$ ./testrun.sh elf/tst-relr
elf/tst-relr: ./libc.so.6: version `GLIBC_ABI_DT_RELR' not found (required by elf/tst-relr)

I suggest you move it to third patch when GLIBC_ABI_DT_RELR is added.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 2/5] elf: Properly handle zero DT_RELA/DT_REL values
  2022-03-10 20:03 ` [PATCH v6 2/5] elf: Properly handle zero DT_RELA/DT_REL values H.J. Lu
@ 2022-03-29 16:38   ` Adhemerval Zanella
  2022-03-29 22:30     ` H.J. Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-29 16:38 UTC (permalink / raw)
  To: libc-alpha, H.J. Lu; +Cc: Joseph Myers



On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> With DT_RELR, there may be no relocations in DT_RELA/DT_REL and their
> entry values are zero.  Don't relocate DT_RELA/DT_REL and update the
> combined relocation start address if their entry values are zero.

Patch looks good with the two small fixes below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/dynamic-link.h     |  6 +++++-
>  elf/get-dynamic-info.h | 18 ++++++++++++++----
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> index d04c457e55..252f407a12 100644
> --- a/elf/dynamic-link.h
> +++ b/elf/dynamic-link.h
> @@ -84,7 +84,9 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
>  	     __typeof (((ElfW(Dyn) *) 0)->d_un.d_val) nrelative; int lazy; }  \
>        ranges[2] = { { 0, 0, 0, 0 }, { 0, 0, 0, 0 } };			      \
>  									      \
> -    if ((map)->l_info[DT_##RELOC])					      \
> +    /* With DT_RELR, DT_RELA/DT_REL can have zero value.  */		      \
> +    if ((map)->l_info[DT_##RELOC]					      \

Compare to NULL here.

> +	&& (map)->l_info[DT_##RELOC]->d_un.d_ptr != 0)			      \
>        {									      \
>  	ranges[0].start = D_PTR ((map), l_info[DT_##RELOC]);		      \
>  	ranges[0].size = (map)->l_info[DT_##RELOC##SZ]->d_un.d_val;	      \
> @@ -98,6 +100,8 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
>  	ElfW(Addr) start = D_PTR ((map), l_info[DT_JMPREL]);		      \
>  	ElfW(Addr) size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val;	      \
>  									      \
> +	if (ranges[0].start == 0)					      \
> +	  ranges[0].start = start;					      \
>  	if (ranges[0].start + ranges[0].size == (start + size))		      \
>  	  ranges[0].size -= size;					      \
>  	if (!(do_lazy)							      \

Ok.

> diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
> index 6c2a3a12b1..f4b957684b 100644
> --- a/elf/get-dynamic-info.h
> +++ b/elf/get-dynamic-info.h
> @@ -83,16 +83,26 @@ elf_get_dynamic_info (struct link_map *l, bool bootstrap,
>        ADJUST_DYN_INFO (DT_PLTGOT);
>        ADJUST_DYN_INFO (DT_STRTAB);
>        ADJUST_DYN_INFO (DT_SYMTAB);
> +      ADJUST_DYN_INFO (DT_RELR);
> +      ADJUST_DYN_INFO (DT_JMPREL);
> +      ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
> +      ADJUST_DYN_INFO (ADDRIDX (DT_GNU_HASH));
> +# undef ADJUST_DYN_INFO
> +
> +      /* DT_RELA/DT_REL are mandatory.  But they may have zero value if
> +	 there is DT_RELR.  Don't relocate them if they are zero.  */
> +# define ADJUST_DYN_INFO(tag) \
> +      do								      \
> +	if (info[tag] != NULL && info[tag]->d_un.d_ptr != 0)		      \
> +         info[tag]->d_un.d_ptr += l_addr;				      \
> +      while (0)
> +

Maybe use '{' and '}' on the do ... while.

>  # if ! ELF_MACHINE_NO_RELA
>        ADJUST_DYN_INFO (DT_RELA);
>  # endif
>  # 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));
>  # undef ADJUST_DYN_INFO
>      }
>    if (info[DT_PLTREL] != NULL)

Ok.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-10 20:03 ` [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support H.J. Lu
@ 2022-03-29 16:52   ` Adhemerval Zanella
  2022-03-29 22:29     ` H.J. Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-29 16:52 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Joseph Myers



On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> The EI_ABIVERSION field of the ELF header in executables and shared
> libraries can be bumped to indicate the minimum ABI requirement on the
> dynamic linker.  However, EI_ABIVERSION in executables isn't checked by
> the Linux kernel ELF loader nor the existing dynamic linker.  Executables
> will crash mysteriously if the dynamic linker doesn't support the ABI
> features required by the EI_ABIVERSION field.  The dynamic linker should
> be changed to check EI_ABIVERSION in executables.
> 
> Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support so
> that the existing dynamic linkers will issue an error on executables with
> GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR entry
> without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.
> 
> Support __placeholder_only_for_empty_version_map as the placeholder symbol
> used only for empty version map to generate GLIBC_ABI_DT_RELR without any
> symbols.

I think it only make sense to add the GLIBC_ABI_DT_RELR if the ABI actually 
supports DT_RELR, otherwise if the ABI start to support old glibcs might
wrongly assumes DT_RELR and fails with undefined behavior at runtime
(which this patch essentially tries to avoid).

> ---
>  elf/Makefile         | 12 ++++++++++--
>  elf/Versions         |  5 +++++
>  elf/dl-version.c     | 33 +++++++++++++++++++++++++++++++--
>  include/link.h       |  6 ++++++
>  scripts/abilist.awk  |  2 ++
>  scripts/versions.awk |  7 ++++++-
>  6 files changed, 60 insertions(+), 5 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index fd462ba315..d3118b9777 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1113,8 +1113,10 @@ $(eval $(call include_dsosort_tests,dso-sort-tests-1.def))
>  $(eval $(call include_dsosort_tests,dso-sort-tests-2.def))
>  endif
>  
> -check-abi: $(objpfx)check-abi-ld.out
> -tests-special += $(objpfx)check-abi-ld.out
> +check-abi: $(objpfx)check-abi-ld.out \
> +	   $(objpfx)check-abi-version-libc.out
> +tests-special += $(objpfx)check-abi-ld.out \
> +	   $(objpfx)check-abi-version-libc.out

One line per definition.

>  update-abi: update-abi-ld
>  update-all-abi: update-all-abi-ld
>  
> @@ -2739,3 +2741,9 @@ $(objpfx)check-tst-relr-pie.out: $(objpfx)tst-relr-pie
>  		| sed -ne '/required from libc.so/,$$ p' \
>  		| grep GLIBC_ABI_DT_RELR > $@; \
>  	$(evaluate-test)
> +
> +$(objpfx)check-abi-version-libc.out: $(common-objpfx)libc.so
> +	LC_ALL=C $(READELF) -V -W $< \
> +		| sed -ne '/.gnu.version_d/, /.gnu.version_r/ p' \
> +		| grep GLIBC_ABI_DT_RELR > $@; \
> +	$(evaluate-test)

This test should be enable iff for $(have-dt-relr) == yes.

> diff --git a/elf/Versions b/elf/Versions
> index 8bed855d8c..a9ff278de7 100644
> --- a/elf/Versions
> +++ b/elf/Versions
> @@ -23,6 +23,11 @@ libc {
>    GLIBC_2.35 {
>      _dl_find_object;
>    }
> +  GLIBC_ABI_DT_RELR {
> +    # This symbol is used only for empty version map and will be removed
> +    # by scripts/versions.awk.
> +    __placeholder_only_for_empty_version_map;
> +  }

Same as before. Maybe use the same strategy we do for time64: add a config.h
define to HAVE_DTRELR (set by configure) and use it to set the symbol if
it is defined.

This define would be used on dl-version.c to also enable the l_abi_version
check.

>    GLIBC_PRIVATE {
>      # functions used in other libraries
>      __libc_early_init;
> diff --git a/elf/dl-version.c b/elf/dl-version.c
> index b47bd91727..720ec596a5 100644
> --- a/elf/dl-version.c
> +++ b/elf/dl-version.c
> @@ -214,12 +214,20 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>  	      while (1)
>  		{
>  		  /* Match the symbol.  */
> +		  const char *string = strtab + aux->vna_name;
>  		  result |= match_symbol (DSO_FILENAME (map->l_name),
>  					  map->l_ns, aux->vna_hash,
> -					  strtab + aux->vna_name,
> -					  needed->l_real, verbose,
> +					  string, needed->l_real, verbose,
>  					  aux->vna_flags & VER_FLG_WEAK);
>  
> +		  if (map->l_abi_version == lav_none
> +		      /* 0xfd0e42: _dl_elf_hash ("GLIBC_ABI_DT_RELR").  */
> +		      && aux->vna_hash == 0xfd0e42
> +		      && __glibc_likely (strcmp (string,
> +						 "GLIBC_ABI_DT_RELR")
> +					 == 0))
> +		    map->l_abi_version = lav_dt_relr_ref;
> +
>  		  /* Compare the version index.  */
>  		  if ((unsigned int) (aux->vna_other & 0x7fff) > ndx_high)
>  		    ndx_high = aux->vna_other & 0x7fff;
> @@ -253,6 +261,16 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>        ent = (ElfW(Verdef) *) (map->l_addr + def->d_un.d_ptr);
>        while (1)
>  	{
> +	  /* 0x0963cf85: _dl_elf_hash ("GLIBC_PRIVATE").  */
> +	  if (ent->vd_hash == 0x0963cf85)
> +	    {
> +	      ElfW(Verdaux) *aux = (ElfW(Verdaux) *) ((char *) ent
> +						      + ent->vd_aux);
> +	      if (__glibc_likely (strcmp ("GLIBC_PRIVATE",
> +					  strtab + aux->vda_name) == 0))
> +		map->l_abi_version = lav_private_def;
> +	    }
> +
>  	  if ((unsigned int) (ent->vd_ndx & 0x7fff) > ndx_high)
>  	    ndx_high = ent->vd_ndx & 0x7fff;
>  
> @@ -352,6 +370,17 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>  	}
>      }
>  
> +  /* Issue an error if there is a DT_RELR entry without GLIBC_ABI_DT_RELR
> +     dependency nor GLIBC_PRIVATE definition.  */
> +  if (map->l_info[DT_RELR] != NULL
> +      && __glibc_unlikely (map->l_abi_version == lav_none))
> +    {
> +      _dl_exception_create
> +	(&exception, DSO_FILENAME (map->l_name),
> +	 N_("DT_RELR without GLIBC_ABI_DT_RELR dependency"));
> +      goto call_error;
> +    }
> +
>    return result;
>  }
>  
> diff --git a/include/link.h b/include/link.h
> index 03db14c7b0..8ec5e35cf2 100644
> --- a/include/link.h
> +++ b/include/link.h
> @@ -177,6 +177,12 @@ struct link_map
>  	lt_library,		/* Library needed by main executable.  */
>  	lt_loaded		/* Extra run-time loaded shared object.  */
>        } l_type:2;
> +    enum			/* ABI dependency of this object.  */
> +      {
> +	lav_none,		/* No ABI dependency.  */
> +	lav_dt_relr_ref,	/* Need GLIBC_ABI_DT_RELR.  */
> +	lav_private_def		/* Define GLIBC_PRIVATE.  */
> +      } l_abi_version:2;
>      unsigned int l_relocated:1;	/* Nonzero if object's relocations done.  */
>      unsigned int l_init_called:1; /* Nonzero if DT_INIT function called.  */
>      unsigned int l_global:1;	/* Nonzero if object in _dl_global_scope.  */
> diff --git a/scripts/abilist.awk b/scripts/abilist.awk
> index 24a34ccbed..6cc7af6ac8 100644
> --- a/scripts/abilist.awk
> +++ b/scripts/abilist.awk
> @@ -55,6 +55,8 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
>    # caused STV_HIDDEN symbols to appear in .dynsym, though that is useless.
>    if (NF > 7 && $7 == ".hidden") next;
>  
> +  if (version ~ /^GLIBC_ABI_/ && !include_abi_version) next;
> +
>    if (version == "GLIBC_PRIVATE" && !include_private) next;
>  
>    desc = "";
> diff --git a/scripts/versions.awk b/scripts/versions.awk
> index 357ad1355e..d70b07bd1a 100644
> --- a/scripts/versions.awk
> +++ b/scripts/versions.awk
> @@ -185,8 +185,13 @@ END {
>  	closeversion(oldver, veryoldver);
>  	veryoldver = oldver;
>        }
> -      printf("%s {\n  global:\n", $2) > outfile;
>        oldver = $2;
> +      # Skip the placeholder symbol used only for empty version map.
> +      if ($3 == "__placeholder_only_for_empty_version_map;") {
> +	printf("%s {\n", $2) > outfile;
> +	continue;
> +      }
> +      printf("%s {\n  global:\n", $2) > outfile;
>      }
>      printf("   ") > outfile;
>      for (n = 3; n <= NF; ++n) {

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 4/5] Add --disable-default-dt-relr
  2022-03-10 20:03 ` [PATCH v6 4/5] Add --disable-default-dt-relr H.J. Lu
@ 2022-03-29 17:19   ` Adhemerval Zanella
  2022-03-29 22:25     ` H.J. Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-29 17:19 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Joseph Myers



On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> Enable DT_RELR in glibc shared libraries and position independent
> executables (PIE) automatically if linker supports -z pack-relative-relocs.
> 
> Also add a new configuration option, --disable-default-dt-relr, to
> avoid DT_RELR usage in glibc shared libraries and PIEs.

LGTM, with two small nits below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  INSTALL             |  6 ++++++
>  Makeconfig          | 19 +++++++++++++++++++
>  Makerules           |  2 ++
>  configure           | 18 ++++++++++++++++++
>  configure.ac        | 13 +++++++++++++
>  elf/Makefile        |  4 +++-
>  manual/install.texi |  5 +++++
>  7 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/INSTALL b/INSTALL
> index 63c022d6b9..4a6506f11f 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -133,6 +133,12 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>       used with the GCC option, -static-pie, which is available with GCC
>       8 or above, to create static PIE.
>  
> +'--disable-default-dt-relr'
> +     Don't enable DT_RELR in glibc shared libraries and position
> +     independent executables (PIE). By default, DT_RELR is enabled in

Two space after period.

> +     glibc shared libraries and position independent executables on
> +     targets that support it.
> +
>  '--enable-cet'
>  '--enable-cet=permissive'
>       Enable Intel Control-flow Enforcement Technology (CET) support.
> diff --git a/Makeconfig b/Makeconfig
> index 47db08d6ae..70c0acc065 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -358,6 +358,23 @@ else
>  real-static-start-installed-name = $(static-start-installed-name)
>  endif
>  
> +# Linker option to enable and disable DT-RELR.
> +ifeq ($(have-dt-relr),yes)
> +dt-relr-ldflag = -Wl,-z,pack-relative-relocs
> +no-dt-relr-ldflag = -Wl,-z,nopack-relative-relocs
> +else
> +dt-relr-ldflag =
> +no-dt-relr-ldflag =
> +endif
> +
> +# Default linker option for DT-RELR.
> +ifeq (yes,$(build-dt-relr-default))
> +default-rt-relr-ldflag = $(dt-relr-ldflag)
> +else
> +default-rt-relr-ldflag = $(no-dt-relr-ldflag)
> +endif
> +LDFLAGS-rtld += $(default-rt-relr-ldflag)
> +
>  ifeq (yesyes,$(build-shared)$(have-z-combreloc))
>  combreloc-LDFLAGS = -Wl,-z,combreloc
>  LDFLAGS.so += $(combreloc-LDFLAGS)
> @@ -419,6 +436,7 @@ link-extra-libs-tests = $(libsupport)
>  # Command for linking PIE programs with the C library.
>  ifndef +link-pie
>  +link-pie-before-inputs = $(if $($(@F)-no-pie),$(no-pie-ldflag),-pie) \
> +	     $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
>  	     -Wl,-O1 -nostdlib -nostartfiles \
>  	     $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
>  	     $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
> @@ -451,6 +469,7 @@ endif
>  ifndef +link-static
>  +link-static-before-inputs = -nostdlib -nostartfiles -static \
>  	      $(if $($(@F)-no-pie),$(no-pie-ldflag),$(static-pie-ldflag)) \
> +	      $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
>  	      $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F))  \
>  	      $(firstword $(CRT-$(@F)) $(csu-objpfx)$(real-static-start-installed-name)) \
>  	      $(+preinit) $(+prectorT)
> diff --git a/Makerules b/Makerules
> index 428464f092..7c1da551bf 100644
> --- a/Makerules
> +++ b/Makerules
> @@ -536,6 +536,7 @@ lib%.so: lib%_pic.a $(+preinit) $(+postinit) $(link-libc-deps)
>  define build-shlib-helper
>  $(LINK.o) -shared -static-libgcc -Wl,-O1 $(sysdep-LDFLAGS) \
>  	  $(if $($(@F)-no-z-defs)$(no-z-defs),,-Wl,-z,defs) $(rtld-LDFLAGS) \
> +	  $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
>  	  $(extra-B-$(@F:lib%.so=%).so) -B$(csu-objpfx) \
>  	  $(extra-B-$(@F:lib%.so=%).so) $(load-map-file) \
>  	  -Wl,-soname=lib$(libprefix)$(@F:lib%.so=%).so$($(@F)-version) \
> @@ -595,6 +596,7 @@ endef
>  define build-module-helper
>  $(LINK.o) -shared -static-libgcc $(sysdep-LDFLAGS) $(rtld-LDFLAGS) \
>  	  $(if $($(@F)-no-z-defs)$(no-z-defs),,-Wl,-z,defs) \
> +	  $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
>  	  -B$(csu-objpfx) $(load-map-file) \
>  	  $(LDFLAGS.so) $(LDFLAGS-$(@F:%.so=%).so) \
>  	  $(link-test-modules-rpath-link) \

Ok.

> diff --git a/configure b/configure
> index 9156e29fe9..ee4137a977 100755
> --- a/configure
> +++ b/configure
> @@ -768,6 +768,7 @@ enable_sanity_checks
>  enable_shared
>  enable_profile
>  enable_default_pie
> +enable_default_dt_relr
>  enable_timezone_tools
>  enable_hardcoded_path_in_tests
>  enable_hidden_plt
> @@ -1425,6 +1426,7 @@ Optional Features:
>    --enable-profile        build profiled library [default=no]
>    --disable-default-pie   Do not build glibc programs and the testsuite as PIE
>                            [default=no]
> +  --disable-dt-relr       Do not enable DT_RELR in glibc[default=no]
>    --disable-timezone-tools
>                            do not install timezone tools [default=install]
>    --enable-hardcoded-path-in-tests
> @@ -3441,6 +3443,13 @@ else
>    default_pie=yes
>  fi
>  
> +# Check whether --enable-default-dt-relr was given.
> +if test "${enable_default_dt_relr+set}" = set; then :
> +  enableval=$enable_default_dt_relr; default_dt_relr=$enableval
> +else
> +  default_dt_relr=yes
> +fi
> +
>  # Check whether --enable-timezone-tools was given.
>  if test "${enable_timezone_tools+set}" = set; then :
>    enableval=$enable_timezone_tools; enable_timezone_tools=$enableval
> @@ -7136,6 +7145,15 @@ fi
>  config_vars="$config_vars
>  enable-static-pie = $libc_cv_static_pie"
>  
> +# Disable build-dt-relr-default if linker does not support it or glibc is
> +# configured with --disable-default-dt-relr.
> +build_dt_relr_default=$default_dt_relr
> +if test "x$build_dt_relr_default" != xno; then
> +  build_dt_relr_default=$libc_cv_dt_relr
> +fi
> +config_vars="$config_vars
> +build-dt-relr-default = $build_dt_relr_default"
> +
>  # Set the `multidir' variable by grabbing the variable from the compiler.
>  # We do it once and save the result in a generated makefile.
>  libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
> diff --git a/configure.ac b/configure.ac
> index 5c09871fee..680b036ffa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -197,6 +197,11 @@ AC_ARG_ENABLE([default-pie],
>  			     [Do not build glibc programs and the testsuite as PIE @<:@default=no@:>@]),
>  	      [default_pie=$enableval],
>  	      [default_pie=yes])
> +AC_ARG_ENABLE([default-dt-relr],
> +	      AS_HELP_STRING([--disable-dt-relr],
> +			     [Do not enable DT_RELR in glibc@<:@default=no@:>@]),
> +	      [default_dt_relr=$enableval],
> +	      [default_dt_relr=yes])
>  AC_ARG_ENABLE([timezone-tools],
>  	      AS_HELP_STRING([--disable-timezone-tools],
>  			     [do not install timezone tools @<:@default=install@:>@]),
> @@ -1914,6 +1919,14 @@ if test "$libc_cv_static_pie" = "yes"; then
>  fi
>  LIBC_CONFIG_VAR([enable-static-pie], [$libc_cv_static_pie])
>  
> +# Disable build-dt-relr-default if linker does not support it or glibc is

Maybe 'or if'.

> +# configured with --disable-default-dt-relr.
> +build_dt_relr_default=$default_dt_relr
> +if test "x$build_dt_relr_default" != xno; then
> +  build_dt_relr_default=$libc_cv_dt_relr
> +fi
> +LIBC_CONFIG_VAR([build-dt-relr-default], [$build_dt_relr_default])
> +
>  # Set the `multidir' variable by grabbing the variable from the compiler.
>  # We do it once and save the result in a generated makefile.
>  libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
> diff --git a/elf/Makefile b/elf/Makefile
> index d3118b9777..c4384536ba 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1567,6 +1567,7 @@ $(objpfx)nodlopen2.out: $(objpfx)nodlopenmod2.so
>  
>  $(objpfx)filtmod1.so: $(objpfx)filtmod1.os $(objpfx)filtmod2.so
>  	$(LINK.o) -shared -o $@ -B$(csu-objpfx) $(LDFLAGS.so) \
> +		  $(default-rt-relr-ldflag) \
>  		  -L$(subst :, -L,$(rpath-link)) \
>  		  -Wl,-rpath-link=$(rpath-link) \
>  		  $< -Wl,-F,$(objpfx)filtmod2.so
> @@ -2366,7 +2367,7 @@ $(objpfx)tst-big-note: $(objpfx)tst-big-note-lib.so
>  # artificial, large note in tst-big-note-lib.o and invalidate the
>  # test.
>  $(objpfx)tst-big-note-lib.so: $(objpfx)tst-big-note-lib.o
> -	$(LINK.o) -shared -o $@ $(LDFLAGS.so) $<
> +	$(LINK.o) -shared -o $@ $(LDFLAGS.so) $(default-rt-relr-ldflag) $<
>  
>  $(objpfx)tst-unwind-ctor: $(objpfx)tst-unwind-ctor-lib.so
>  
> @@ -2672,6 +2673,7 @@ $(objpfx)tst-ro-dynamic: $(objpfx)tst-ro-dynamic-mod.so
>  $(objpfx)tst-ro-dynamic-mod.so: $(objpfx)tst-ro-dynamic-mod.os \
>    tst-ro-dynamic-mod.map
>  	$(LINK.o) -nostdlib -nostartfiles -shared -o $@ \
> +		$(default-rt-relr-ldflag) \
>  		-Wl,--script=tst-ro-dynamic-mod.map \
>  		$(objpfx)tst-ro-dynamic-mod.os
>  
> diff --git a/manual/install.texi b/manual/install.texi
> index 29c52f2927..04ea996561 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -161,6 +161,11 @@ and architecture support it, static executables are built as static PIE and the
>  resulting glibc can be used with the GCC option, -static-pie, which is
>  available with GCC 8 or above, to create static PIE.
>  
> +@item --disable-default-dt-relr
> +Don't enable DT_RELR in glibc shared libraries and position independent
> +executables (PIE).  By default, DT_RELR is enabled in glibc shared
> +libraries and position independent executables on targets that support it.
> +
>  @item --enable-cet
>  @itemx --enable-cet=permissive
>  Enable Intel Control-flow Enforcement Technology (CET) support.  When

Ok.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 5/5] NEWS: Mention DT_RELR support
  2022-03-10 20:03 ` [PATCH v6 5/5] NEWS: Mention DT_RELR support H.J. Lu
@ 2022-03-29 17:25   ` Adhemerval Zanella
  2022-03-29 22:22     ` H.J. Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-29 17:25 UTC (permalink / raw)
  To: libc-alpha



On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> ---
>  NEWS | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/NEWS b/NEWS
> index 626eeabf5d..5d64b794f1 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -9,7 +9,8 @@ Version 2.36
>  
>  Major new features:
>  
> -  [Add new features here]
> +* Support DT_RELR relative relocation format generated with the linker

Maybe

 * Support for DT_RELR relocation relocation format has been added to
   glibc.  This is a new ELF section type that improves the size of relative
   relocations in shared object files and position independent
   executables (PIE).  DT_RELR support requires linker support for
   -z pack-relative-relocs option, which is supported for some abi in recent
   binutils versions.


> +  option, -z pack-relative-relocs.
>  
>  Deprecated and removed features, and other changes affecting compatibility:
>  

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 5/5] NEWS: Mention DT_RELR support
  2022-03-29 17:25   ` Adhemerval Zanella
@ 2022-03-29 22:22     ` H.J. Lu
  2022-03-29 23:34       ` Fangrui Song
  0 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-29 22:22 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library

On Tue, Mar 29, 2022 at 10:25 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> > ---
> >  NEWS | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 626eeabf5d..5d64b794f1 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -9,7 +9,8 @@ Version 2.36
> >
> >  Major new features:
> >
> > -  [Add new features here]
> > +* Support DT_RELR relative relocation format generated with the linker
>
> Maybe
>
>  * Support for DT_RELR relocation relocation format has been added to
>    glibc.  This is a new ELF section type that improves the size of relative
>    relocations in shared object files and position independent
>    executables (PIE).  DT_RELR support requires linker support for
>    -z pack-relative-relocs option, which is supported for some abi in recent
>    binutils versions.
>
>
> > +  option, -z pack-relative-relocs.
> >
> >  Deprecated and removed features, and other changes affecting compatibility:
> >

How about this?

* Support for DT_RELR relative relocation format has been added to
  glibc.  This is a new ELF dynamic tag that improves the size of
  relative relocations in shared object files and position independent
  executables (PIE).  DT_RELR generation requires linker support for
  -z pack-relative-relocs option, which is supported for some targets
  in recent binutils versions.

-- 
H.J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 4/5] Add --disable-default-dt-relr
  2022-03-29 17:19   ` Adhemerval Zanella
@ 2022-03-29 22:25     ` H.J. Lu
  0 siblings, 0 replies; 27+ messages in thread
From: H.J. Lu @ 2022-03-29 22:25 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Joseph Myers

On Tue, Mar 29, 2022 at 10:19 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> > Enable DT_RELR in glibc shared libraries and position independent
> > executables (PIE) automatically if linker supports -z pack-relative-relocs.
> >
> > Also add a new configuration option, --disable-default-dt-relr, to
> > avoid DT_RELR usage in glibc shared libraries and PIEs.
>
> LGTM, with two small nits below.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
> > ---
> >  INSTALL             |  6 ++++++
> >  Makeconfig          | 19 +++++++++++++++++++
> >  Makerules           |  2 ++
> >  configure           | 18 ++++++++++++++++++
> >  configure.ac        | 13 +++++++++++++
> >  elf/Makefile        |  4 +++-
> >  manual/install.texi |  5 +++++
> >  7 files changed, 66 insertions(+), 1 deletion(-)
> >
> > diff --git a/INSTALL b/INSTALL
> > index 63c022d6b9..4a6506f11f 100644
> > --- a/INSTALL
> > +++ b/INSTALL
> > @@ -133,6 +133,12 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
> >       used with the GCC option, -static-pie, which is available with GCC
> >       8 or above, to create static PIE.
> >
> > +'--disable-default-dt-relr'
> > +     Don't enable DT_RELR in glibc shared libraries and position
> > +     independent executables (PIE). By default, DT_RELR is enabled in
>
> Two space after period.

INSTALL is a generated file.  The original text in manual/install.texi has
2 spaces.

> > +     glibc shared libraries and position independent executables on
> > +     targets that support it.
> > +
> >  '--enable-cet'
> >  '--enable-cet=permissive'
> >       Enable Intel Control-flow Enforcement Technology (CET) support.
> > diff --git a/Makeconfig b/Makeconfig
> > index 47db08d6ae..70c0acc065 100644
> > --- a/Makeconfig
> > +++ b/Makeconfig
> > @@ -358,6 +358,23 @@ else
> >  real-static-start-installed-name = $(static-start-installed-name)
> >  endif
> >
> > +# Linker option to enable and disable DT-RELR.
> > +ifeq ($(have-dt-relr),yes)
> > +dt-relr-ldflag = -Wl,-z,pack-relative-relocs
> > +no-dt-relr-ldflag = -Wl,-z,nopack-relative-relocs
> > +else
> > +dt-relr-ldflag =
> > +no-dt-relr-ldflag =
> > +endif
> > +
> > +# Default linker option for DT-RELR.
> > +ifeq (yes,$(build-dt-relr-default))
> > +default-rt-relr-ldflag = $(dt-relr-ldflag)
> > +else
> > +default-rt-relr-ldflag = $(no-dt-relr-ldflag)
> > +endif
> > +LDFLAGS-rtld += $(default-rt-relr-ldflag)
> > +
> >  ifeq (yesyes,$(build-shared)$(have-z-combreloc))
> >  combreloc-LDFLAGS = -Wl,-z,combreloc
> >  LDFLAGS.so += $(combreloc-LDFLAGS)
> > @@ -419,6 +436,7 @@ link-extra-libs-tests = $(libsupport)
> >  # Command for linking PIE programs with the C library.
> >  ifndef +link-pie
> >  +link-pie-before-inputs = $(if $($(@F)-no-pie),$(no-pie-ldflag),-pie) \
> > +          $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
> >            -Wl,-O1 -nostdlib -nostartfiles \
> >            $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F)) \
> >            $(combreloc-LDFLAGS) $(relro-LDFLAGS) $(hashstyle-LDFLAGS) \
> > @@ -451,6 +469,7 @@ endif
> >  ifndef +link-static
> >  +link-static-before-inputs = -nostdlib -nostartfiles -static \
> >             $(if $($(@F)-no-pie),$(no-pie-ldflag),$(static-pie-ldflag)) \
> > +           $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
> >             $(sysdep-LDFLAGS) $(LDFLAGS) $(LDFLAGS-$(@F))  \
> >             $(firstword $(CRT-$(@F)) $(csu-objpfx)$(real-static-start-installed-name)) \
> >             $(+preinit) $(+prectorT)
> > diff --git a/Makerules b/Makerules
> > index 428464f092..7c1da551bf 100644
> > --- a/Makerules
> > +++ b/Makerules
> > @@ -536,6 +536,7 @@ lib%.so: lib%_pic.a $(+preinit) $(+postinit) $(link-libc-deps)
> >  define build-shlib-helper
> >  $(LINK.o) -shared -static-libgcc -Wl,-O1 $(sysdep-LDFLAGS) \
> >         $(if $($(@F)-no-z-defs)$(no-z-defs),,-Wl,-z,defs) $(rtld-LDFLAGS) \
> > +       $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
> >         $(extra-B-$(@F:lib%.so=%).so) -B$(csu-objpfx) \
> >         $(extra-B-$(@F:lib%.so=%).so) $(load-map-file) \
> >         -Wl,-soname=lib$(libprefix)$(@F:lib%.so=%).so$($(@F)-version) \
> > @@ -595,6 +596,7 @@ endef
> >  define build-module-helper
> >  $(LINK.o) -shared -static-libgcc $(sysdep-LDFLAGS) $(rtld-LDFLAGS) \
> >         $(if $($(@F)-no-z-defs)$(no-z-defs),,-Wl,-z,defs) \
> > +       $(if $($(@F)-no-dt-relr),$(no-dt-relr-ldflag),$(default-rt-relr-ldflag)) \
> >         -B$(csu-objpfx) $(load-map-file) \
> >         $(LDFLAGS.so) $(LDFLAGS-$(@F:%.so=%).so) \
> >         $(link-test-modules-rpath-link) \
>
> Ok.
>
> > diff --git a/configure b/configure
> > index 9156e29fe9..ee4137a977 100755
> > --- a/configure
> > +++ b/configure
> > @@ -768,6 +768,7 @@ enable_sanity_checks
> >  enable_shared
> >  enable_profile
> >  enable_default_pie
> > +enable_default_dt_relr
> >  enable_timezone_tools
> >  enable_hardcoded_path_in_tests
> >  enable_hidden_plt
> > @@ -1425,6 +1426,7 @@ Optional Features:
> >    --enable-profile        build profiled library [default=no]
> >    --disable-default-pie   Do not build glibc programs and the testsuite as PIE
> >                            [default=no]
> > +  --disable-dt-relr       Do not enable DT_RELR in glibc[default=no]
> >    --disable-timezone-tools
> >                            do not install timezone tools [default=install]
> >    --enable-hardcoded-path-in-tests
> > @@ -3441,6 +3443,13 @@ else
> >    default_pie=yes
> >  fi
> >
> > +# Check whether --enable-default-dt-relr was given.
> > +if test "${enable_default_dt_relr+set}" = set; then :
> > +  enableval=$enable_default_dt_relr; default_dt_relr=$enableval
> > +else
> > +  default_dt_relr=yes
> > +fi
> > +
> >  # Check whether --enable-timezone-tools was given.
> >  if test "${enable_timezone_tools+set}" = set; then :
> >    enableval=$enable_timezone_tools; enable_timezone_tools=$enableval
> > @@ -7136,6 +7145,15 @@ fi
> >  config_vars="$config_vars
> >  enable-static-pie = $libc_cv_static_pie"
> >
> > +# Disable build-dt-relr-default if linker does not support it or glibc is
> > +# configured with --disable-default-dt-relr.
> > +build_dt_relr_default=$default_dt_relr
> > +if test "x$build_dt_relr_default" != xno; then
> > +  build_dt_relr_default=$libc_cv_dt_relr
> > +fi
> > +config_vars="$config_vars
> > +build-dt-relr-default = $build_dt_relr_default"
> > +
> >  # Set the `multidir' variable by grabbing the variable from the compiler.
> >  # We do it once and save the result in a generated makefile.
> >  libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
> > diff --git a/configure.ac b/configure.ac
> > index 5c09871fee..680b036ffa 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -197,6 +197,11 @@ AC_ARG_ENABLE([default-pie],
> >                            [Do not build glibc programs and the testsuite as PIE @<:@default=no@:>@]),
> >             [default_pie=$enableval],
> >             [default_pie=yes])
> > +AC_ARG_ENABLE([default-dt-relr],
> > +           AS_HELP_STRING([--disable-dt-relr],
> > +                          [Do not enable DT_RELR in glibc@<:@default=no@:>@]),
> > +           [default_dt_relr=$enableval],
> > +           [default_dt_relr=yes])
> >  AC_ARG_ENABLE([timezone-tools],
> >             AS_HELP_STRING([--disable-timezone-tools],
> >                            [do not install timezone tools @<:@default=install@:>@]),
> > @@ -1914,6 +1919,14 @@ if test "$libc_cv_static_pie" = "yes"; then
> >  fi
> >  LIBC_CONFIG_VAR([enable-static-pie], [$libc_cv_static_pie])
> >
> > +# Disable build-dt-relr-default if linker does not support it or glibc is
>
> Maybe 'or if'.

Fixed in v7.

> > +# configured with --disable-default-dt-relr.
> > +build_dt_relr_default=$default_dt_relr
> > +if test "x$build_dt_relr_default" != xno; then
> > +  build_dt_relr_default=$libc_cv_dt_relr
> > +fi
> > +LIBC_CONFIG_VAR([build-dt-relr-default], [$build_dt_relr_default])
> > +
> >  # Set the `multidir' variable by grabbing the variable from the compiler.
> >  # We do it once and save the result in a generated makefile.
> >  libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
> > diff --git a/elf/Makefile b/elf/Makefile
> > index d3118b9777..c4384536ba 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -1567,6 +1567,7 @@ $(objpfx)nodlopen2.out: $(objpfx)nodlopenmod2.so
> >
> >  $(objpfx)filtmod1.so: $(objpfx)filtmod1.os $(objpfx)filtmod2.so
> >       $(LINK.o) -shared -o $@ -B$(csu-objpfx) $(LDFLAGS.so) \
> > +               $(default-rt-relr-ldflag) \
> >                 -L$(subst :, -L,$(rpath-link)) \
> >                 -Wl,-rpath-link=$(rpath-link) \
> >                 $< -Wl,-F,$(objpfx)filtmod2.so
> > @@ -2366,7 +2367,7 @@ $(objpfx)tst-big-note: $(objpfx)tst-big-note-lib.so
> >  # artificial, large note in tst-big-note-lib.o and invalidate the
> >  # test.
> >  $(objpfx)tst-big-note-lib.so: $(objpfx)tst-big-note-lib.o
> > -     $(LINK.o) -shared -o $@ $(LDFLAGS.so) $<
> > +     $(LINK.o) -shared -o $@ $(LDFLAGS.so) $(default-rt-relr-ldflag) $<
> >
> >  $(objpfx)tst-unwind-ctor: $(objpfx)tst-unwind-ctor-lib.so
> >
> > @@ -2672,6 +2673,7 @@ $(objpfx)tst-ro-dynamic: $(objpfx)tst-ro-dynamic-mod.so
> >  $(objpfx)tst-ro-dynamic-mod.so: $(objpfx)tst-ro-dynamic-mod.os \
> >    tst-ro-dynamic-mod.map
> >       $(LINK.o) -nostdlib -nostartfiles -shared -o $@ \
> > +             $(default-rt-relr-ldflag) \
> >               -Wl,--script=tst-ro-dynamic-mod.map \
> >               $(objpfx)tst-ro-dynamic-mod.os
> >
> > diff --git a/manual/install.texi b/manual/install.texi
> > index 29c52f2927..04ea996561 100644
> > --- a/manual/install.texi
> > +++ b/manual/install.texi
> > @@ -161,6 +161,11 @@ and architecture support it, static executables are built as static PIE and the
> >  resulting glibc can be used with the GCC option, -static-pie, which is
> >  available with GCC 8 or above, to create static PIE.
> >
> > +@item --disable-default-dt-relr
> > +Don't enable DT_RELR in glibc shared libraries and position independent
> > +executables (PIE).  By default, DT_RELR is enabled in glibc shared
> > +libraries and position independent executables on targets that support it.
> > +
> >  @item --enable-cet
> >  @itemx --enable-cet=permissive
> >  Enable Intel Control-flow Enforcement Technology (CET) support.  When
>
> Ok.



-- 
H.J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-29 16:52   ` Adhemerval Zanella
@ 2022-03-29 22:29     ` H.J. Lu
  2022-03-30 14:18       ` Adhemerval Zanella
  0 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-29 22:29 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Joseph Myers

On Tue, Mar 29, 2022 at 9:52 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> > The EI_ABIVERSION field of the ELF header in executables and shared
> > libraries can be bumped to indicate the minimum ABI requirement on the
> > dynamic linker.  However, EI_ABIVERSION in executables isn't checked by
> > the Linux kernel ELF loader nor the existing dynamic linker.  Executables
> > will crash mysteriously if the dynamic linker doesn't support the ABI
> > features required by the EI_ABIVERSION field.  The dynamic linker should
> > be changed to check EI_ABIVERSION in executables.
> >
> > Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support so
> > that the existing dynamic linkers will issue an error on executables with
> > GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR entry
> > without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.
> >
> > Support __placeholder_only_for_empty_version_map as the placeholder symbol
> > used only for empty version map to generate GLIBC_ABI_DT_RELR without any
> > symbols.
>
> I think it only make sense to add the GLIBC_ABI_DT_RELR if the ABI actually
> supports DT_RELR, otherwise if the ABI start to support old glibcs might
> wrongly assumes DT_RELR and fails with undefined behavior at runtime
> (which this patch essentially tries to avoid).

The DT_RELR set enables DT_RELR in glibc for all targets.

> > ---
> >  elf/Makefile         | 12 ++++++++++--
> >  elf/Versions         |  5 +++++
> >  elf/dl-version.c     | 33 +++++++++++++++++++++++++++++++--
> >  include/link.h       |  6 ++++++
> >  scripts/abilist.awk  |  2 ++
> >  scripts/versions.awk |  7 ++++++-
> >  6 files changed, 60 insertions(+), 5 deletions(-)
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index fd462ba315..d3118b9777 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -1113,8 +1113,10 @@ $(eval $(call include_dsosort_tests,dso-sort-tests-1.def))
> >  $(eval $(call include_dsosort_tests,dso-sort-tests-2.def))
> >  endif
> >
> > -check-abi: $(objpfx)check-abi-ld.out
> > -tests-special += $(objpfx)check-abi-ld.out
> > +check-abi: $(objpfx)check-abi-ld.out \
> > +        $(objpfx)check-abi-version-libc.out
> > +tests-special += $(objpfx)check-abi-ld.out \
> > +        $(objpfx)check-abi-version-libc.out
>
> One line per definition.

Fixed in v7.

> >  update-abi: update-abi-ld
> >  update-all-abi: update-all-abi-ld
> >
> > @@ -2739,3 +2741,9 @@ $(objpfx)check-tst-relr-pie.out: $(objpfx)tst-relr-pie
> >               | sed -ne '/required from libc.so/,$$ p' \
> >               | grep GLIBC_ABI_DT_RELR > $@; \
> >       $(evaluate-test)
> > +
> > +$(objpfx)check-abi-version-libc.out: $(common-objpfx)libc.so
> > +     LC_ALL=C $(READELF) -V -W $< \
> > +             | sed -ne '/.gnu.version_d/, /.gnu.version_r/ p' \
> > +             | grep GLIBC_ABI_DT_RELR > $@; \
> > +     $(evaluate-test)
>
> This test should be enable iff for $(have-dt-relr) == yes.

This test checks if libc.so has the GLIBC_ABI_DT_RELR
version definition.   Since it doesn't require a new linker,
there is no need to check $(have-dt-relr).   This test
passes with an old linker.

> > diff --git a/elf/Versions b/elf/Versions
> > index 8bed855d8c..a9ff278de7 100644
> > --- a/elf/Versions
> > +++ b/elf/Versions
> > @@ -23,6 +23,11 @@ libc {
> >    GLIBC_2.35 {
> >      _dl_find_object;
> >    }
> > +  GLIBC_ABI_DT_RELR {
> > +    # This symbol is used only for empty version map and will be removed
> > +    # by scripts/versions.awk.
> > +    __placeholder_only_for_empty_version_map;
> > +  }
>
> Same as before. Maybe use the same strategy we do for time64: add a config.h
> define to HAVE_DTRELR (set by configure) and use it to set the symbol if
> it is defined.
>
> This define would be used on dl-version.c to also enable the l_abi_version
> check.
>
> >    GLIBC_PRIVATE {
> >      # functions used in other libraries
> >      __libc_early_init;
> > diff --git a/elf/dl-version.c b/elf/dl-version.c
> > index b47bd91727..720ec596a5 100644
> > --- a/elf/dl-version.c
> > +++ b/elf/dl-version.c
> > @@ -214,12 +214,20 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
> >             while (1)
> >               {
> >                 /* Match the symbol.  */
> > +               const char *string = strtab + aux->vna_name;
> >                 result |= match_symbol (DSO_FILENAME (map->l_name),
> >                                         map->l_ns, aux->vna_hash,
> > -                                       strtab + aux->vna_name,
> > -                                       needed->l_real, verbose,
> > +                                       string, needed->l_real, verbose,
> >                                         aux->vna_flags & VER_FLG_WEAK);
> >
> > +               if (map->l_abi_version == lav_none
> > +                   /* 0xfd0e42: _dl_elf_hash ("GLIBC_ABI_DT_RELR").  */
> > +                   && aux->vna_hash == 0xfd0e42
> > +                   && __glibc_likely (strcmp (string,
> > +                                              "GLIBC_ABI_DT_RELR")
> > +                                      == 0))
> > +                 map->l_abi_version = lav_dt_relr_ref;
> > +
> >                 /* Compare the version index.  */
> >                 if ((unsigned int) (aux->vna_other & 0x7fff) > ndx_high)
> >                   ndx_high = aux->vna_other & 0x7fff;
> > @@ -253,6 +261,16 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
> >        ent = (ElfW(Verdef) *) (map->l_addr + def->d_un.d_ptr);
> >        while (1)
> >       {
> > +       /* 0x0963cf85: _dl_elf_hash ("GLIBC_PRIVATE").  */
> > +       if (ent->vd_hash == 0x0963cf85)
> > +         {
> > +           ElfW(Verdaux) *aux = (ElfW(Verdaux) *) ((char *) ent
> > +                                                   + ent->vd_aux);
> > +           if (__glibc_likely (strcmp ("GLIBC_PRIVATE",
> > +                                       strtab + aux->vda_name) == 0))
> > +             map->l_abi_version = lav_private_def;
> > +         }
> > +
> >         if ((unsigned int) (ent->vd_ndx & 0x7fff) > ndx_high)
> >           ndx_high = ent->vd_ndx & 0x7fff;
> >
> > @@ -352,6 +370,17 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
> >       }
> >      }
> >
> > +  /* Issue an error if there is a DT_RELR entry without GLIBC_ABI_DT_RELR
> > +     dependency nor GLIBC_PRIVATE definition.  */
> > +  if (map->l_info[DT_RELR] != NULL
> > +      && __glibc_unlikely (map->l_abi_version == lav_none))
> > +    {
> > +      _dl_exception_create
> > +     (&exception, DSO_FILENAME (map->l_name),
> > +      N_("DT_RELR without GLIBC_ABI_DT_RELR dependency"));
> > +      goto call_error;
> > +    }
> > +
> >    return result;
> >  }
> >
> > diff --git a/include/link.h b/include/link.h
> > index 03db14c7b0..8ec5e35cf2 100644
> > --- a/include/link.h
> > +++ b/include/link.h
> > @@ -177,6 +177,12 @@ struct link_map
> >       lt_library,             /* Library needed by main executable.  */
> >       lt_loaded               /* Extra run-time loaded shared object.  */
> >        } l_type:2;
> > +    enum                     /* ABI dependency of this object.  */
> > +      {
> > +     lav_none,               /* No ABI dependency.  */
> > +     lav_dt_relr_ref,        /* Need GLIBC_ABI_DT_RELR.  */
> > +     lav_private_def         /* Define GLIBC_PRIVATE.  */
> > +      } l_abi_version:2;
> >      unsigned int l_relocated:1;      /* Nonzero if object's relocations done.  */
> >      unsigned int l_init_called:1; /* Nonzero if DT_INIT function called.  */
> >      unsigned int l_global:1; /* Nonzero if object in _dl_global_scope.  */
> > diff --git a/scripts/abilist.awk b/scripts/abilist.awk
> > index 24a34ccbed..6cc7af6ac8 100644
> > --- a/scripts/abilist.awk
> > +++ b/scripts/abilist.awk
> > @@ -55,6 +55,8 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
> >    # caused STV_HIDDEN symbols to appear in .dynsym, though that is useless.
> >    if (NF > 7 && $7 == ".hidden") next;
> >
> > +  if (version ~ /^GLIBC_ABI_/ && !include_abi_version) next;
> > +
> >    if (version == "GLIBC_PRIVATE" && !include_private) next;
> >
> >    desc = "";
> > diff --git a/scripts/versions.awk b/scripts/versions.awk
> > index 357ad1355e..d70b07bd1a 100644
> > --- a/scripts/versions.awk
> > +++ b/scripts/versions.awk
> > @@ -185,8 +185,13 @@ END {
> >       closeversion(oldver, veryoldver);
> >       veryoldver = oldver;
> >        }
> > -      printf("%s {\n  global:\n", $2) > outfile;
> >        oldver = $2;
> > +      # Skip the placeholder symbol used only for empty version map.
> > +      if ($3 == "__placeholder_only_for_empty_version_map;") {
> > +     printf("%s {\n", $2) > outfile;
> > +     continue;
> > +      }
> > +      printf("%s {\n  global:\n", $2) > outfile;
> >      }
> >      printf("   ") > outfile;
> >      for (n = 3; n <= NF; ++n) {



-- 
H.J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 2/5] elf: Properly handle zero DT_RELA/DT_REL values
  2022-03-29 16:38   ` Adhemerval Zanella
@ 2022-03-29 22:30     ` H.J. Lu
  0 siblings, 0 replies; 27+ messages in thread
From: H.J. Lu @ 2022-03-29 22:30 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Joseph Myers

On Tue, Mar 29, 2022 at 9:38 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> > With DT_RELR, there may be no relocations in DT_RELA/DT_REL and their
> > entry values are zero.  Don't relocate DT_RELA/DT_REL and update the
> > combined relocation start address if their entry values are zero.
>
> Patch looks good with the two small fixes below.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
> > ---
> >  elf/dynamic-link.h     |  6 +++++-
> >  elf/get-dynamic-info.h | 18 ++++++++++++++----
> >  2 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> > index d04c457e55..252f407a12 100644
> > --- a/elf/dynamic-link.h
> > +++ b/elf/dynamic-link.h
> > @@ -84,7 +84,9 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> >            __typeof (((ElfW(Dyn) *) 0)->d_un.d_val) nrelative; int lazy; }  \
> >        ranges[2] = { { 0, 0, 0, 0 }, { 0, 0, 0, 0 } };                              \
> >                                                                             \
> > -    if ((map)->l_info[DT_##RELOC])                                         \
> > +    /* With DT_RELR, DT_RELA/DT_REL can have zero value.  */               \
> > +    if ((map)->l_info[DT_##RELOC]                                          \
>
> Compare to NULL here.

Fixed in v7.

> > +     && (map)->l_info[DT_##RELOC]->d_un.d_ptr != 0)                        \
> >        {                                                                            \
> >       ranges[0].start = D_PTR ((map), l_info[DT_##RELOC]);                  \
> >       ranges[0].size = (map)->l_info[DT_##RELOC##SZ]->d_un.d_val;           \
> > @@ -98,6 +100,8 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> >       ElfW(Addr) start = D_PTR ((map), l_info[DT_JMPREL]);                  \
> >       ElfW(Addr) size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val;             \
> >                                                                             \
> > +     if (ranges[0].start == 0)                                             \
> > +       ranges[0].start = start;                                            \
> >       if (ranges[0].start + ranges[0].size == (start + size))               \
> >         ranges[0].size -= size;                                             \
> >       if (!(do_lazy)                                                        \
>
> Ok.
>
> > diff --git a/elf/get-dynamic-info.h b/elf/get-dynamic-info.h
> > index 6c2a3a12b1..f4b957684b 100644
> > --- a/elf/get-dynamic-info.h
> > +++ b/elf/get-dynamic-info.h
> > @@ -83,16 +83,26 @@ elf_get_dynamic_info (struct link_map *l, bool bootstrap,
> >        ADJUST_DYN_INFO (DT_PLTGOT);
> >        ADJUST_DYN_INFO (DT_STRTAB);
> >        ADJUST_DYN_INFO (DT_SYMTAB);
> > +      ADJUST_DYN_INFO (DT_RELR);
> > +      ADJUST_DYN_INFO (DT_JMPREL);
> > +      ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
> > +      ADJUST_DYN_INFO (ADDRIDX (DT_GNU_HASH));
> > +# undef ADJUST_DYN_INFO
> > +
> > +      /* DT_RELA/DT_REL are mandatory.  But they may have zero value if
> > +      there is DT_RELR.  Don't relocate them if they are zero.  */
> > +# define ADJUST_DYN_INFO(tag) \
> > +      do                                                                   \
> > +     if (info[tag] != NULL && info[tag]->d_un.d_ptr != 0)                  \
> > +         info[tag]->d_un.d_ptr += l_addr;                                  \
> > +      while (0)
> > +
>
> Maybe use '{' and '}' on the do ... while.

Fixed in v7.

> >  # if ! ELF_MACHINE_NO_RELA
> >        ADJUST_DYN_INFO (DT_RELA);
> >  # endif
> >  # 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));
> >  # undef ADJUST_DYN_INFO
> >      }
> >    if (info[DT_PLTREL] != NULL)
>
> Ok.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924]
  2022-03-29 16:34   ` Adhemerval Zanella
@ 2022-03-29 22:34     ` H.J. Lu
  0 siblings, 0 replies; 27+ messages in thread
From: H.J. Lu @ 2022-03-29 22:34 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Joseph Myers

On Tue, Mar 29, 2022 at 9:34 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 10/03/2022 17:03, H.J. Lu via Libc-alpha 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.
>
> I really don't think ia64 ELFCLASS32 should be blocker for this change.
>
> >
> > * 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:
>
> Agreed.
>
> >
> > * 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 ok, some comments below.
>
> > ---
> >  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
> > +
>
> I think you don't need to setup have_dt_relr, it does not accomplish anything here
> (LIBC_CONFIG_VAR already sets the required have-dt-relr on config.make).

Fixed in v7.

> >  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
>
> Please use one line per test:
>
>  tests += \
>    tst-relr \
>    tst-relr-pie \
>    # tests
>  tests-pie +=
>    tst-relr-pie \
>    # tests-pie
>  tests-special += \
>    $(objpfx)check-tst-relr-pie.out \
>    # tests-special
>
> Also, the tst-rel-pie should only be added if $(have-fpie):
>
>   ifeq ($(have-fpie),yes)
>   tests-pie += \
>     tst-relr-pie \
>     #tests-pie
>   endf

Fixed in v7.

> > +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)
>
> Ok.
>
> > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> > index 25dd7ca4f2..d04c457e55 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])                                           \
> > +      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));            \
> > +    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/elf.h b/elf/elf.h
> > index 0735f6b579..0195029188 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;
> >
>
> Ok.
>
> > +/* 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 */
>
> 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
>
> Ok.
>
> > new file mode 100644
> > index 0000000000..56addd2ad4
> > --- /dev/null
> > +++ b/elf/tst-relr.c
> > @@ -0,0 +1,64 @@
> > +/* 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
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <link.h>
> > +#include <stdbool.h>
> > +#include <support/check.h>
> > +
> > +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 < sizeof (arr) / sizeof (arr[0]); i++)
>
> Use array_length here.

Fixed in v7.

> > +    TEST_VERIFY ((arr[i] == 0 && val[i] == 0)
> > +              || (arr[i] == &o && val[i] == 1)
> > +              || (arr[i] == &x && val[i] == 2));
> > +
> > +  return 0;
> > +}
> > +
> > +#include <support/test-driver.c>
>
> This fails without the rest of the patchset applied:
>
> $ ./testrun.sh elf/tst-relr
> elf/tst-relr: ./libc.so.6: version `GLIBC_ABI_DT_RELR' not found (required by elf/tst-relr)
>
> I suggest you move it to third patch when GLIBC_ABI_DT_RELR is added.

Fixed in v7.

The first patch just defines DT_RELR related macros and types.   The
second patch adds GLIBC_ABI_DT_RELR.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 5/5] NEWS: Mention DT_RELR support
  2022-03-29 22:22     ` H.J. Lu
@ 2022-03-29 23:34       ` Fangrui Song
  2022-03-30 14:20         ` Adhemerval Zanella
  0 siblings, 1 reply; 27+ messages in thread
From: Fangrui Song @ 2022-03-29 23:34 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library

On 2022-03-29, H.J. Lu via Libc-alpha wrote:
>On Tue, Mar 29, 2022 at 10:25 AM Adhemerval Zanella via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
>> > ---
>> >  NEWS | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/NEWS b/NEWS
>> > index 626eeabf5d..5d64b794f1 100644
>> > --- a/NEWS
>> > +++ b/NEWS
>> > @@ -9,7 +9,8 @@ Version 2.36
>> >
>> >  Major new features:
>> >
>> > -  [Add new features here]
>> > +* Support DT_RELR relative relocation format generated with the linker
>>
>> Maybe
>>
>>  * Support for DT_RELR relocation relocation format has been added to
>>    glibc.  This is a new ELF section type that improves the size of relative
>>    relocations in shared object files and position independent
>>    executables (PIE).  DT_RELR support requires linker support for
>>    -z pack-relative-relocs option, which is supported for some abi in recent
>>    binutils versions.
>>
>>
>> > +  option, -z pack-relative-relocs.
>> >
>> >  Deprecated and removed features, and other changes affecting compatibility:
>> >
>
>How about this?
>
>* Support for DT_RELR relative relocation format has been added to
>  glibc.  This is a new ELF dynamic tag that improves the size of
>  relative relocations in shared object files and position independent
>  executables (PIE).  DT_RELR generation requires linker support for
>  -z pack-relative-relocs option, which is supported for some targets
>  in recent binutils versions.
>

Perhaps more specific (a user may want to know what "recent" means): for x86-64 in binutils 2.38.
It may or may not be worth adding words to imply that ld support for other targets will come in the future:)

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-29 22:29     ` H.J. Lu
@ 2022-03-30 14:18       ` Adhemerval Zanella
  2022-03-30 14:41         ` H.J. Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-30 14:18 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library, Joseph Myers



On 29/03/2022 19:29, H.J. Lu wrote:
> On Tue, Mar 29, 2022 at 9:52 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
>>> The EI_ABIVERSION field of the ELF header in executables and shared
>>> libraries can be bumped to indicate the minimum ABI requirement on the
>>> dynamic linker.  However, EI_ABIVERSION in executables isn't checked by
>>> the Linux kernel ELF loader nor the existing dynamic linker.  Executables
>>> will crash mysteriously if the dynamic linker doesn't support the ABI
>>> features required by the EI_ABIVERSION field.  The dynamic linker should
>>> be changed to check EI_ABIVERSION in executables.
>>>
>>> Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support so
>>> that the existing dynamic linkers will issue an error on executables with
>>> GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR entry
>>> without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.
>>>
>>> Support __placeholder_only_for_empty_version_map as the placeholder symbol
>>> used only for empty version map to generate GLIBC_ABI_DT_RELR without any
>>> symbols.
>>
>> I think it only make sense to add the GLIBC_ABI_DT_RELR if the ABI actually
>> supports DT_RELR, otherwise if the ABI start to support old glibcs might
>> wrongly assumes DT_RELR and fails with undefined behavior at runtime
>> (which this patch essentially tries to avoid).
> 
> The DT_RELR set enables DT_RELR in glibc for all targets.
If I understood correctly, if we add GLIBC_ABI_DT_RELR on 2.36 but only enable
it on some architecture on 2.37 and try to run a binary with DT_RELR enabled
on glibc 2.36, _dl_check_map_versions won't accuse a failure and it will
eventually only fail at runtime (since relocation won't be applied).  My
understanding we are trying to avoid such failures.

> 
>>> ---
>>>  elf/Makefile         | 12 ++++++++++--
>>>  elf/Versions         |  5 +++++
>>>  elf/dl-version.c     | 33 +++++++++++++++++++++++++++++++--
>>>  include/link.h       |  6 ++++++
>>>  scripts/abilist.awk  |  2 ++
>>>  scripts/versions.awk |  7 ++++++-
>>>  6 files changed, 60 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/elf/Makefile b/elf/Makefile
>>> index fd462ba315..d3118b9777 100644
>>> --- a/elf/Makefile
>>> +++ b/elf/Makefile
>>> @@ -1113,8 +1113,10 @@ $(eval $(call include_dsosort_tests,dso-sort-tests-1.def))
>>>  $(eval $(call include_dsosort_tests,dso-sort-tests-2.def))
>>>  endif
>>>
>>> -check-abi: $(objpfx)check-abi-ld.out
>>> -tests-special += $(objpfx)check-abi-ld.out
>>> +check-abi: $(objpfx)check-abi-ld.out \
>>> +        $(objpfx)check-abi-version-libc.out
>>> +tests-special += $(objpfx)check-abi-ld.out \
>>> +        $(objpfx)check-abi-version-libc.out
>>
>> One line per definition.
> 
> Fixed in v7.
> 
>>>  update-abi: update-abi-ld
>>>  update-all-abi: update-all-abi-ld
>>>
>>> @@ -2739,3 +2741,9 @@ $(objpfx)check-tst-relr-pie.out: $(objpfx)tst-relr-pie
>>>               | sed -ne '/required from libc.so/,$$ p' \
>>>               | grep GLIBC_ABI_DT_RELR > $@; \
>>>       $(evaluate-test)
>>> +
>>> +$(objpfx)check-abi-version-libc.out: $(common-objpfx)libc.so
>>> +     LC_ALL=C $(READELF) -V -W $< \
>>> +             | sed -ne '/.gnu.version_d/, /.gnu.version_r/ p' \
>>> +             | grep GLIBC_ABI_DT_RELR > $@; \
>>> +     $(evaluate-test)
>>
>> This test should be enable iff for $(have-dt-relr) == yes.
> 
> This test checks if libc.so has the GLIBC_ABI_DT_RELR
> version definition.   Since it doesn't require a new linker,
> there is no need to check $(have-dt-relr).   This test
> passes with an old linker.
> 
>>> diff --git a/elf/Versions b/elf/Versions
>>> index 8bed855d8c..a9ff278de7 100644
>>> --- a/elf/Versions
>>> +++ b/elf/Versions
>>> @@ -23,6 +23,11 @@ libc {
>>>    GLIBC_2.35 {
>>>      _dl_find_object;
>>>    }
>>> +  GLIBC_ABI_DT_RELR {
>>> +    # This symbol is used only for empty version map and will be removed
>>> +    # by scripts/versions.awk.
>>> +    __placeholder_only_for_empty_version_map;
>>> +  }
>>
>> Same as before. Maybe use the same strategy we do for time64: add a config.h
>> define to HAVE_DTRELR (set by configure) and use it to set the symbol if
>> it is defined.
>>
>> This define would be used on dl-version.c to also enable the l_abi_version
>> check.
>>
>>>    GLIBC_PRIVATE {
>>>      # functions used in other libraries
>>>      __libc_early_init;
>>> diff --git a/elf/dl-version.c b/elf/dl-version.c
>>> index b47bd91727..720ec596a5 100644
>>> --- a/elf/dl-version.c
>>> +++ b/elf/dl-version.c
>>> @@ -214,12 +214,20 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>>>             while (1)
>>>               {
>>>                 /* Match the symbol.  */
>>> +               const char *string = strtab + aux->vna_name;
>>>                 result |= match_symbol (DSO_FILENAME (map->l_name),
>>>                                         map->l_ns, aux->vna_hash,
>>> -                                       strtab + aux->vna_name,
>>> -                                       needed->l_real, verbose,
>>> +                                       string, needed->l_real, verbose,
>>>                                         aux->vna_flags & VER_FLG_WEAK);
>>>
>>> +               if (map->l_abi_version == lav_none
>>> +                   /* 0xfd0e42: _dl_elf_hash ("GLIBC_ABI_DT_RELR").  */
>>> +                   && aux->vna_hash == 0xfd0e42
>>> +                   && __glibc_likely (strcmp (string,
>>> +                                              "GLIBC_ABI_DT_RELR")
>>> +                                      == 0))
>>> +                 map->l_abi_version = lav_dt_relr_ref;
>>> +
>>>                 /* Compare the version index.  */
>>>                 if ((unsigned int) (aux->vna_other & 0x7fff) > ndx_high)
>>>                   ndx_high = aux->vna_other & 0x7fff;
>>> @@ -253,6 +261,16 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>>>        ent = (ElfW(Verdef) *) (map->l_addr + def->d_un.d_ptr);
>>>        while (1)
>>>       {
>>> +       /* 0x0963cf85: _dl_elf_hash ("GLIBC_PRIVATE").  */
>>> +       if (ent->vd_hash == 0x0963cf85)
>>> +         {
>>> +           ElfW(Verdaux) *aux = (ElfW(Verdaux) *) ((char *) ent
>>> +                                                   + ent->vd_aux);
>>> +           if (__glibc_likely (strcmp ("GLIBC_PRIVATE",
>>> +                                       strtab + aux->vda_name) == 0))
>>> +             map->l_abi_version = lav_private_def;
>>> +         }
>>> +
>>>         if ((unsigned int) (ent->vd_ndx & 0x7fff) > ndx_high)
>>>           ndx_high = ent->vd_ndx & 0x7fff;
>>>
>>> @@ -352,6 +370,17 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
>>>       }
>>>      }
>>>
>>> +  /* Issue an error if there is a DT_RELR entry without GLIBC_ABI_DT_RELR
>>> +     dependency nor GLIBC_PRIVATE definition.  */
>>> +  if (map->l_info[DT_RELR] != NULL
>>> +      && __glibc_unlikely (map->l_abi_version == lav_none))
>>> +    {
>>> +      _dl_exception_create
>>> +     (&exception, DSO_FILENAME (map->l_name),
>>> +      N_("DT_RELR without GLIBC_ABI_DT_RELR dependency"));
>>> +      goto call_error;
>>> +    }
>>> +
>>>    return result;
>>>  }
>>>
>>> diff --git a/include/link.h b/include/link.h
>>> index 03db14c7b0..8ec5e35cf2 100644
>>> --- a/include/link.h
>>> +++ b/include/link.h
>>> @@ -177,6 +177,12 @@ struct link_map
>>>       lt_library,             /* Library needed by main executable.  */
>>>       lt_loaded               /* Extra run-time loaded shared object.  */
>>>        } l_type:2;
>>> +    enum                     /* ABI dependency of this object.  */
>>> +      {
>>> +     lav_none,               /* No ABI dependency.  */
>>> +     lav_dt_relr_ref,        /* Need GLIBC_ABI_DT_RELR.  */
>>> +     lav_private_def         /* Define GLIBC_PRIVATE.  */
>>> +      } l_abi_version:2;
>>>      unsigned int l_relocated:1;      /* Nonzero if object's relocations done.  */
>>>      unsigned int l_init_called:1; /* Nonzero if DT_INIT function called.  */
>>>      unsigned int l_global:1; /* Nonzero if object in _dl_global_scope.  */
>>> diff --git a/scripts/abilist.awk b/scripts/abilist.awk
>>> index 24a34ccbed..6cc7af6ac8 100644
>>> --- a/scripts/abilist.awk
>>> +++ b/scripts/abilist.awk
>>> @@ -55,6 +55,8 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
>>>    # caused STV_HIDDEN symbols to appear in .dynsym, though that is useless.
>>>    if (NF > 7 && $7 == ".hidden") next;
>>>
>>> +  if (version ~ /^GLIBC_ABI_/ && !include_abi_version) next;
>>> +
>>>    if (version == "GLIBC_PRIVATE" && !include_private) next;
>>>
>>>    desc = "";
>>> diff --git a/scripts/versions.awk b/scripts/versions.awk
>>> index 357ad1355e..d70b07bd1a 100644
>>> --- a/scripts/versions.awk
>>> +++ b/scripts/versions.awk
>>> @@ -185,8 +185,13 @@ END {
>>>       closeversion(oldver, veryoldver);
>>>       veryoldver = oldver;
>>>        }
>>> -      printf("%s {\n  global:\n", $2) > outfile;
>>>        oldver = $2;
>>> +      # Skip the placeholder symbol used only for empty version map.
>>> +      if ($3 == "__placeholder_only_for_empty_version_map;") {
>>> +     printf("%s {\n", $2) > outfile;
>>> +     continue;
>>> +      }
>>> +      printf("%s {\n  global:\n", $2) > outfile;
>>>      }
>>>      printf("   ") > outfile;
>>>      for (n = 3; n <= NF; ++n) {
> 
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 5/5] NEWS: Mention DT_RELR support
  2022-03-29 23:34       ` Fangrui Song
@ 2022-03-30 14:20         ` Adhemerval Zanella
  0 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-30 14:20 UTC (permalink / raw)
  To: Fangrui Song, H.J. Lu; +Cc: GNU C Library



On 29/03/2022 20:34, Fangrui Song wrote:
> On 2022-03-29, H.J. Lu via Libc-alpha wrote:
>> On Tue, Mar 29, 2022 at 10:25 AM Adhemerval Zanella via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>>
>>>
>>> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
>>> > ---
>>> >  NEWS | 3 ++-
>>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/NEWS b/NEWS
>>> > index 626eeabf5d..5d64b794f1 100644
>>> > --- a/NEWS
>>> > +++ b/NEWS
>>> > @@ -9,7 +9,8 @@ Version 2.36
>>> >
>>> >  Major new features:
>>> >
>>> > -  [Add new features here]
>>> > +* Support DT_RELR relative relocation format generated with the linker
>>>
>>> Maybe
>>>
>>>  * Support for DT_RELR relocation relocation format has been added to
>>>    glibc.  This is a new ELF section type that improves the size of relative
>>>    relocations in shared object files and position independent
>>>    executables (PIE).  DT_RELR support requires linker support for
>>>    -z pack-relative-relocs option, which is supported for some abi in recent
>>>    binutils versions.
>>>
>>>
>>> > +  option, -z pack-relative-relocs.
>>> >
>>> >  Deprecated and removed features, and other changes affecting compatibility:
>>> >
>>
>> How about this?
>>
>> * Support for DT_RELR relative relocation format has been added to
>>  glibc.  This is a new ELF dynamic tag that improves the size of
>>  relative relocations in shared object files and position independent
>>  executables (PIE).  DT_RELR generation requires linker support for
>>  -z pack-relative-relocs option, which is supported for some targets
>>  in recent binutils versions.
>>
> 

Look ok.

> Perhaps more specific (a user may want to know what "recent" means): for x86-64 in binutils 2.38.
> It may or may not be worth adding words to imply that ld support for other targets will come in the future:)

I think 'recent' should be ok since we don't know if distros will backport it
(so tying to a version might be misleading).

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-30 14:18       ` Adhemerval Zanella
@ 2022-03-30 14:41         ` H.J. Lu
  2022-03-30 17:17           ` Adhemerval Zanella
  0 siblings, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-30 14:41 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Joseph Myers

On Wed, Mar 30, 2022 at 7:18 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 29/03/2022 19:29, H.J. Lu wrote:
> > On Tue, Mar 29, 2022 at 9:52 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> >>> The EI_ABIVERSION field of the ELF header in executables and shared
> >>> libraries can be bumped to indicate the minimum ABI requirement on the
> >>> dynamic linker.  However, EI_ABIVERSION in executables isn't checked by
> >>> the Linux kernel ELF loader nor the existing dynamic linker.  Executables
> >>> will crash mysteriously if the dynamic linker doesn't support the ABI
> >>> features required by the EI_ABIVERSION field.  The dynamic linker should
> >>> be changed to check EI_ABIVERSION in executables.
> >>>
> >>> Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support so
> >>> that the existing dynamic linkers will issue an error on executables with
> >>> GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR entry
> >>> without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.
> >>>
> >>> Support __placeholder_only_for_empty_version_map as the placeholder symbol
> >>> used only for empty version map to generate GLIBC_ABI_DT_RELR without any
> >>> symbols.
> >>
> >> I think it only make sense to add the GLIBC_ABI_DT_RELR if the ABI actually
> >> supports DT_RELR, otherwise if the ABI start to support old glibcs might
> >> wrongly assumes DT_RELR and fails with undefined behavior at runtime
> >> (which this patch essentially tries to avoid).
> >
> > The DT_RELR set enables DT_RELR in glibc for all targets.
> If I understood correctly, if we add GLIBC_ABI_DT_RELR on 2.36 but only enable
> it on some architecture on 2.37 and try to run a binary with DT_RELR enabled
> on glibc 2.36, _dl_check_map_versions won't accuse a failure and it will
> eventually only fail at runtime (since relocation won't be applied).  My
> understanding we are trying to avoid such failures.

My patch set enables DT_RELR and GLIBC_ABI_DT_RELR on glibc 2.36
for all, regardless of binutils version.   There is no per-arch
behavior.    Only
DT_RELR run-time tests are enabled for linkers with -z pack-relative-relocs.

> >
> >>> ---
> >>>  elf/Makefile         | 12 ++++++++++--
> >>>  elf/Versions         |  5 +++++
> >>>  elf/dl-version.c     | 33 +++++++++++++++++++++++++++++++--
> >>>  include/link.h       |  6 ++++++
> >>>  scripts/abilist.awk  |  2 ++
> >>>  scripts/versions.awk |  7 ++++++-
> >>>  6 files changed, 60 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/elf/Makefile b/elf/Makefile
> >>> index fd462ba315..d3118b9777 100644
> >>> --- a/elf/Makefile
> >>> +++ b/elf/Makefile
> >>> @@ -1113,8 +1113,10 @@ $(eval $(call include_dsosort_tests,dso-sort-tests-1.def))
> >>>  $(eval $(call include_dsosort_tests,dso-sort-tests-2.def))
> >>>  endif
> >>>
> >>> -check-abi: $(objpfx)check-abi-ld.out
> >>> -tests-special += $(objpfx)check-abi-ld.out
> >>> +check-abi: $(objpfx)check-abi-ld.out \
> >>> +        $(objpfx)check-abi-version-libc.out
> >>> +tests-special += $(objpfx)check-abi-ld.out \
> >>> +        $(objpfx)check-abi-version-libc.out
> >>
> >> One line per definition.
> >
> > Fixed in v7.
> >
> >>>  update-abi: update-abi-ld
> >>>  update-all-abi: update-all-abi-ld
> >>>
> >>> @@ -2739,3 +2741,9 @@ $(objpfx)check-tst-relr-pie.out: $(objpfx)tst-relr-pie
> >>>               | sed -ne '/required from libc.so/,$$ p' \
> >>>               | grep GLIBC_ABI_DT_RELR > $@; \
> >>>       $(evaluate-test)
> >>> +
> >>> +$(objpfx)check-abi-version-libc.out: $(common-objpfx)libc.so
> >>> +     LC_ALL=C $(READELF) -V -W $< \
> >>> +             | sed -ne '/.gnu.version_d/, /.gnu.version_r/ p' \
> >>> +             | grep GLIBC_ABI_DT_RELR > $@; \
> >>> +     $(evaluate-test)
> >>
> >> This test should be enable iff for $(have-dt-relr) == yes.
> >
> > This test checks if libc.so has the GLIBC_ABI_DT_RELR
> > version definition.   Since it doesn't require a new linker,
> > there is no need to check $(have-dt-relr).   This test
> > passes with an old linker.
> >
> >>> diff --git a/elf/Versions b/elf/Versions
> >>> index 8bed855d8c..a9ff278de7 100644
> >>> --- a/elf/Versions
> >>> +++ b/elf/Versions
> >>> @@ -23,6 +23,11 @@ libc {
> >>>    GLIBC_2.35 {
> >>>      _dl_find_object;
> >>>    }
> >>> +  GLIBC_ABI_DT_RELR {
> >>> +    # This symbol is used only for empty version map and will be removed
> >>> +    # by scripts/versions.awk.
> >>> +    __placeholder_only_for_empty_version_map;
> >>> +  }
> >>
> >> Same as before. Maybe use the same strategy we do for time64: add a config.h
> >> define to HAVE_DTRELR (set by configure) and use it to set the symbol if
> >> it is defined.
> >>
> >> This define would be used on dl-version.c to also enable the l_abi_version
> >> check.
> >>
> >>>    GLIBC_PRIVATE {
> >>>      # functions used in other libraries
> >>>      __libc_early_init;
> >>> diff --git a/elf/dl-version.c b/elf/dl-version.c
> >>> index b47bd91727..720ec596a5 100644
> >>> --- a/elf/dl-version.c
> >>> +++ b/elf/dl-version.c
> >>> @@ -214,12 +214,20 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
> >>>             while (1)
> >>>               {
> >>>                 /* Match the symbol.  */
> >>> +               const char *string = strtab + aux->vna_name;
> >>>                 result |= match_symbol (DSO_FILENAME (map->l_name),
> >>>                                         map->l_ns, aux->vna_hash,
> >>> -                                       strtab + aux->vna_name,
> >>> -                                       needed->l_real, verbose,
> >>> +                                       string, needed->l_real, verbose,
> >>>                                         aux->vna_flags & VER_FLG_WEAK);
> >>>
> >>> +               if (map->l_abi_version == lav_none
> >>> +                   /* 0xfd0e42: _dl_elf_hash ("GLIBC_ABI_DT_RELR").  */
> >>> +                   && aux->vna_hash == 0xfd0e42
> >>> +                   && __glibc_likely (strcmp (string,
> >>> +                                              "GLIBC_ABI_DT_RELR")
> >>> +                                      == 0))
> >>> +                 map->l_abi_version = lav_dt_relr_ref;
> >>> +
> >>>                 /* Compare the version index.  */
> >>>                 if ((unsigned int) (aux->vna_other & 0x7fff) > ndx_high)
> >>>                   ndx_high = aux->vna_other & 0x7fff;
> >>> @@ -253,6 +261,16 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
> >>>        ent = (ElfW(Verdef) *) (map->l_addr + def->d_un.d_ptr);
> >>>        while (1)
> >>>       {
> >>> +       /* 0x0963cf85: _dl_elf_hash ("GLIBC_PRIVATE").  */
> >>> +       if (ent->vd_hash == 0x0963cf85)
> >>> +         {
> >>> +           ElfW(Verdaux) *aux = (ElfW(Verdaux) *) ((char *) ent
> >>> +                                                   + ent->vd_aux);
> >>> +           if (__glibc_likely (strcmp ("GLIBC_PRIVATE",
> >>> +                                       strtab + aux->vda_name) == 0))
> >>> +             map->l_abi_version = lav_private_def;
> >>> +         }
> >>> +
> >>>         if ((unsigned int) (ent->vd_ndx & 0x7fff) > ndx_high)
> >>>           ndx_high = ent->vd_ndx & 0x7fff;
> >>>
> >>> @@ -352,6 +370,17 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
> >>>       }
> >>>      }
> >>>
> >>> +  /* Issue an error if there is a DT_RELR entry without GLIBC_ABI_DT_RELR
> >>> +     dependency nor GLIBC_PRIVATE definition.  */
> >>> +  if (map->l_info[DT_RELR] != NULL
> >>> +      && __glibc_unlikely (map->l_abi_version == lav_none))
> >>> +    {
> >>> +      _dl_exception_create
> >>> +     (&exception, DSO_FILENAME (map->l_name),
> >>> +      N_("DT_RELR without GLIBC_ABI_DT_RELR dependency"));
> >>> +      goto call_error;
> >>> +    }
> >>> +
> >>>    return result;
> >>>  }
> >>>
> >>> diff --git a/include/link.h b/include/link.h
> >>> index 03db14c7b0..8ec5e35cf2 100644
> >>> --- a/include/link.h
> >>> +++ b/include/link.h
> >>> @@ -177,6 +177,12 @@ struct link_map
> >>>       lt_library,             /* Library needed by main executable.  */
> >>>       lt_loaded               /* Extra run-time loaded shared object.  */
> >>>        } l_type:2;
> >>> +    enum                     /* ABI dependency of this object.  */
> >>> +      {
> >>> +     lav_none,               /* No ABI dependency.  */
> >>> +     lav_dt_relr_ref,        /* Need GLIBC_ABI_DT_RELR.  */
> >>> +     lav_private_def         /* Define GLIBC_PRIVATE.  */
> >>> +      } l_abi_version:2;
> >>>      unsigned int l_relocated:1;      /* Nonzero if object's relocations done.  */
> >>>      unsigned int l_init_called:1; /* Nonzero if DT_INIT function called.  */
> >>>      unsigned int l_global:1; /* Nonzero if object in _dl_global_scope.  */
> >>> diff --git a/scripts/abilist.awk b/scripts/abilist.awk
> >>> index 24a34ccbed..6cc7af6ac8 100644
> >>> --- a/scripts/abilist.awk
> >>> +++ b/scripts/abilist.awk
> >>> @@ -55,6 +55,8 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
> >>>    # caused STV_HIDDEN symbols to appear in .dynsym, though that is useless.
> >>>    if (NF > 7 && $7 == ".hidden") next;
> >>>
> >>> +  if (version ~ /^GLIBC_ABI_/ && !include_abi_version) next;
> >>> +
> >>>    if (version == "GLIBC_PRIVATE" && !include_private) next;
> >>>
> >>>    desc = "";
> >>> diff --git a/scripts/versions.awk b/scripts/versions.awk
> >>> index 357ad1355e..d70b07bd1a 100644
> >>> --- a/scripts/versions.awk
> >>> +++ b/scripts/versions.awk
> >>> @@ -185,8 +185,13 @@ END {
> >>>       closeversion(oldver, veryoldver);
> >>>       veryoldver = oldver;
> >>>        }
> >>> -      printf("%s {\n  global:\n", $2) > outfile;
> >>>        oldver = $2;
> >>> +      # Skip the placeholder symbol used only for empty version map.
> >>> +      if ($3 == "__placeholder_only_for_empty_version_map;") {
> >>> +     printf("%s {\n", $2) > outfile;
> >>> +     continue;
> >>> +      }
> >>> +      printf("%s {\n  global:\n", $2) > outfile;
> >>>      }
> >>>      printf("   ") > outfile;
> >>>      for (n = 3; n <= NF; ++n) {
> >
> >
> >



-- 
H.J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-30 14:41         ` H.J. Lu
@ 2022-03-30 17:17           ` Adhemerval Zanella
  2022-03-30 17:32             ` H.J. Lu
  2022-03-30 19:22             ` Joseph Myers
  0 siblings, 2 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-30 17:17 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library, Joseph Myers



On 30/03/2022 11:41, H.J. Lu wrote:
> On Wed, Mar 30, 2022 at 7:18 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 29/03/2022 19:29, H.J. Lu wrote:
>>> On Tue, Mar 29, 2022 at 9:52 AM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
>>>>> The EI_ABIVERSION field of the ELF header in executables and shared
>>>>> libraries can be bumped to indicate the minimum ABI requirement on the
>>>>> dynamic linker.  However, EI_ABIVERSION in executables isn't checked by
>>>>> the Linux kernel ELF loader nor the existing dynamic linker.  Executables
>>>>> will crash mysteriously if the dynamic linker doesn't support the ABI
>>>>> features required by the EI_ABIVERSION field.  The dynamic linker should
>>>>> be changed to check EI_ABIVERSION in executables.
>>>>>
>>>>> Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support so
>>>>> that the existing dynamic linkers will issue an error on executables with
>>>>> GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR entry
>>>>> without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.
>>>>>
>>>>> Support __placeholder_only_for_empty_version_map as the placeholder symbol
>>>>> used only for empty version map to generate GLIBC_ABI_DT_RELR without any
>>>>> symbols.
>>>>
>>>> I think it only make sense to add the GLIBC_ABI_DT_RELR if the ABI actually
>>>> supports DT_RELR, otherwise if the ABI start to support old glibcs might
>>>> wrongly assumes DT_RELR and fails with undefined behavior at runtime
>>>> (which this patch essentially tries to avoid).
>>>
>>> The DT_RELR set enables DT_RELR in glibc for all targets.
>> If I understood correctly, if we add GLIBC_ABI_DT_RELR on 2.36 but only enable
>> it on some architecture on 2.37 and try to run a binary with DT_RELR enabled
>> on glibc 2.36, _dl_check_map_versions won't accuse a failure and it will
>> eventually only fail at runtime (since relocation won't be applied).  My
>> understanding we are trying to avoid such failures.
> 
> My patch set enables DT_RELR and GLIBC_ABI_DT_RELR on glibc 2.36
> for all, regardless of binutils version.   There is no per-arch
> behavior.    Only
> DT_RELR run-time tests are enabled for linkers with -z pack-relative-relocs.

But it still does not fail when the architecture does not actually implement
RELR even when static linker does:

$ clang --target=aarch64-linux-gnu -fuse-ld=lld -Wl,-z,pack-relative-relocs -fpie -o test -pie test.c
$ aarch64-glibc-linux-gnu-readelf -S test | grep relr\.dyn
  [ 9] .relr.dyn         RELR             0000000000000528  00000528
$ ./elf/ld.so --library-path . ./test
$

I would expect that even when binary adds GLIBC_ABI_DT_RELR, if dynamic linker
does not have DT_RELR it would fail early.


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-30 17:17           ` Adhemerval Zanella
@ 2022-03-30 17:32             ` H.J. Lu
  2022-03-30 17:39               ` Adhemerval Zanella
  2022-03-30 19:22             ` Joseph Myers
  1 sibling, 1 reply; 27+ messages in thread
From: H.J. Lu @ 2022-03-30 17:32 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Joseph Myers

Does the linker add the version dependency? Does ld.so support DT_RELR?

On Wed, Mar 30, 2022, 10:17 AM Adhemerval Zanella <
adhemerval.zanella@linaro.org> wrote:

>
>
> On 30/03/2022 11:41, H.J. Lu wrote:
> > On Wed, Mar 30, 2022 at 7:18 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 29/03/2022 19:29, H.J. Lu wrote:
> >>> On Tue, Mar 29, 2022 at 9:52 AM Adhemerval Zanella
> >>> <adhemerval.zanella@linaro.org> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
> >>>>> The EI_ABIVERSION field of the ELF header in executables and shared
> >>>>> libraries can be bumped to indicate the minimum ABI requirement on
> the
> >>>>> dynamic linker.  However, EI_ABIVERSION in executables isn't checked
> by
> >>>>> the Linux kernel ELF loader nor the existing dynamic linker.
> Executables
> >>>>> will crash mysteriously if the dynamic linker doesn't support the ABI
> >>>>> features required by the EI_ABIVERSION field.  The dynamic linker
> should
> >>>>> be changed to check EI_ABIVERSION in executables.
> >>>>>
> >>>>> Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support
> so
> >>>>> that the existing dynamic linkers will issue an error on executables
> with
> >>>>> GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR
> entry
> >>>>> without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.
> >>>>>
> >>>>> Support __placeholder_only_for_empty_version_map as the placeholder
> symbol
> >>>>> used only for empty version map to generate GLIBC_ABI_DT_RELR
> without any
> >>>>> symbols.
> >>>>
> >>>> I think it only make sense to add the GLIBC_ABI_DT_RELR if the ABI
> actually
> >>>> supports DT_RELR, otherwise if the ABI start to support old glibcs
> might
> >>>> wrongly assumes DT_RELR and fails with undefined behavior at runtime
> >>>> (which this patch essentially tries to avoid).
> >>>
> >>> The DT_RELR set enables DT_RELR in glibc for all targets.
> >> If I understood correctly, if we add GLIBC_ABI_DT_RELR on 2.36 but only
> enable
> >> it on some architecture on 2.37 and try to run a binary with DT_RELR
> enabled
> >> on glibc 2.36, _dl_check_map_versions won't accuse a failure and it will
> >> eventually only fail at runtime (since relocation won't be applied).  My
> >> understanding we are trying to avoid such failures.
> >
> > My patch set enables DT_RELR and GLIBC_ABI_DT_RELR on glibc 2.36
> > for all, regardless of binutils version.   There is no per-arch
> > behavior.    Only
> > DT_RELR run-time tests are enabled for linkers with -z
> pack-relative-relocs.
>
> But it still does not fail when the architecture does not actually
> implement
> RELR even when static linker does:
>
> $ clang --target=aarch64-linux-gnu -fuse-ld=lld
> -Wl,-z,pack-relative-relocs -fpie -o test -pie test.c
> $ aarch64-glibc-linux-gnu-readelf -S test | grep relr\.dyn
>   [ 9] .relr.dyn         RELR             0000000000000528  00000528
> $ ./elf/ld.so --library-path . ./test
> $
>
> I would expect that even when binary adds GLIBC_ABI_DT_RELR, if dynamic
> linker
> does not have DT_RELR it would fail early.
>
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-30 17:32             ` H.J. Lu
@ 2022-03-30 17:39               ` Adhemerval Zanella
  0 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-30 17:39 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library, Joseph Myers



On 30/03/2022 14:32, H.J. Lu wrote:
> Does the linker add the version dependency? Does ld.so support DT_RELR?
> 

$ readelf -V test | grep GLIBC_ABI_DT_RELR
  0x0030:   Name: GLIBC_ABI_DT_RELR  Flags: none  Version: 4

But ld.so does not support DT_RELR

$ grep libc_cv_dt_relr config.log 
libc_cv_dt_relr=no


> On Wed, Mar 30, 2022, 10:17 AM Adhemerval Zanella <adhemerval.zanella@linaro.org <mailto:adhemerval.zanella@linaro.org>> wrote:
> 
> 
> 
>     On 30/03/2022 11:41, H.J. Lu wrote:
>     > On Wed, Mar 30, 2022 at 7:18 AM Adhemerval Zanella
>     > <adhemerval.zanella@linaro.org <mailto:adhemerval.zanella@linaro.org>> wrote:
>     >>
>     >>
>     >>
>     >> On 29/03/2022 19:29, H.J. Lu wrote:
>     >>> On Tue, Mar 29, 2022 at 9:52 AM Adhemerval Zanella
>     >>> <adhemerval.zanella@linaro.org <mailto:adhemerval.zanella@linaro.org>> wrote:
>     >>>>
>     >>>>
>     >>>>
>     >>>> On 10/03/2022 17:03, H.J. Lu via Libc-alpha wrote:
>     >>>>> The EI_ABIVERSION field of the ELF header in executables and shared
>     >>>>> libraries can be bumped to indicate the minimum ABI requirement on the
>     >>>>> dynamic linker.  However, EI_ABIVERSION in executables isn't checked by
>     >>>>> the Linux kernel ELF loader nor the existing dynamic linker.  Executables
>     >>>>> will crash mysteriously if the dynamic linker doesn't support the ABI
>     >>>>> features required by the EI_ABIVERSION field.  The dynamic linker should
>     >>>>> be changed to check EI_ABIVERSION in executables.
>     >>>>>
>     >>>>> Add a glibc version, GLIBC_ABI_DT_RELR, to indicate DT_RELR support so
>     >>>>> that the existing dynamic linkers will issue an error on executables with
>     >>>>> GLIBC_ABI_DT_RELR dependency.  Issue an error if there is a DT_RELR entry
>     >>>>> without GLIBC_ABI_DT_RELR dependency nor GLIBC_PRIVATE definition.
>     >>>>>
>     >>>>> Support __placeholder_only_for_empty_version_map as the placeholder symbol
>     >>>>> used only for empty version map to generate GLIBC_ABI_DT_RELR without any
>     >>>>> symbols.
>     >>>>
>     >>>> I think it only make sense to add the GLIBC_ABI_DT_RELR if the ABI actually
>     >>>> supports DT_RELR, otherwise if the ABI start to support old glibcs might
>     >>>> wrongly assumes DT_RELR and fails with undefined behavior at runtime
>     >>>> (which this patch essentially tries to avoid).
>     >>>
>     >>> The DT_RELR set enables DT_RELR in glibc for all targets.
>     >> If I understood correctly, if we add GLIBC_ABI_DT_RELR on 2.36 but only enable
>     >> it on some architecture on 2.37 and try to run a binary with DT_RELR enabled
>     >> on glibc 2.36, _dl_check_map_versions won't accuse a failure and it will
>     >> eventually only fail at runtime (since relocation won't be applied).  My
>     >> understanding we are trying to avoid such failures.
>     >
>     > My patch set enables DT_RELR and GLIBC_ABI_DT_RELR on glibc 2.36
>     > for all, regardless of binutils version.   There is no per-arch
>     > behavior.    Only
>     > DT_RELR run-time tests are enabled for linkers with -z pack-relative-relocs.
> 
>     But it still does not fail when the architecture does not actually implement
>     RELR even when static linker does:
> 
>     $ clang --target=aarch64-linux-gnu -fuse-ld=lld -Wl,-z,pack-relative-relocs -fpie -o test -pie test.c
>     $ aarch64-glibc-linux-gnu-readelf -S test | grep relr\.dyn
>       [ 9] .relr.dyn         RELR             0000000000000528  00000528
>     $ ./elf/ld.so --library-path . ./test
>     $
> 
>     I would expect that even when binary adds GLIBC_ABI_DT_RELR, if dynamic linker
>     does not have DT_RELR it would fail early.
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-30 17:17           ` Adhemerval Zanella
  2022-03-30 17:32             ` H.J. Lu
@ 2022-03-30 19:22             ` Joseph Myers
  2022-03-30 20:38               ` Adhemerval Zanella
  1 sibling, 1 reply; 27+ messages in thread
From: Joseph Myers @ 2022-03-30 19:22 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: H.J. Lu, GNU C Library

On Wed, 30 Mar 2022, Adhemerval Zanella via Libc-alpha wrote:

> But it still does not fail when the architecture does not actually implement
> RELR even when static linker does:

The dynamic linker is supposed to support executing binaries using RELR 
relocations on all architectures - there's not meant to be anything 
architecture-specific about that support.  What's architecture-specific is 
the static linker support (thus, if missing in the static linker used to 
build and test glibc, then (a) the dynamic linker could still run binaries 
using RELR if they were built using some other static linker, but (b) none 
of the glibc binaries, either installed or test, will contain any RELR 
relocations themselves).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-30 19:22             ` Joseph Myers
@ 2022-03-30 20:38               ` Adhemerval Zanella
  2022-03-30 21:32                 ` Fangrui Song
  0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2022-03-30 20:38 UTC (permalink / raw)
  To: Joseph Myers; +Cc: H.J. Lu, GNU C Library



On 30/03/2022 16:22, Joseph Myers wrote:
> On Wed, 30 Mar 2022, Adhemerval Zanella via Libc-alpha wrote:
> 
>> But it still does not fail when the architecture does not actually implement
>> RELR even when static linker does:
> 
> The dynamic linker is supposed to support executing binaries using RELR 
> relocations on all architectures - there's not meant to be anything 
> architecture-specific about that support.  What's architecture-specific is 
> the static linker support (thus, if missing in the static linker used to 
> build and test glibc, then (a) the dynamic linker could still run binaries 
> using RELR if they were built using some other static linker, but (b) none 
> of the glibc binaries, either installed or test, will contain any RELR 
> relocations themselves).
> 

Indeed, I was confused by the expected support.  This part seems ok H.J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support
  2022-03-30 20:38               ` Adhemerval Zanella
@ 2022-03-30 21:32                 ` Fangrui Song
  0 siblings, 0 replies; 27+ messages in thread
From: Fangrui Song @ 2022-03-30 21:32 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Joseph Myers, GNU C Library

On 2022-03-30, Adhemerval Zanella via Libc-alpha wrote:
>
>
>On 30/03/2022 16:22, Joseph Myers wrote:
>> On Wed, 30 Mar 2022, Adhemerval Zanella via Libc-alpha wrote:
>>
>>> But it still does not fail when the architecture does not actually implement
>>> RELR even when static linker does:
>>
>> The dynamic linker is supposed to support executing binaries using RELR
>> relocations on all architectures - there's not meant to be anything
>> architecture-specific about that support.  What's architecture-specific is
>> the static linker support (thus, if missing in the static linker used to
>> build and test glibc, then (a) the dynamic linker could still run binaries
>> using RELR if they were built using some other static linker, but (b) none
>> of the glibc binaries, either installed or test, will contain any RELR
>> relocations themselves).
>>
>
>Indeed, I was confused by the expected support.  This part seems ok H.J.

Agree. Having the generic support helps GNU ld developers to contribute
RELR for non-x86. 
When I first seen "Add --disable-default-dt-relr" I was thinking whether
it is necessary to add this so early, but then I realized that this
essentially turns the whole glibc testsuite into tests
for GNU ld's RELR support. binutils itself has very few run-time tests
and bugs tend to be difficult to surface.

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-03-30 21:32 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10 20:03 [PATCH v6 0/5] Support DT_RELR relative relocation format H.J. Lu
2022-03-10 20:03 ` [PATCH v6 1/5] elf: Support DT_RELR relative relocation format [BZ #27924] H.J. Lu
2022-03-29 16:34   ` Adhemerval Zanella
2022-03-29 22:34     ` H.J. Lu
2022-03-10 20:03 ` [PATCH v6 2/5] elf: Properly handle zero DT_RELA/DT_REL values H.J. Lu
2022-03-29 16:38   ` Adhemerval Zanella
2022-03-29 22:30     ` H.J. Lu
2022-03-10 20:03 ` [PATCH v6 3/5] Add GLIBC_ABI_DT_RELR for DT_RELR support H.J. Lu
2022-03-29 16:52   ` Adhemerval Zanella
2022-03-29 22:29     ` H.J. Lu
2022-03-30 14:18       ` Adhemerval Zanella
2022-03-30 14:41         ` H.J. Lu
2022-03-30 17:17           ` Adhemerval Zanella
2022-03-30 17:32             ` H.J. Lu
2022-03-30 17:39               ` Adhemerval Zanella
2022-03-30 19:22             ` Joseph Myers
2022-03-30 20:38               ` Adhemerval Zanella
2022-03-30 21:32                 ` Fangrui Song
2022-03-10 20:03 ` [PATCH v6 4/5] Add --disable-default-dt-relr H.J. Lu
2022-03-29 17:19   ` Adhemerval Zanella
2022-03-29 22:25     ` H.J. Lu
2022-03-10 20:03 ` [PATCH v6 5/5] NEWS: Mention DT_RELR support H.J. Lu
2022-03-29 17:25   ` Adhemerval Zanella
2022-03-29 22:22     ` H.J. Lu
2022-03-29 23:34       ` Fangrui Song
2022-03-30 14:20         ` Adhemerval Zanella
2022-03-10 20:09 ` [PATCH v6 0/5] Support DT_RELR relative relocation format Fangrui Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).