[PATCH 00/10] Stack setup & misc fixes for x86_64-gnu

[PATCH 00/10] Stack setup & misc fixes for x86_64-gnu

[Sergey Bugaev]

And these are the rest of my x86_64-gnu glibc fixes! (well, for now)

With these (+ the BRK_START change), I'm able build a system that boots
to /bin/sh and runs some commands (coreutils) successfully.

Enjoy :)

Sergey

[PATCH 01/10] Remove sysdeps/generic/thread_state.h

[Sergey Bugaev]

This is a Mach-specific thread state definitions, and it's already
handled by sysdeps/mach/thread_state.h.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/generic/thread_state.h | 51 ----------------------------------
 1 file changed, 51 deletions(-)
 delete mode 100644 sysdeps/generic/thread_state.h

diff --git a/sysdeps/generic/thread_state.h b/sysdeps/generic/thread_state.h
deleted file mode 100644
index 18926efb..00000000
--- a/sysdeps/generic/thread_state.h
+++ /dev/null
@@ -1,51 +0,0 @@
-/* Mach thread state definitions for machine-independent code.  Stub version.
-   Copyright (C) 1994-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/>.  */
-
-/* Everything else is called `thread_state', but CMU's header file is
-   called `thread_status'.  Oh boy.  */
-#include <mach/thread_state.h>
-
-/* Replace <machine> with "i386" or "mips" or whatever.  */
-
-/* This lets the kernel define architecture-specific registers for a new
-   thread.  */
-#define MACHINE_NEW_THREAD_STATE_FLAVOR	<machine>_NEW_THREAD_STATE
-/* This makes the kernel load all architectures-specific registers for the
-   thread.  */
-#define MACHINE_THREAD_STATE_FLAVOR	<machine>_THREAD_STATE
-#define MACHINE_THREAD_STATE_COUNT	<machine>_THREAD_STATE_COUNT
-
-#define machine_thread_state <machine>_thread_state
-
-/* Define these to the member names in `struct <machine>_thread_state'
-   for the PC and stack pointer.  */
-#define PC ?
-#define SP ?
-
-/* This structure should contain all of the different flavors of thread
-   state structures which are meaningful for this machine.  Every machine's
-   definition of this structure should have a member `int set' which is a
-   bit mask (1 << FLAVOR) of the flavors of thread state in the structure
-   which are filled in; and a member `struct machine_thread_state basic'.
-   On some machines those are the only members (e.g. i386); on others,
-   there are several relevant flavors of thread state (e.g. mips).  */
-struct machine_thread_all_state
-  {
-    int set;			/* Mask of bits (1 << FLAVOR).  */
-    struct <machine>_thread_state basic;
-  };
-- 
2.40.1

Re: [PATCH 01/10] Remove sysdeps/generic/thread_state.h

[Samuel Thibault]

Sergey Bugaev, le mer. 17 mai 2023 22:14:27 +0300, a ecrit:
> This is a Mach-specific thread state definitions, and it's already
> handled by sysdeps/mach/thread_state.h.

?
No, this is the template file for the per-arch files, that you'd fall
down on when building for another arch, so you know what you're supposed
to fill in.

> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/generic/thread_state.h | 51 ----------------------------------
>  1 file changed, 51 deletions(-)
>  delete mode 100644 sysdeps/generic/thread_state.h
> 
> diff --git a/sysdeps/generic/thread_state.h b/sysdeps/generic/thread_state.h
> deleted file mode 100644
> index 18926efb..00000000
> --- a/sysdeps/generic/thread_state.h
> +++ /dev/null
> @@ -1,51 +0,0 @@
> -/* Mach thread state definitions for machine-independent code.  Stub version.
> -   Copyright (C) 1994-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/>.  */
> -
> -/* Everything else is called `thread_state', but CMU's header file is
> -   called `thread_status'.  Oh boy.  */
> -#include <mach/thread_state.h>
> -
> -/* Replace <machine> with "i386" or "mips" or whatever.  */
> -
> -/* This lets the kernel define architecture-specific registers for a new
> -   thread.  */
> -#define MACHINE_NEW_THREAD_STATE_FLAVOR	<machine>_NEW_THREAD_STATE
> -/* This makes the kernel load all architectures-specific registers for the
> -   thread.  */
> -#define MACHINE_THREAD_STATE_FLAVOR	<machine>_THREAD_STATE
> -#define MACHINE_THREAD_STATE_COUNT	<machine>_THREAD_STATE_COUNT
> -
> -#define machine_thread_state <machine>_thread_state
> -
> -/* Define these to the member names in `struct <machine>_thread_state'
> -   for the PC and stack pointer.  */
> -#define PC ?
> -#define SP ?
> -
> -/* This structure should contain all of the different flavors of thread
> -   state structures which are meaningful for this machine.  Every machine's
> -   definition of this structure should have a member `int set' which is a
> -   bit mask (1 << FLAVOR) of the flavors of thread state in the structure
> -   which are filled in; and a member `struct machine_thread_state basic'.
> -   On some machines those are the only members (e.g. i386); on others,
> -   there are several relevant flavors of thread state (e.g. mips).  */
> -struct machine_thread_all_state
> -  {
> -    int set;			/* Mask of bits (1 << FLAVOR).  */
> -    struct <machine>_thread_state basic;
> -  };
> -- 
> 2.40.1
> 
> 

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

[PATCH 02/10] mach: Define MACHINE_THREAD_STATE_SETUP_CALL

[Sergey Bugaev]

The existing two macros, MACHINE_THREAD_STATE_SET_PC and
MACHINE_THREAD_STATE_SET_SP, can be used to set program counter and the
stack pointer registers in a machine-specific thread state structure.

Useful as it is, this may not be enough to set up the thread to make a
function call, because the machine-specific ABI may impose additional
requirements. In particular, x86_64 ABI requires that upon function
entry, the stack pointer is 8 less than 16-byte aligned (sp & 15 == 8).

To deal with this, introduce a new macro,
MACHINE_THREAD_STATE_SETUP_CALL (), which sets both stack and
instruction pointers, and also applies any machine-specific requirements
to make a valid function call. The default implementation simply
forwards to MACHINE_THREAD_STATE_SET_PC and MACHINE_THREAD_STATE_SET_SP,
but on x86_64 we additionally align the stack pointer.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
Any ideas for a better name than PTR_ALIGN_DOWN_8_16?

 sysdeps/mach/thread_state.h     |  9 +++++++++
 sysdeps/mach/x86/thread_state.h | 13 +++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/sysdeps/mach/thread_state.h b/sysdeps/mach/thread_state.h
index 9fa3d4e1..431aaf82 100644
--- a/sysdeps/mach/thread_state.h
+++ b/sysdeps/mach/thread_state.h
@@ -38,6 +38,15 @@
 #endif
 #endif
 
+/* Set up the thread state to call the given function on the given state.
+   Dependning on architecture, this may imply more than just setting PC
+   and SP.  */
+#ifndef MACHINE_THREAD_STATE_SETUP_CALL
+#define MACHINE_THREAD_STATE_SETUP_CALL(ts, stack, size, func) \
+  (MACHINE_THREAD_STATE_SET_PC (ts, func), \
+   MACHINE_THREAD_STATE_SET_SP (ts, stack, size))
+#endif
+
 /* This copies architecture-specific bits from the current thread to the new
    thread state.  */
 #ifndef MACHINE_THREAD_STATE_FIX_NEW
diff --git a/sysdeps/mach/x86/thread_state.h b/sysdeps/mach/x86/thread_state.h
index 5be0bec1..8c419515 100644
--- a/sysdeps/mach/x86/thread_state.h
+++ b/sysdeps/mach/x86/thread_state.h
@@ -20,6 +20,7 @@
 #define _MACH_X86_THREAD_STATE_H 1
 
 #include <mach/machine/thread_status.h>
+#include <libc-pointer-arith.h>
 
 /* This lets the kernel define segments for a new thread.  */
 #define MACHINE_NEW_THREAD_STATE_FLAVOR	i386_THREAD_STATE
@@ -54,6 +55,18 @@ struct machine_thread_all_state
     struct i386_float_state fpu;
   };
 
+#ifdef __x86_64__
+/* We're setting up the stack to perform a function call.  On function entry,
+   the stack pointer must be 8 bytes less than 16-aligned.  */
+#define PTR_ALIGN_DOWN_8_16(ptr)					      \
+ ({ uintptr_t __ptr = PTR_ALIGN_DOWN (ptr, 8);				      \
+    PTR_IS_ALIGNED (__ptr, 16) ? (__ptr - 8) : __ptr; })
+
+#define MACHINE_THREAD_STATE_SETUP_CALL(ts, stack, size, func)		      \
+  ((ts)->SP = PTR_ALIGN_DOWN_8_16 ((uintptr_t) (stack) + (size)),	      \
+   (ts)->PC = (uintptr_t) func)
+#endif
+
 #include <sysdeps/mach/thread_state.h>
 
 #endif /* mach/x86/thread_state.h */
-- 
2.40.1

Re: [PATCH 02/10] mach: Define MACHINE_THREAD_STATE_SETUP_CALL

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mer. 17 mai 2023 22:14:28 +0300, a ecrit:
> The existing two macros, MACHINE_THREAD_STATE_SET_PC and
> MACHINE_THREAD_STATE_SET_SP, can be used to set program counter and the
> stack pointer registers in a machine-specific thread state structure.
> 
> Useful as it is, this may not be enough to set up the thread to make a
> function call, because the machine-specific ABI may impose additional
> requirements. In particular, x86_64 ABI requires that upon function
> entry, the stack pointer is 8 less than 16-byte aligned (sp & 15 == 8).
> 
> To deal with this, introduce a new macro,
> MACHINE_THREAD_STATE_SETUP_CALL (), which sets both stack and
> instruction pointers, and also applies any machine-specific requirements
> to make a valid function call. The default implementation simply
> forwards to MACHINE_THREAD_STATE_SET_PC and MACHINE_THREAD_STATE_SET_SP,
> but on x86_64 we additionally align the stack pointer.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
> Any ideas for a better name than PTR_ALIGN_DOWN_8_16?
> 
>  sysdeps/mach/thread_state.h     |  9 +++++++++
>  sysdeps/mach/x86/thread_state.h | 13 +++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/sysdeps/mach/thread_state.h b/sysdeps/mach/thread_state.h
> index 9fa3d4e1..431aaf82 100644
> --- a/sysdeps/mach/thread_state.h
> +++ b/sysdeps/mach/thread_state.h
> @@ -38,6 +38,15 @@
>  #endif
>  #endif
>  
> +/* Set up the thread state to call the given function on the given state.
> +   Dependning on architecture, this may imply more than just setting PC
> +   and SP.  */
> +#ifndef MACHINE_THREAD_STATE_SETUP_CALL
> +#define MACHINE_THREAD_STATE_SETUP_CALL(ts, stack, size, func) \
> +  (MACHINE_THREAD_STATE_SET_PC (ts, func), \
> +   MACHINE_THREAD_STATE_SET_SP (ts, stack, size))
> +#endif
> +
>  /* This copies architecture-specific bits from the current thread to the new
>     thread state.  */
>  #ifndef MACHINE_THREAD_STATE_FIX_NEW
> diff --git a/sysdeps/mach/x86/thread_state.h b/sysdeps/mach/x86/thread_state.h
> index 5be0bec1..8c419515 100644
> --- a/sysdeps/mach/x86/thread_state.h
> +++ b/sysdeps/mach/x86/thread_state.h
> @@ -20,6 +20,7 @@
>  #define _MACH_X86_THREAD_STATE_H 1
>  
>  #include <mach/machine/thread_status.h>
> +#include <libc-pointer-arith.h>
>  
>  /* This lets the kernel define segments for a new thread.  */
>  #define MACHINE_NEW_THREAD_STATE_FLAVOR	i386_THREAD_STATE
> @@ -54,6 +55,18 @@ struct machine_thread_all_state
>      struct i386_float_state fpu;
>    };
>  
> +#ifdef __x86_64__
> +/* We're setting up the stack to perform a function call.  On function entry,
> +   the stack pointer must be 8 bytes less than 16-aligned.  */
> +#define PTR_ALIGN_DOWN_8_16(ptr)					      \
> + ({ uintptr_t __ptr = PTR_ALIGN_DOWN (ptr, 8);				      \
> +    PTR_IS_ALIGNED (__ptr, 16) ? (__ptr - 8) : __ptr; })
> +
> +#define MACHINE_THREAD_STATE_SETUP_CALL(ts, stack, size, func)		      \
> +  ((ts)->SP = PTR_ALIGN_DOWN_8_16 ((uintptr_t) (stack) + (size)),	      \
> +   (ts)->PC = (uintptr_t) func)
> +#endif
> +
>  #include <sysdeps/mach/thread_state.h>
>  
>  #endif /* mach/x86/thread_state.h */
> -- 
> 2.40.1
> 

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

[PATCH 03/10] hurd: Use MACHINE_THREAD_STATE_SETUP_CALL

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 hurd/hurdfault.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hurd/hurdfault.c b/hurd/hurdfault.c
index 4340897d..dae889a9 100644
--- a/hurd/hurdfault.c
+++ b/hurd/hurdfault.c
@@ -205,8 +205,8 @@ _hurdsig_fault_init (void)
      It runs the function above.  */
   memset (&state, 0, sizeof state);
   MACHINE_THREAD_STATE_FIX_NEW (&state);
-  MACHINE_THREAD_STATE_SET_PC (&state, faulted);
-  MACHINE_THREAD_STATE_SET_SP (&state, faultstack, sizeof faultstack);
+  MACHINE_THREAD_STATE_SETUP_CALL (&state, faultstack,
+				   sizeof faultstack, faulted);
 
   err = __USEPORT
     (PROC,
-- 
2.40.1

Re: [PATCH 03/10] hurd: Use MACHINE_THREAD_STATE_SETUP_CALLo

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mer. 17 mai 2023 22:14:29 +0300, a ecrit:
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  hurd/hurdfault.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hurd/hurdfault.c b/hurd/hurdfault.c
> index 4340897d..dae889a9 100644
> --- a/hurd/hurdfault.c
> +++ b/hurd/hurdfault.c
> @@ -205,8 +205,8 @@ _hurdsig_fault_init (void)
>       It runs the function above.  */
>    memset (&state, 0, sizeof state);
>    MACHINE_THREAD_STATE_FIX_NEW (&state);
> -  MACHINE_THREAD_STATE_SET_PC (&state, faulted);
> -  MACHINE_THREAD_STATE_SET_SP (&state, faultstack, sizeof faultstack);
> +  MACHINE_THREAD_STATE_SETUP_CALL (&state, faultstack,
> +				   sizeof faultstack, faulted);
>  
>    err = __USEPORT
>      (PROC,
> -- 
> 2.40.1
> 

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

[PATCH 04/10] mach: Add __mach_setup_thread_call ()

[Sergey Bugaev]

This is just like mach_setup_thread (), but it's suitable for making the
thread call a function correctly, as opposed to explicitly setting the
thread's stack and instruction pointers to the given values. Internally,
it uses MACHINE_THREAD_STATE_SETUP_CALL.

Unlike mach_setup_thread (), which is exported via mach.h for the
benefit of the Hurd exec server, __mach_setup_thread_call () is private
to glibc for the time being.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 mach/mach.h         |  6 +++++-
 mach/setup-thread.c | 52 ++++++++++++++++++++++++++++++++-------------
 mach/setup-thread.h | 32 ++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 16 deletions(-)
 create mode 100644 mach/setup-thread.h

diff --git a/mach/mach.h b/mach/mach.h
index d115f5a1..348f0196 100644
--- a/mach/mach.h
+++ b/mach/mach.h
@@ -88,7 +88,11 @@ extern FILE *mach_open_devstream (mach_port_t device_port, const char *mode);
    If STACK_BASE is not null it is filled in with the chosen stack base.
    If STACK_SIZE is not null it is filled in with the chosen stack size.
    Regardless, an extra page of red zone is allocated off the end; this
-   is not included in *STACK_SIZE.  */
+   is not included in *STACK_SIZE.
+
+   Mote: this function is unsuitable for setting up the thread to call a
+   function at PC, since the architecture ABI may impose additional
+   requirements beyond setting PC and stack.  */
 kern_return_t __mach_setup_thread (task_t task, thread_t thread, void *pc,
 				   vm_address_t *stack_base,
 				   vm_size_t *stack_size);
diff --git a/mach/setup-thread.c b/mach/setup-thread.c
index ae24a149..0e149787 100644
--- a/mach/setup-thread.c
+++ b/mach/setup-thread.c
@@ -16,6 +16,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <mach.h>
+#include <mach/setup-thread.h>
 #include <thread_state.h>
 #include <string.h>
 #include <mach/machine/vm_param.h>
@@ -24,17 +25,10 @@
 
 #define	STACK_SIZE	(16 * 1024 * 1024) /* 16MB, arbitrary.  */
 
-/* Give THREAD a stack and set it to run at PC when resumed.
-   If *STACK_SIZE is nonzero, that size of stack is allocated.
-   If *STACK_BASE is nonzero, that stack location is used.
-   If STACK_BASE is not null it is filled in with the chosen stack base.
-   If STACK_SIZE is not null it is filled in with the chosen stack size.
-   Regardless, an extra page of red zone is allocated off the end; this
-   is not included in *STACK_SIZE.  */
-
-kern_return_t
-__mach_setup_thread (task_t task, thread_t thread, void *pc,
-		     vm_address_t *stack_base, vm_size_t *stack_size)
+static kern_return_t
+mach_setup_thread_impl (task_t task, thread_t thread, int is_call,
+			void *pc, vm_address_t *stack_base,
+			vm_size_t *stack_size)
 {
   kern_return_t error;
   struct machine_thread_state ts;
@@ -43,6 +37,8 @@ __mach_setup_thread (task_t task, thread_t thread, void *pc,
   vm_size_t size;
   int anywhere;
 
+  memset (&ts, 0, sizeof (ts));
+
   size = stack_size ? *stack_size ? : STACK_SIZE : STACK_SIZE;
   stack = stack_base ? *stack_base ? : 0 : 0;
   anywhere = !stack_base || !*stack_base;
@@ -54,21 +50,25 @@ __mach_setup_thread (task_t task, thread_t thread, void *pc,
   if (stack_size)
     *stack_size = size;
 
-  memset (&ts, 0, sizeof (ts));
-  MACHINE_THREAD_STATE_SET_PC (&ts, pc);
 #ifdef STACK_GROWTH_DOWN
   if (stack_base)
     *stack_base = stack + __vm_page_size;
-  ts.SP = stack + __vm_page_size + size;
 #elif defined (STACK_GROWTH_UP)
   if (stack_base)
     *stack_base = stack;
-  ts.SP = stack;
   stack += size;
 #else
   #error stack direction unknown
 #endif
 
+  if (is_call)
+    MACHINE_THREAD_STATE_SETUP_CALL (&ts, *stack_base, size, pc);
+  else
+    {
+      MACHINE_THREAD_STATE_SET_PC (&ts, pc);
+      MACHINE_THREAD_STATE_SET_SP (&ts, *stack_base, size);
+    }
+
   /* Create the red zone.  */
   if (error = __vm_protect (task, stack, __vm_page_size, 0, VM_PROT_NONE))
     return error;
@@ -77,8 +77,30 @@ __mach_setup_thread (task_t task, thread_t thread, void *pc,
 			     (natural_t *) &ts, tssize);
 }
 
+/* Give THREAD a stack and set it to run at PC when resumed.
+   If *STACK_SIZE is nonzero, that size of stack is allocated.
+   If *STACK_BASE is nonzero, that stack location is used.
+   If STACK_BASE is not null it is filled in with the chosen stack base.
+   If STACK_SIZE is not null it is filled in with the chosen stack size.
+   Regardless, an extra page of red zone is allocated off the end; this
+   is not included in *STACK_SIZE.  */
+
+kern_return_t
+__mach_setup_thread (task_t task, thread_t thread, void *pc,
+		     vm_address_t *stack_base, vm_size_t *stack_size)
+{
+  return mach_setup_thread_impl (task, thread, 0, pc, stack_base, stack_size);
+}
+
 weak_alias (__mach_setup_thread, mach_setup_thread)
 
+kern_return_t
+__mach_setup_thread_call (task_t task, thread_t thread, void *pc,
+			  vm_address_t *stack_base, vm_size_t *stack_size)
+{
+  return mach_setup_thread_impl (task, thread, 1, pc, stack_base, stack_size);
+}
+
 /* Give THREAD a TLS area.  */
 kern_return_t
 __mach_setup_tls (thread_t thread)
diff --git a/mach/setup-thread.h b/mach/setup-thread.h
new file mode 100644
index 00000000..b4c94d1d
--- /dev/null
+++ b/mach/setup-thread.h
@@ -0,0 +1,32 @@
+/* Setup a Mach thread.
+   Copyright (C) 1993-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	_MACH_SETUP_THREAD_H
+
+#define	_MACH_SETUP_THREAD_H	1
+
+#include <mach.h>
+
+/* Like mach_setup_thread (), but suitable for setting up function
+   calls.  */
+kern_return_t __mach_setup_thread_call (task_t task, thread_t thread,
+					void *function,
+					vm_address_t *stack_base,
+					vm_size_t *stack_size);
+
+#endif	/* mach/setup-thread.h */
-- 
2.40.1

Re: [PATCH 04/10] mach: Add __mach_setup_thread_call ()

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mer. 17 mai 2023 22:14:30 +0300, a ecrit:
> This is just like mach_setup_thread (), but it's suitable for making the
> thread call a function correctly, as opposed to explicitly setting the
> thread's stack and instruction pointers to the given values. Internally,
> it uses MACHINE_THREAD_STATE_SETUP_CALL.
> 
> Unlike mach_setup_thread (), which is exported via mach.h for the
> benefit of the Hurd exec server, __mach_setup_thread_call () is private
> to glibc for the time being.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  mach/mach.h         |  6 +++++-
>  mach/setup-thread.c | 52 ++++++++++++++++++++++++++++++++-------------
>  mach/setup-thread.h | 32 ++++++++++++++++++++++++++++
>  3 files changed, 74 insertions(+), 16 deletions(-)
>  create mode 100644 mach/setup-thread.h
> 
> diff --git a/mach/mach.h b/mach/mach.h
> index d115f5a1..348f0196 100644
> --- a/mach/mach.h
> +++ b/mach/mach.h
> @@ -88,7 +88,11 @@ extern FILE *mach_open_devstream (mach_port_t device_port, const char *mode);
>     If STACK_BASE is not null it is filled in with the chosen stack base.
>     If STACK_SIZE is not null it is filled in with the chosen stack size.
>     Regardless, an extra page of red zone is allocated off the end; this
> -   is not included in *STACK_SIZE.  */
> +   is not included in *STACK_SIZE.
> +
> +   Mote: this function is unsuitable for setting up the thread to call a
> +   function at PC, since the architecture ABI may impose additional
> +   requirements beyond setting PC and stack.  */
>  kern_return_t __mach_setup_thread (task_t task, thread_t thread, void *pc,
>  				   vm_address_t *stack_base,
>  				   vm_size_t *stack_size);
> diff --git a/mach/setup-thread.c b/mach/setup-thread.c
> index ae24a149..0e149787 100644
> --- a/mach/setup-thread.c
> +++ b/mach/setup-thread.c
> @@ -16,6 +16,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <mach.h>
> +#include <mach/setup-thread.h>
>  #include <thread_state.h>
>  #include <string.h>
>  #include <mach/machine/vm_param.h>
> @@ -24,17 +25,10 @@
>  
>  #define	STACK_SIZE	(16 * 1024 * 1024) /* 16MB, arbitrary.  */
>  
> -/* Give THREAD a stack and set it to run at PC when resumed.
> -   If *STACK_SIZE is nonzero, that size of stack is allocated.
> -   If *STACK_BASE is nonzero, that stack location is used.
> -   If STACK_BASE is not null it is filled in with the chosen stack base.
> -   If STACK_SIZE is not null it is filled in with the chosen stack size.
> -   Regardless, an extra page of red zone is allocated off the end; this
> -   is not included in *STACK_SIZE.  */
> -
> -kern_return_t
> -__mach_setup_thread (task_t task, thread_t thread, void *pc,
> -		     vm_address_t *stack_base, vm_size_t *stack_size)
> +static kern_return_t
> +mach_setup_thread_impl (task_t task, thread_t thread, int is_call,
> +			void *pc, vm_address_t *stack_base,
> +			vm_size_t *stack_size)
>  {
>    kern_return_t error;
>    struct machine_thread_state ts;
> @@ -43,6 +37,8 @@ __mach_setup_thread (task_t task, thread_t thread, void *pc,
>    vm_size_t size;
>    int anywhere;
>  
> +  memset (&ts, 0, sizeof (ts));
> +
>    size = stack_size ? *stack_size ? : STACK_SIZE : STACK_SIZE;
>    stack = stack_base ? *stack_base ? : 0 : 0;
>    anywhere = !stack_base || !*stack_base;
> @@ -54,21 +50,25 @@ __mach_setup_thread (task_t task, thread_t thread, void *pc,
>    if (stack_size)
>      *stack_size = size;
>  
> -  memset (&ts, 0, sizeof (ts));
> -  MACHINE_THREAD_STATE_SET_PC (&ts, pc);
>  #ifdef STACK_GROWTH_DOWN
>    if (stack_base)
>      *stack_base = stack + __vm_page_size;
> -  ts.SP = stack + __vm_page_size + size;
>  #elif defined (STACK_GROWTH_UP)
>    if (stack_base)
>      *stack_base = stack;
> -  ts.SP = stack;
>    stack += size;
>  #else
>    #error stack direction unknown
>  #endif
>  
> +  if (is_call)
> +    MACHINE_THREAD_STATE_SETUP_CALL (&ts, *stack_base, size, pc);
> +  else
> +    {
> +      MACHINE_THREAD_STATE_SET_PC (&ts, pc);
> +      MACHINE_THREAD_STATE_SET_SP (&ts, *stack_base, size);
> +    }
> +
>    /* Create the red zone.  */
>    if (error = __vm_protect (task, stack, __vm_page_size, 0, VM_PROT_NONE))
>      return error;
> @@ -77,8 +77,30 @@ __mach_setup_thread (task_t task, thread_t thread, void *pc,
>  			     (natural_t *) &ts, tssize);
>  }
>  
> +/* Give THREAD a stack and set it to run at PC when resumed.
> +   If *STACK_SIZE is nonzero, that size of stack is allocated.
> +   If *STACK_BASE is nonzero, that stack location is used.
> +   If STACK_BASE is not null it is filled in with the chosen stack base.
> +   If STACK_SIZE is not null it is filled in with the chosen stack size.
> +   Regardless, an extra page of red zone is allocated off the end; this
> +   is not included in *STACK_SIZE.  */
> +
> +kern_return_t
> +__mach_setup_thread (task_t task, thread_t thread, void *pc,
> +		     vm_address_t *stack_base, vm_size_t *stack_size)
> +{
> +  return mach_setup_thread_impl (task, thread, 0, pc, stack_base, stack_size);
> +}
> +
>  weak_alias (__mach_setup_thread, mach_setup_thread)
>  
> +kern_return_t
> +__mach_setup_thread_call (task_t task, thread_t thread, void *pc,
> +			  vm_address_t *stack_base, vm_size_t *stack_size)
> +{
> +  return mach_setup_thread_impl (task, thread, 1, pc, stack_base, stack_size);
> +}
> +
>  /* Give THREAD a TLS area.  */
>  kern_return_t
>  __mach_setup_tls (thread_t thread)
> diff --git a/mach/setup-thread.h b/mach/setup-thread.h
> new file mode 100644
> index 00000000..b4c94d1d
> --- /dev/null
> +++ b/mach/setup-thread.h
> @@ -0,0 +1,32 @@
> +/* Setup a Mach thread.
> +   Copyright (C) 1993-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	_MACH_SETUP_THREAD_H
> +
> +#define	_MACH_SETUP_THREAD_H	1
> +
> +#include <mach.h>
> +
> +/* Like mach_setup_thread (), but suitable for setting up function
> +   calls.  */
> +kern_return_t __mach_setup_thread_call (task_t task, thread_t thread,
> +					void *function,
> +					vm_address_t *stack_base,
> +					vm_size_t *stack_size);
> +
> +#endif	/* mach/setup-thread.h */
> -- 
> 2.40.1
> 

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

[PATCH 05/10] hurd: Use __mach_setup_thread_call ()

[Sergey Bugaev]

...instead of mach_setup_thread (), which is unsuitable for setting up
function calls.

Checked on x86_64-gnu: the signal thread no longer crashes upon trying
to process a message.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 hurd/hurdsig.c                | 10 ++++++----
 sysdeps/mach/hurd/profil.c    |  5 +++--
 sysdeps/mach/hurd/setitimer.c | 11 ++++++-----
 3 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
index 78ea59d9..313e95ae 100644
--- a/hurd/hurdsig.c
+++ b/hurd/hurdsig.c
@@ -22,6 +22,7 @@
 #include <lock-intern.h>	/* For `struct mutex'.  */
 #include <pthreadP.h>
 #include <mach.h>
+#include <mach/setup-thread.h>
 #include <mach/thread_switch.h>
 #include <mach/mig_support.h>
 #include <mach/vm_param.h>
@@ -1525,10 +1526,11 @@ _hurdsig_init (const int *intarray, size_t intarraysize)
       assert_perror (err);
 
       stacksize = __vm_page_size * 8; /* Small stack for signal thread.  */
-      err = __mach_setup_thread (__mach_task_self (), _hurd_msgport_thread,
-				 _hurd_msgport_receive,
-				 (vm_address_t *) &__hurd_sigthread_stack_base,
-				 &stacksize);
+      err = __mach_setup_thread_call (__mach_task_self (),
+				      _hurd_msgport_thread,
+				      _hurd_msgport_receive,
+				      (vm_address_t *) &__hurd_sigthread_stack_base,
+				      &stacksize);
       assert_perror (err);
       err = __mach_setup_tls (_hurd_msgport_thread);
       assert_perror (err);
diff --git a/sysdeps/mach/hurd/profil.c b/sysdeps/mach/hurd/profil.c
index 64fe76f7..8092df0a 100644
--- a/sysdeps/mach/hurd/profil.c
+++ b/sysdeps/mach/hurd/profil.c
@@ -22,6 +22,7 @@
 #include <hurd.h>
 #include <mach/mach4.h>
 #include <mach/pc_sample.h>
+#include <mach/setup-thread.h>
 #include <lock-intern.h>
 #include <assert.h>
 #include <libc-internal.h>
@@ -68,8 +69,8 @@ update_waiter (u_short *sample_buffer, size_t size, size_t offset, u_int scale)
       /* Set up the profiling collector thread.  */
       err = __thread_create (__mach_task_self (), &profile_thread);
       if (! err)
-	err = __mach_setup_thread (__mach_task_self (), profile_thread,
-				   &profile_waiter, NULL, NULL);
+	err = __mach_setup_thread_call (__mach_task_self (), profile_thread,
+					&profile_waiter, NULL, NULL);
       if (! err)
 	err = __mach_setup_tls(profile_thread);
     }
diff --git a/sysdeps/mach/hurd/setitimer.c b/sysdeps/mach/hurd/setitimer.c
index d09c59dc..03191b91 100644
--- a/sysdeps/mach/hurd/setitimer.c
+++ b/sysdeps/mach/hurd/setitimer.c
@@ -25,6 +25,7 @@
 #include <hurd/msg_request.h>
 #include <mach.h>
 #include <mach/message.h>
+#include <mach/setup-thread.h>
 
 /* XXX Temporary cheezoid implementation of ITIMER_REAL/SIGALRM.  */
 
@@ -227,11 +228,11 @@ setitimer_locked (const struct itimerval *new, struct itimerval *old,
 	    goto out;
 	  _hurd_itimer_thread_stack_base = 0; /* Anywhere.  */
 	  _hurd_itimer_thread_stack_size = __vm_page_size; /* Small stack.  */
-	  if ((err = __mach_setup_thread (__mach_task_self (),
-					 _hurd_itimer_thread,
-					 &timer_thread,
-					 &_hurd_itimer_thread_stack_base,
-					 &_hurd_itimer_thread_stack_size))
+	  if ((err = __mach_setup_thread_call (__mach_task_self (),
+					       _hurd_itimer_thread,
+					       &timer_thread,
+					       &_hurd_itimer_thread_stack_base,
+					       &_hurd_itimer_thread_stack_size))
 	      || (err = __mach_setup_tls(_hurd_itimer_thread)))
 	    {
 	      __thread_terminate (_hurd_itimer_thread);
-- 
2.40.1

Re: [PATCH 05/10] hurd: Use __mach_setup_thread_call ()

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le mer. 17 mai 2023 22:14:31 +0300, a ecrit:
> ...instead of mach_setup_thread (), which is unsuitable for setting up
> function calls.
> 
> Checked on x86_64-gnu: the signal thread no longer crashes upon trying
> to process a message.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  hurd/hurdsig.c                | 10 ++++++----
>  sysdeps/mach/hurd/profil.c    |  5 +++--
>  sysdeps/mach/hurd/setitimer.c | 11 ++++++-----
>  3 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
> index 78ea59d9..313e95ae 100644
> --- a/hurd/hurdsig.c
> +++ b/hurd/hurdsig.c
> @@ -22,6 +22,7 @@
>  #include <lock-intern.h>	/* For `struct mutex'.  */
>  #include <pthreadP.h>
>  #include <mach.h>
> +#include <mach/setup-thread.h>
>  #include <mach/thread_switch.h>
>  #include <mach/mig_support.h>
>  #include <mach/vm_param.h>
> @@ -1525,10 +1526,11 @@ _hurdsig_init (const int *intarray, size_t intarraysize)
>        assert_perror (err);
>  
>        stacksize = __vm_page_size * 8; /* Small stack for signal thread.  */
> -      err = __mach_setup_thread (__mach_task_self (), _hurd_msgport_thread,
> -				 _hurd_msgport_receive,
> -				 (vm_address_t *) &__hurd_sigthread_stack_base,
> -				 &stacksize);
> +      err = __mach_setup_thread_call (__mach_task_self (),
> +				      _hurd_msgport_thread,
> +				      _hurd_msgport_receive,
> +				      (vm_address_t *) &__hurd_sigthread_stack_base,
> +				      &stacksize);
>        assert_perror (err);
>        err = __mach_setup_tls (_hurd_msgport_thread);
>        assert_perror (err);
> diff --git a/sysdeps/mach/hurd/profil.c b/sysdeps/mach/hurd/profil.c
> index 64fe76f7..8092df0a 100644
> --- a/sysdeps/mach/hurd/profil.c
> +++ b/sysdeps/mach/hurd/profil.c
> @@ -22,6 +22,7 @@
>  #include <hurd.h>
>  #include <mach/mach4.h>
>  #include <mach/pc_sample.h>
> +#include <mach/setup-thread.h>
>  #include <lock-intern.h>
>  #include <assert.h>
>  #include <libc-internal.h>
> @@ -68,8 +69,8 @@ update_waiter (u_short *sample_buffer, size_t size, size_t offset, u_int scale)
>        /* Set up the profiling collector thread.  */
>        err = __thread_create (__mach_task_self (), &profile_thread);
>        if (! err)
> -	err = __mach_setup_thread (__mach_task_self (), profile_thread,
> -				   &profile_waiter, NULL, NULL);
> +	err = __mach_setup_thread_call (__mach_task_self (), profile_thread,
> +					&profile_waiter, NULL, NULL);
>        if (! err)
>  	err = __mach_setup_tls(profile_thread);
>      }
> diff --git a/sysdeps/mach/hurd/setitimer.c b/sysdeps/mach/hurd/setitimer.c
> index d09c59dc..03191b91 100644
> --- a/sysdeps/mach/hurd/setitimer.c
> +++ b/sysdeps/mach/hurd/setitimer.c
> @@ -25,6 +25,7 @@
>  #include <hurd/msg_request.h>
>  #include <mach.h>
>  #include <mach/message.h>
> +#include <mach/setup-thread.h>
>  
>  /* XXX Temporary cheezoid implementation of ITIMER_REAL/SIGALRM.  */
>  
> @@ -227,11 +228,11 @@ setitimer_locked (const struct itimerval *new, struct itimerval *old,
>  	    goto out;
>  	  _hurd_itimer_thread_stack_base = 0; /* Anywhere.  */
>  	  _hurd_itimer_thread_stack_size = __vm_page_size; /* Small stack.  */
> -	  if ((err = __mach_setup_thread (__mach_task_self (),
> -					 _hurd_itimer_thread,
> -					 &timer_thread,
> -					 &_hurd_itimer_thread_stack_base,
> -					 &_hurd_itimer_thread_stack_size))
> +	  if ((err = __mach_setup_thread_call (__mach_task_self (),
> +					       _hurd_itimer_thread,
> +					       &timer_thread,
> +					       &_hurd_itimer_thread_stack_base,
> +					       &_hurd_itimer_thread_stack_size))
>  	      || (err = __mach_setup_tls(_hurd_itimer_thread)))
>  	    {
>  	      __thread_terminate (_hurd_itimer_thread);
> -- 
> 2.40.1
> 
> 

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

[RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Sergey Bugaev]

Unlike sigstate->thread, tcb->self did not hold a Mach port reference on
the thread port it names. This means that the port can be deallocated,
and the name reused for something else, without anyone noticing. Using
tcb->self will then lead to port use-after-free.

Fortunately nothing was accessing tcb->self, other than it being
intially set to then-valid thread port name upon TCB initialization. To
assert that this keeps being the case without altering TCB layout,
rename self -> self_do_not_use, and stop initializing it.

Also, do not (re-)allocate a whole separate and unused stack for the
main thread, and just exit __pthread_setup early in this case.

Found upon attempting to use tcb->self and getting unexpected crashes.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/i386/tls.h         |  3 +--
 sysdeps/mach/hurd/x86/htl/pt-setup.c | 34 ++++++++++------------------
 sysdeps/mach/hurd/x86_64/tls.h       |  3 +--
 3 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h
index e124fb10..ba283008 100644
--- a/sysdeps/mach/hurd/i386/tls.h
+++ b/sysdeps/mach/hurd/i386/tls.h
@@ -32,7 +32,7 @@ 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.  */
+  thread_t self_do_not_use;	/* This thread's control port.  */
   int multiple_threads;
   uintptr_t sysinfo;
   uintptr_t stack_guard;
@@ -419,7 +419,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb)
   HURD_TLS_DESC_DECL (desc, tcb);
 
   tcb->tcb = tcb;
-  tcb->self = child;
 
   if (HURD_SEL_LDT (sel))
     err = __i386_set_ldt (child, sel, &desc, 1);
diff --git a/sysdeps/mach/hurd/x86/htl/pt-setup.c b/sysdeps/mach/hurd/x86/htl/pt-setup.c
index 3abd92b2..686124d7 100644
--- a/sysdeps/mach/hurd/x86/htl/pt-setup.c
+++ b/sysdeps/mach/hurd/x86/htl/pt-setup.c
@@ -19,6 +19,7 @@
 #include <stdint.h>
 #include <assert.h>
 #include <mach.h>
+#include <hurd.h>
 
 #include <pt-internal.h>
 
@@ -76,35 +77,24 @@ __pthread_setup (struct __pthread *thread,
 				      void *), void *(*start_routine) (void *),
 		 void *arg)
 {
-  tcbhead_t *tcb;
   error_t err;
-  mach_port_t ktid;
 
-  thread->mcontext.pc = entry_point;
-  thread->mcontext.sp = stack_setup (thread, start_routine, arg);
-
-  ktid = __mach_thread_self ();
-  if (thread->kernel_thread == ktid)
+  if (thread->kernel_thread == hurd_thread_self ())
     /* Fix up the TCB for the main thread.  The C library has already
        installed a TCB, which we want to keep using.  This TCB must not
        be freed so don't register it in the thread structure.  On the
        other hand, it's not yet possible to reliably release a TCB.
-       Leave the unused one registered so that it doesn't leak.  The
-       only thing left to do is to correctly set the `self' member in
-       the already existing TCB.  */
-    tcb = THREAD_SELF;
-  else
-    {
-      err = __thread_set_pcsptp (thread->kernel_thread,
-				 1, thread->mcontext.pc,
-				 1, thread->mcontext.sp,
-				 1, thread->tcb);
-      assert_perror (err);
-      tcb = thread->tcb;
-    }
-  __mach_port_deallocate (__mach_task_self (), ktid);
+       Leave the unused one registered so that it doesn't leak.  */
+    return 0;
+
+  thread->mcontext.pc = entry_point;
+  thread->mcontext.sp = stack_setup (thread, start_routine, arg);
 
-  tcb->self = thread->kernel_thread;
+  err = __thread_set_pcsptp (thread->kernel_thread,
+			     1, thread->mcontext.pc,
+			     1, thread->mcontext.sp,
+			     1, thread->tcb);
+  assert_perror (err);
 
   return 0;
 }
diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
index 1274723a..35dcef44 100644
--- a/sysdeps/mach/hurd/x86_64/tls.h
+++ b/sysdeps/mach/hurd/x86_64/tls.h
@@ -35,7 +35,7 @@ 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.  */
+  thread_t self_do_no_use;	/* This thread's control port.  */
   int __glibc_padding1;
   int multiple_threads;
   int gscope_flag;
@@ -158,7 +158,6 @@ _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;
-- 
2.40.1

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le mer. 17 mai 2023 22:14:32 +0300, a ecrit:
> Unlike sigstate->thread, tcb->self did not hold a Mach port reference on
> the thread port it names. This means that the port can be deallocated,
> and the name reused for something else, without anyone noticing. Using
> tcb->self will then lead to port use-after-free.
> 
> Fortunately nothing was accessing tcb->self, other than it being
> intially set to then-valid thread port name upon TCB initialization. To
> assert that this keeps being the case without altering TCB layout,
> rename self -> self_do_not_use, and stop initializing it.
> 
> Also, do not (re-)allocate a whole separate and unused stack for the
> main thread, and just exit __pthread_setup early in this case.
> 
> Found upon attempting to use tcb->self and getting unexpected crashes.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/i386/tls.h         |  3 +--
>  sysdeps/mach/hurd/x86/htl/pt-setup.c | 34 ++++++++++------------------
>  sysdeps/mach/hurd/x86_64/tls.h       |  3 +--
>  3 files changed, 14 insertions(+), 26 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h
> index e124fb10..ba283008 100644
> --- a/sysdeps/mach/hurd/i386/tls.h
> +++ b/sysdeps/mach/hurd/i386/tls.h
> @@ -32,7 +32,7 @@ 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.  */
> +  thread_t self_do_not_use;	/* This thread's control port.  */
>    int multiple_threads;
>    uintptr_t sysinfo;
>    uintptr_t stack_guard;
> @@ -419,7 +419,6 @@ _hurd_tls_new (thread_t child, tcbhead_t *tcb)
>    HURD_TLS_DESC_DECL (desc, tcb);
>  
>    tcb->tcb = tcb;
> -  tcb->self = child;
>  
>    if (HURD_SEL_LDT (sel))
>      err = __i386_set_ldt (child, sel, &desc, 1);
> diff --git a/sysdeps/mach/hurd/x86/htl/pt-setup.c b/sysdeps/mach/hurd/x86/htl/pt-setup.c
> index 3abd92b2..686124d7 100644
> --- a/sysdeps/mach/hurd/x86/htl/pt-setup.c
> +++ b/sysdeps/mach/hurd/x86/htl/pt-setup.c
> @@ -19,6 +19,7 @@
>  #include <stdint.h>
>  #include <assert.h>
>  #include <mach.h>
> +#include <hurd.h>
>  
>  #include <pt-internal.h>
>  
> @@ -76,35 +77,24 @@ __pthread_setup (struct __pthread *thread,
>  				      void *), void *(*start_routine) (void *),
>  		 void *arg)
>  {
> -  tcbhead_t *tcb;
>    error_t err;
> -  mach_port_t ktid;
>  
> -  thread->mcontext.pc = entry_point;
> -  thread->mcontext.sp = stack_setup (thread, start_routine, arg);
> -
> -  ktid = __mach_thread_self ();
> -  if (thread->kernel_thread == ktid)
> +  if (thread->kernel_thread == hurd_thread_self ())
>      /* Fix up the TCB for the main thread.  The C library has already
>         installed a TCB, which we want to keep using.  This TCB must not
>         be freed so don't register it in the thread structure.  On the
>         other hand, it's not yet possible to reliably release a TCB.
> -       Leave the unused one registered so that it doesn't leak.  The
> -       only thing left to do is to correctly set the `self' member in
> -       the already existing TCB.  */
> -    tcb = THREAD_SELF;
> -  else
> -    {
> -      err = __thread_set_pcsptp (thread->kernel_thread,
> -				 1, thread->mcontext.pc,
> -				 1, thread->mcontext.sp,
> -				 1, thread->tcb);
> -      assert_perror (err);
> -      tcb = thread->tcb;
> -    }
> -  __mach_port_deallocate (__mach_task_self (), ktid);
> +       Leave the unused one registered so that it doesn't leak.  */
> +    return 0;
> +
> +  thread->mcontext.pc = entry_point;
> +  thread->mcontext.sp = stack_setup (thread, start_routine, arg);
>  
> -  tcb->self = thread->kernel_thread;
> +  err = __thread_set_pcsptp (thread->kernel_thread,
> +			     1, thread->mcontext.pc,
> +			     1, thread->mcontext.sp,
> +			     1, thread->tcb);
> +  assert_perror (err);
>  
>    return 0;
>  }
> diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
> index 1274723a..35dcef44 100644
> --- a/sysdeps/mach/hurd/x86_64/tls.h
> +++ b/sysdeps/mach/hurd/x86_64/tls.h
> @@ -35,7 +35,7 @@ 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.  */
> +  thread_t self_do_no_use;	/* This thread's control port.  */
>    int __glibc_padding1;
>    int multiple_threads;
>    int gscope_flag;
> @@ -158,7 +158,6 @@ _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;
> -- 
> 2.40.1
> 
> 

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

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Joseph Myers]

I suspect this of causing linknamespace test failures:

Contents of conform/POSIX2008/pthread.h/linknamespace.out:

[initial] pthread_create -> [libpthread.a(pt-create.o)] __pthread_setup -> [libpthread.a(pt-setup.o)] hurd_thread_self

(On x86_64 there's also a localplt test failure: "Extra PLT reference: 
libc.so: hurd_thread_self".  In addition, x86_64 has a c++-types-check 
failure.  Thus, neither Hurd configuration has clean results for 
compilation tests from build-many-glibcs.py at present.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Sergey Bugaev]

Hello,

On Thu, May 18, 2023 at 9:55 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> I suspect this of causing linknamespace test failures:
>
> Contents of conform/POSIX2008/pthread.h/linknamespace.out:
>
> [initial] pthread_create -> [libpthread.a(pt-create.o)] __pthread_setup -> [libpthread.a(pt-setup.o)] hurd_thread_self
>
> (On x86_64 there's also a localplt test failure: "Extra PLT reference:
> libc.so: hurd_thread_self".  In addition, x86_64 has a c++-types-check
> failure.  Thus, neither Hurd configuration has clean results for
> compilation tests from build-many-glibcs.py at present.)

Thank you, and sorry :|

Do I understand it right that this is because of hurd_thread_self
being publicly exported and interposable? A quick grep shows that
nothing else inside glibc was using hurd_thread_self indeed.

Would the right way to resolve this be introducing a hidden
__hurd_thread_self and using that? We could also make it
__mach_thread_self () + __mach_port_deallocate (), but why do +2
syscalls when we already have all the required info in userspace.

What's the C++ type check failure? Surely this patch (nor
2f8ecb58a59eb82c43214d000842d99644a662d1 "hurd: Fix x86_64
_hurd_tls_fork") has modified any public headers?

Sergey

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 2500 bytes --]

On Thu, 18 May 2023, Sergey Bugaev via Libc-alpha wrote:

> Hello,
> 
> On Thu, May 18, 2023 at 9:55 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > I suspect this of causing linknamespace test failures:
> >
> > Contents of conform/POSIX2008/pthread.h/linknamespace.out:
> >
> > [initial] pthread_create -> [libpthread.a(pt-create.o)] __pthread_setup -> [libpthread.a(pt-setup.o)] hurd_thread_self
> >
> > (On x86_64 there's also a localplt test failure: "Extra PLT reference:
> > libc.so: hurd_thread_self".  In addition, x86_64 has a c++-types-check
> > failure.  Thus, neither Hurd configuration has clean results for
> > compilation tests from build-many-glibcs.py at present.)
> 
> Thank you, and sorry :|
> 
> Do I understand it right that this is because of hurd_thread_self
> being publicly exported and interposable? A quick grep shows that
> nothing else inside glibc was using hurd_thread_self indeed.
> 
> Would the right way to resolve this be introducing a hidden
> __hurd_thread_self and using that? We could also make it

Yes.

Strictly there are two separate issues: (a) calling an exported symbol 
should be done via a hidden alias, to avoid going via the PLT, and (b) 
functions in a standard namespace should not call names in the user's 
namespace, which requires using a name such as __hurd_thread_self instead 
(which should also be marked hidden to make the call optimally efficient).

> What's the C++ type check failure? Surely this patch (nor
> 2f8ecb58a59eb82c43214d000842d99644a662d1 "hurd: Fix x86_64
> _hurd_tls_fork") has modified any public headers?

The C++ type check failure was already present before this patch.

--- sysdeps/mach/hurd/x86_64/c++-types.data     2023-05-02 09:14:30.246903708 +0000
+++ -   2023-05-18 02:08:06.184068438 +0000
@@ -1 +1 @@
-blkcnt64_t:x
+blkcnt64_t:l
@@ -8 +8 @@
-dev_t:j
+dev_t:m
@@ -10 +10 @@
-fsblkcnt64_t:y
+fsblkcnt64_t:m
@@ -12 +12 @@
-fsfilcnt64_t:y
+fsfilcnt64_t:m
@@ -14 +14 @@
-fsid_t:y
+fsid_t:m
@@ -17 +17 @@
-ino64_t:y
+ino64_t:m
@@ -21 +21 @@
-int64_t:x
+int64_t:l
@@ -23 +23 @@
-intptr_t:i
+intptr_t:l
@@ -25 +25 @@
-loff_t:x
+loff_t:l
@@ -27,2 +27,2 @@
-nlink_t:j
-off64_t:x
+nlink_t:m
+off64_t:l
@@ -43,4 +43,4 @@
-pthread_t:i
-quad_t:x
-register_t:i
-rlim64_t:y
+pthread_t:l
+quad_t:l
+register_t:l
+rlim64_t:m
@@ -49 +49 @@
-size_t:j
+size_t:m
@@ -51 +51 @@
-ssize_t:i
+ssize_t:l
@@ -60 +60 @@
-u_int64_t:y
+u_int64_t:m
@@ -64 +64 @@
-u_quad_t:y
+u_quad_t:m

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Samuel Thibault]

Joseph Myers, le jeu. 18 mai 2023 20:16:03 +0000, a ecrit:
> The C++ type check failure was already present before this patch.
> 
> --- sysdeps/mach/hurd/x86_64/c++-types.data     2023-05-02 09:14:30.246903708 +0000
> +++ -   2023-05-18 02:08:06.184068438 +0000
> @@ -1 +1 @@
> -blkcnt64_t:x
> +blkcnt64_t:l

etc.

Uh, I'm confused. I do remember seeing a c++-types test failing and thus
adding the c++ data file, but somehow apparently only added the 32bit
one. And I'm not actually seeing the c++-types failure in my test box,
thus it went unnoticed... I commited the update to 64, thanks!

Samuel

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Sergey Bugaev]

On Thu, May 18, 2023 at 11:16 PM Joseph Myers <joseph@codesourcery.com> wrote:
> Strictly there are two separate issues: (a) calling an exported symbol
> should be done via a hidden alias, to avoid going via the PLT, and (b)
> functions in a standard namespace should not call names in the user's
> namespace, which requires using a name such as __hurd_thread_self instead
> (which should also be marked hidden to make the call optimally efficient).

While we're talking about this, could you please say if my
understanding below is correct (and correct me if not)?

'foo' is a public symbol, to be used by the user, and also
interposable by the user. Always called via PLT (except from inside
the user's code when redefined inside the executable, in which case
the compiler/linker will see that no PLT is needed), and should not be
called from inside glibc.

'__foo' is a (public? private? semi-private?) symbol, the user code is
not supposed to use it, but it's exported from libc.so for the benefit
of other glibc code that ends up in different DSOs and still wants to
call the original version even when 'foo' gets interposed. Invoked via
PLT from other DSOs, not invoked from libc.so because of...

'__GI___foo' is a private non-exported symbol created by the
hidden_def/hidden_proto macro being used on '__foo', this is what the
code in libc.so (or: whatever DSO the symbol is hidden_def'ed in)
calls to avoid PLT jumps.

Q: why '__foo', why not use hidden_def/hidden_proto on the 'foo' directly?
A: that would only work for code that ends up in libc.so (or rather,
the same DSOs as 'foo'), but we still want other code to also call the
non-interposed, original version of 'foo'

Is that ^^ correct? Should each and every function that is exported to
the user and also used inside libc's own code have '__foo' and
'__GI___foo' versions? What does 'GI' even stand for?

Thanks in advance,
Sergey

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Florian Weimer]

* Sergey Bugaev:

> On Thu, May 18, 2023 at 11:16 PM Joseph Myers <joseph@codesourcery.com> wrote:
>> Strictly there are two separate issues: (a) calling an exported symbol
>> should be done via a hidden alias, to avoid going via the PLT, and (b)
>> functions in a standard namespace should not call names in the user's
>> namespace, which requires using a name such as __hurd_thread_self instead
>> (which should also be marked hidden to make the call optimally efficient).
>
> While we're talking about this, could you please say if my
> understanding below is correct (and correct me if not)?
>
> 'foo' is a public symbol, to be used by the user, and also
> interposable by the user. Always called via PLT (except from inside
> the user's code when redefined inside the executable, in which case
> the compiler/linker will see that no PLT is needed), and should not be
> called from inside glibc.
>
> '__foo' is a (public? private? semi-private?) symbol, the user code is
> not supposed to use it, but it's exported from libc.so for the benefit
> of other glibc code that ends up in different DSOs and still wants to
> call the original version even when 'foo' gets interposed. Invoked via
> PLT from other DSOs, not invoked from libc.so because of...

__foo may be exported or not.  We have some __ symbols that are used by
the implementation (so GCC etc.), and probably some that are expected to
be used by users.  Truely exported symbols have a GLIBC_2.y symbol
version, internal exported symbols (usually added for coordination
between different shared objects that make up glibc) have a
GLIBC_PRIVATE symbol version.

Some old internal symbols have GLIBC_2.0 or similar early versions
because GLIBC_PRIVATE did not exist then.

> '__GI___foo' is a private non-exported symbol created by the
> hidden_def/hidden_proto macro being used on '__foo', this is what the
> code in libc.so (or: whatever DSO the symbol is hidden_def'ed in)
> calls to avoid PLT jumps.

Correct.

> Q: why '__foo', why not use hidden_def/hidden_proto on the 'foo' directly?
> A: that would only work for code that ends up in libc.so (or rather,
> the same DSOs as 'foo'), but we still want other code to also call the
> non-interposed, original version of 'foo'

hidden_def/hidden_proto does not do anything for static linking, given
the macros are defined today.

It's of course possible to do all this quite differently.

Thanks,
Florian

Re: [RFC PATCH 06/10] hurd: Make sure to not use tcb->self

[Joseph Myers]

On Fri, 19 May 2023, Sergey Bugaev via Libc-alpha wrote:

> 'foo' is a public symbol, to be used by the user, and also
> interposable by the user. Always called via PLT (except from inside
> the user's code when redefined inside the executable, in which case
> the compiler/linker will see that no PLT is needed), and should not be
> called from inside glibc.

Should not be called from within glibc via the PLT within the same 
library.

It's OK for foo to be called from bar if foo is part of all the standards 
that contain bar.  But in that case, if the call is from the same library, 
*_hidden_def / *_hidden_proto should be used to avoid calling via the PLT.

If foo is not part of all the standards that contain bar, then glibc code 
for bar should use __foo instead to avoid namespace issues, especially for 
static linking.

If __foo is needed by executables, *_nonshared.a or other shared 
libraries, then it also needs to be exported from the library it's in (in 
which case PLT avoidance is also relevant for __foo, so *_hidden_* should 
be used for __foo).

If __foo is only used within a single library and doesn't need to be 
exported, it's OK to declare it directly with attribute_hidden rather than 
using *_hidden_* to create __GI___foo as an alias.  (In this case, if you 
forget to use attribute_hidden, you won't get test failures - the symbol 
in fact won't get exported, because only symbols explicitly listed in the 
version maps get exported, and so it won't get a PLT entry.  But in some 
cases, code generation is more efficient if the compiler knows that the 
symbol is hidden.  So when you're calling an unexported symbol in the same 
library, it's still desirable for it to be declared as hidden in a way 
visible to the compiler.)  The more complicated hidden_proto / hidden_def 
approach also works for unexported symbols used within a single library, 
it's just more complicated than necessary in that case.

-- 
Joseph S. Myers
joseph@codesourcery.com

[PATCH] hurd: Fix using interposable hurd_thread_self

[Sergey Bugaev]

Create a private hidden __hurd_thread_self alias, and use that one.

Fixes 2f8ecb58a59eb82c43214d000842d99644a662d1
"hurd: Fix x86_64 _hurd_tls_fork" and
c7fcce38c83a2bb665ef5dc4981bf20c7e586123
"hurd: Make sure to not use tcb->self"

Reported-by: Joseph Myers <joseph@codesourcery.com>
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---

This should hopefully fix it (but please test!); I have only checked
that it builds on x86_64-gnu, and that _Fork now calls
__GI___hurd_thread_self without the PLT, and that libpthread.so only
references __hurd_thread_self@GLIBC_PRIVATE.

 hurd/Versions                           | 1 +
 hurd/thread-self.c                      | 5 ++++-
 sysdeps/hurd/include/hurd.h             | 3 +++
 sysdeps/mach/hurd/i386/htl/pt-setup.c   | 2 +-
 sysdeps/mach/hurd/x86_64/htl/pt-setup.c | 2 +-
 sysdeps/mach/hurd/x86_64/tls.h          | 7 +++++--
 6 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hurd/Versions b/hurd/Versions
index 3d8b412d..439e8abf 100644
--- a/hurd/Versions
+++ b/hurd/Versions
@@ -156,6 +156,7 @@ libc {
     __libc_open; __libc_close;
 
     # Used by libpthread.
+    __hurd_thread_self;
     _hurd_sigstate_set_global_rcv;
     _hurd_sigstate_lock;
     _hurd_sigstate_pending;
diff --git a/hurd/thread-self.c b/hurd/thread-self.c
index f3718165..af013503 100644
--- a/hurd/thread-self.c
+++ b/hurd/thread-self.c
@@ -19,7 +19,10 @@
 #include <hurd/signal.h>
 
 thread_t
-hurd_thread_self (void)
+__hurd_thread_self (void)
 {
   return _hurd_self_sigstate ()->thread;
 }
+
+libc_hidden_def (__hurd_thread_self)
+weak_alias (__hurd_thread_self, hurd_thread_self)
diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h
index 7da9af26..5d904e0e 100644
--- a/sysdeps/hurd/include/hurd.h
+++ b/sysdeps/hurd/include/hurd.h
@@ -11,5 +11,8 @@ void _hurd_libc_proc_init (char **argv);
 libc_hidden_proto (_hurd_exec_paths)
 libc_hidden_proto (_hurd_init)
 libc_hidden_proto (_hurd_libc_proc_init)
+
+extern thread_t __hurd_thread_self (void);
+libc_hidden_proto (__hurd_thread_self)
 #endif
 #endif
diff --git a/sysdeps/mach/hurd/i386/htl/pt-setup.c b/sysdeps/mach/hurd/i386/htl/pt-setup.c
index ba108b96..27e5c98b 100644
--- a/sysdeps/mach/hurd/i386/htl/pt-setup.c
+++ b/sysdeps/mach/hurd/i386/htl/pt-setup.c
@@ -79,7 +79,7 @@ __pthread_setup (struct __pthread *thread,
 {
   error_t err;
 
-  if (thread->kernel_thread == hurd_thread_self ())
+  if (thread->kernel_thread == __hurd_thread_self ())
     /* Fix up the TCB for the main thread.  The C library has already
        installed a TCB, which we want to keep using.  This TCB must not
        be freed so don't register it in the thread structure.  On the
diff --git a/sysdeps/mach/hurd/x86_64/htl/pt-setup.c b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
index 7dc62912..8eebaf97 100644
--- a/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
+++ b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
@@ -56,7 +56,7 @@ __pthread_setup (struct __pthread *thread,
   struct i386_thread_state state;
   struct i386_fsgs_base_state fsgs_state;
 
-  if (thread->kernel_thread == hurd_thread_self ())
+  if (thread->kernel_thread == __hurd_thread_self ())
     /* Fix up the TCB for the main thread.  The C library has already
        installed a TCB, which we want to keep using.  This TCB must not
        be freed so don't register it in the thread structure.  On the
diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
index 468f703f..a5a49a47 100644
--- a/sysdeps/mach/hurd/x86_64/tls.h
+++ b/sysdeps/mach/hurd/x86_64/tls.h
@@ -132,6 +132,10 @@ THREAD_TCB (thread_t thread,
   ((descr)->pointer_guard						\
    = THREAD_GETMEM (THREAD_SELF, pointer_guard))
 
+/* From hurd.h, reproduced here to avoid a circular include.  */
+extern thread_t __hurd_thread_self (void);
+libc_hidden_proto (__hurd_thread_self)
+
 /* 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,
@@ -141,8 +145,7 @@ _hurd_tls_fork (thread_t child, thread_t orig,
   struct i386_fsgs_base_state state;
   mach_msg_type_number_t state_count = i386_FSGS_BASE_STATE_COUNT;
 
-  extern thread_t hurd_thread_self (void);
-  if (orig != hurd_thread_self ())
+  if (orig != __hurd_thread_self ())
     {
       err = __thread_get_state (orig, i386_FSGS_BASE_STATE,
                                 (thread_state_t) &state,
-- 
2.40.1

Re: [PATCH] hurd: Fix using interposable hurd_thread_self

[Samuel Thibault]

Checked and applied, thanks!

Sergey Bugaev, le ven. 19 mai 2023 17:47:24 +0300, a ecrit:
> Create a private hidden __hurd_thread_self alias, and use that one.
> 
> Fixes 2f8ecb58a59eb82c43214d000842d99644a662d1
> "hurd: Fix x86_64 _hurd_tls_fork" and
> c7fcce38c83a2bb665ef5dc4981bf20c7e586123
> "hurd: Make sure to not use tcb->self"
> 
> Reported-by: Joseph Myers <joseph@codesourcery.com>
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
> 
> This should hopefully fix it (but please test!); I have only checked
> that it builds on x86_64-gnu, and that _Fork now calls
> __GI___hurd_thread_self without the PLT, and that libpthread.so only
> references __hurd_thread_self@GLIBC_PRIVATE.
> 
>  hurd/Versions                           | 1 +
>  hurd/thread-self.c                      | 5 ++++-
>  sysdeps/hurd/include/hurd.h             | 3 +++
>  sysdeps/mach/hurd/i386/htl/pt-setup.c   | 2 +-
>  sysdeps/mach/hurd/x86_64/htl/pt-setup.c | 2 +-
>  sysdeps/mach/hurd/x86_64/tls.h          | 7 +++++--
>  6 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/hurd/Versions b/hurd/Versions
> index 3d8b412d..439e8abf 100644
> --- a/hurd/Versions
> +++ b/hurd/Versions
> @@ -156,6 +156,7 @@ libc {
>      __libc_open; __libc_close;
>  
>      # Used by libpthread.
> +    __hurd_thread_self;
>      _hurd_sigstate_set_global_rcv;
>      _hurd_sigstate_lock;
>      _hurd_sigstate_pending;
> diff --git a/hurd/thread-self.c b/hurd/thread-self.c
> index f3718165..af013503 100644
> --- a/hurd/thread-self.c
> +++ b/hurd/thread-self.c
> @@ -19,7 +19,10 @@
>  #include <hurd/signal.h>
>  
>  thread_t
> -hurd_thread_self (void)
> +__hurd_thread_self (void)
>  {
>    return _hurd_self_sigstate ()->thread;
>  }
> +
> +libc_hidden_def (__hurd_thread_self)
> +weak_alias (__hurd_thread_self, hurd_thread_self)
> diff --git a/sysdeps/hurd/include/hurd.h b/sysdeps/hurd/include/hurd.h
> index 7da9af26..5d904e0e 100644
> --- a/sysdeps/hurd/include/hurd.h
> +++ b/sysdeps/hurd/include/hurd.h
> @@ -11,5 +11,8 @@ void _hurd_libc_proc_init (char **argv);
>  libc_hidden_proto (_hurd_exec_paths)
>  libc_hidden_proto (_hurd_init)
>  libc_hidden_proto (_hurd_libc_proc_init)
> +
> +extern thread_t __hurd_thread_self (void);
> +libc_hidden_proto (__hurd_thread_self)
>  #endif
>  #endif
> diff --git a/sysdeps/mach/hurd/i386/htl/pt-setup.c b/sysdeps/mach/hurd/i386/htl/pt-setup.c
> index ba108b96..27e5c98b 100644
> --- a/sysdeps/mach/hurd/i386/htl/pt-setup.c
> +++ b/sysdeps/mach/hurd/i386/htl/pt-setup.c
> @@ -79,7 +79,7 @@ __pthread_setup (struct __pthread *thread,
>  {
>    error_t err;
>  
> -  if (thread->kernel_thread == hurd_thread_self ())
> +  if (thread->kernel_thread == __hurd_thread_self ())
>      /* Fix up the TCB for the main thread.  The C library has already
>         installed a TCB, which we want to keep using.  This TCB must not
>         be freed so don't register it in the thread structure.  On the
> diff --git a/sysdeps/mach/hurd/x86_64/htl/pt-setup.c b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
> index 7dc62912..8eebaf97 100644
> --- a/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
> +++ b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
> @@ -56,7 +56,7 @@ __pthread_setup (struct __pthread *thread,
>    struct i386_thread_state state;
>    struct i386_fsgs_base_state fsgs_state;
>  
> -  if (thread->kernel_thread == hurd_thread_self ())
> +  if (thread->kernel_thread == __hurd_thread_self ())
>      /* Fix up the TCB for the main thread.  The C library has already
>         installed a TCB, which we want to keep using.  This TCB must not
>         be freed so don't register it in the thread structure.  On the
> diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
> index 468f703f..a5a49a47 100644
> --- a/sysdeps/mach/hurd/x86_64/tls.h
> +++ b/sysdeps/mach/hurd/x86_64/tls.h
> @@ -132,6 +132,10 @@ THREAD_TCB (thread_t thread,
>    ((descr)->pointer_guard						\
>     = THREAD_GETMEM (THREAD_SELF, pointer_guard))
>  
> +/* From hurd.h, reproduced here to avoid a circular include.  */
> +extern thread_t __hurd_thread_self (void);
> +libc_hidden_proto (__hurd_thread_self)
> +
>  /* 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,
> @@ -141,8 +145,7 @@ _hurd_tls_fork (thread_t child, thread_t orig,
>    struct i386_fsgs_base_state state;
>    mach_msg_type_number_t state_count = i386_FSGS_BASE_STATE_COUNT;
>  
> -  extern thread_t hurd_thread_self (void);
> -  if (orig != hurd_thread_self ())
> +  if (orig != __hurd_thread_self ())
>      {
>        err = __thread_get_state (orig, i386_FSGS_BASE_STATE,
>                                  (thread_state_t) &state,
> -- 
> 2.40.1
> 

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

[PATCH 07/10] hurd: Fix x86_64 _hurd_tls_fork

[Sergey Bugaev]

It is illegal to call thread_get_state () on mach_thread_self (), so
this codepath cannot be used as-is to fork the calling thread's TLS.
Fortunately we can use THREAD_SELF (aka %fs:0x0) to find out the value
of our fs_base without calling into the kernel.

Fixes: f6cf701efc61c9ad910372bda14b9a235db310a8
"hurd: Implement TLS for x86_64"

Checked on x86_64-gnu: fork () now works!

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
It would be nice if GNU Mach allowed

thread_get_state (mach_thread_self (), i386_FSGS_BASE_STATE)

like it already allows thread_set_state for this case. But even if
gnumach adds suppport for this, doing it in userspace without an extra
RPC is still better.

 sysdeps/mach/hurd/x86_64/tls.h | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
index 35dcef44..6487ed35 100644
--- a/sysdeps/mach/hurd/x86_64/tls.h
+++ b/sysdeps/mach/hurd/x86_64/tls.h
@@ -140,12 +140,25 @@ _hurd_tls_fork (thread_t child, thread_t orig,
   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);
+
+  extern thread_t hurd_thread_self (void);
+  if (orig != hurd_thread_self ())
+    {
+      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);
+    }
+  else
+    {
+      /* It is illegal to call thread_get_state () on mach_thread_self ().
+         But we're only interested in the value of fs_base, and since we're
+         this thread, we know it points to our TCB.  */
+      state.fs_base = (unsigned long) THREAD_SELF;
+      state.gs_base = 0;
+    }
 
   return __thread_set_state (child, i386_FSGS_BASE_STATE,
                              (thread_state_t) &state,
-- 
2.40.1

Re: [PATCH 07/10] hurd: Fix x86_64 _hurd_tls_fork

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le mer. 17 mai 2023 22:14:33 +0300, a ecrit:
> It is illegal to call thread_get_state () on mach_thread_self (), so
> this codepath cannot be used as-is to fork the calling thread's TLS.
> Fortunately we can use THREAD_SELF (aka %fs:0x0) to find out the value
> of our fs_base without calling into the kernel.
> 
> Fixes: f6cf701efc61c9ad910372bda14b9a235db310a8
> "hurd: Implement TLS for x86_64"
> 
> Checked on x86_64-gnu: fork () now works!
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
> It would be nice if GNU Mach allowed
> 
> thread_get_state (mach_thread_self (), i386_FSGS_BASE_STATE)
> 
> like it already allows thread_set_state for this case. But even if
> gnumach adds suppport for this, doing it in userspace without an extra
> RPC is still better.
> 
>  sysdeps/mach/hurd/x86_64/tls.h | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
> index 35dcef44..6487ed35 100644
> --- a/sysdeps/mach/hurd/x86_64/tls.h
> +++ b/sysdeps/mach/hurd/x86_64/tls.h
> @@ -140,12 +140,25 @@ _hurd_tls_fork (thread_t child, thread_t orig,
>    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);
> +
> +  extern thread_t hurd_thread_self (void);
> +  if (orig != hurd_thread_self ())
> +    {
> +      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);
> +    }
> +  else
> +    {
> +      /* It is illegal to call thread_get_state () on mach_thread_self ().
> +         But we're only interested in the value of fs_base, and since we're
> +         this thread, we know it points to our TCB.  */
> +      state.fs_base = (unsigned long) THREAD_SELF;
> +      state.gs_base = 0;
> +    }
>  
>    return __thread_set_state (child, i386_FSGS_BASE_STATE,
>                               (thread_state_t) &state,
> -- 
> 2.40.1
> 

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

[PATCH 08/10] hurd: Fix setting up pthreads

[Sergey Bugaev]

On x86_64, we have to pass function arguments in registers, not on the
stack. We also have to align the stack pointer in a specific way. Since
sharing the logic with i386 does not bring much benefit, split the file
back into i386- and x86_64-specific versions, and fix the x86_64 version
to set up the thread properly.

Bonus: i386 keeps doing the extra RPC inside __thread_set_pcsptp to
fetch the state of the thread before setting it; but x86_64 no lnoger
does that.

Checked on x86_64-gnu and i686-gnu.

Fixes be6d002ca277ffc90058d382396150cb0e785b9c
"hurd: Set up the basic tree for x86_64-gnu"

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
__thread_set_pcsptp is also used by pthread_cancel (), which suffers
from the same issue. Eventually I'd like to remove __thread_set_pcsptp
entirely.

 .../mach/hurd/{x86 => i386}/htl/pt-setup.c    |  2 +-
 sysdeps/mach/hurd/x86_64/htl/pt-setup.c       | 93 +++++++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)
 rename sysdeps/mach/hurd/{x86 => i386}/htl/pt-setup.c (98%)
 create mode 100644 sysdeps/mach/hurd/x86_64/htl/pt-setup.c

diff --git a/sysdeps/mach/hurd/x86/htl/pt-setup.c b/sysdeps/mach/hurd/i386/htl/pt-setup.c
similarity index 98%
rename from sysdeps/mach/hurd/x86/htl/pt-setup.c
rename to sysdeps/mach/hurd/i386/htl/pt-setup.c
index 686124d7..ba108b96 100644
--- a/sysdeps/mach/hurd/x86/htl/pt-setup.c
+++ b/sysdeps/mach/hurd/i386/htl/pt-setup.c
@@ -1,4 +1,4 @@
-/* Setup thread stack.  Hurd/x86 version.
+/* Setup thread stack.  Hurd/i386 version.
    Copyright (C) 2000-2023 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
diff --git a/sysdeps/mach/hurd/x86_64/htl/pt-setup.c b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
new file mode 100644
index 00000000..7dc62912
--- /dev/null
+++ b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
@@ -0,0 +1,93 @@
+/* Setup thread stack.  Hurd/x86_64 version.
+   Copyright (C) 2000-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/>.  */
+
+#include <stdint.h>
+#include <assert.h>
+#include <mach.h>
+#include <hurd.h>
+
+#include <thread_state.h>
+#include <pt-internal.h>
+
+/* Set up the stack for THREAD.  Return the stack pointer
+   for the new thread.  */
+static void *
+stack_setup (struct __pthread *thread)
+{
+  error_t err;
+  uintptr_t bottom, top;
+
+  /* Calculate the top of the new stack.  */
+  bottom = (uintptr_t) thread->stackaddr;
+  top = bottom + thread->stacksize + round_page (thread->guardsize);
+
+  if (thread->guardsize)
+    {
+      err = __vm_protect (__mach_task_self (), (vm_address_t) bottom,
+			  thread->guardsize, 0, 0);
+      assert_perror (err);
+    }
+
+  return (void *) PTR_ALIGN_DOWN_8_16 (top);
+}
+
+int
+__pthread_setup (struct __pthread *thread,
+		 void (*entry_point) (struct __pthread *, void *(*)(void *),
+				      void *), void *(*start_routine) (void *),
+		 void *arg)
+{
+  error_t err;
+  struct i386_thread_state state;
+  struct i386_fsgs_base_state fsgs_state;
+
+  if (thread->kernel_thread == hurd_thread_self ())
+    /* Fix up the TCB for the main thread.  The C library has already
+       installed a TCB, which we want to keep using.  This TCB must not
+       be freed so don't register it in the thread structure.  On the
+       other hand, it's not yet possible to reliably release a TCB.
+       Leave the unused one registered so that it doesn't leak.  */
+    return 0;
+
+
+  thread->mcontext.pc = entry_point;
+  thread->mcontext.sp = stack_setup (thread);
+
+  /* Set up the state to call entry_point (thread, start_routine, arg) */
+  memset (&state, 0, sizeof (state));
+  state.ursp = (uintptr_t) thread->mcontext.sp;
+  state.rip = (uintptr_t) thread->mcontext.pc;
+  state.rdi = (uintptr_t) thread;
+  state.rsi = (uintptr_t) start_routine;
+  state.rdx = (uintptr_t) arg;
+
+  err = __thread_set_state (thread->kernel_thread, i386_THREAD_STATE,
+			    (thread_state_t) &state,
+			    i386_THREAD_STATE_COUNT);
+  assert_perror (err);
+
+  /* Set fs_base to the TCB pointer for the thread.  */
+  memset (&fsgs_state, 0, sizeof (fsgs_state));
+  fsgs_state.fs_base = (uintptr_t) thread->tcb;
+  err = __thread_set_state (thread->kernel_thread, i386_FSGS_BASE_STATE,
+			    (thread_state_t) &fsgs_state,
+			    i386_FSGS_BASE_STATE_COUNT);
+  assert_perror (err);
+
+  return 0;
+}
-- 
2.40.1

Re: [PATCH 08/10] hurd: Fix setting up pthreads

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le mer. 17 mai 2023 22:14:34 +0300, a ecrit:
> On x86_64, we have to pass function arguments in registers, not on the
> stack. We also have to align the stack pointer in a specific way. Since
> sharing the logic with i386 does not bring much benefit, split the file
> back into i386- and x86_64-specific versions, and fix the x86_64 version
> to set up the thread properly.
> 
> Bonus: i386 keeps doing the extra RPC inside __thread_set_pcsptp to
> fetch the state of the thread before setting it; but x86_64 no lnoger
> does that.
> 
> Checked on x86_64-gnu and i686-gnu.
> 
> Fixes be6d002ca277ffc90058d382396150cb0e785b9c
> "hurd: Set up the basic tree for x86_64-gnu"
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
> __thread_set_pcsptp is also used by pthread_cancel (), which suffers
> from the same issue. Eventually I'd like to remove __thread_set_pcsptp
> entirely.
> 
>  .../mach/hurd/{x86 => i386}/htl/pt-setup.c    |  2 +-
>  sysdeps/mach/hurd/x86_64/htl/pt-setup.c       | 93 +++++++++++++++++++
>  2 files changed, 94 insertions(+), 1 deletion(-)
>  rename sysdeps/mach/hurd/{x86 => i386}/htl/pt-setup.c (98%)
>  create mode 100644 sysdeps/mach/hurd/x86_64/htl/pt-setup.c
> 
> diff --git a/sysdeps/mach/hurd/x86/htl/pt-setup.c b/sysdeps/mach/hurd/i386/htl/pt-setup.c
> similarity index 98%
> rename from sysdeps/mach/hurd/x86/htl/pt-setup.c
> rename to sysdeps/mach/hurd/i386/htl/pt-setup.c
> index 686124d7..ba108b96 100644
> --- a/sysdeps/mach/hurd/x86/htl/pt-setup.c
> +++ b/sysdeps/mach/hurd/i386/htl/pt-setup.c
> @@ -1,4 +1,4 @@
> -/* Setup thread stack.  Hurd/x86 version.
> +/* Setup thread stack.  Hurd/i386 version.
>     Copyright (C) 2000-2023 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> diff --git a/sysdeps/mach/hurd/x86_64/htl/pt-setup.c b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
> new file mode 100644
> index 00000000..7dc62912
> --- /dev/null
> +++ b/sysdeps/mach/hurd/x86_64/htl/pt-setup.c
> @@ -0,0 +1,93 @@
> +/* Setup thread stack.  Hurd/x86_64 version.
> +   Copyright (C) 2000-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/>.  */
> +
> +#include <stdint.h>
> +#include <assert.h>
> +#include <mach.h>
> +#include <hurd.h>
> +
> +#include <thread_state.h>
> +#include <pt-internal.h>
> +
> +/* Set up the stack for THREAD.  Return the stack pointer
> +   for the new thread.  */
> +static void *
> +stack_setup (struct __pthread *thread)
> +{
> +  error_t err;
> +  uintptr_t bottom, top;
> +
> +  /* Calculate the top of the new stack.  */
> +  bottom = (uintptr_t) thread->stackaddr;
> +  top = bottom + thread->stacksize + round_page (thread->guardsize);
> +
> +  if (thread->guardsize)
> +    {
> +      err = __vm_protect (__mach_task_self (), (vm_address_t) bottom,
> +			  thread->guardsize, 0, 0);
> +      assert_perror (err);
> +    }
> +
> +  return (void *) PTR_ALIGN_DOWN_8_16 (top);
> +}
> +
> +int
> +__pthread_setup (struct __pthread *thread,
> +		 void (*entry_point) (struct __pthread *, void *(*)(void *),
> +				      void *), void *(*start_routine) (void *),
> +		 void *arg)
> +{
> +  error_t err;
> +  struct i386_thread_state state;
> +  struct i386_fsgs_base_state fsgs_state;
> +
> +  if (thread->kernel_thread == hurd_thread_self ())
> +    /* Fix up the TCB for the main thread.  The C library has already
> +       installed a TCB, which we want to keep using.  This TCB must not
> +       be freed so don't register it in the thread structure.  On the
> +       other hand, it's not yet possible to reliably release a TCB.
> +       Leave the unused one registered so that it doesn't leak.  */
> +    return 0;
> +
> +
> +  thread->mcontext.pc = entry_point;
> +  thread->mcontext.sp = stack_setup (thread);
> +
> +  /* Set up the state to call entry_point (thread, start_routine, arg) */
> +  memset (&state, 0, sizeof (state));
> +  state.ursp = (uintptr_t) thread->mcontext.sp;
> +  state.rip = (uintptr_t) thread->mcontext.pc;
> +  state.rdi = (uintptr_t) thread;
> +  state.rsi = (uintptr_t) start_routine;
> +  state.rdx = (uintptr_t) arg;
> +
> +  err = __thread_set_state (thread->kernel_thread, i386_THREAD_STATE,
> +			    (thread_state_t) &state,
> +			    i386_THREAD_STATE_COUNT);
> +  assert_perror (err);
> +
> +  /* Set fs_base to the TCB pointer for the thread.  */
> +  memset (&fsgs_state, 0, sizeof (fsgs_state));
> +  fsgs_state.fs_base = (uintptr_t) thread->tcb;
> +  err = __thread_set_state (thread->kernel_thread, i386_FSGS_BASE_STATE,
> +			    (thread_state_t) &fsgs_state,
> +			    i386_FSGS_BASE_STATE_COUNT);
> +  assert_perror (err);
> +
> +  return 0;
> +}
> -- 
> 2.40.1
> 
> 

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

[PATCH 09/10] hurd: Also make it possible to call strlen very early

[Sergey Bugaev]

strlen, which is another ifunc-selected function, is invoked during
early static executable startup if the argv arrives from the exec
server. Make it not crash.

Checked on x86_64-gnu: statically linked executables launched after the
exec server is up now start up successfully.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/x86_64/static-start.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sysdeps/mach/hurd/x86_64/static-start.S b/sysdeps/mach/hurd/x86_64/static-start.S
index cc8e2410..0fed375c 100644
--- a/sysdeps/mach/hurd/x86_64/static-start.S
+++ b/sysdeps/mach/hurd/x86_64/static-start.S
@@ -22,6 +22,9 @@ _start:
 
 	leaq __memcpy_sse2_unaligned(%rip), %rax
 	movq %rax, memcpy@GOTPCREL(%rip)
+	leaq __strlen_sse2(%rip), %rax
+	movq %rax, strlen@GOTPCREL(%rip)
+
 	call _hurd_stack_setup
 	xorq %rdx, %rdx
 	jmp _start1
-- 
2.40.1

Re: [PATCH 09/10] hurd: Also make it possible to call strlen very early

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le mer. 17 mai 2023 22:14:35 +0300, a ecrit:
> strlen, which is another ifunc-selected function, is invoked during
> early static executable startup if the argv arrives from the exec
> server. Make it not crash.
> 
> Checked on x86_64-gnu: statically linked executables launched after the
> exec server is up now start up successfully.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/x86_64/static-start.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sysdeps/mach/hurd/x86_64/static-start.S b/sysdeps/mach/hurd/x86_64/static-start.S
> index cc8e2410..0fed375c 100644
> --- a/sysdeps/mach/hurd/x86_64/static-start.S
> +++ b/sysdeps/mach/hurd/x86_64/static-start.S
> @@ -22,6 +22,9 @@ _start:
>  
>  	leaq __memcpy_sse2_unaligned(%rip), %rax
>  	movq %rax, memcpy@GOTPCREL(%rip)
> +	leaq __strlen_sse2(%rip), %rax
> +	movq %rax, strlen@GOTPCREL(%rip)
> +
>  	call _hurd_stack_setup
>  	xorq %rdx, %rdx
>  	jmp _start1
> -- 
> 2.40.1
> 
> 

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

[RFC PATCH 10/10] hurd: Regenerate errno.h

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
My build keeps insisting that stdc-predef.h should be next to sysdeps and
sys/cdefs.h; maybe it's right? If not, how do I stop it from changing?

 sysdeps/mach/hurd/bits/errno.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/mach/hurd/bits/errno.h b/sysdeps/mach/hurd/bits/errno.h
index a0794f96..080daebe 100644
--- a/sysdeps/mach/hurd/bits/errno.h
+++ b/sysdeps/mach/hurd/bits/errno.h
@@ -1,6 +1,5 @@
 /* This file generated by errnos.awk from
      errno.texi
-     stdc-predef.h
      libc-symbols.h
      mach/message.h
      mach/kern_return.h
@@ -12,6 +11,7 @@
      features.h
      features-time64.h
      ../sysdeps/generic/features-time64.h
+     stdc-predef.h
      sys/cdefs.h
      ../misc/sys/cdefs.h
      ../sysdeps/x86/bits/wordsize.h
-- 
2.40.1

Re: [RFC PATCH 10/10] hurd: Regenerate errno.h

[Joseph Myers]

On Wed, 17 May 2023, Sergey Bugaev via Libc-alpha wrote:

> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
> My build keeps insisting that stdc-predef.h should be next to sysdeps and
> sys/cdefs.h; maybe it's right? If not, how do I stop it from changing?

I suggest changing the generator code not to put this list in the file at 
all.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC PATCH 10/10] hurd: Regenerate errno.h

[Samuel Thibault]

Joseph Myers, le mer. 17 mai 2023 19:39:23 +0000, a ecrit:
> On Wed, 17 May 2023, Sergey Bugaev via Libc-alpha wrote:
> 
> > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> > ---
> > My build keeps insisting that stdc-predef.h should be next to sysdeps and
> > sys/cdefs.h; maybe it's right? If not, how do I stop it from changing?
> 
> I suggest changing the generator code not to put this list in the file at 
> all.

Yes, if it poses problem we can just rule it out.

Samuel