[PATCH v2 0/4] More x86_64-gnu glibc work

[PATCH v2 0/4] More x86_64-gnu glibc work

[Sergey Bugaev]

Changes compared to v1:
* Drop patches that have been pushed
* Address the review comments (hopefully...)
* Some more effort towards splitting changes into commits properly, and not
  like last time
* Rewrite/simplify the heck of init-first.c

I *think* I understood enough about how init-first.c worked to change it with
some confidence, rather than just trying to replicate what the i386 version was
doing, but for x86_64. I was able to get the much simplified i386 version to
work -- there no longer are any return address rewriting tricks, and only a
single weird jump out of a call stack in the !SHARED config. init & init1 are
unified into one function / code path, as are doinit and doinit1. call_init is
gone. There are no longer any varargs, nor assumptions that function args are
passed on the stack -- everything is dealt with through a pointer. Overall, I
think this version is much cleaner.

I've tried to write some comments about how _hurd_stack_setup () works. There's
really not much code to it, but it is tricky, so better have it documented.

In my testing, both SHARED and !SHARED versions still work (on i386). Early
boot !SHARED seems to work too -- specifically, I replaced the stock
/hurd/ext2fs.static with the one I cross-built with this version of glibc in
the boot script, and it seemed to find its arguments just fine (though the
system did not actually fully boot up, but that was likely for some unrealted
reason...).

So please don't trust my (brief and unreliable) testing, and do your own.

As for x86_64, yes, I have verified that on x86_64-pc-linux-gnu argc/argv/env
arrive on-stack just like the code in _hurd_stack_setup () expects them to.

I still don't understand why __LIBC_NO_TLS () is supposed to return 0 when we
have the early TLS configured, but this seems to be what the i386 version does.

Note: this (TLS) still depends on the gnumach patch adding
i386_fsgs_base_state. If the API is alright, can we push it without an
implementation, so that userland code (glibc) can be built against it?

Note: the other still unpushed patches I'm carrying locally are
"htl: Make pthread_mutex_t pointer-aligned" and the mach-machine part of
"mach: Look for mach_i386.defs on x86_64 too". You may need to apply them to
actually build any of this on x86_64.

Sergey Bugaev (4):
  hurd: Simplify init-first.c further
  hurd: Generalize init-first.c to support x86_64
  hurd: Implement TLS for x86_64
  htl: Add pthreadtypes-arch.h for x86_64

 sysdeps/mach/hurd/dl-sysdep.c                |   5 +-
 sysdeps/mach/hurd/i386/dl-machine.h          |   7 -
 sysdeps/mach/hurd/{i386 => x86}/init-first.c | 220 ++++++++-----------
 sysdeps/mach/hurd/x86_64/tls.h               | 215 ++++++++++++++++++
 sysdeps/x86_64/htl/bits/pthreadtypes-arch.h  |  36 +++
 5 files changed, 345 insertions(+), 138 deletions(-)
 delete mode 100644 sysdeps/mach/hurd/i386/dl-machine.h
 rename sysdeps/mach/hurd/{i386 => x86}/init-first.c (67%)
 create mode 100644 sysdeps/mach/hurd/x86_64/tls.h
 create mode 100644 sysdeps/x86_64/htl/bits/pthreadtypes-arch.h

-- 
2.39.2

[PATCH v2 1/4] hurd: Simplify init-first.c further

[Sergey Bugaev]

This drops all of the return address rewriting kludges. The only
remaining hack is the jump out of a call stack while adjusting the
stack pointer.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/dl-sysdep.c       |   5 +-
 sysdeps/mach/hurd/i386/dl-machine.h |   7 -
 sysdeps/mach/hurd/i386/init-first.c | 200 ++++++++++------------------
 3 files changed, 75 insertions(+), 137 deletions(-)
 delete mode 100644 sysdeps/mach/hurd/i386/dl-machine.h

diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c
index 9e591708..18b35ddb 100644
--- a/sysdeps/mach/hurd/dl-sysdep.c
+++ b/sysdeps/mach/hurd/dl-sysdep.c
@@ -207,6 +207,9 @@ _dl_sysdep_start (void **start_argptr,
 	    }
 	}
 
+      extern void _dl_init_first (void *data);
+      _dl_init_first (argdata);
+
       {
 	extern void _dl_start_user (void);
 	/* Unwind the stack to ARGDATA and simulate a return from _dl_start
@@ -793,7 +796,7 @@ _dl_show_auxv (void)
 
 
 void weak_function
-_dl_init_first (int argc, ...)
+_dl_init_first (void *p)
 {
   /* This no-op definition only gets used if libc is not linked in.  */
 }
diff --git a/sysdeps/mach/hurd/i386/dl-machine.h b/sysdeps/mach/hurd/i386/dl-machine.h
deleted file mode 100644
index 40f2ff29..00000000
--- a/sysdeps/mach/hurd/i386/dl-machine.h
+++ /dev/null
@@ -1,7 +0,0 @@
-/* Dynamic linker magic for Hurd/i386.
-   This file just gets us a call to _dl_first_init inserted
-   into the asm in sysdeps/i386/dl-machine.h that contains
-   the initializer code.  */
-
-#define RTLD_START_SPECIAL_INIT "call _dl_init_first@PLT; movl (%esp), %edx"
-#include_next "dl-machine.h"
diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c
index a558da16..34e8dcc0 100644
--- a/sysdeps/mach/hurd/i386/init-first.c
+++ b/sysdeps/mach/hurd/i386/init-first.c
@@ -24,8 +24,6 @@
 #include <sysdep.h>
 #include <set-hooks.h>
 #include "hurdstartup.h"
-#include "hurdmalloc.h"		/* XXX */
-#include "../locale/localeinfo.h"
 
 #include <ldsodefs.h>
 #include <fpu_control.h>
@@ -87,68 +85,13 @@ posixland_init (int argc, char **argv, char **envp)
   __init_misc (argc, argv, envp);
 }
 
-
 static void
-init1 (int argc, char *arg0, ...)
-{
-  char **argv = &arg0;
-  char **envp = &argv[argc + 1];
-  struct hurd_startup_data *d;
-
-  while (*envp)
-    ++envp;
-  d = (void *) ++envp;
-
-  if ((void *) d == argv[0])
-    {
-      /* No Hurd data block to process.  */
-#ifndef SHARED
-      __libc_enable_secure = 0;
-#endif
-      return;
-    }
-
-#ifndef SHARED
-  __libc_enable_secure = d->flags & EXEC_SECURE;
-#endif
-
-  _hurd_init_dtable = d->dtable;
-  _hurd_init_dtablesize = d->dtablesize;
-
-  {
-    /* Check if the stack we are now on is different from
-       the one described by _hurd_stack_{base,size}.  */
-
-    char dummy;
-    const vm_address_t newsp = (vm_address_t) &dummy;
-
-    if (d->stack_size != 0 && (newsp < d->stack_base
-			       || newsp - d->stack_base > d->stack_size))
-      /* The new stack pointer does not intersect with the
-	 stack the exec server set up for us, so free that stack.  */
-      __vm_deallocate (__mach_task_self (), d->stack_base, d->stack_size);
-  }
-
-  if (d->portarray || d->intarray)
-    /* Initialize library data structures, start signal processing, etc.  */
-    _hurd_init (d->flags, argv,
-		d->portarray, d->portarraysize,
-		d->intarray, d->intarraysize);
-}
-
-
-static inline void
 init (int *data)
 {
-  /* data is the address of the argc parameter to _dl_init_first or
-     doinit1 in _hurd_stack_setup, so the array subscripts are
-     undefined.  */
-  DIAG_PUSH_NEEDS_COMMENT;
-  DIAG_IGNORE_NEEDS_COMMENT (10, "-Warray-bounds");
-
   int argc = *data;
   char **argv = (void *) (data + 1);
   char **envp = &argv[argc + 1];
+  struct hurd_startup_data *d;
 
   /* Since the cthreads initialization code uses malloc, and the
      malloc initialization code needs to get at the environment, make
@@ -157,18 +100,18 @@ init (int *data)
      stored.  */
   __environ = envp;
 
-#ifndef SHARED
-  struct hurd_startup_data *d;
-
   while (*envp)
     ++envp;
   d = (void *) ++envp;
 
+#ifndef SHARED
+
   /* If we are the bootstrap task started by the kernel,
      then after the environment pointers there is no Hurd
      data block; the argument strings start there.  */
   if ((void *) d == argv[0] || d->phdr == 0)
     {
+      __libc_enable_secure = 0;
       /* With a new enough linker (binutils-2.23 or better),
          the magic __ehdr_start symbol will be available and
          __libc_start_main will have done this that way already.  */
@@ -186,51 +129,39 @@ init (int *data)
     }
   else
     {
+      __libc_enable_secure = d->flags & EXEC_SECURE;
       _dl_phdr = (ElfW(Phdr) *) d->phdr;
       _dl_phnum = d->phdrsz / sizeof (ElfW(Phdr));
       assert (d->phdrsz % sizeof (ElfW(Phdr)) == 0);
     }
 #endif
 
-  /* Call `init1' (above) with the user code as the return address, and the
-     argument data immediately above that on the stack.  */
-
-  void *usercode, **ret_address;
-
-  void call_init1 (void);
-
-  /* The argument data is just above the stack frame we will unwind by
-     returning.  Mutate our own return address to run the code below.  */
-  /* The following expression would typically be written as
-     ``__builtin_return_address (0)''.  But, for example, GCC 4.4.6 doesn't
-     recognize that this read operation may alias the following write
-     operation, and thus is free to reorder the two, clobbering the
-     original return address.  */
-  ret_address = (void **) __builtin_frame_address (0) + 1;
-  usercode = *ret_address;
-  /* GCC 4.4.6 also wants us to force loading USERCODE already here.  */
-  asm volatile ("# %0" : : "X" (usercode));
-  *ret_address = &call_init1;
-  /* Force USERCODE into %eax and &init1 into %ecx, which are not
-     restored by function return.  */
-  asm volatile ("# a %0 c %1" : : "a" (usercode), "c" (&init1));
-
-  DIAG_POP_NEEDS_COMMENT;	/* -Warray-bounds.  */
-}
+  if ((void *) d == argv[0])
+    return;
 
-/* These bits of inline assembler used to be located inside `init'.
-   However they were optimized away by gcc 2.95.  */
+  _hurd_init_dtable = d->dtable;
+  _hurd_init_dtablesize = d->dtablesize;
 
-/* The return address of `init' above, was redirected to here, so at
-   this point our stack is unwound and callers' registers restored.
-   Only %ecx and %eax are call-clobbered and thus still have the
-   values we set just above.  We have stashed in %eax the user code
-   return address.  Push it on the top of the stack so it acts as
-   init1's return address, and then jump there.  */
-asm ("call_init1:\n"
-     "	push %eax\n"
-     "	jmp *%ecx\n");
+  {
+    /* Check if the stack we are now on is different from
+       the one described by _hurd_stack_{base,size}.  */
 
+    char dummy;
+    const vm_address_t newsp = (vm_address_t) &dummy;
+
+    if (d->stack_size != 0 && (newsp < d->stack_base
+			       || newsp - d->stack_base > d->stack_size))
+      /* The new stack pointer does not intersect with the
+	 stack the exec server set up for us, so free that stack.  */
+      __vm_deallocate (__mach_task_self (), d->stack_base, d->stack_size);
+  }
+
+  if (d->portarray || d->intarray)
+    /* Initialize library data structures, start signal processing, etc.  */
+    _hurd_init (d->flags, argv,
+		d->portarray, d->portarraysize,
+		d->intarray, d->intarraysize);
+}
 
 /* Do the first essential initializations that must precede all else.  */
 static inline void
@@ -242,7 +173,7 @@ first_init (void)
 #ifndef SHARED
   /* In the static case, we need to set up TLS early so that the stack
      protection guard can be read at gs:0x14 by the gcc-generated snippets.  */
-  _hurd_tls_init(&__init1_tcbhead);
+  _hurd_tls_init (&__init1_tcbhead);
   asm ("movw %%gs,%w0" : "=m" (__init1_desc));
 #endif
 
@@ -252,21 +183,15 @@ first_init (void)
 #ifdef SHARED
 /* This function is called specially by the dynamic linker to do early
    initialization of the shared C library before normal initializers
-   expecting a Posixoid environment can run.  It gets called with the
-   stack set up just as the user will see it, so it can switch stacks.  */
+   expecting a Posixoid environment can run.  */
 
 void
-_dl_init_first (int argc, ...)
+_dl_init_first (void **p)
 {
   first_init ();
-
-  /* If we use ``__builtin_frame_address (0) + 2'' here, GCC gets confused.  */
-  init (&argc);
+  init ((int *) p);
 }
-#endif
-
 
-#ifdef SHARED
 /* The regular posixland initialization is what goes into libc's
    normal initializer.  */
 /* NOTE!  The linker notices the magical name `_init' and sets the DT_INIT
@@ -280,9 +205,10 @@ __libc_init_first (int argc, char **argv, char **envp)
 {
   /* Everything was done in the shared library initializer, _init.  */
 }
-#else
-strong_alias (posixland_init, __libc_init_first);
 
+#else /* SHARED */
+
+strong_alias (posixland_init, __libc_init_first);
 
 /* XXX This is all a crock and I am not happy with it.
    This poorly-named function is called by static-start.S,
@@ -291,32 +217,48 @@ void
 inhibit_stack_protector
 _hurd_stack_setup (void)
 {
-  intptr_t caller = (intptr_t) __builtin_return_address (0);
+  /* This is the very first C code that runs in a statically linked
+     executable -- calling this function is the first thing that _start in
+     static-start.S does.  Once this function returns, the unusual way that it
+     does (see below), _start jumps to _start1, the regular start-up code.
+
+     _start1 expects the arguments, environment, and a Hurd data block to be
+     located at the top of the stack.  The data may already be located there,
+     or we may need to receive it from the exec server.  */
+  void *caller = __builtin_extract_return_addr (__builtin_return_address (0));
+  /* If the arguments and environment are already located on the stack, this is
+     where they are, just above our call frame.  Note that this may not be a
+     valid pointer in case we're supposed to receive the arguments from the exec
+     server, so we can not dereference it yet.  */
+  void **p = (void **) __builtin_frame_address (0) + 2;
+
+  /* Init the essential things.  */
+  first_init ();
 
   void doinit (intptr_t *data)
     {
-      /* This function gets called with the argument data at TOS.  */
-      void doinit1 (int argc, ...)
-	{
-	  /* If we use ``__builtin_frame_address (0) + 2'' here, GCC gets
-	     confused.  */
-	  init ((int *) &argc);
-	}
-
-      /* Push the user return address after the argument data, and then
-	 jump to `doinit1' (above), so it is as if __libc_init_first's
-	 caller had called `doinit1' with the argument data already on the
-	 stack.  */
-      *--data = caller;
+      init ((int *) data);
       asm volatile ("movl %0, %%esp\n" /* Switch to new outermost stack.  */
-		    "movl $0, %%ebp\n" /* Clear outermost frame pointer.  */
-		    "jmp *%1" : : "r" (data), "r" (&doinit1));
-      /* NOTREACHED */
+		    "xorl %%ebp, %%ebp\n" /* Clear outermost frame pointer.  */
+		    "jmp *%1" : : "r" (data), "r" (caller));
+      __builtin_unreachable ();
     }
 
-  first_init ();
-
-  _hurd_startup ((void **) __builtin_frame_address (0) + 2, &doinit);
+  /* _hurd_startup () will attempt to receive the data block from the exec
+     server; or if that is not possible, will take the data from the pointer
+     we pass it here.  The important point here is that the data
+     _hurd_startup () collects may be allocated in its stack frame (with
+     alloca), which is why _hurd_startup () does not return the normal way.
+     Instead, it invokes a callback (which is not expected to return normally
+     either).
+
+     Our callback not only passes the data pointer to init (), but also jumps
+     out of the call stack back to our caller (i.e. to _start1), while setting
+     the stack pointer to the data (which is somewhere on teh current stack
+     anyway).  This way, _start1 find the data on the top of the stack, just as
+     it expects to.  */
+  _hurd_startup (p, &doinit);
+  __builtin_unreachable ();
 }
 #endif
 
-- 
2.39.2

[PATCH v2 2/4] hurd: Generalize init-first.c to support x86_64

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/{i386 => x86}/init-first.c | 6 ++++++
 1 file changed, 6 insertions(+)
 rename sysdeps/mach/hurd/{i386 => x86}/init-first.c (97%)

diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/x86/init-first.c
similarity index 97%
rename from sysdeps/mach/hurd/i386/init-first.c
rename to sysdeps/mach/hurd/x86/init-first.c
index 34e8dcc0..a187af82 100644
--- a/sysdeps/mach/hurd/i386/init-first.c
+++ b/sysdeps/mach/hurd/x86/init-first.c
@@ -238,9 +238,15 @@ _hurd_stack_setup (void)
   void doinit (intptr_t *data)
     {
       init ((int *) data);
+# ifdef __x86_64__
+      asm volatile ("movq %0, %%rsp\n" /* Switch to new outermost stack.  */
+                    "xorq %%rbp, %%rbp\n" /* Clear outermost frame pointer.  */
+                    "jmp *%1" : : "r" (data), "r" (caller));
+# else
       asm volatile ("movl %0, %%esp\n" /* Switch to new outermost stack.  */
 		    "xorl %%ebp, %%ebp\n" /* Clear outermost frame pointer.  */
 		    "jmp *%1" : : "r" (data), "r" (caller));
+# endif
       __builtin_unreachable ();
     }
 
-- 
2.39.2

[PATCH v2 3/4] hurd: Implement TLS for x86_64

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/x86/init-first.c |  14 +-
 sysdeps/mach/hurd/x86_64/tls.h     | 215 +++++++++++++++++++++++++++++
 2 files changed, 228 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/mach/hurd/x86_64/tls.h

diff --git a/sysdeps/mach/hurd/x86/init-first.c b/sysdeps/mach/hurd/x86/init-first.c
index a187af82..04480ed3 100644
--- a/sysdeps/mach/hurd/x86/init-first.c
+++ b/sysdeps/mach/hurd/x86/init-first.c
@@ -41,8 +41,14 @@ extern char **__libc_argv attribute_hidden;
 extern char **_dl_argv;
 
 #ifndef SHARED
-unsigned short __init1_desc;
 static tcbhead_t __init1_tcbhead;
+# ifndef __x86_64__
+unsigned short __init1_desc;
+# endif
+#endif
+
+#ifdef __x86_64__
+unsigned char __libc_tls_initialized;
 #endif
 
 /* Things that want to be run before _hurd_init or much anything else.
@@ -174,7 +180,13 @@ first_init (void)
   /* In the static case, we need to set up TLS early so that the stack
      protection guard can be read at gs:0x14 by the gcc-generated snippets.  */
   _hurd_tls_init (&__init1_tcbhead);
+
+  /* Make sure __LIBC_NO_TLS () keeps evaluating to 1.  */
+# ifdef __x86_64__
+  __libc_tls_initialized = 0;
+# else
   asm ("movw %%gs,%w0" : "=m" (__init1_desc));
+# endif
 #endif
 
   RUN_RELHOOK (_hurd_preinit_hook, ());
diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
new file mode 100644
index 00000000..cf74e1f4
--- /dev/null
+++ b/sysdeps/mach/hurd/x86_64/tls.h
@@ -0,0 +1,215 @@
+/* Definitions for thread-local data handling.  Hurd/x86_64 version.
+   Copyright (C) 2003-2023 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 _X86_64_TLS_H
+#define _X86_64_TLS_H
+
+
+/* Some things really need not be machine-dependent.  */
+#include <sysdeps/mach/hurd/tls.h>
+
+
+#ifndef __ASSEMBLER__
+# include <dl-dtv.h>
+# include <mach/machine/thread_status.h>
+# include <errno.h>
+# include <assert.h>
+
+/* Type of the TCB.  */
+typedef struct
+{
+  void *tcb;			/* Points to this structure.  */
+  dtv_t *dtv;			/* Vector of pointers to TLS data.  */
+  thread_t self;		/* This thread's control port.  */
+  int __glibc_padding1;
+  int multiple_threads;
+  int gscope_flag;
+  uintptr_t sysinfo;
+  uintptr_t stack_guard;
+  uintptr_t pointer_guard;
+  long __glibc_padding2[2];
+  int private_futex;
+  int __glibc_padding3;
+  /* Reservation of some values for the TM ABI.  */
+  void *__private_tm[4];
+  /* GCC split stack support.  */
+  void *__private_ss;
+  /* The lowest address of shadow stack.  */
+  unsigned long long int ssp_base;
+
+  /* Keep these fields last, so offsets of fields above can continue being
+     compatible with the x86_64 NPTL version.  */
+  mach_port_t reply_port;      /* This thread's reply port.  */
+  struct hurd_sigstate *_hurd_sigstate;
+
+  /* Used by the exception handling implementation in the dynamic loader.  */
+  struct rtld_catch *rtld_catch;
+} tcbhead_t;
+
+/* GCC generates %fs:0x28 to access the stack guard.  */
+_Static_assert (offsetof (tcbhead_t, stack_guard) == 0x28,
+                "stack guard offset");
+/* libgcc uses %fs:0x70 to access the split stack pointer.  */
+_Static_assert (offsetof (tcbhead_t, __private_ss) == 0x70,
+                "split stack pointer offset");
+
+extern unsigned char __libc_tls_initialized;
+
+# define __LIBC_NO_TLS() __builtin_expect (!__libc_tls_initialized, 0)
+
+/* The TCB can have any size and the memory following the address the
+   thread pointer points to is unspecified.  Allocate the TCB there.  */
+# define TLS_TCB_AT_TP	1
+# define TLS_DTV_AT_TP	0
+
+/* Alignment requirement for TCB.
+
+   Some processors such as Intel Atom pay a big penalty on every
+   access using a segment override if that segment's base is not
+   aligned to the size of a cache line.  (See Intel 64 and IA-32
+   Architectures Optimization Reference Manual, section 13.3.3.3,
+   "Segment Base".)  On such machines, a cache line is 64 bytes.  */
+# define TCB_ALIGNMENT	64
+
+
+# define TLS_INIT_TP(descr) _hurd_tls_init ((tcbhead_t *) (descr))
+
+# define THREAD_SELF							\
+  (*(tcbhead_t * __seg_fs *) offsetof (tcbhead_t, tcb))
+/* Read member of the thread descriptor directly.  */
+# define THREAD_GETMEM(descr, member)					\
+  (*(__typeof (descr->member) __seg_fs *) offsetof (tcbhead_t, member))
+/* Write member of the thread descriptor directly.  */
+# define THREAD_SETMEM(descr, member, value)				\
+  (*(__typeof (descr->member) __seg_fs *) offsetof (tcbhead_t, member) = value)
+
+
+/* Return the TCB address of a thread given its state.
+   Note: this is expensive.  */
+static inline tcbhead_t * __attribute__ ((unused))
+THREAD_TCB (thread_t thread,
+            const void *all_state __attribute__ ((unused)))
+{
+  error_t err;
+  /* Fetch the target thread's state.  */
+  struct i386_fsgs_base_state state;
+  mach_msg_type_number_t state_count = i386_FSGS_BASE_STATE_COUNT;
+  err = __thread_get_state (thread, i386_FSGS_BASE_STATE,
+                            (thread_state_t) &state,
+                            &state_count);
+  assert_perror (err);
+  assert (state_count == i386_FSGS_BASE_STATE_COUNT);
+  return (tcbhead_t *) state.fs_base;
+}
+
+/* Install new dtv for current thread.  */
+# define INSTALL_NEW_DTV(dtvp) THREAD_SETMEM (THREAD_SELF, dtv, dtvp)
+
+/* Return the address of the dtv for the current thread.  */
+# define THREAD_DTV() THREAD_GETMEM (THREAD_SELF, dtv)
+
+
+/* Set the stack guard field in TCB head.  */
+# define THREAD_SET_STACK_GUARD(value)					\
+  THREAD_SETMEM (THREAD_SELF, stack_guard, value)
+# define THREAD_COPY_STACK_GUARD(descr)					\
+  ((descr)->stack_guard							\
+   = THREAD_GETMEM (THREAD_SELF, stack_guard))
+
+/* Set the pointer guard field in the TCB head.  */
+# define THREAD_SET_POINTER_GUARD(value)				\
+  THREAD_SETMEM (THREAD_SELF, pointer_guard, value)
+# define THREAD_COPY_POINTER_GUARD(descr)				\
+  ((descr)->pointer_guard						\
+   = THREAD_GETMEM (THREAD_SELF, pointer_guard))
+
+/* Set up TLS in the new thread of a fork child, copying from the original.  */
+static inline kern_return_t __attribute__ ((unused))
+_hurd_tls_fork (thread_t child, thread_t orig,
+                void *machine_state __attribute__ ((unused)))
+{
+  error_t err;
+  struct i386_fsgs_base_state state;
+  mach_msg_type_number_t state_count = i386_FSGS_BASE_STATE_COUNT;
+  err = __thread_get_state (orig, i386_FSGS_BASE_STATE,
+                            (thread_state_t) &state,
+                            &state_count);
+  if (err)
+    return err;
+  assert (state_count == i386_FSGS_BASE_STATE_COUNT);
+
+  return __thread_set_state (child, i386_FSGS_BASE_STATE,
+                             (thread_state_t) &state,
+                             state_count);
+}
+
+static inline kern_return_t __attribute__ ((unused))
+_hurd_tls_new (thread_t child, tcbhead_t *tcb)
+{
+  struct i386_fsgs_base_state state;
+
+  tcb->tcb = tcb;
+  tcb->self = child;
+
+  /* Install the TCB address into FS base.  */
+  state.fs_base = (uintptr_t) tcb;
+  state.gs_base = 0;
+  return __thread_set_state (child, i386_FSGS_BASE_STATE,
+                             (thread_state_t) &state,
+                             i386_FSGS_BASE_STATE_COUNT);
+}
+
+static inline bool __attribute__ ((unused))
+_hurd_tls_init (tcbhead_t *tcb)
+{
+  error_t err;
+  thread_t self = __mach_thread_self ();
+
+  /* We always at least start the sigthread anyway.  */
+  tcb->multiple_threads = 1;
+
+  err = _hurd_tls_new (self, tcb);
+  __mach_port_deallocate (__mach_task_self (), self);
+  __libc_tls_initialized = 1;
+  return err == 0;
+}
+
+
+/* Global scope switch support.  */
+# define THREAD_GSCOPE_FLAG_UNUSED 0
+# define THREAD_GSCOPE_FLAG_USED   1
+# define THREAD_GSCOPE_FLAG_WAIT   2
+
+# define THREAD_GSCOPE_SET_FLAG() \
+  THREAD_SETMEM (THREAD_SELF, gscope_flag, THREAD_GSCOPE_FLAG_USED)
+
+# define THREAD_GSCOPE_RESET_FLAG() \
+  ({									\
+    int __flag;								\
+    asm volatile ("xchgl %0, %%fs:%P1"					\
+                  : "=r" (__flag)					\
+                  : "i" (offsetof (tcbhead_t, gscope_flag)),		\
+                    "0" (THREAD_GSCOPE_FLAG_UNUSED));			\
+    if (__flag == THREAD_GSCOPE_FLAG_WAIT)				\
+      lll_wake (THREAD_SELF->gscope_flag, LLL_PRIVATE);			\
+  })
+
+
+
+#endif	/* __ASSEMBLER__ */
+#endif	/* x86_64/tls.h */
-- 
2.39.2

[PATCH v2 4/4] htl: Add pthreadtypes-arch.h for x86_64

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/x86_64/htl/bits/pthreadtypes-arch.h | 36 +++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 sysdeps/x86_64/htl/bits/pthreadtypes-arch.h

diff --git a/sysdeps/x86_64/htl/bits/pthreadtypes-arch.h b/sysdeps/x86_64/htl/bits/pthreadtypes-arch.h
new file mode 100644
index 00000000..1d402cc6
--- /dev/null
+++ b/sysdeps/x86_64/htl/bits/pthreadtypes-arch.h
@@ -0,0 +1,36 @@
+/* Machine-specific pthread type layouts.  Hurd x86_64 version.
+   Copyright (C) 2002-2023 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 _BITS_PTHREADTYPES_ARCH_H
+#define _BITS_PTHREADTYPES_ARCH_H	1
+
+#define __SIZEOF_PTHREAD_MUTEX_T 32
+#define __SIZEOF_PTHREAD_ATTR_T 48
+#define __SIZEOF_PTHREAD_RWLOCK_T 48
+#define __SIZEOF_PTHREAD_BARRIER_T 40
+#define __SIZEOF_PTHREAD_MUTEXATTR_T 16
+#define __SIZEOF_PTHREAD_COND_T 40
+#define __SIZEOF_PTHREAD_CONDATTR_T 8
+#define __SIZEOF_PTHREAD_RWLOCKATTR_T 4
+#define __SIZEOF_PTHREAD_BARRIERATTR_T 4
+#define __SIZEOF_PTHREAD_ONCE_T 8
+
+#define __LOCK_ALIGNMENT
+#define __ONCE_ALIGNMENT
+
+#endif /* bits/pthreadtypes.h */
-- 
2.39.2

Re: [PATCH v2 1/4] hurd: Simplify init-first.c further

[Samuel Thibault]

Hello,

Thanks for this!

Did you try to run make check?

Sergey Bugaev via Libc-alpha, le mer. 22 févr. 2023 00:19:29 +0300, a ecrit:
> This drops all of the return address rewriting kludges. The only
> remaining hack is the jump out of a call stack while adjusting the
> stack pointer.

Is this hack really still needed? Since we don't switch stack any more,
we could as well just return normally?

> --- a/sysdeps/mach/hurd/dl-sysdep.c
> +++ b/sysdeps/mach/hurd/dl-sysdep.c
> @@ -207,6 +207,9 @@ _dl_sysdep_start (void **start_argptr,
>  	    }
>  	}
>  
> +      extern void _dl_init_first (void *data);

Please put extern function declaration into a header, dl-sysdep.h for
instance.

> diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c
> index a558da16..34e8dcc0 100644
> --- a/sysdeps/mach/hurd/i386/init-first.c
> +++ b/sysdeps/mach/hurd/i386/init-first.c
> +  {
> +    /* Check if the stack we are now on is different from
> +       the one described by _hurd_stack_{base,size}.  */
>  
> +    char dummy;
> +    const vm_address_t newsp = (vm_address_t) &dummy;
> +
> +    if (d->stack_size != 0 && (newsp < d->stack_base
> +			       || newsp - d->stack_base > d->stack_size))
> +      /* The new stack pointer does not intersect with the
> +	 stack the exec server set up for us, so free that stack.  */
> +      __vm_deallocate (__mach_task_self (), d->stack_base, d->stack_size);
> +  }

Again, I don't think this is needed any more since we don't switch stack
any more?

Samuel

Re: [PATCH v2 0/4] More x86_64-gnu glibc work

[Samuel Thibault]

Sergey Bugaev via Libc-alpha, le mer. 22 févr. 2023 00:19:28 +0300, a ecrit:
> Overall, I think this version is much cleaner.

Definitely :D

> I've tried to write some comments about how _hurd_stack_setup () works. There's
> really not much code to it, but it is tricky, so better have it documented.

Thanks!

> As for x86_64, yes, I have verified that on x86_64-pc-linux-gnu argc/argv/env
> arrive on-stack just like the code in _hurd_stack_setup () expects them to.

Ok, good :)

> I still don't understand why __LIBC_NO_TLS () is supposed to return 0 when we
> have the early TLS configured, but this seems to be what the i386 version does.

I don't remember the details, but most probably that's because there is
more to TLS than the mere early TLS. The gcc accesses are constants from
gs/fs so early TLS is enough. But e.g. _hurd_critical_section_lock would
try to access the sigstate, which will be in the eventual tcbhead, and
not in the early tcbhead.

> Note: this (TLS) still depends on the gnumach patch adding
> i386_fsgs_base_state. If the API is alright, can we push it without an
> implementation, so that userland code (glibc) can be built against it?

We can probably do that, yes.

Samuel

Re: [PATCH v2 1/4] hurd: Simplify init-first.c further

[Sergey Bugaev]

On Thu, Feb 23, 2023 at 2:26 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Did you try to run make check?

No.

I'm cross-compiling, so I don't think make check would be able to run
any tests. And from what I remember from building glibc on the Hurd
itself back in 2021, make check takes a very long time and either
never really completes or brings the system into some weird state.

If you're able to run make check on your end, please do so (but wait
until I send v3 with the changes you've requested below).

Are there specific tests for the various combinations of startup
variants? (shared vs static, args already on the stack vs not, exec
server present vs not) If so, maybe I could run just them and not the
full thing; that would be easier on my system.

> Sergey Bugaev via Libc-alpha, le mer. 22 févr. 2023 00:19:29 +0300, a ecrit:
> > This drops all of the return address rewriting kludges. The only
> > remaining hack is the jump out of a call stack while adjusting the
> > stack pointer.
>
> Is this hack really still needed? Since we don't switch stack any more,
> we could as well just return normally?

No, we can't just return normally, we have to adjust the stack pointer
and jump out. We don't _switch_ the stack as in use an entirely
different area of memory for the stack, but we do adjust the stack
pointer within the existing stack area.

Are the comments I have added not clear enough about this? If so,
maybe they should be expanded further. The reason we have to return in
this weird way is that:
1. The normal startup code (_start1) expects to find args/env on the
top of the stack. Like, literally, argc at sp[0], argv[0] at sp[1] and
so on.
2. _hurd_startup, which is the code that receives args/env from the
exec server, allocates them in its own stack frame, with alloca. So it
cannot return, and neither can _hurd_stack_setup.
Instead of returning, _hurd_startup invokes a callback (doinit) that
(eventually) just sets the stack pointer to point to this data (so it
now is on the top of the stack, just as _start1 expects) and jumps to
_hurd_stack_setup's caller (i.e. to _start). This would very much
break things if _start was itself using the stack for anything, since
the stack pointer is now different, but the only thing _start does is
it calls _hurd_startup and then jumps to _start1, so that works.

And the same is done in the SHARED startup path by _dl_sysdep_start,
except that it uses the RETURN_TO macro instead of direct inline
assembly. That macro only really differs in that it does *not* zero
out ebp/rbp, so I wonder how come that doesn't break backtraces.

> > --- a/sysdeps/mach/hurd/dl-sysdep.c
> > +++ b/sysdeps/mach/hurd/dl-sysdep.c
> > @@ -207,6 +207,9 @@ _dl_sysdep_start (void **start_argptr,
> >           }
> >       }
> >
> > +      extern void _dl_init_first (void *data);
>
> Please put extern function declaration into a header, dl-sysdep.h for
> instance.

Makes sense, thanks.

>
> > diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c
> > index a558da16..34e8dcc0 100644
> > --- a/sysdeps/mach/hurd/i386/init-first.c
> > +++ b/sysdeps/mach/hurd/i386/init-first.c
> > +  {
> > +    /* Check if the stack we are now on is different from
> > +       the one described by _hurd_stack_{base,size}.  */
> >
> > +    char dummy;
> > +    const vm_address_t newsp = (vm_address_t) &dummy;
> > +
> > +    if (d->stack_size != 0 && (newsp < d->stack_base
> > +                            || newsp - d->stack_base > d->stack_size))
> > +      /* The new stack pointer does not intersect with the
> > +      stack the exec server set up for us, so free that stack.  */
> > +      __vm_deallocate (__mach_task_self (), d->stack_base, d->stack_size);
> > +  }
>
> Again, I don't think this is needed any more since we don't switch stack
> any more?

Good point, most likely not. I'll drop it and see if anything breaks.

Sergey

[PATCH v3 1/2] hurd: Simplify init-first.c further

[Sergey Bugaev]

This drops all of the return address rewriting kludges. The only
remaining hack is the jump out of a call stack while adjusting the
stack pointer.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/dl-sysdep.c       |   4 +-
 sysdeps/mach/hurd/dl-sysdep.h       |   4 +
 sysdeps/mach/hurd/i386/dl-machine.h |   7 -
 sysdeps/mach/hurd/i386/init-first.c | 193 +++++++++-------------------
 4 files changed, 68 insertions(+), 140 deletions(-)
 delete mode 100644 sysdeps/mach/hurd/i386/dl-machine.h

diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c
index 9e591708..a2115f6e 100644
--- a/sysdeps/mach/hurd/dl-sysdep.c
+++ b/sysdeps/mach/hurd/dl-sysdep.c
@@ -207,6 +207,8 @@ _dl_sysdep_start (void **start_argptr,
 	    }
 	}
 
+      _dl_init_first (argdata);
+
       {
 	extern void _dl_start_user (void);
 	/* Unwind the stack to ARGDATA and simulate a return from _dl_start
@@ -793,7 +795,7 @@ _dl_show_auxv (void)
 
 
 void weak_function
-_dl_init_first (int argc, ...)
+_dl_init_first (void *p)
 {
   /* This no-op definition only gets used if libc is not linked in.  */
 }
diff --git a/sysdeps/mach/hurd/dl-sysdep.h b/sysdeps/mach/hurd/dl-sysdep.h
index 3baf56b7..fa35a71c 100644
--- a/sysdeps/mach/hurd/dl-sysdep.h
+++ b/sysdeps/mach/hurd/dl-sysdep.h
@@ -22,3 +22,7 @@
    (open, mmap, etc).  */
 
 #define RTLD_PRIVATE_ERRNO 0
+
+#ifndef __ASSEMBLER__
+void _dl_init_first (void *data);
+#endif
diff --git a/sysdeps/mach/hurd/i386/dl-machine.h b/sysdeps/mach/hurd/i386/dl-machine.h
deleted file mode 100644
index 40f2ff29..00000000
--- a/sysdeps/mach/hurd/i386/dl-machine.h
+++ /dev/null
@@ -1,7 +0,0 @@
-/* Dynamic linker magic for Hurd/i386.
-   This file just gets us a call to _dl_first_init inserted
-   into the asm in sysdeps/i386/dl-machine.h that contains
-   the initializer code.  */
-
-#define RTLD_START_SPECIAL_INIT "call _dl_init_first@PLT; movl (%esp), %edx"
-#include_next "dl-machine.h"
diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i386/init-first.c
index a558da16..05424563 100644
--- a/sysdeps/mach/hurd/i386/init-first.c
+++ b/sysdeps/mach/hurd/i386/init-first.c
@@ -22,10 +22,9 @@
 #include <unistd.h>
 #include <string.h>
 #include <sysdep.h>
+#include <dl-sysdep.h>
 #include <set-hooks.h>
 #include "hurdstartup.h"
-#include "hurdmalloc.h"		/* XXX */
-#include "../locale/localeinfo.h"
 
 #include <ldsodefs.h>
 #include <fpu_control.h>
@@ -87,68 +86,13 @@ posixland_init (int argc, char **argv, char **envp)
   __init_misc (argc, argv, envp);
 }
 
-
 static void
-init1 (int argc, char *arg0, ...)
+init (void **data)
 {
-  char **argv = &arg0;
-  char **envp = &argv[argc + 1];
-  struct hurd_startup_data *d;
-
-  while (*envp)
-    ++envp;
-  d = (void *) ++envp;
-
-  if ((void *) d == argv[0])
-    {
-      /* No Hurd data block to process.  */
-#ifndef SHARED
-      __libc_enable_secure = 0;
-#endif
-      return;
-    }
-
-#ifndef SHARED
-  __libc_enable_secure = d->flags & EXEC_SECURE;
-#endif
-
-  _hurd_init_dtable = d->dtable;
-  _hurd_init_dtablesize = d->dtablesize;
-
-  {
-    /* Check if the stack we are now on is different from
-       the one described by _hurd_stack_{base,size}.  */
-
-    char dummy;
-    const vm_address_t newsp = (vm_address_t) &dummy;
-
-    if (d->stack_size != 0 && (newsp < d->stack_base
-			       || newsp - d->stack_base > d->stack_size))
-      /* The new stack pointer does not intersect with the
-	 stack the exec server set up for us, so free that stack.  */
-      __vm_deallocate (__mach_task_self (), d->stack_base, d->stack_size);
-  }
-
-  if (d->portarray || d->intarray)
-    /* Initialize library data structures, start signal processing, etc.  */
-    _hurd_init (d->flags, argv,
-		d->portarray, d->portarraysize,
-		d->intarray, d->intarraysize);
-}
-
-
-static inline void
-init (int *data)
-{
-  /* data is the address of the argc parameter to _dl_init_first or
-     doinit1 in _hurd_stack_setup, so the array subscripts are
-     undefined.  */
-  DIAG_PUSH_NEEDS_COMMENT;
-  DIAG_IGNORE_NEEDS_COMMENT (10, "-Warray-bounds");
-
-  int argc = *data;
+  int argc = (int) *data;
   char **argv = (void *) (data + 1);
   char **envp = &argv[argc + 1];
+  struct hurd_startup_data *d;
 
   /* Since the cthreads initialization code uses malloc, and the
      malloc initialization code needs to get at the environment, make
@@ -157,18 +101,18 @@ init (int *data)
      stored.  */
   __environ = envp;
 
-#ifndef SHARED
-  struct hurd_startup_data *d;
-
   while (*envp)
     ++envp;
   d = (void *) ++envp;
 
+#ifndef SHARED
+
   /* If we are the bootstrap task started by the kernel,
      then after the environment pointers there is no Hurd
      data block; the argument strings start there.  */
   if ((void *) d == argv[0] || d->phdr == 0)
     {
+      __libc_enable_secure = 0;
       /* With a new enough linker (binutils-2.23 or better),
          the magic __ehdr_start symbol will be available and
          __libc_start_main will have done this that way already.  */
@@ -186,51 +130,25 @@ init (int *data)
     }
   else
     {
+      __libc_enable_secure = d->flags & EXEC_SECURE;
       _dl_phdr = (ElfW(Phdr) *) d->phdr;
       _dl_phnum = d->phdrsz / sizeof (ElfW(Phdr));
       assert (d->phdrsz % sizeof (ElfW(Phdr)) == 0);
     }
 #endif
 
-  /* Call `init1' (above) with the user code as the return address, and the
-     argument data immediately above that on the stack.  */
-
-  void *usercode, **ret_address;
-
-  void call_init1 (void);
-
-  /* The argument data is just above the stack frame we will unwind by
-     returning.  Mutate our own return address to run the code below.  */
-  /* The following expression would typically be written as
-     ``__builtin_return_address (0)''.  But, for example, GCC 4.4.6 doesn't
-     recognize that this read operation may alias the following write
-     operation, and thus is free to reorder the two, clobbering the
-     original return address.  */
-  ret_address = (void **) __builtin_frame_address (0) + 1;
-  usercode = *ret_address;
-  /* GCC 4.4.6 also wants us to force loading USERCODE already here.  */
-  asm volatile ("# %0" : : "X" (usercode));
-  *ret_address = &call_init1;
-  /* Force USERCODE into %eax and &init1 into %ecx, which are not
-     restored by function return.  */
-  asm volatile ("# a %0 c %1" : : "a" (usercode), "c" (&init1));
-
-  DIAG_POP_NEEDS_COMMENT;	/* -Warray-bounds.  */
-}
-
-/* These bits of inline assembler used to be located inside `init'.
-   However they were optimized away by gcc 2.95.  */
+  if ((void *) d == argv[0])
+    return;
 
-/* The return address of `init' above, was redirected to here, so at
-   this point our stack is unwound and callers' registers restored.
-   Only %ecx and %eax are call-clobbered and thus still have the
-   values we set just above.  We have stashed in %eax the user code
-   return address.  Push it on the top of the stack so it acts as
-   init1's return address, and then jump there.  */
-asm ("call_init1:\n"
-     "	push %eax\n"
-     "	jmp *%ecx\n");
+  _hurd_init_dtable = d->dtable;
+  _hurd_init_dtablesize = d->dtablesize;
 
+  if (d->portarray || d->intarray)
+    /* Initialize library data structures, start signal processing, etc.  */
+    _hurd_init (d->flags, argv,
+		d->portarray, d->portarraysize,
+		d->intarray, d->intarraysize);
+}
 
 /* Do the first essential initializations that must precede all else.  */
 static inline void
@@ -242,7 +160,7 @@ first_init (void)
 #ifndef SHARED
   /* In the static case, we need to set up TLS early so that the stack
      protection guard can be read at gs:0x14 by the gcc-generated snippets.  */
-  _hurd_tls_init(&__init1_tcbhead);
+  _hurd_tls_init (&__init1_tcbhead);
   asm ("movw %%gs,%w0" : "=m" (__init1_desc));
 #endif
 
@@ -252,21 +170,15 @@ first_init (void)
 #ifdef SHARED
 /* This function is called specially by the dynamic linker to do early
    initialization of the shared C library before normal initializers
-   expecting a Posixoid environment can run.  It gets called with the
-   stack set up just as the user will see it, so it can switch stacks.  */
+   expecting a Posixoid environment can run.  */
 
 void
-_dl_init_first (int argc, ...)
+_dl_init_first (void *data)
 {
   first_init ();
-
-  /* If we use ``__builtin_frame_address (0) + 2'' here, GCC gets confused.  */
-  init (&argc);
+  init (data);
 }
-#endif
-
 
-#ifdef SHARED
 /* The regular posixland initialization is what goes into libc's
    normal initializer.  */
 /* NOTE!  The linker notices the magical name `_init' and sets the DT_INIT
@@ -280,9 +192,10 @@ __libc_init_first (int argc, char **argv, char **envp)
 {
   /* Everything was done in the shared library initializer, _init.  */
 }
-#else
-strong_alias (posixland_init, __libc_init_first);
 
+#else /* SHARED */
+
+strong_alias (posixland_init, __libc_init_first);
 
 /* XXX This is all a crock and I am not happy with it.
    This poorly-named function is called by static-start.S,
@@ -291,32 +204,48 @@ void
 inhibit_stack_protector
 _hurd_stack_setup (void)
 {
-  intptr_t caller = (intptr_t) __builtin_return_address (0);
+  /* This is the very first C code that runs in a statically linked
+     executable -- calling this function is the first thing that _start in
+     static-start.S does.  Once this function returns, the unusual way that it
+     does (see below), _start jumps to _start1, the regular start-up code.
+
+     _start1 expects the arguments, environment, and a Hurd data block to be
+     located at the top of the stack.  The data may already be located there,
+     or we may need to receive it from the exec server.  */
+  void *caller = __builtin_extract_return_addr (__builtin_return_address (0));
+  /* If the arguments and environment are already located on the stack, this is
+     where they are, just above our call frame.  Note that this may not be a
+     valid pointer in case we're supposed to receive the arguments from the exec
+     server, so we can not dereference it yet.  */
+  void **p = (void **) __builtin_frame_address (0) + 2;
+
+  /* Init the essential things.  */
+  first_init ();
 
   void doinit (intptr_t *data)
     {
-      /* This function gets called with the argument data at TOS.  */
-      void doinit1 (int argc, ...)
-	{
-	  /* If we use ``__builtin_frame_address (0) + 2'' here, GCC gets
-	     confused.  */
-	  init ((int *) &argc);
-	}
-
-      /* Push the user return address after the argument data, and then
-	 jump to `doinit1' (above), so it is as if __libc_init_first's
-	 caller had called `doinit1' with the argument data already on the
-	 stack.  */
-      *--data = caller;
+      init ((void **) data);
       asm volatile ("movl %0, %%esp\n" /* Switch to new outermost stack.  */
-		    "movl $0, %%ebp\n" /* Clear outermost frame pointer.  */
-		    "jmp *%1" : : "r" (data), "r" (&doinit1));
-      /* NOTREACHED */
+		    "xorl %%ebp, %%ebp\n" /* Clear outermost frame pointer.  */
+		    "jmp *%1" : : "r" (data), "r" (caller));
+      __builtin_unreachable ();
     }
 
-  first_init ();
-
-  _hurd_startup ((void **) __builtin_frame_address (0) + 2, &doinit);
+  /* _hurd_startup () will attempt to receive the data block from the exec
+     server; or if that is not possible, will take the data from the pointer
+     we pass it here.  The important point here is that the data
+     _hurd_startup () collects may be allocated in its stack frame (with
+     alloca), which is why _hurd_startup () does not return the normal way.
+     Instead, it invokes a callback (which is not expected to return normally
+     either).
+
+     Our callback not only passes the data pointer to init (), but also jumps
+     out of the call stack back to our caller (i.e. to _start1), while setting
+     the stack pointer to the data (which is somewhere on the current stack
+     anyway).  This way, _start1 find the data on the top of the stack, just as
+     it expects to.  */
+  _hurd_startup (p, &doinit);
+  __builtin_unreachable ();
 }
 #endif
 
-- 
2.39.2

[PATCH v3 2/2] hurd: Generalize init-first.c to support x86_64

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/{i386 => x86}/init-first.c | 6 ++++++
 1 file changed, 6 insertions(+)
 rename sysdeps/mach/hurd/{i386 => x86}/init-first.c (97%)

diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/x86/init-first.c
similarity index 97%
rename from sysdeps/mach/hurd/i386/init-first.c
rename to sysdeps/mach/hurd/x86/init-first.c
index 05424563..2fccaefd 100644
--- a/sysdeps/mach/hurd/i386/init-first.c
+++ b/sysdeps/mach/hurd/x86/init-first.c
@@ -225,9 +225,15 @@ _hurd_stack_setup (void)
   void doinit (intptr_t *data)
     {
       init ((void **) data);
+# ifdef __x86_64__
+      asm volatile ("movq %0, %%rsp\n" /* Switch to new outermost stack.  */
+                    "xorq %%rbp, %%rbp\n" /* Clear outermost frame pointer.  */
+                    "jmp *%1" : : "r" (data), "r" (caller));
+# else
       asm volatile ("movl %0, %%esp\n" /* Switch to new outermost stack.  */
 		    "xorl %%ebp, %%ebp\n" /* Clear outermost frame pointer.  */
 		    "jmp *%1" : : "r" (data), "r" (caller));
+# endif
       __builtin_unreachable ();
     }
 
-- 
2.39.2

Re: [PATCH v2 1/4] hurd: Simplify init-first.c further

[Samuel Thibault]

Sergey Bugaev, le jeu. 23 févr. 2023 16:54:05 +0300, a ecrit:
> And from what I remember from building glibc on the Hurd
> itself back in 2021, make check takes a very long time and either
> never really completes or brings the system into some weird state.

Some checks pose problem indeed, see the "unsupported" lines in debian's
debian/testsuite-xfail-debian.mk

> If you're able to run make check on your end, please do so (but wait
> until I send v3 with the changes you've requested below).

I'll let it run through the night.

> Are there specific tests for the various combinations of startup
> variants? (shared vs static, args already on the stack vs not, exec
> server present vs not)

Basically it's the elf/ directory that tests things. I don't know which
ones would exactly test what you'd want, there are various tests for
various situations.

> Instead of returning, _hurd_startup invokes a callback (doinit) that
> (eventually) just sets the stack pointer to point to this data (so it
> now is on the top of the stack, just as _start1 expects) and jumps to
> _hurd_stack_setup's caller (i.e. to _start).

Ah, ok, I had misunderstood and I hadn't really dived into the code. I
thought _hurd_startup was eventually calling main() itself, but actually
no it's sorta-returning to _start, but lower in the stack.

Your current comments are enough, I was just stuck with older
assumptions with the old stack switching code.

Thanks!
Samuel

Re: [PATCH v2 1/4] hurd: Simplify init-first.c further

[Samuel Thibault]

Samuel Thibault, le ven. 24 févr. 2023 02:08:09 +0100, a ecrit:
> Sergey Bugaev, le jeu. 23 févr. 2023 16:54:05 +0300, a ecrit:
> > And from what I remember from building glibc on the Hurd
> > itself back in 2021, make check takes a very long time and either
> > never really completes or brings the system into some weird state.
> 
> Some checks pose problem indeed, see the "unsupported" lines in debian's
> debian/testsuite-xfail-debian.mk

I have added the bugreports on bugzilla, and added unsupported lines in
sysdeps/hurd/mach/Makefile.

> > If you're able to run make check on your end, please do so (but wait
> > until I send v3 with the changes you've requested below).
> 
> I'll let it run through the night.

It went fine! I also rebuilt hurd to exercise ext2fs.static, and that
went fine too. I thus pushed your commit, many thanks!

Samuel

Re: [PATCH v2 3/4] hurd: Implement TLS for x86_64

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mer. 22 févr. 2023 00:19:31 +0300, a ecrit:
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/x86/init-first.c |  14 +-
>  sysdeps/mach/hurd/x86_64/tls.h     | 215 +++++++++++++++++++++++++++++
>  2 files changed, 228 insertions(+), 1 deletion(-)
>  create mode 100644 sysdeps/mach/hurd/x86_64/tls.h
> 
> diff --git a/sysdeps/mach/hurd/x86/init-first.c b/sysdeps/mach/hurd/x86/init-first.c
> index a187af82..04480ed3 100644
> --- a/sysdeps/mach/hurd/x86/init-first.c
> +++ b/sysdeps/mach/hurd/x86/init-first.c
> @@ -41,8 +41,14 @@ extern char **__libc_argv attribute_hidden;
>  extern char **_dl_argv;
>  
>  #ifndef SHARED
> -unsigned short __init1_desc;
>  static tcbhead_t __init1_tcbhead;
> +# ifndef __x86_64__
> +unsigned short __init1_desc;
> +# endif
> +#endif
> +
> +#ifdef __x86_64__
> +unsigned char __libc_tls_initialized;
>  #endif
>  
>  /* Things that want to be run before _hurd_init or much anything else.
> @@ -174,7 +180,13 @@ first_init (void)
>    /* In the static case, we need to set up TLS early so that the stack
>       protection guard can be read at gs:0x14 by the gcc-generated snippets.  */
>    _hurd_tls_init (&__init1_tcbhead);
> +
> +  /* Make sure __LIBC_NO_TLS () keeps evaluating to 1.  */
> +# ifdef __x86_64__
> +  __libc_tls_initialized = 0;
> +# else
>    asm ("movw %%gs,%w0" : "=m" (__init1_desc));
> +# endif
>  #endif
>  
>    RUN_RELHOOK (_hurd_preinit_hook, ());
> diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
> new file mode 100644
> index 00000000..cf74e1f4
> --- /dev/null
> +++ b/sysdeps/mach/hurd/x86_64/tls.h
> @@ -0,0 +1,215 @@
> +/* Definitions for thread-local data handling.  Hurd/x86_64 version.
> +   Copyright (C) 2003-2023 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 _X86_64_TLS_H
> +#define _X86_64_TLS_H
> +
> +
> +/* Some things really need not be machine-dependent.  */
> +#include <sysdeps/mach/hurd/tls.h>
> +
> +
> +#ifndef __ASSEMBLER__
> +# include <dl-dtv.h>
> +# include <mach/machine/thread_status.h>
> +# include <errno.h>
> +# include <assert.h>
> +
> +/* Type of the TCB.  */
> +typedef struct
> +{
> +  void *tcb;			/* Points to this structure.  */
> +  dtv_t *dtv;			/* Vector of pointers to TLS data.  */
> +  thread_t self;		/* This thread's control port.  */
> +  int __glibc_padding1;
> +  int multiple_threads;
> +  int gscope_flag;
> +  uintptr_t sysinfo;
> +  uintptr_t stack_guard;
> +  uintptr_t pointer_guard;
> +  long __glibc_padding2[2];
> +  int private_futex;
> +  int __glibc_padding3;
> +  /* Reservation of some values for the TM ABI.  */
> +  void *__private_tm[4];
> +  /* GCC split stack support.  */
> +  void *__private_ss;
> +  /* The lowest address of shadow stack.  */
> +  unsigned long long int ssp_base;
> +
> +  /* Keep these fields last, so offsets of fields above can continue being
> +     compatible with the x86_64 NPTL version.  */
> +  mach_port_t reply_port;      /* This thread's reply port.  */
> +  struct hurd_sigstate *_hurd_sigstate;
> +
> +  /* Used by the exception handling implementation in the dynamic loader.  */
> +  struct rtld_catch *rtld_catch;
> +} tcbhead_t;
> +
> +/* GCC generates %fs:0x28 to access the stack guard.  */
> +_Static_assert (offsetof (tcbhead_t, stack_guard) == 0x28,
> +                "stack guard offset");
> +/* libgcc uses %fs:0x70 to access the split stack pointer.  */
> +_Static_assert (offsetof (tcbhead_t, __private_ss) == 0x70,
> +                "split stack pointer offset");
> +
> +extern unsigned char __libc_tls_initialized;
> +
> +# define __LIBC_NO_TLS() __builtin_expect (!__libc_tls_initialized, 0)
> +
> +/* The TCB can have any size and the memory following the address the
> +   thread pointer points to is unspecified.  Allocate the TCB there.  */
> +# define TLS_TCB_AT_TP	1
> +# define TLS_DTV_AT_TP	0
> +
> +/* Alignment requirement for TCB.
> +
> +   Some processors such as Intel Atom pay a big penalty on every
> +   access using a segment override if that segment's base is not
> +   aligned to the size of a cache line.  (See Intel 64 and IA-32
> +   Architectures Optimization Reference Manual, section 13.3.3.3,
> +   "Segment Base".)  On such machines, a cache line is 64 bytes.  */
> +# define TCB_ALIGNMENT	64
> +
> +
> +# define TLS_INIT_TP(descr) _hurd_tls_init ((tcbhead_t *) (descr))
> +
> +# define THREAD_SELF							\
> +  (*(tcbhead_t * __seg_fs *) offsetof (tcbhead_t, tcb))
> +/* Read member of the thread descriptor directly.  */
> +# define THREAD_GETMEM(descr, member)					\
> +  (*(__typeof (descr->member) __seg_fs *) offsetof (tcbhead_t, member))
> +/* Write member of the thread descriptor directly.  */
> +# define THREAD_SETMEM(descr, member, value)				\
> +  (*(__typeof (descr->member) __seg_fs *) offsetof (tcbhead_t, member) = value)
> +
> +
> +/* Return the TCB address of a thread given its state.
> +   Note: this is expensive.  */
> +static inline tcbhead_t * __attribute__ ((unused))
> +THREAD_TCB (thread_t thread,
> +            const void *all_state __attribute__ ((unused)))
> +{
> +  error_t err;
> +  /* Fetch the target thread's state.  */
> +  struct i386_fsgs_base_state state;
> +  mach_msg_type_number_t state_count = i386_FSGS_BASE_STATE_COUNT;
> +  err = __thread_get_state (thread, i386_FSGS_BASE_STATE,
> +                            (thread_state_t) &state,
> +                            &state_count);
> +  assert_perror (err);
> +  assert (state_count == i386_FSGS_BASE_STATE_COUNT);
> +  return (tcbhead_t *) state.fs_base;
> +}
> +
> +/* Install new dtv for current thread.  */
> +# define INSTALL_NEW_DTV(dtvp) THREAD_SETMEM (THREAD_SELF, dtv, dtvp)
> +
> +/* Return the address of the dtv for the current thread.  */
> +# define THREAD_DTV() THREAD_GETMEM (THREAD_SELF, dtv)
> +
> +
> +/* Set the stack guard field in TCB head.  */
> +# define THREAD_SET_STACK_GUARD(value)					\
> +  THREAD_SETMEM (THREAD_SELF, stack_guard, value)
> +# define THREAD_COPY_STACK_GUARD(descr)					\
> +  ((descr)->stack_guard							\
> +   = THREAD_GETMEM (THREAD_SELF, stack_guard))
> +
> +/* Set the pointer guard field in the TCB head.  */
> +# define THREAD_SET_POINTER_GUARD(value)				\
> +  THREAD_SETMEM (THREAD_SELF, pointer_guard, value)
> +# define THREAD_COPY_POINTER_GUARD(descr)				\
> +  ((descr)->pointer_guard						\
> +   = THREAD_GETMEM (THREAD_SELF, pointer_guard))
> +
> +/* Set up TLS in the new thread of a fork child, copying from the original.  */
> +static inline kern_return_t __attribute__ ((unused))
> +_hurd_tls_fork (thread_t child, thread_t orig,
> +                void *machine_state __attribute__ ((unused)))
> +{
> +  error_t err;
> +  struct i386_fsgs_base_state state;
> +  mach_msg_type_number_t state_count = i386_FSGS_BASE_STATE_COUNT;
> +  err = __thread_get_state (orig, i386_FSGS_BASE_STATE,
> +                            (thread_state_t) &state,
> +                            &state_count);
> +  if (err)
> +    return err;
> +  assert (state_count == i386_FSGS_BASE_STATE_COUNT);
> +
> +  return __thread_set_state (child, i386_FSGS_BASE_STATE,
> +                             (thread_state_t) &state,
> +                             state_count);
> +}
> +
> +static inline kern_return_t __attribute__ ((unused))
> +_hurd_tls_new (thread_t child, tcbhead_t *tcb)
> +{
> +  struct i386_fsgs_base_state state;
> +
> +  tcb->tcb = tcb;
> +  tcb->self = child;
> +
> +  /* Install the TCB address into FS base.  */
> +  state.fs_base = (uintptr_t) tcb;
> +  state.gs_base = 0;
> +  return __thread_set_state (child, i386_FSGS_BASE_STATE,
> +                             (thread_state_t) &state,
> +                             i386_FSGS_BASE_STATE_COUNT);
> +}
> +
> +static inline bool __attribute__ ((unused))
> +_hurd_tls_init (tcbhead_t *tcb)
> +{
> +  error_t err;
> +  thread_t self = __mach_thread_self ();
> +
> +  /* We always at least start the sigthread anyway.  */
> +  tcb->multiple_threads = 1;
> +
> +  err = _hurd_tls_new (self, tcb);
> +  __mach_port_deallocate (__mach_task_self (), self);
> +  __libc_tls_initialized = 1;
> +  return err == 0;
> +}
> +
> +
> +/* Global scope switch support.  */
> +# define THREAD_GSCOPE_FLAG_UNUSED 0
> +# define THREAD_GSCOPE_FLAG_USED   1
> +# define THREAD_GSCOPE_FLAG_WAIT   2
> +
> +# define THREAD_GSCOPE_SET_FLAG() \
> +  THREAD_SETMEM (THREAD_SELF, gscope_flag, THREAD_GSCOPE_FLAG_USED)
> +
> +# define THREAD_GSCOPE_RESET_FLAG() \
> +  ({									\
> +    int __flag;								\
> +    asm volatile ("xchgl %0, %%fs:%P1"					\
> +                  : "=r" (__flag)					\
> +                  : "i" (offsetof (tcbhead_t, gscope_flag)),		\
> +                    "0" (THREAD_GSCOPE_FLAG_UNUSED));			\
> +    if (__flag == THREAD_GSCOPE_FLAG_WAIT)				\
> +      lll_wake (THREAD_SELF->gscope_flag, LLL_PRIVATE);			\
> +  })
> +
> +
> +
> +#endif	/* __ASSEMBLER__ */
> +#endif	/* x86_64/tls.h */
> -- 
> 2.39.2
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

Re: [PATCH v2 4/4] htl: Add pthreadtypes-arch.h for x86_64

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mer. 22 févr. 2023 00:19:32 +0300, a ecrit:
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/x86_64/htl/bits/pthreadtypes-arch.h | 36 +++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 sysdeps/x86_64/htl/bits/pthreadtypes-arch.h
> 
> diff --git a/sysdeps/x86_64/htl/bits/pthreadtypes-arch.h b/sysdeps/x86_64/htl/bits/pthreadtypes-arch.h
> new file mode 100644
> index 00000000..1d402cc6
> --- /dev/null
> +++ b/sysdeps/x86_64/htl/bits/pthreadtypes-arch.h
> @@ -0,0 +1,36 @@
> +/* Machine-specific pthread type layouts.  Hurd x86_64 version.
> +   Copyright (C) 2002-2023 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 _BITS_PTHREADTYPES_ARCH_H
> +#define _BITS_PTHREADTYPES_ARCH_H	1
> +
> +#define __SIZEOF_PTHREAD_MUTEX_T 32
> +#define __SIZEOF_PTHREAD_ATTR_T 48
> +#define __SIZEOF_PTHREAD_RWLOCK_T 48
> +#define __SIZEOF_PTHREAD_BARRIER_T 40
> +#define __SIZEOF_PTHREAD_MUTEXATTR_T 16
> +#define __SIZEOF_PTHREAD_COND_T 40
> +#define __SIZEOF_PTHREAD_CONDATTR_T 8
> +#define __SIZEOF_PTHREAD_RWLOCKATTR_T 4
> +#define __SIZEOF_PTHREAD_BARRIERATTR_T 4
> +#define __SIZEOF_PTHREAD_ONCE_T 8
> +
> +#define __LOCK_ALIGNMENT
> +#define __ONCE_ALIGNMENT
> +
> +#endif /* bits/pthreadtypes.h */
> -- 
> 2.39.2
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.