[hurd,commited] hurd: Fix static-PIE startup

Message ID 20211228094254.1900948-1-samuel.thibault@ens-lyon.org
State Committed, archived
Headers
Series [hurd,commited] hurd: Fix static-PIE startup |

Checks

Context Check Description
dj/TryBot-apply_patch fail Patch failed to apply to master at the time it was sent
dj/TryBot-32bit fail Patch series failed to apply

Commit Message

Samuel Thibault Dec. 28, 2021, 9:42 a.m. UTC
  hurd initialization stages use RUN_HOOK to run various initialization
functions.  That is however using absolute addresses which need to be
relocated, which is done later by csu.  We can however easily make the
linker compute relative addresses which thus don't need a relocation.
The new SET_RELHOOK and RUN_RELHOOK macros implement this.
---
 hurd/dtable.c                       |  8 +++-----
 hurd/hurdid.c                       |  7 +++----
 hurd/hurdinit.c                     |  6 +++---
 hurd/hurdmalloc.c                   | 10 +++-------
 hurd/hurdpid.c                      |  7 +++----
 hurd/hurdrlimit.c                   |  7 +++----
 hurd/hurdsock.c                     |  7 +++----
 include/set-hooks.h                 | 25 +++++++++++++++++++++++
 sysdeps/generic/set-hooks-arch.h    | 31 +++++++++++++++++++++++++++++
 sysdeps/i386/set-hooks-arch.h       | 28 ++++++++++++++++++++++++++
 sysdeps/mach/hurd/brk.c             |  8 ++++----
 sysdeps/mach/hurd/check_fds.c       |  6 ++----
 sysdeps/mach/hurd/i386/init-first.c |  2 +-
 sysdeps/x86_64/set-hooks-arch.h     | 28 ++++++++++++++++++++++++++
 14 files changed, 140 insertions(+), 40 deletions(-)
 create mode 100644 sysdeps/generic/set-hooks-arch.h
 create mode 100644 sysdeps/i386/set-hooks-arch.h
 create mode 100644 sysdeps/x86_64/set-hooks-arch.h
  

Comments

H.J. Lu Feb. 1, 2022, 12:14 a.m. UTC | #1
On Tue, Dec 28, 2021 at 1:43 AM Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
>
> hurd initialization stages use RUN_HOOK to run various initialization
> functions.  That is however using absolute addresses which need to be
> relocated, which is done later by csu.  We can however easily make the
> linker compute relative addresses which thus don't need a relocation.
> The new SET_RELHOOK and RUN_RELHOOK macros implement this.
> ---
>  hurd/dtable.c                       |  8 +++-----
>  hurd/hurdid.c                       |  7 +++----
>  hurd/hurdinit.c                     |  6 +++---
>  hurd/hurdmalloc.c                   | 10 +++-------
>  hurd/hurdpid.c                      |  7 +++----
>  hurd/hurdrlimit.c                   |  7 +++----
>  hurd/hurdsock.c                     |  7 +++----
>  include/set-hooks.h                 | 25 +++++++++++++++++++++++
>  sysdeps/generic/set-hooks-arch.h    | 31 +++++++++++++++++++++++++++++
>  sysdeps/i386/set-hooks-arch.h       | 28 ++++++++++++++++++++++++++
>  sysdeps/mach/hurd/brk.c             |  8 ++++----
>  sysdeps/mach/hurd/check_fds.c       |  6 ++----
>  sysdeps/mach/hurd/i386/init-first.c |  2 +-
>  sysdeps/x86_64/set-hooks-arch.h     | 28 ++++++++++++++++++++++++++
>  14 files changed, 140 insertions(+), 40 deletions(-)
>  create mode 100644 sysdeps/generic/set-hooks-arch.h
>  create mode 100644 sysdeps/i386/set-hooks-arch.h
>  create mode 100644 sysdeps/x86_64/set-hooks-arch.h
>
> diff --git a/hurd/dtable.c b/hurd/dtable.c
> index bbd3bfc892..4f081f09fd 100644
> --- a/hurd/dtable.c
> +++ b/hurd/dtable.c
> @@ -36,7 +36,7 @@ DEFINE_HOOK (_hurd_fd_subinit, (void));
>
>  /* Initialize the file descriptor table at startup.  */
>
> -static void
> +static void attribute_used_retain
>  init_dtable (void)
>  {
>    int i;
> @@ -91,12 +91,10 @@ init_dtable (void)
>
>    /* Run things that want to run after the file descriptor table
>       is initialized.  */
> -  RUN_HOOK (_hurd_fd_subinit, ());
> -
> -  (void) &init_dtable;         /* Avoid "defined but not used" warning.  */
> +  RUN_RELHOOK (_hurd_fd_subinit, ());
>  }
>
> -text_set_element (_hurd_subinit, init_dtable);
> +SET_RELHOOK (_hurd_subinit, init_dtable);
>
>  /* XXX when the linker supports it, the following functions should all be
>     elsewhere and just have text_set_elements here.  */
> diff --git a/hurd/hurdid.c b/hurd/hurdid.c
> index 70c46c0740..b220532c35 100644
> --- a/hurd/hurdid.c
> +++ b/hurd/hurdid.c
> @@ -17,6 +17,7 @@
>
>  #include <hurd.h>
>  #include <hurd/id.h>
> +#include "set-hooks.h"
>
>  struct hurd_id_data _hurd_id;
>
> @@ -74,7 +75,7 @@ _hurd_check_ids (void)
>    return 0;
>  }
>
> -static void
> +static void attribute_used_retain
>  init_id (void)
>  {
>    __mutex_init (&_hurd_id.lock);
> @@ -84,7 +85,5 @@ init_id (void)
>    _hurd_id.gen.nuids = _hurd_id.aux.nuids = 0;
>    _hurd_id.gen.gids = _hurd_id.aux.gids = NULL;
>    _hurd_id.gen.ngids = _hurd_id.aux.ngids = 0;
> -
> -  (void) &init_id;             /* Avoid "defined but not used" warning.  */
>  }
> -text_set_element (_hurd_preinit_hook, init_id);
> +SET_RELHOOK (_hurd_preinit_hook, init_id);
> diff --git a/hurd/hurdinit.c b/hurd/hurdinit.c
> index d4d219a083..9377d902d2 100644
> --- a/hurd/hurdinit.c
> +++ b/hurd/hurdinit.c
> @@ -108,7 +108,7 @@ _hurd_init (int flags, char **argv,
>    /* Call other things which want to do some initialization.  These are not
>       on the __libc_subinit hook because things there like to be able to
>       assume the availability of the POSIX.1 services we provide.  */
> -  RUN_HOOK (_hurd_subinit, ());
> +  RUN_RELHOOK (_hurd_subinit, ());
>  }
>  libc_hidden_def (_hurd_init)
>
> @@ -190,7 +190,7 @@ _hurd_new_proc_init (char **argv,
>    /* Call other things which want to do some initialization.  These are not
>       on the _hurd_subinit hook because things there assume that things done
>       here, like _hurd_pid, are already initialized.  */
> -  RUN_HOOK (_hurd_proc_subinit, ());
> +  RUN_RELHOOK (_hurd_proc_subinit, ());
>
>    /* XXX This code should probably be removed entirely at some point.  This
>       conditional should make it reasonably usable with old gdb's for a
> @@ -242,7 +242,7 @@ _hurd_setproc (process_t procserver)
>
>      /* Call these functions again so they can fetch the
>         new information from the new proc server.  */
> -    RUN_HOOK (_hurd_proc_subinit, ());
> +    RUN_RELHOOK (_hurd_proc_subinit, ());
>
>      if (_hurd_pgrp != oldpgrp)
>        {
> diff --git a/hurd/hurdmalloc.c b/hurd/hurdmalloc.c
> index 7046bcef33..ccb8fc8b54 100644
> --- a/hurd/hurdmalloc.c
> +++ b/hurd/hurdmalloc.c
> @@ -1,5 +1,6 @@
>  #include <stdlib.h>
>  #include <string.h>
> +#include "set-hooks.h"
>
>  #include "hurdmalloc.h"                /* XXX see that file */
>
> @@ -148,7 +149,7 @@ static struct free_list malloc_free_list[NBUCKETS];
>     It preserves the values of data variables like malloc_free_list, but
>     does not save the vm_allocate'd space allocated by this malloc.  */
>
> -static void
> +static void attribute_used_retain
>  malloc_init (void)
>  {
>    int i;
> @@ -160,11 +161,6 @@ malloc_init (void)
>        malloc_free_list[i].in_use = 0;
>  #endif
>      }
> -
> -  /* This not only suppresses a `defined but not used' warning,
> -     but it is ABSOLUTELY NECESSARY to avoid the hyperclever
> -     compiler from "optimizing out" the entire function!  */
> -  (void) &malloc_init;
>  }
>
>  static void
> @@ -445,4 +441,4 @@ _hurd_malloc_fork_child(void)
>  }
>
>
> -text_set_element (_hurd_preinit_hook, malloc_init);
> +SET_RELHOOK (_hurd_preinit_hook, malloc_init);
> diff --git a/hurd/hurdpid.c b/hurd/hurdpid.c
> index 9014de58e9..613ca357c6 100644
> --- a/hurd/hurdpid.c
> +++ b/hurd/hurdpid.c
> @@ -17,11 +17,12 @@
>
>  #include <hurd.h>
>  #include <lowlevellock.h>
> +#include "set-hooks.h"
>
>  pid_t _hurd_pid, _hurd_ppid, _hurd_pgrp;
>  int _hurd_orphaned;
>
> -static void
> +static void attribute_used_retain
>  init_pids (void)
>  {
>    __USEPORT (PROC,
> @@ -29,11 +30,9 @@ init_pids (void)
>                __proc_getpids (port, &_hurd_pid, &_hurd_ppid, &_hurd_orphaned);
>                __proc_getpgrp (port, _hurd_pid, &_hurd_pgrp);
>              }));
> -
> -  (void) &init_pids;           /* Avoid "defined but not used" warning.  */
>  }
>
> -text_set_element (_hurd_proc_subinit, init_pids);
> +SET_RELHOOK (_hurd_proc_subinit, init_pids);
>
>  #include <hurd/msg_server.h>
>  #include "set-hooks.h"
> diff --git a/hurd/hurdrlimit.c b/hurd/hurdrlimit.c
> index 17535c2851..4d16bd4f3f 100644
> --- a/hurd/hurdrlimit.c
> +++ b/hurd/hurdrlimit.c
> @@ -19,6 +19,7 @@
>  #include <hurd.h>
>  #include <lock-intern.h>
>  #include <hurd/resource.h>
> +#include "set-hooks.h"
>
>  /* This must be given an initializer, or the a.out linking rules will
>     not include the entire file when this symbol is referenced. */
> @@ -29,7 +30,7 @@ struct rlimit _hurd_rlimits[RLIM_NLIMITS] = { { 0, }, };
>     mutex_init is still required below just in case of unexec.  */
>  struct mutex _hurd_rlimit_lock = { SPIN_LOCK_INITIALIZER, };
>
> -static void
> +static void attribute_used_retain
>  init_rlimit (void)
>  {
>    int i;
> @@ -52,7 +53,5 @@ init_rlimit (void)
>           }
>  #undef I
>      }
> -
> -  (void) &init_rlimit;
>  }
> -text_set_element (_hurd_preinit_hook, init_rlimit);
> +SET_RELHOOK (_hurd_preinit_hook, init_rlimit);
> diff --git a/hurd/hurdsock.c b/hurd/hurdsock.c
> index 04e86b4324..116326b2f2 100644
> --- a/hurd/hurdsock.c
> +++ b/hurd/hurdsock.c
> @@ -25,6 +25,7 @@
>  #include <_itoa.h>
>  #include <lock-intern.h>       /* For `struct mutex'.  */
>  #include "hurdmalloc.h"                /* XXX */
> +#include "set-hooks.h"
>
>  static struct mutex lock;
>
> @@ -109,7 +110,7 @@ retry:
>    return server;
>  }
>
> -static void
> +static void attribute_used_retain
>  init (void)
>  {
>    int i;
> @@ -118,7 +119,5 @@ init (void)
>
>    for (i = 0; i < max_domain; ++i)
>      servers[i] = MACH_PORT_NULL;
> -
> -  (void) &init;                        /* Avoid "defined but not used" warning.  */
>  }
> -text_set_element (_hurd_preinit_hook, init);
> +SET_RELHOOK (_hurd_preinit_hook, init);
> diff --git a/include/set-hooks.h b/include/set-hooks.h
> index a60b5ae19f..1a2f6ad56b 100644
> --- a/include/set-hooks.h
> +++ b/include/set-hooks.h
> @@ -24,6 +24,8 @@
>  #include <sys/cdefs.h>
>  #include <libc-symbols.h>
>
> +#include "set-hooks-arch.h"
> +
>  #ifdef symbol_set_define
>  /* Define a hook variable called NAME.  Functions put on this hook take
>     arguments described by PROTO.  Use `text_set_element (NAME, FUNCTION)'
> @@ -55,6 +57,25 @@ do {                                                                       \
>  DEFINE_HOOK (name, proto); \
>  extern void runner proto; void runner proto { RUN_HOOK (name, args); }
>
> +# ifdef SET_RELHOOK
> +/* This is similar to RUN_RELHOOK, but the hooks were registered with
> + * SET_RELHOOK so that a relative offset was computed by the linker
> + * rather than an absolute address by the dynamic linker. */
> +#  define RUN_RELHOOK(NAME, ARGS)                                    \
> +do {                                                                 \
> +  void *const *ptr;                                                  \
> +  for (ptr = (void *const *) symbol_set_first_element (NAME);        \
> +       ! symbol_set_end_p (NAME, ptr); ++ptr) {                              \
> +    __##NAME##_hook_function_t *f =                                  \
> +       (void*) ((uintptr_t) ptr + (ptrdiff_t) *ptr);                 \
> +    (*f) ARGS;                                                       \
> +  } \
> +} while (0)
> +# else
> +#  define SET_RELHOOK(NAME, HOOK) text_set_element (NAME, HOOK)
> +#  define RUN_RELHOOK(NAME, ARGS) RUN_HOOK(NAME, ARGS)
> +# endif
> +
>  #else
>
>  /* The system does not provide necessary support for this.  */
> @@ -66,6 +87,10 @@ extern void runner proto; void runner proto { RUN_HOOK (name, args); }
>
>  # define DEFINE_HOOK_RUNNER(name, runner, proto, args)
>
> +# define SET_RELHOOK(NAME, HOOK)
> +
> +# define RUN_RELHOOK(NAME, ARGS)
> +
>  #endif
>
>  #endif /* set-hooks.h */
> diff --git a/sysdeps/generic/set-hooks-arch.h b/sysdeps/generic/set-hooks-arch.h
> new file mode 100644
> index 0000000000..4b47fa2f2e
> --- /dev/null
> +++ b/sysdeps/generic/set-hooks-arch.h
> @@ -0,0 +1,31 @@
> +/* Machine-dependent macros for using symbol sets for running lists of
> +   functions. Generic/stub version.
> +   Copyright (C) 2021 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/>.  */
> +
> +#ifndef _SET_HOOKS_ARCH_H
> +#define _SET_HOOKS_ARCH_H
> +
> +/* Define SET_RELHOOK to a variant of text_set_element that records a relative
> +   offset rather than an absolute address. See sysdeps/i386/set-hooks-arch.h
> +   for an example.
> +
> +#define SET_RELHOOK(NAME, HOOK) ...
> +
> + */
> +
> +#endif /* set_hooks_arch.h */
> diff --git a/sysdeps/i386/set-hooks-arch.h b/sysdeps/i386/set-hooks-arch.h
> new file mode 100644
> index 0000000000..97513bf897
> --- /dev/null
> +++ b/sysdeps/i386/set-hooks-arch.h
> @@ -0,0 +1,28 @@
> +/* Machine-dependent macros for using symbol sets for running lists of
> +   functions. i386 version.
> +   Copyright (C) 2021 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/>.  */
> +
> +#ifndef _SET_HOOKS_ARCH_H
> +#define _SET_HOOKS_ARCH_H
> +
> +#define SET_RELHOOK(NAME, HOOK) \
> +       asm(".section " #NAME",\"aR\"\n" \
> +           ".long "#HOOK" - .\n" \
> +           ".section .text");
> +
> +#endif /* set_hooks_arch.h */
> diff --git a/sysdeps/mach/hurd/brk.c b/sysdeps/mach/hurd/brk.c
> index a4294b9eae..088c99b2c8 100644
> --- a/sysdeps/mach/hurd/brk.c
> +++ b/sysdeps/mach/hurd/brk.c
> @@ -21,6 +21,8 @@
>  #include <lock-intern.h>       /* For `struct mutex'.  */
>  #include <vm_param.h>
>
> +#include "set-hooks.h"
> +
>
>  /* Initial maximum size of the data segment (this is arbitrary).  */
>  #define        DATA_SIZE       (128 * 1024 * 1024)
> @@ -130,7 +132,7 @@ _hurd_set_brk (vm_address_t addr)
>    return 0;
>  }
>
> -static void
> +static void attribute_used_retain
>  init_brk (void)
>  {
>    vm_address_t pagend;
> @@ -160,7 +162,5 @@ init_brk (void)
>         /* Couldn't allocate the memory.  The break will be very short.  */
>         _hurd_data_end = pagend;
>      }
> -
> -  (void) &init_brk;            /* Avoid ``defined but not used'' warning.  */
>  }
> -text_set_element (_hurd_preinit_hook, init_brk);
> +SET_RELHOOK (_hurd_preinit_hook, init_brk);
> diff --git a/sysdeps/mach/hurd/check_fds.c b/sysdeps/mach/hurd/check_fds.c
> index 155e9dd3e9..61e6055d35 100644
> --- a/sysdeps/mach/hurd/check_fds.c
> +++ b/sysdeps/mach/hurd/check_fds.c
> @@ -79,7 +79,7 @@ check_standard_fds (void)
>    check_one_fd (STDERR_FILENO, O_RDWR);
>  }
>
> -static void
> +static void attribute_used_retain
>  init_standard_fds (void)
>  {
>    /* Now that we have FDs, make sure that, if this is a SUID program,
> @@ -87,10 +87,8 @@ init_standard_fds (void)
>       ourselves.  If that's not possible we stop the program.  */
>    if (__builtin_expect (__libc_enable_secure, 0))
>      check_standard_fds ();
> -
> -  (void) &init_standard_fds;   /* Avoid "defined but not used" warning.  */
>  }
> -text_set_element (_hurd_fd_subinit, init_standard_fds);
> +SET_RELHOOK (_hurd_fd_subinit, init_standard_fds);
>
>
>  #ifndef SHARED
> diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c
> index 5e85aa2bc5..c6ae370daf 100644
> --- a/sysdeps/mach/hurd/i386/init-first.c
> +++ b/sysdeps/mach/hurd/i386/init-first.c
> @@ -242,7 +242,7 @@ first_init (void)
>    /* Initialize data structures so we can do RPCs.  */
>    __mach_init ();
>
> -  RUN_HOOK (_hurd_preinit_hook, ());
> +  RUN_RELHOOK (_hurd_preinit_hook, ());
>  }
>
>  #ifdef SHARED
> diff --git a/sysdeps/x86_64/set-hooks-arch.h b/sysdeps/x86_64/set-hooks-arch.h
> new file mode 100644
> index 0000000000..227d4f8e08
> --- /dev/null
> +++ b/sysdeps/x86_64/set-hooks-arch.h
> @@ -0,0 +1,28 @@
> +/* Machine-dependent macros for using symbol sets for running lists of
> +   functions. x86-64 version.
> +   Copyright (C) 2021 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/>.  */
> +
> +#ifndef _SET_HOOKS_ARCH_H
> +#define _SET_HOOKS_ARCH_H
> +
> +#define SET_RELHOOK(NAME, HOOK) \
> +       asm(".section " #NAME",\"aR\"\n" \
> +           ".quad "#HOOK" - .\n" \
> +           ".section .text");
> +
> +#endif /* set_hooks_arch.h */

SET_RELHOOK is used only for hurd which doesn't support x86-64.
Why not remove sysdeps/x86_64/set-hooks-arch.h and move
sysdeps/i386/set-hooks-arch.h to sysdeps/mach/hurd/i386?
  
Samuel Thibault Feb. 1, 2022, 12:49 a.m. UTC | #2
Hello,

H.J. Lu, le lun. 31 janv. 2022 16:14:59 -0800, a ecrit:
> > +#define SET_RELHOOK(NAME, HOOK) \
> > +       asm(".section " #NAME",\"aR\"\n" \
> > +           ".quad "#HOOK" - .\n" \
> > +           ".section .text");
> > +
> > +#endif /* set_hooks_arch.h */
> 
> SET_RELHOOK is used only for hurd which doesn't support x86-64.

We plan to support x86-64 relatively soon.

Samuel
  
H.J. Lu Feb. 1, 2022, 12:53 a.m. UTC | #3
On Mon, Jan 31, 2022 at 4:49 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Hello,
>
> H.J. Lu, le lun. 31 janv. 2022 16:14:59 -0800, a ecrit:
> > > +#define SET_RELHOOK(NAME, HOOK) \
> > > +       asm(".section " #NAME",\"aR\"\n" \
> > > +           ".quad "#HOOK" - .\n" \
> > > +           ".section .text");
> > > +
> > > +#endif /* set_hooks_arch.h */
> >
> > SET_RELHOOK is used only for hurd which doesn't support x86-64.
>
> We plan to support x86-64 relatively soon.
>

Then please put it under the x86-64 directory for hurd.   It doesn't
belong to sysdeps/x86_64.
  
Samuel Thibault Feb. 1, 2022, 12:58 a.m. UTC | #4
H.J. Lu via Libc-alpha, le lun. 31 janv. 2022 16:53:19 -0800, a ecrit:
> On Mon, Jan 31, 2022 at 4:49 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > H.J. Lu, le lun. 31 janv. 2022 16:14:59 -0800, a ecrit:
> > > > +#define SET_RELHOOK(NAME, HOOK) \
> > > > +       asm(".section " #NAME",\"aR\"\n" \
> > > > +           ".quad "#HOOK" - .\n" \
> > > > +           ".section .text");
> > > > +
> > > > +#endif /* set_hooks_arch.h */
> > >
> > > SET_RELHOOK is used only for hurd which doesn't support x86-64.
> >
> > We plan to support x86-64 relatively soon.
> >
> 
> Then please put it under the x86-64 directory for hurd.   It doesn't
> belong to sysdeps/x86_64.

Why not?  The problem is the same for all x86-64 ports, it's not
specific to the Hurd port.  It happens that DEFINE_HOOK is not widely
used in the glibc, and notably not for initialization steps that are
before the csu relocation.  But that's still something that could happen
someday.  Why hiding this solution behind a hurd/ directory?

Samuel
  
H.J. Lu Feb. 1, 2022, 2:19 p.m. UTC | #5
On Mon, Jan 31, 2022 at 4:58 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> H.J. Lu via Libc-alpha, le lun. 31 janv. 2022 16:53:19 -0800, a ecrit:
> > On Mon, Jan 31, 2022 at 4:49 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > > H.J. Lu, le lun. 31 janv. 2022 16:14:59 -0800, a ecrit:
> > > > > +#define SET_RELHOOK(NAME, HOOK) \
> > > > > +       asm(".section " #NAME",\"aR\"\n" \
> > > > > +           ".quad "#HOOK" - .\n" \
> > > > > +           ".section .text");
> > > > > +
> > > > > +#endif /* set_hooks_arch.h */
> > > >
> > > > SET_RELHOOK is used only for hurd which doesn't support x86-64.
> > >
> > > We plan to support x86-64 relatively soon.
> > >
> >
> > Then please put it under the x86-64 directory for hurd.   It doesn't
> > belong to sysdeps/x86_64.
>
> Why not?  The problem is the same for all x86-64 ports, it's not
> specific to the Hurd port.  It happens that DEFINE_HOOK is not widely
> used in the glibc, and notably not for initialization steps that are
> before the csu relocation.  But that's still something that could happen
> someday.  Why hiding this solution behind a hurd/ directory?

1. We don't need separate files for i386 and x86-64.

#define SET_RELHOOK(NAME, HOOK) \
asm(".section " #NAME",\"aR\"\n" \
    ".dc.a "#HOOK" - .\n" \
    ".section .text");

works for both.

2. If Linux needs it, it can be put in sysdeps/generic/set-hooks-arch.h

Please create sysdeps/mach/hurd/x86/set-hooks-arch.h with
SET_RELHOOK.
  

Patch

diff --git a/hurd/dtable.c b/hurd/dtable.c
index bbd3bfc892..4f081f09fd 100644
--- a/hurd/dtable.c
+++ b/hurd/dtable.c
@@ -36,7 +36,7 @@  DEFINE_HOOK (_hurd_fd_subinit, (void));
 
 /* Initialize the file descriptor table at startup.  */
 
-static void
+static void attribute_used_retain
 init_dtable (void)
 {
   int i;
@@ -91,12 +91,10 @@  init_dtable (void)
 
   /* Run things that want to run after the file descriptor table
      is initialized.  */
-  RUN_HOOK (_hurd_fd_subinit, ());
-
-  (void) &init_dtable;		/* Avoid "defined but not used" warning.  */
+  RUN_RELHOOK (_hurd_fd_subinit, ());
 }
 
-text_set_element (_hurd_subinit, init_dtable);
+SET_RELHOOK (_hurd_subinit, init_dtable);
 
 /* XXX when the linker supports it, the following functions should all be
    elsewhere and just have text_set_elements here.  */
diff --git a/hurd/hurdid.c b/hurd/hurdid.c
index 70c46c0740..b220532c35 100644
--- a/hurd/hurdid.c
+++ b/hurd/hurdid.c
@@ -17,6 +17,7 @@ 
 
 #include <hurd.h>
 #include <hurd/id.h>
+#include "set-hooks.h"
 
 struct hurd_id_data _hurd_id;
 
@@ -74,7 +75,7 @@  _hurd_check_ids (void)
   return 0;
 }
 
-static void
+static void attribute_used_retain
 init_id (void)
 {
   __mutex_init (&_hurd_id.lock);
@@ -84,7 +85,5 @@  init_id (void)
   _hurd_id.gen.nuids = _hurd_id.aux.nuids = 0;
   _hurd_id.gen.gids = _hurd_id.aux.gids = NULL;
   _hurd_id.gen.ngids = _hurd_id.aux.ngids = 0;
-
-  (void) &init_id;		/* Avoid "defined but not used" warning.  */
 }
-text_set_element (_hurd_preinit_hook, init_id);
+SET_RELHOOK (_hurd_preinit_hook, init_id);
diff --git a/hurd/hurdinit.c b/hurd/hurdinit.c
index d4d219a083..9377d902d2 100644
--- a/hurd/hurdinit.c
+++ b/hurd/hurdinit.c
@@ -108,7 +108,7 @@  _hurd_init (int flags, char **argv,
   /* Call other things which want to do some initialization.  These are not
      on the __libc_subinit hook because things there like to be able to
      assume the availability of the POSIX.1 services we provide.  */
-  RUN_HOOK (_hurd_subinit, ());
+  RUN_RELHOOK (_hurd_subinit, ());
 }
 libc_hidden_def (_hurd_init)
 
@@ -190,7 +190,7 @@  _hurd_new_proc_init (char **argv,
   /* Call other things which want to do some initialization.  These are not
      on the _hurd_subinit hook because things there assume that things done
      here, like _hurd_pid, are already initialized.  */
-  RUN_HOOK (_hurd_proc_subinit, ());
+  RUN_RELHOOK (_hurd_proc_subinit, ());
 
   /* XXX This code should probably be removed entirely at some point.  This
      conditional should make it reasonably usable with old gdb's for a
@@ -242,7 +242,7 @@  _hurd_setproc (process_t procserver)
 
     /* Call these functions again so they can fetch the
        new information from the new proc server.  */
-    RUN_HOOK (_hurd_proc_subinit, ());
+    RUN_RELHOOK (_hurd_proc_subinit, ());
 
     if (_hurd_pgrp != oldpgrp)
       {
diff --git a/hurd/hurdmalloc.c b/hurd/hurdmalloc.c
index 7046bcef33..ccb8fc8b54 100644
--- a/hurd/hurdmalloc.c
+++ b/hurd/hurdmalloc.c
@@ -1,5 +1,6 @@ 
 #include <stdlib.h>
 #include <string.h>
+#include "set-hooks.h"
 
 #include "hurdmalloc.h"		/* XXX see that file */
 
@@ -148,7 +149,7 @@  static struct free_list malloc_free_list[NBUCKETS];
    It preserves the values of data variables like malloc_free_list, but
    does not save the vm_allocate'd space allocated by this malloc.  */
 
-static void
+static void attribute_used_retain
 malloc_init (void)
 {
   int i;
@@ -160,11 +161,6 @@  malloc_init (void)
       malloc_free_list[i].in_use = 0;
 #endif
     }
-
-  /* This not only suppresses a `defined but not used' warning,
-     but it is ABSOLUTELY NECESSARY to avoid the hyperclever
-     compiler from "optimizing out" the entire function!  */
-  (void) &malloc_init;
 }
 
 static void
@@ -445,4 +441,4 @@  _hurd_malloc_fork_child(void)
 }
 
 
-text_set_element (_hurd_preinit_hook, malloc_init);
+SET_RELHOOK (_hurd_preinit_hook, malloc_init);
diff --git a/hurd/hurdpid.c b/hurd/hurdpid.c
index 9014de58e9..613ca357c6 100644
--- a/hurd/hurdpid.c
+++ b/hurd/hurdpid.c
@@ -17,11 +17,12 @@ 
 
 #include <hurd.h>
 #include <lowlevellock.h>
+#include "set-hooks.h"
 
 pid_t _hurd_pid, _hurd_ppid, _hurd_pgrp;
 int _hurd_orphaned;
 
-static void
+static void attribute_used_retain
 init_pids (void)
 {
   __USEPORT (PROC,
@@ -29,11 +30,9 @@  init_pids (void)
 	       __proc_getpids (port, &_hurd_pid, &_hurd_ppid, &_hurd_orphaned);
 	       __proc_getpgrp (port, _hurd_pid, &_hurd_pgrp);
 	     }));
-
-  (void) &init_pids;		/* Avoid "defined but not used" warning.  */
 }
 
-text_set_element (_hurd_proc_subinit, init_pids);
+SET_RELHOOK (_hurd_proc_subinit, init_pids);
 
 #include <hurd/msg_server.h>
 #include "set-hooks.h"
diff --git a/hurd/hurdrlimit.c b/hurd/hurdrlimit.c
index 17535c2851..4d16bd4f3f 100644
--- a/hurd/hurdrlimit.c
+++ b/hurd/hurdrlimit.c
@@ -19,6 +19,7 @@ 
 #include <hurd.h>
 #include <lock-intern.h>
 #include <hurd/resource.h>
+#include "set-hooks.h"
 
 /* This must be given an initializer, or the a.out linking rules will
    not include the entire file when this symbol is referenced. */
@@ -29,7 +30,7 @@  struct rlimit _hurd_rlimits[RLIM_NLIMITS] = { { 0, }, };
    mutex_init is still required below just in case of unexec.  */
 struct mutex _hurd_rlimit_lock = { SPIN_LOCK_INITIALIZER, };
 
-static void
+static void attribute_used_retain
 init_rlimit (void)
 {
   int i;
@@ -52,7 +53,5 @@  init_rlimit (void)
 	  }
 #undef	I
     }
-
-  (void) &init_rlimit;
 }
-text_set_element (_hurd_preinit_hook, init_rlimit);
+SET_RELHOOK (_hurd_preinit_hook, init_rlimit);
diff --git a/hurd/hurdsock.c b/hurd/hurdsock.c
index 04e86b4324..116326b2f2 100644
--- a/hurd/hurdsock.c
+++ b/hurd/hurdsock.c
@@ -25,6 +25,7 @@ 
 #include <_itoa.h>
 #include <lock-intern.h>	/* For `struct mutex'.  */
 #include "hurdmalloc.h"		/* XXX */
+#include "set-hooks.h"
 
 static struct mutex lock;
 
@@ -109,7 +110,7 @@  retry:
   return server;
 }
 
-static void
+static void attribute_used_retain
 init (void)
 {
   int i;
@@ -118,7 +119,5 @@  init (void)
 
   for (i = 0; i < max_domain; ++i)
     servers[i] = MACH_PORT_NULL;
-
-  (void) &init;			/* Avoid "defined but not used" warning.  */
 }
-text_set_element (_hurd_preinit_hook, init);
+SET_RELHOOK (_hurd_preinit_hook, init);
diff --git a/include/set-hooks.h b/include/set-hooks.h
index a60b5ae19f..1a2f6ad56b 100644
--- a/include/set-hooks.h
+++ b/include/set-hooks.h
@@ -24,6 +24,8 @@ 
 #include <sys/cdefs.h>
 #include <libc-symbols.h>
 
+#include "set-hooks-arch.h"
+
 #ifdef symbol_set_define
 /* Define a hook variable called NAME.  Functions put on this hook take
    arguments described by PROTO.  Use `text_set_element (NAME, FUNCTION)'
@@ -55,6 +57,25 @@  do {									      \
 DEFINE_HOOK (name, proto); \
 extern void runner proto; void runner proto { RUN_HOOK (name, args); }
 
+# ifdef SET_RELHOOK
+/* This is similar to RUN_RELHOOK, but the hooks were registered with
+ * SET_RELHOOK so that a relative offset was computed by the linker
+ * rather than an absolute address by the dynamic linker. */
+#  define RUN_RELHOOK(NAME, ARGS)				      \
+do {								      \
+  void *const *ptr;						      \
+  for (ptr = (void *const *) symbol_set_first_element (NAME);	      \
+       ! symbol_set_end_p (NAME, ptr); ++ptr) {			      \
+    __##NAME##_hook_function_t *f =				      \
+	(void*) ((uintptr_t) ptr + (ptrdiff_t) *ptr);		      \
+    (*f) ARGS;							      \
+  } \
+} while (0)
+# else
+#  define SET_RELHOOK(NAME, HOOK) text_set_element (NAME, HOOK)
+#  define RUN_RELHOOK(NAME, ARGS) RUN_HOOK(NAME, ARGS)
+# endif
+
 #else
 
 /* The system does not provide necessary support for this.  */
@@ -66,6 +87,10 @@  extern void runner proto; void runner proto { RUN_HOOK (name, args); }
 
 # define DEFINE_HOOK_RUNNER(name, runner, proto, args)
 
+# define SET_RELHOOK(NAME, HOOK)
+
+# define RUN_RELHOOK(NAME, ARGS)
+
 #endif
 
 #endif /* set-hooks.h */
diff --git a/sysdeps/generic/set-hooks-arch.h b/sysdeps/generic/set-hooks-arch.h
new file mode 100644
index 0000000000..4b47fa2f2e
--- /dev/null
+++ b/sysdeps/generic/set-hooks-arch.h
@@ -0,0 +1,31 @@ 
+/* Machine-dependent macros for using symbol sets for running lists of
+   functions. Generic/stub version.
+   Copyright (C) 2021 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/>.  */
+
+#ifndef _SET_HOOKS_ARCH_H
+#define _SET_HOOKS_ARCH_H
+
+/* Define SET_RELHOOK to a variant of text_set_element that records a relative
+   offset rather than an absolute address. See sysdeps/i386/set-hooks-arch.h
+   for an example.
+
+#define SET_RELHOOK(NAME, HOOK) ...
+
+ */
+
+#endif /* set_hooks_arch.h */
diff --git a/sysdeps/i386/set-hooks-arch.h b/sysdeps/i386/set-hooks-arch.h
new file mode 100644
index 0000000000..97513bf897
--- /dev/null
+++ b/sysdeps/i386/set-hooks-arch.h
@@ -0,0 +1,28 @@ 
+/* Machine-dependent macros for using symbol sets for running lists of
+   functions. i386 version.
+   Copyright (C) 2021 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/>.  */
+
+#ifndef _SET_HOOKS_ARCH_H
+#define _SET_HOOKS_ARCH_H
+
+#define SET_RELHOOK(NAME, HOOK) \
+	asm(".section " #NAME",\"aR\"\n" \
+	    ".long "#HOOK" - .\n" \
+	    ".section .text");
+
+#endif /* set_hooks_arch.h */
diff --git a/sysdeps/mach/hurd/brk.c b/sysdeps/mach/hurd/brk.c
index a4294b9eae..088c99b2c8 100644
--- a/sysdeps/mach/hurd/brk.c
+++ b/sysdeps/mach/hurd/brk.c
@@ -21,6 +21,8 @@ 
 #include <lock-intern.h>	/* For `struct mutex'.  */
 #include <vm_param.h>
 
+#include "set-hooks.h"
+
 
 /* Initial maximum size of the data segment (this is arbitrary).  */
 #define	DATA_SIZE	(128 * 1024 * 1024)
@@ -130,7 +132,7 @@  _hurd_set_brk (vm_address_t addr)
   return 0;
 }
 
-static void
+static void attribute_used_retain
 init_brk (void)
 {
   vm_address_t pagend;
@@ -160,7 +162,5 @@  init_brk (void)
 	/* Couldn't allocate the memory.  The break will be very short.  */
 	_hurd_data_end = pagend;
     }
-
-  (void) &init_brk;		/* Avoid ``defined but not used'' warning.  */
 }
-text_set_element (_hurd_preinit_hook, init_brk);
+SET_RELHOOK (_hurd_preinit_hook, init_brk);
diff --git a/sysdeps/mach/hurd/check_fds.c b/sysdeps/mach/hurd/check_fds.c
index 155e9dd3e9..61e6055d35 100644
--- a/sysdeps/mach/hurd/check_fds.c
+++ b/sysdeps/mach/hurd/check_fds.c
@@ -79,7 +79,7 @@  check_standard_fds (void)
   check_one_fd (STDERR_FILENO, O_RDWR);
 }
 
-static void
+static void attribute_used_retain
 init_standard_fds (void)
 {
   /* Now that we have FDs, make sure that, if this is a SUID program,
@@ -87,10 +87,8 @@  init_standard_fds (void)
      ourselves.  If that's not possible we stop the program.  */
   if (__builtin_expect (__libc_enable_secure, 0))
     check_standard_fds ();
-
-  (void) &init_standard_fds;	/* Avoid "defined but not used" warning.  */
 }
-text_set_element (_hurd_fd_subinit, init_standard_fds);
+SET_RELHOOK (_hurd_fd_subinit, init_standard_fds);
 
 
 #ifndef SHARED
diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c
index 5e85aa2bc5..c6ae370daf 100644
--- a/sysdeps/mach/hurd/i386/init-first.c
+++ b/sysdeps/mach/hurd/i386/init-first.c
@@ -242,7 +242,7 @@  first_init (void)
   /* Initialize data structures so we can do RPCs.  */
   __mach_init ();
 
-  RUN_HOOK (_hurd_preinit_hook, ());
+  RUN_RELHOOK (_hurd_preinit_hook, ());
 }
 
 #ifdef SHARED
diff --git a/sysdeps/x86_64/set-hooks-arch.h b/sysdeps/x86_64/set-hooks-arch.h
new file mode 100644
index 0000000000..227d4f8e08
--- /dev/null
+++ b/sysdeps/x86_64/set-hooks-arch.h
@@ -0,0 +1,28 @@ 
+/* Machine-dependent macros for using symbol sets for running lists of
+   functions. x86-64 version.
+   Copyright (C) 2021 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/>.  */
+
+#ifndef _SET_HOOKS_ARCH_H
+#define _SET_HOOKS_ARCH_H
+
+#define SET_RELHOOK(NAME, HOOK) \
+	asm(".section " #NAME",\"aR\"\n" \
+	    ".quad "#HOOK" - .\n" \
+	    ".section .text");
+
+#endif /* set_hooks_arch.h */