[RFC PATCH 0/12] Towards glibc on x86_64-gnu

[RFC PATCH 0/12] Towards glibc on x86_64-gnu

[Sergey Bugaev]

Hello!

Seeing that the patches to make binutils and GCC support --target=x86_64-gnu
have recently landed, I thought it would be interesting to try to configure
glibc with --host=x86_64-gnu and see what breaks.

To make things work at least somewhat, I've tried to put some basic directory
& Implies structure in place, then fixed various 64-bit issues in generic code
(these fixes should hopefully be uncontroversial!). Then I have even tried to
actually write some of the missing arch-specific code for x86_64-gnu.

With this patch series, I'm able to 'make install-headers' successfully.
Moreover, I'm able to 'make io/subdir_objs' successfully - which builds a lot
of Hurd-specific, but mostly arch-independent, routines. 'subdir_objs' in
'mach', 'hurd', and 'htl' do not succeed, but a lof of files do get built.

The main missing pieces seem to be, ordered by scariness increasing:
- missing x86_64 thread state definition in the kernel headers;
- TLS things;
- INTR_MSG_TRAP.

Thread state (i.e. a struct with all the register values) should not be that
hard for GNU Mach to define (unless it is defined somewhere already and I'm
just not seeing it).

It seems that GCC expects TLS on x86_64 to be done relative to %fs, not %gs, so
that's what I attempted to do in tls.h. The main thing missing there is the
ability to actually set (and read) the %fs base address of a thread. It is my
understanding (but note that I have no idea what I'm talking about) that on
x86_64 the segment descriptors (as in GDT/LDT) are not used for this, and
instead the address can be set by writing to a MSR. Linux exposes the
arch_prctl (ARCH_[GS]ET_[FG]S) syscall for this; so maybe GNU Mach could also
have an explicit routine for this, perhaps like this:

routine i386_set_fgs_base (
	target_thread: thread_t;
	which: int;
	value: rpc_vm_address_t);

We should not need a getter routine, because one can simply inspect the target
thread's state (unless, again, I misunderstand things horribly).

Anyway; this is an RFC. I don't expect the latter patches to actually get
merged. "I have no idea what I'm doing" applies. Maybe I'm doing things
horribly wrong; tell me!

Note: I'm sending this as a single patch series that consists of patches for
the glibc, MIG, and the Hurd repos. The repo that a patch is for is indicated
in the subject line of each patch.

Sergey

[RFC PATCH glibc 1/12] hurd: Refactor readlinkat()

[Sergey Bugaev]

Make the code flow more linear using early returns where possible. This
makes it so much easier to reason about what runs on error / successful
code paths.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/readlinkat.c | 55 ++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/sysdeps/mach/hurd/readlinkat.c b/sysdeps/mach/hurd/readlinkat.c
index 1efb09ca..dabdbb37 100644
--- a/sysdeps/mach/hurd/readlinkat.c
+++ b/sysdeps/mach/hurd/readlinkat.c
@@ -31,38 +31,47 @@ __readlinkat (int fd, const char *file_name, char *buf, size_t len)
   error_t err;
   file_t file_stat;
   struct stat64 st;
+  enum retry_type doretry;
+  char retryname[1024];
+  file_t file;
+  char *rbuf = buf;
 
   file_stat = __file_name_lookup_at (fd, 0, file_name, O_NOLINK, 0);
   if (file_stat == MACH_PORT_NULL)
     return -1;
 
   err = __io_stat (file_stat, &st);
-  if (! err)
-    if (S_ISLNK (st.st_mode))
-      {
-	enum retry_type doretry;
-	char retryname[1024];
-	file_t file;
-	char *rbuf = buf;
+  if (err)
+    goto out;
+  if (!S_ISLNK (st.st_mode))
+    {
+      err = EINVAL;
+      goto out;
+    }
 
-	err = __dir_lookup (file_stat, "", O_READ | O_NOLINK, 0, &doretry, retryname, &file);
-	if (! err && (doretry != FS_RETRY_NORMAL || retryname[0] != '\0'))
-	  err = EGRATUITOUS;
-	if (! err)
-	  {
-	    err = __io_read (file, &rbuf, &len, 0, len);
-	    if (!err && rbuf != buf)
-	      {
-		memcpy (buf, rbuf, len);
-		__vm_deallocate (__mach_task_self (), (vm_address_t)rbuf, len);
-	      }
+  err = __dir_lookup (file_stat, "", O_READ | O_NOLINK,
+                      0, &doretry, retryname, &file);
+  if (err)
+    goto out;
+  if (doretry != FS_RETRY_NORMAL || retryname[0] != '\0')
+    {
+      err = EGRATUITOUS;
+      goto out;
+    }
+
+  err = __io_read (file, &rbuf, &len, 0, len);
+  __mach_port_deallocate (__mach_task_self (), file);
+  if (err)
+    goto out;
+
+  if (rbuf != buf)
+    {
+      memcpy (buf, rbuf, len);
+      __vm_deallocate (__mach_task_self (), (vm_address_t) rbuf, len);
+    }
 
-	    __mach_port_deallocate (__mach_task_self (), file);
-	  }
-      }
-    else
-      err = EINVAL;
 
+ out:
   __mach_port_deallocate (__mach_task_self (), file_stat);
 
   return err ? __hurd_fail (err) : len;
-- 
2.39.1

Re: [RFC PATCH glibc 1/12] hurd: Refactor readlinkat()

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le dim. 12 févr. 2023 14:10:32 +0300, a ecrit:
> Make the code flow more linear using early returns where possible. This
> makes it so much easier to reason about what runs on error / successful
> code paths.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/readlinkat.c | 55 ++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 23 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/readlinkat.c b/sysdeps/mach/hurd/readlinkat.c
> index 1efb09ca..dabdbb37 100644
> --- a/sysdeps/mach/hurd/readlinkat.c
> +++ b/sysdeps/mach/hurd/readlinkat.c
> @@ -31,38 +31,47 @@ __readlinkat (int fd, const char *file_name, char *buf, size_t len)
>    error_t err;
>    file_t file_stat;
>    struct stat64 st;
> +  enum retry_type doretry;
> +  char retryname[1024];
> +  file_t file;
> +  char *rbuf = buf;
>  
>    file_stat = __file_name_lookup_at (fd, 0, file_name, O_NOLINK, 0);
>    if (file_stat == MACH_PORT_NULL)
>      return -1;
>  
>    err = __io_stat (file_stat, &st);
> -  if (! err)
> -    if (S_ISLNK (st.st_mode))
> -      {
> -	enum retry_type doretry;
> -	char retryname[1024];
> -	file_t file;
> -	char *rbuf = buf;
> +  if (err)
> +    goto out;
> +  if (!S_ISLNK (st.st_mode))
> +    {
> +      err = EINVAL;
> +      goto out;
> +    }
>  
> -	err = __dir_lookup (file_stat, "", O_READ | O_NOLINK, 0, &doretry, retryname, &file);
> -	if (! err && (doretry != FS_RETRY_NORMAL || retryname[0] != '\0'))
> -	  err = EGRATUITOUS;
> -	if (! err)
> -	  {
> -	    err = __io_read (file, &rbuf, &len, 0, len);
> -	    if (!err && rbuf != buf)
> -	      {
> -		memcpy (buf, rbuf, len);
> -		__vm_deallocate (__mach_task_self (), (vm_address_t)rbuf, len);
> -	      }
> +  err = __dir_lookup (file_stat, "", O_READ | O_NOLINK,
> +                      0, &doretry, retryname, &file);
> +  if (err)
> +    goto out;
> +  if (doretry != FS_RETRY_NORMAL || retryname[0] != '\0')
> +    {
> +      err = EGRATUITOUS;
> +      goto out;
> +    }
> +
> +  err = __io_read (file, &rbuf, &len, 0, len);
> +  __mach_port_deallocate (__mach_task_self (), file);
> +  if (err)
> +    goto out;
> +
> +  if (rbuf != buf)
> +    {
> +      memcpy (buf, rbuf, len);
> +      __vm_deallocate (__mach_task_self (), (vm_address_t) rbuf, len);
> +    }
>  
> -	    __mach_port_deallocate (__mach_task_self (), file);
> -	  }
> -      }
> -    else
> -      err = EINVAL;
>  
> + out:
>    __mach_port_deallocate (__mach_task_self (), file_stat);
>  
>    return err ? __hurd_fail (err) : len;
> -- 
> 2.39.1
> 

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

[RFC PATCH glibc 2/12] hurd: Use mach_msg_type_number_t where appropriate

[Sergey Bugaev]

It has been decided that on x86_64, mach_msg_type_number_t stays 32-bit.
Therefore, it's not possible to use mach_msg_type_number_t
interchangeably with size_t, in particular this breaks when a pointer to
a variable is passed to a MIG routine.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 hurd/hurdioctl.c               | 2 +-
 hurd/hurdprio.c                | 2 +-
 hurd/lookup-retry.c            | 2 +-
 hurd/xattr.c                   | 4 ++--
 sysdeps/mach/hurd/getcwd.c     | 2 +-
 sysdeps/mach/hurd/readlinkat.c | 6 ++++--
 sysdeps/mach/hurd/sendfile64.c | 2 +-
 7 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/hurd/hurdioctl.c b/hurd/hurdioctl.c
index e9a6a7ea..59e6a5c1 100644
--- a/hurd/hurdioctl.c
+++ b/hurd/hurdioctl.c
@@ -311,7 +311,7 @@ static int
 siocgifconf (int fd, int request, struct ifconf *ifc)
 {
   error_t err;
-  size_t data_len = ifc->ifc_len;
+  mach_msg_type_number_t data_len = ifc->ifc_len;
   char *data = ifc->ifc_buf;
 
   if (data_len <= 0)
diff --git a/hurd/hurdprio.c b/hurd/hurdprio.c
index dbeb272b..954d3987 100644
--- a/hurd/hurdprio.c
+++ b/hurd/hurdprio.c
@@ -58,7 +58,7 @@ _hurd_priority_which_map (enum __priority_which which, int who,
 	  int *oldpi = pi;
 	  mach_msg_type_number_t oldpisize = pisize;
 	  char *tw = 0;
-	  size_t twsz = 0;
+	  mach_msg_type_number_t twsz = 0;
 	  err = __USEPORT (PROC, __proc_getprocinfo (port, pids[i],
 						     &pi_flags,
 						     &pi, &pisize,
diff --git a/hurd/lookup-retry.c b/hurd/lookup-retry.c
index 0d344026..8850c4fd 100644
--- a/hurd/lookup-retry.c
+++ b/hurd/lookup-retry.c
@@ -162,7 +162,7 @@ __hurd_file_name_lookup_retry (error_t (*use_init_port)
 			    {
 			      char buf[1024];
 			      char *trans = buf;
-			      size_t translen = sizeof buf;
+			      mach_msg_type_number_t translen = sizeof buf;
 			      err = __file_get_translator (*result,
 							   &trans, &translen);
 			      if (!err
diff --git a/hurd/xattr.c b/hurd/xattr.c
index f98e548c..48914bcf 100644
--- a/hurd/xattr.c
+++ b/hurd/xattr.c
@@ -60,7 +60,7 @@ _hurd_xattr_get (io_t port, const char *name, void *value, size_t *size)
   if (!strcmp (name, "translator"))
     {
       char *buf = value;
-      size_t bufsz = value ? *size : 0;
+      mach_msg_type_number_t bufsz = value ? *size : 0;
       error_t err = __file_get_translator (port, &buf, &bufsz);
       if (err)
 	return err;
@@ -144,7 +144,7 @@ _hurd_xattr_set (io_t port, const char *name, const void *value, size_t size,
 	{
 	  /* Must make sure it's already there.  */
 	  char *buf = NULL;
-	  size_t bufsz = 0;
+	  mach_msg_type_number_t bufsz = 0;
 	  error_t err = __file_get_translator (port, &buf, &bufsz);
 	  if (err)
 	    return err;
diff --git a/sysdeps/mach/hurd/getcwd.c b/sysdeps/mach/hurd/getcwd.c
index fc5e78e7..f24b35b3 100644
--- a/sysdeps/mach/hurd/getcwd.c
+++ b/sysdeps/mach/hurd/getcwd.c
@@ -117,7 +117,7 @@ __hurd_canonicalize_directory_name_internal (file_t thisdir,
       int mount_point;
       file_t newp;
       char *dirdata;
-      size_t dirdatasize;
+      mach_msg_type_number_t dirdatasize;
       int direntry, nentries;
 
 
diff --git a/sysdeps/mach/hurd/readlinkat.c b/sysdeps/mach/hurd/readlinkat.c
index dabdbb37..5bb8b753 100644
--- a/sysdeps/mach/hurd/readlinkat.c
+++ b/sysdeps/mach/hurd/readlinkat.c
@@ -35,6 +35,7 @@ __readlinkat (int fd, const char *file_name, char *buf, size_t len)
   char retryname[1024];
   file_t file;
   char *rbuf = buf;
+  mach_msg_type_number_t nread = len;
 
   file_stat = __file_name_lookup_at (fd, 0, file_name, O_NOLINK, 0);
   if (file_stat == MACH_PORT_NULL)
@@ -59,15 +60,16 @@ __readlinkat (int fd, const char *file_name, char *buf, size_t len)
       goto out;
     }
 
-  err = __io_read (file, &rbuf, &len, 0, len);
+  err = __io_read (file, &rbuf, &nread, 0, len);
   __mach_port_deallocate (__mach_task_self (), file);
   if (err)
     goto out;
 
+  len = nread;
   if (rbuf != buf)
     {
       memcpy (buf, rbuf, len);
-      __vm_deallocate (__mach_task_self (), (vm_address_t) rbuf, len);
+      __vm_deallocate (__mach_task_self (), (vm_address_t) rbuf, nread);
     }
 
 
diff --git a/sysdeps/mach/hurd/sendfile64.c b/sysdeps/mach/hurd/sendfile64.c
index 658b0282..e36b9790 100644
--- a/sysdeps/mach/hurd/sendfile64.c
+++ b/sysdeps/mach/hurd/sendfile64.c
@@ -35,7 +35,7 @@ __sendfile64 (int out_fd, int in_fd, off64_t *offset, size_t count)
      which might blow if it's huge or address space is real tight.  */
 
   char *data = 0;
-  size_t datalen = 0;
+  mach_msg_type_number_t datalen = 0;
   error_t err = HURD_DPORT_USE (in_fd,
 				__io_read (port, &data, &datalen,
 					   offset ? *offset : (off_t) -1,
-- 
2.39.1

Re: [RFC PATCH glibc 2/12] hurd: Use mach_msg_type_number_t where appropriate

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le dim. 12 févr. 2023 14:10:33 +0300, a ecrit:
> It has been decided that on x86_64, mach_msg_type_number_t stays 32-bit.
> Therefore, it's not possible to use mach_msg_type_number_t
> interchangeably with size_t, in particular this breaks when a pointer to
> a variable is passed to a MIG routine.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  hurd/hurdioctl.c               | 2 +-
>  hurd/hurdprio.c                | 2 +-
>  hurd/lookup-retry.c            | 2 +-
>  hurd/xattr.c                   | 4 ++--
>  sysdeps/mach/hurd/getcwd.c     | 2 +-
>  sysdeps/mach/hurd/readlinkat.c | 6 ++++--
>  sysdeps/mach/hurd/sendfile64.c | 2 +-
>  7 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/hurd/hurdioctl.c b/hurd/hurdioctl.c
> index e9a6a7ea..59e6a5c1 100644
> --- a/hurd/hurdioctl.c
> +++ b/hurd/hurdioctl.c
> @@ -311,7 +311,7 @@ static int
>  siocgifconf (int fd, int request, struct ifconf *ifc)
>  {
>    error_t err;
> -  size_t data_len = ifc->ifc_len;
> +  mach_msg_type_number_t data_len = ifc->ifc_len;
>    char *data = ifc->ifc_buf;
>  
>    if (data_len <= 0)
> diff --git a/hurd/hurdprio.c b/hurd/hurdprio.c
> index dbeb272b..954d3987 100644
> --- a/hurd/hurdprio.c
> +++ b/hurd/hurdprio.c
> @@ -58,7 +58,7 @@ _hurd_priority_which_map (enum __priority_which which, int who,
>  	  int *oldpi = pi;
>  	  mach_msg_type_number_t oldpisize = pisize;
>  	  char *tw = 0;
> -	  size_t twsz = 0;
> +	  mach_msg_type_number_t twsz = 0;
>  	  err = __USEPORT (PROC, __proc_getprocinfo (port, pids[i],
>  						     &pi_flags,
>  						     &pi, &pisize,
> diff --git a/hurd/lookup-retry.c b/hurd/lookup-retry.c
> index 0d344026..8850c4fd 100644
> --- a/hurd/lookup-retry.c
> +++ b/hurd/lookup-retry.c
> @@ -162,7 +162,7 @@ __hurd_file_name_lookup_retry (error_t (*use_init_port)
>  			    {
>  			      char buf[1024];
>  			      char *trans = buf;
> -			      size_t translen = sizeof buf;
> +			      mach_msg_type_number_t translen = sizeof buf;
>  			      err = __file_get_translator (*result,
>  							   &trans, &translen);
>  			      if (!err
> diff --git a/hurd/xattr.c b/hurd/xattr.c
> index f98e548c..48914bcf 100644
> --- a/hurd/xattr.c
> +++ b/hurd/xattr.c
> @@ -60,7 +60,7 @@ _hurd_xattr_get (io_t port, const char *name, void *value, size_t *size)
>    if (!strcmp (name, "translator"))
>      {
>        char *buf = value;
> -      size_t bufsz = value ? *size : 0;
> +      mach_msg_type_number_t bufsz = value ? *size : 0;
>        error_t err = __file_get_translator (port, &buf, &bufsz);
>        if (err)
>  	return err;
> @@ -144,7 +144,7 @@ _hurd_xattr_set (io_t port, const char *name, const void *value, size_t size,
>  	{
>  	  /* Must make sure it's already there.  */
>  	  char *buf = NULL;
> -	  size_t bufsz = 0;
> +	  mach_msg_type_number_t bufsz = 0;
>  	  error_t err = __file_get_translator (port, &buf, &bufsz);
>  	  if (err)
>  	    return err;
> diff --git a/sysdeps/mach/hurd/getcwd.c b/sysdeps/mach/hurd/getcwd.c
> index fc5e78e7..f24b35b3 100644
> --- a/sysdeps/mach/hurd/getcwd.c
> +++ b/sysdeps/mach/hurd/getcwd.c
> @@ -117,7 +117,7 @@ __hurd_canonicalize_directory_name_internal (file_t thisdir,
>        int mount_point;
>        file_t newp;
>        char *dirdata;
> -      size_t dirdatasize;
> +      mach_msg_type_number_t dirdatasize;
>        int direntry, nentries;
>  
>  
> diff --git a/sysdeps/mach/hurd/readlinkat.c b/sysdeps/mach/hurd/readlinkat.c
> index dabdbb37..5bb8b753 100644
> --- a/sysdeps/mach/hurd/readlinkat.c
> +++ b/sysdeps/mach/hurd/readlinkat.c
> @@ -35,6 +35,7 @@ __readlinkat (int fd, const char *file_name, char *buf, size_t len)
>    char retryname[1024];
>    file_t file;
>    char *rbuf = buf;
> +  mach_msg_type_number_t nread = len;
>  
>    file_stat = __file_name_lookup_at (fd, 0, file_name, O_NOLINK, 0);
>    if (file_stat == MACH_PORT_NULL)
> @@ -59,15 +60,16 @@ __readlinkat (int fd, const char *file_name, char *buf, size_t len)
>        goto out;
>      }
>  
> -  err = __io_read (file, &rbuf, &len, 0, len);
> +  err = __io_read (file, &rbuf, &nread, 0, len);
>    __mach_port_deallocate (__mach_task_self (), file);
>    if (err)
>      goto out;
>  
> +  len = nread;
>    if (rbuf != buf)
>      {
>        memcpy (buf, rbuf, len);
> -      __vm_deallocate (__mach_task_self (), (vm_address_t) rbuf, len);
> +      __vm_deallocate (__mach_task_self (), (vm_address_t) rbuf, nread);
>      }
>  
>  
> diff --git a/sysdeps/mach/hurd/sendfile64.c b/sysdeps/mach/hurd/sendfile64.c
> index 658b0282..e36b9790 100644
> --- a/sysdeps/mach/hurd/sendfile64.c
> +++ b/sysdeps/mach/hurd/sendfile64.c
> @@ -35,7 +35,7 @@ __sendfile64 (int out_fd, int in_fd, off64_t *offset, size_t count)
>       which might blow if it's huge or address space is real tight.  */
>  
>    char *data = 0;
> -  size_t datalen = 0;
> +  mach_msg_type_number_t datalen = 0;
>    error_t err = HURD_DPORT_USE (in_fd,
>  				__io_read (port, &data, &datalen,
>  					   offset ? *offset : (off_t) -1,
> -- 
> 2.39.1
> 

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

[RFC PATCH glibc 3/12] mach, hurd: Cast through uintptr_t

[Sergey Bugaev]

When casting between a pointer and an integer of a different size, GCC
emits a warning (which is escalated to a build failure by -Werror).
Indeed, if what you start with is a pointer, which you then cast to a
shorter integer and then back again, you're going to cut off some bits
of the pointer.

But if you start with an integer (such as mach_port_t), then cast it to
a longer pointer (void *), and then back to a shorter integer, you are
fine. To keep GCC happy, cast through an intermediary uintptr_t, which
is always the same size as a pointer.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 htl/cthreads-compat.c |  5 +++--
 hurd/fopenport.c      | 15 ++++++++++-----
 hurd/hurd/port.h      |  2 +-
 hurd/port-cleanup.c   |  3 ++-
 hurd/vpprintf.c       |  6 ++++--
 mach/devstream.c      |  9 +++++----
 6 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/htl/cthreads-compat.c b/htl/cthreads-compat.c
index 55043a8b..2a0a95b6 100644
--- a/htl/cthreads-compat.c
+++ b/htl/cthreads-compat.c
@@ -25,8 +25,9 @@ void
 __cthread_detach (__cthread_t thread)
 {
   int err;
+  pthread_t pthread = (pthread_t) (uintptr_t) thread;
 
-  err = __pthread_detach ((pthread_t) thread);
+  err = __pthread_detach (pthread);
   assert_perror (err);
 }
 weak_alias (__cthread_detach, cthread_detach)
@@ -40,7 +41,7 @@ __cthread_fork (__cthread_fn_t func, void *arg)
   err = __pthread_create (&thread, NULL, func, arg);
   assert_perror (err);
 
-  return (__cthread_t) thread;
+  return (__cthread_t) (uintptr_t) thread;
 }
 weak_alias (__cthread_fork, cthread_fork)
 
diff --git a/hurd/fopenport.c b/hurd/fopenport.c
index 60afb445..be6aa30c 100644
--- a/hurd/fopenport.c
+++ b/hurd/fopenport.c
@@ -28,9 +28,10 @@ readio (void *cookie, char *buf, size_t n)
   mach_msg_type_number_t nread;
   error_t err;
   char *bufp = buf;
+  io_t io = (io_t) (uintptr_t) cookie;
 
   nread = n;
-  if (err = __io_read ((io_t) cookie, &bufp, &nread, -1, n))
+  if (err = __io_read (io, &bufp, &nread, -1, n))
     return __hurd_fail (err);
 
   if (bufp != buf)
@@ -50,8 +51,9 @@ writeio (void *cookie, const char *buf, size_t n)
 {
   vm_size_t wrote;
   error_t err;
+  io_t io = (io_t) (uintptr_t) cookie;
 
-  if (err = __io_write ((io_t) cookie, buf, n, -1, &wrote))
+  if (err = __io_write (io, buf, n, -1, &wrote))
     return __hurd_fail (err);
 
   return wrote;
@@ -65,7 +67,8 @@ seekio (void *cookie,
 	off64_t *pos,
 	int whence)
 {
-  error_t err = __io_seek ((file_t) cookie, *pos, whence, pos);
+  io_t io = (io_t) (uintptr_t) cookie;
+  error_t err = __io_seek (io, *pos, whence, pos);
   return err ? __hurd_fail (err) : 0;
 }
 
@@ -74,8 +77,9 @@ seekio (void *cookie,
 static int
 closeio (void *cookie)
 {
+  io_t io = (io_t) (uintptr_t) cookie;
   error_t error = __mach_port_deallocate (__mach_task_self (),
-					  (mach_port_t) cookie);
+					  io);
   if (error)
     return __hurd_fail (error);
   return 0;
@@ -127,6 +131,7 @@ __fopenport (mach_port_t port, const char *mode)
       return NULL;
     }
 
-  return fopencookie ((void *) port, mode, funcsio);
+  return fopencookie ((void *) (uintptr_t) port,
+                      mode, funcsio);
 }
 weak_alias (__fopenport, fopenport)
diff --git a/hurd/hurd/port.h b/hurd/hurd/port.h
index fdba8db5..1e473978 100644
--- a/hurd/hurd/port.h
+++ b/hurd/hurd/port.h
@@ -96,7 +96,7 @@ _hurd_port_locked_get (struct hurd_port *port,
   if (result != MACH_PORT_NULL)
     {
       link->cleanup = &_hurd_port_cleanup;
-      link->cleanup_data = (void *) result;
+      link->cleanup_data = (void *) (uintptr_t) result;
       _hurd_userlink_link (&port->users, link);
     }
   __spin_unlock (&port->lock);
diff --git a/hurd/port-cleanup.c b/hurd/port-cleanup.c
index 08ab3d47..ad43b830 100644
--- a/hurd/port-cleanup.c
+++ b/hurd/port-cleanup.c
@@ -26,7 +26,8 @@
 void
 _hurd_port_cleanup (void *cleanup_data, jmp_buf env, int val)
 {
-  __mach_port_deallocate (__mach_task_self (), (mach_port_t) cleanup_data);
+  mach_port_t port = (mach_port_t) (uintptr_t) cleanup_data;
+  __mach_port_deallocate (__mach_task_self (), port);
 }
 
 /* We were cancelled while using a port, and called from the cleanup unwinding.
diff --git a/hurd/vpprintf.c b/hurd/vpprintf.c
index fe734946..010f04ca 100644
--- a/hurd/vpprintf.c
+++ b/hurd/vpprintf.c
@@ -25,8 +25,9 @@
 static ssize_t
 do_write (void *cookie,	const char *buf, size_t n)
 {
+  io_t io = (io_t) (uintptr_t) cookie;
   vm_size_t amount = n;
-  error_t error = __io_write ((io_t) cookie, buf, n, -1, &amount);
+  error_t error = __io_write (io, buf, n, -1, &amount);
   if (error)
     return __hurd_fail (error);
   return n;
@@ -51,7 +52,8 @@ vpprintf (io_t port, const char *format, va_list arg)
 #endif
 
   _IO_cookie_init (&temp_f.cfile, _IO_NO_READS,
-		   (void *) port, (cookie_io_functions_t) { write: do_write });
+                   (void *) (uintptr_t) port,
+                   (cookie_io_functions_t) { write: do_write });
 
   done = __vfprintf_internal (&temp_f.cfile.__fp.file, format, arg, 0);
 
diff --git a/mach/devstream.c b/mach/devstream.c
index 09272d10..cfa9a4f4 100644
--- a/mach/devstream.c
+++ b/mach/devstream.c
@@ -28,7 +28,7 @@
 static ssize_t
 devstream_write (void *cookie, const char *buffer, size_t n)
 {
-  const device_t dev = (device_t) cookie;
+  const device_t dev = (device_t) (uintptr_t) cookie;
 
   int write_some (const char *p, size_t to_write)
     {
@@ -83,7 +83,7 @@ devstream_write (void *cookie, const char *buffer, size_t n)
 static ssize_t
 devstream_read (void *cookie, char *buffer, size_t to_read)
 {
-  const device_t dev = (device_t) cookie;
+  const device_t dev = (device_t) (uintptr_t) cookie;
 
   kern_return_t err;
   mach_msg_type_number_t nread = to_read;
@@ -112,7 +112,8 @@ devstream_read (void *cookie, char *buffer, size_t to_read)
 static int
 dealloc_ref (void *cookie)
 {
-  if (__mach_port_deallocate (mach_task_self (), (mach_port_t) cookie))
+  const device_t dev = (device_t) (uintptr_t) cookie;
+  if (__mach_port_deallocate (mach_task_self (), dev))
     {
       errno = EINVAL;
       return -1;
@@ -131,7 +132,7 @@ mach_open_devstream (mach_port_t dev, const char *mode)
       return NULL;
     }
 
-  stream = _IO_fopencookie ((void *) dev, mode,
+  stream = _IO_fopencookie ((void *) (uintptr_t) dev, mode,
 			    (cookie_io_functions_t) { write: devstream_write,
 						      read: devstream_read,
 						      close: dealloc_ref });
-- 
2.39.1

Re: [RFC PATCH glibc 3/12] mach, hurd: Cast through uintptr_t

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le dim. 12 févr. 2023 14:10:34 +0300, a ecrit:
> When casting between a pointer and an integer of a different size, GCC
> emits a warning (which is escalated to a build failure by -Werror).
> Indeed, if what you start with is a pointer, which you then cast to a
> shorter integer and then back again, you're going to cut off some bits
> of the pointer.
> 
> But if you start with an integer (such as mach_port_t), then cast it to
> a longer pointer (void *), and then back to a shorter integer, you are
> fine. To keep GCC happy, cast through an intermediary uintptr_t, which
> is always the same size as a pointer.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  htl/cthreads-compat.c |  5 +++--
>  hurd/fopenport.c      | 15 ++++++++++-----
>  hurd/hurd/port.h      |  2 +-
>  hurd/port-cleanup.c   |  3 ++-
>  hurd/vpprintf.c       |  6 ++++--
>  mach/devstream.c      |  9 +++++----
>  6 files changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/htl/cthreads-compat.c b/htl/cthreads-compat.c
> index 55043a8b..2a0a95b6 100644
> --- a/htl/cthreads-compat.c
> +++ b/htl/cthreads-compat.c
> @@ -25,8 +25,9 @@ void
>  __cthread_detach (__cthread_t thread)
>  {
>    int err;
> +  pthread_t pthread = (pthread_t) (uintptr_t) thread;
>  
> -  err = __pthread_detach ((pthread_t) thread);
> +  err = __pthread_detach (pthread);
>    assert_perror (err);
>  }
>  weak_alias (__cthread_detach, cthread_detach)
> @@ -40,7 +41,7 @@ __cthread_fork (__cthread_fn_t func, void *arg)
>    err = __pthread_create (&thread, NULL, func, arg);
>    assert_perror (err);
>  
> -  return (__cthread_t) thread;
> +  return (__cthread_t) (uintptr_t) thread;
>  }
>  weak_alias (__cthread_fork, cthread_fork)
>  
> diff --git a/hurd/fopenport.c b/hurd/fopenport.c
> index 60afb445..be6aa30c 100644
> --- a/hurd/fopenport.c
> +++ b/hurd/fopenport.c
> @@ -28,9 +28,10 @@ readio (void *cookie, char *buf, size_t n)
>    mach_msg_type_number_t nread;
>    error_t err;
>    char *bufp = buf;
> +  io_t io = (io_t) (uintptr_t) cookie;
>  
>    nread = n;
> -  if (err = __io_read ((io_t) cookie, &bufp, &nread, -1, n))
> +  if (err = __io_read (io, &bufp, &nread, -1, n))
>      return __hurd_fail (err);
>  
>    if (bufp != buf)
> @@ -50,8 +51,9 @@ writeio (void *cookie, const char *buf, size_t n)
>  {
>    vm_size_t wrote;
>    error_t err;
> +  io_t io = (io_t) (uintptr_t) cookie;
>  
> -  if (err = __io_write ((io_t) cookie, buf, n, -1, &wrote))
> +  if (err = __io_write (io, buf, n, -1, &wrote))
>      return __hurd_fail (err);
>  
>    return wrote;
> @@ -65,7 +67,8 @@ seekio (void *cookie,
>  	off64_t *pos,
>  	int whence)
>  {
> -  error_t err = __io_seek ((file_t) cookie, *pos, whence, pos);
> +  io_t io = (io_t) (uintptr_t) cookie;
> +  error_t err = __io_seek (io, *pos, whence, pos);
>    return err ? __hurd_fail (err) : 0;
>  }
>  
> @@ -74,8 +77,9 @@ seekio (void *cookie,
>  static int
>  closeio (void *cookie)
>  {
> +  io_t io = (io_t) (uintptr_t) cookie;
>    error_t error = __mach_port_deallocate (__mach_task_self (),
> -					  (mach_port_t) cookie);
> +					  io);
>    if (error)
>      return __hurd_fail (error);
>    return 0;
> @@ -127,6 +131,7 @@ __fopenport (mach_port_t port, const char *mode)
>        return NULL;
>      }
>  
> -  return fopencookie ((void *) port, mode, funcsio);
> +  return fopencookie ((void *) (uintptr_t) port,
> +                      mode, funcsio);
>  }
>  weak_alias (__fopenport, fopenport)
> diff --git a/hurd/hurd/port.h b/hurd/hurd/port.h
> index fdba8db5..1e473978 100644
> --- a/hurd/hurd/port.h
> +++ b/hurd/hurd/port.h
> @@ -96,7 +96,7 @@ _hurd_port_locked_get (struct hurd_port *port,
>    if (result != MACH_PORT_NULL)
>      {
>        link->cleanup = &_hurd_port_cleanup;
> -      link->cleanup_data = (void *) result;
> +      link->cleanup_data = (void *) (uintptr_t) result;
>        _hurd_userlink_link (&port->users, link);
>      }
>    __spin_unlock (&port->lock);
> diff --git a/hurd/port-cleanup.c b/hurd/port-cleanup.c
> index 08ab3d47..ad43b830 100644
> --- a/hurd/port-cleanup.c
> +++ b/hurd/port-cleanup.c
> @@ -26,7 +26,8 @@
>  void
>  _hurd_port_cleanup (void *cleanup_data, jmp_buf env, int val)
>  {
> -  __mach_port_deallocate (__mach_task_self (), (mach_port_t) cleanup_data);
> +  mach_port_t port = (mach_port_t) (uintptr_t) cleanup_data;
> +  __mach_port_deallocate (__mach_task_self (), port);
>  }
>  
>  /* We were cancelled while using a port, and called from the cleanup unwinding.
> diff --git a/hurd/vpprintf.c b/hurd/vpprintf.c
> index fe734946..010f04ca 100644
> --- a/hurd/vpprintf.c
> +++ b/hurd/vpprintf.c
> @@ -25,8 +25,9 @@
>  static ssize_t
>  do_write (void *cookie,	const char *buf, size_t n)
>  {
> +  io_t io = (io_t) (uintptr_t) cookie;
>    vm_size_t amount = n;
> -  error_t error = __io_write ((io_t) cookie, buf, n, -1, &amount);
> +  error_t error = __io_write (io, buf, n, -1, &amount);
>    if (error)
>      return __hurd_fail (error);
>    return n;
> @@ -51,7 +52,8 @@ vpprintf (io_t port, const char *format, va_list arg)
>  #endif
>  
>    _IO_cookie_init (&temp_f.cfile, _IO_NO_READS,
> -		   (void *) port, (cookie_io_functions_t) { write: do_write });
> +                   (void *) (uintptr_t) port,
> +                   (cookie_io_functions_t) { write: do_write });
>  
>    done = __vfprintf_internal (&temp_f.cfile.__fp.file, format, arg, 0);
>  
> diff --git a/mach/devstream.c b/mach/devstream.c
> index 09272d10..cfa9a4f4 100644
> --- a/mach/devstream.c
> +++ b/mach/devstream.c
> @@ -28,7 +28,7 @@
>  static ssize_t
>  devstream_write (void *cookie, const char *buffer, size_t n)
>  {
> -  const device_t dev = (device_t) cookie;
> +  const device_t dev = (device_t) (uintptr_t) cookie;
>  
>    int write_some (const char *p, size_t to_write)
>      {
> @@ -83,7 +83,7 @@ devstream_write (void *cookie, const char *buffer, size_t n)
>  static ssize_t
>  devstream_read (void *cookie, char *buffer, size_t to_read)
>  {
> -  const device_t dev = (device_t) cookie;
> +  const device_t dev = (device_t) (uintptr_t) cookie;
>  
>    kern_return_t err;
>    mach_msg_type_number_t nread = to_read;
> @@ -112,7 +112,8 @@ devstream_read (void *cookie, char *buffer, size_t to_read)
>  static int
>  dealloc_ref (void *cookie)
>  {
> -  if (__mach_port_deallocate (mach_task_self (), (mach_port_t) cookie))
> +  const device_t dev = (device_t) (uintptr_t) cookie;
> +  if (__mach_port_deallocate (mach_task_self (), dev))
>      {
>        errno = EINVAL;
>        return -1;
> @@ -131,7 +132,7 @@ mach_open_devstream (mach_port_t dev, const char *mode)
>        return NULL;
>      }
>  
> -  stream = _IO_fopencookie ((void *) dev, mode,
> +  stream = _IO_fopencookie ((void *) (uintptr_t) dev, mode,
>  			    (cookie_io_functions_t) { write: devstream_write,
>  						      read: devstream_read,
>  						      close: dealloc_ref });
> -- 
> 2.39.1
> 
> 

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

[RFC PATCH glibc 4/12] hurd: Fix xattr error value

[Sergey Bugaev]

This does not seem like it is supposed to return negative error codes.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 hurd/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hurd/xattr.c b/hurd/xattr.c
index 48914bcf..5a0fc263 100644
--- a/hurd/xattr.c
+++ b/hurd/xattr.c
@@ -68,7 +68,7 @@ _hurd_xattr_get (io_t port, const char *name, void *value, size_t *size)
 	{
 	  if (buf != value)
 	    __munmap (buf, bufsz);
-	  return -ERANGE;
+	  return ERANGE;
 	}
       if (buf != value && bufsz > 0)
 	{
-- 
2.39.1

Re: [RFC PATCH glibc 4/12] hurd: Fix xattr error value

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le dim. 12 févr. 2023 14:10:35 +0300, a ecrit:
> This does not seem like it is supposed to return negative error codes.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  hurd/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hurd/xattr.c b/hurd/xattr.c
> index 48914bcf..5a0fc263 100644
> --- a/hurd/xattr.c
> +++ b/hurd/xattr.c
> @@ -68,7 +68,7 @@ _hurd_xattr_get (io_t port, const char *name, void *value, size_t *size)
>  	{
>  	  if (buf != value)
>  	    __munmap (buf, bufsz);
> -	  return -ERANGE;
> +	  return ERANGE;
>  	}
>        if (buf != value && bufsz > 0)
>  	{
> -- 
> 2.39.1
> 
> 

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

[RFC PATCH glibc 5/12] htl: Fix semaphore reference

[Sergey Bugaev]

'sem' is the opaque 'sem_t', 'isem' is the actual 'struct new_sem'.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/htl/sem-timedwait.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sysdeps/htl/sem-timedwait.c b/sysdeps/htl/sem-timedwait.c
index 8f2df6e7..9974e9ae 100644
--- a/sysdeps/htl/sem-timedwait.c
+++ b/sysdeps/htl/sem-timedwait.c
@@ -60,7 +60,7 @@ __sem_timedwait_internal (sem_t *restrict sem,
   int cancel_oldtype = LIBC_CANCEL_ASYNC();
 
 #if __HAVE_64B_ATOMICS
-  uint64_t d = atomic_fetch_add_relaxed (&sem->data,
+  uint64_t d = atomic_fetch_add_relaxed (&isem->data,
 		 (uint64_t) 1 << SEM_NWAITERS_SHIFT);
 
   pthread_cleanup_push (__sem_wait_cleanup, isem);
@@ -72,11 +72,11 @@ __sem_timedwait_internal (sem_t *restrict sem,
 	  /* No token, sleep.  */
 	  if (timeout)
 	    err = __lll_abstimed_wait_intr (
-		      ((unsigned int *) &sem->data) + SEM_VALUE_OFFSET,
+		      ((unsigned int *) &isem->data) + SEM_VALUE_OFFSET,
 		      0, timeout, flags, clock_id);
 	  else
 	    err = __lll_wait_intr (
-		      ((unsigned int *) &sem->data) + SEM_VALUE_OFFSET,
+		      ((unsigned int *) &isem->data) + SEM_VALUE_OFFSET,
 		      0, flags);
 
 	  if (err != 0 && err != KERN_INVALID_ARGUMENT)
@@ -92,12 +92,12 @@ __sem_timedwait_internal (sem_t *restrict sem,
 	    }
 
 	  /* Token changed */
-	  d = atomic_load_relaxed (&sem->data);
+	  d = atomic_load_relaxed (&isem->data);
 	}
       else
 	{
 	  /* Try to acquire and dequeue.  */
-	  if (atomic_compare_exchange_weak_acquire (&sem->data,
+	  if (atomic_compare_exchange_weak_acquire (&isem->data,
 	      &d, d - 1 - ((uint64_t) 1 << SEM_NWAITERS_SHIFT)))
 	    {
 	      /* Success */
-- 
2.39.1

Re: [RFC PATCH glibc 5/12] htl: Fix semaphore reference

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le dim. 12 févr. 2023 14:10:36 +0300, a ecrit:
> 'sem' is the opaque 'sem_t', 'isem' is the actual 'struct new_sem'.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/htl/sem-timedwait.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/sysdeps/htl/sem-timedwait.c b/sysdeps/htl/sem-timedwait.c
> index 8f2df6e7..9974e9ae 100644
> --- a/sysdeps/htl/sem-timedwait.c
> +++ b/sysdeps/htl/sem-timedwait.c
> @@ -60,7 +60,7 @@ __sem_timedwait_internal (sem_t *restrict sem,
>    int cancel_oldtype = LIBC_CANCEL_ASYNC();
>  
>  #if __HAVE_64B_ATOMICS
> -  uint64_t d = atomic_fetch_add_relaxed (&sem->data,
> +  uint64_t d = atomic_fetch_add_relaxed (&isem->data,
>  		 (uint64_t) 1 << SEM_NWAITERS_SHIFT);
>  
>    pthread_cleanup_push (__sem_wait_cleanup, isem);
> @@ -72,11 +72,11 @@ __sem_timedwait_internal (sem_t *restrict sem,
>  	  /* No token, sleep.  */
>  	  if (timeout)
>  	    err = __lll_abstimed_wait_intr (
> -		      ((unsigned int *) &sem->data) + SEM_VALUE_OFFSET,
> +		      ((unsigned int *) &isem->data) + SEM_VALUE_OFFSET,
>  		      0, timeout, flags, clock_id);
>  	  else
>  	    err = __lll_wait_intr (
> -		      ((unsigned int *) &sem->data) + SEM_VALUE_OFFSET,
> +		      ((unsigned int *) &isem->data) + SEM_VALUE_OFFSET,
>  		      0, flags);
>  
>  	  if (err != 0 && err != KERN_INVALID_ARGUMENT)
> @@ -92,12 +92,12 @@ __sem_timedwait_internal (sem_t *restrict sem,
>  	    }
>  
>  	  /* Token changed */
> -	  d = atomic_load_relaxed (&sem->data);
> +	  d = atomic_load_relaxed (&isem->data);
>  	}
>        else
>  	{
>  	  /* Try to acquire and dequeue.  */
> -	  if (atomic_compare_exchange_weak_acquire (&sem->data,
> +	  if (atomic_compare_exchange_weak_acquire (&isem->data,
>  	      &d, d - 1 - ((uint64_t) 1 << SEM_NWAITERS_SHIFT)))
>  	    {
>  	      /* Success */
> -- 
> 2.39.1
> 
> 

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

[RFC PATCH hurd 6/12] hurd: Fix modes_t and speeds_t types on 64-bit

[Sergey Bugaev]

---
 hurd/tioctl.defs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hurd/tioctl.defs b/hurd/tioctl.defs
index 905a4a38..a04f5ae4 100644
--- a/hurd/tioctl.defs
+++ b/hurd/tioctl.defs
@@ -34,9 +34,9 @@ import <hurd/ioctl_types.h>; /* XXX */
 
 /* These are the pieces of a struct termios as specified by the
    definition of _IOT_termios in <termbits.h>. */
-type modes_t = array[4] of int;
+type modes_t = array[4] of long;
 type ccs_t = array[20] of char;
-type speeds_t = array[2] of int;
+type speeds_t = array[2] of long;
 
 /* This is the arg for a struct winsize as specified by the
    definition of _IOT_winsize in <sys/ioctl.h>. */
-- 
2.39.1

Re: [RFC PATCH hurd 6/12] hurd: Fix modes_t and speeds_t types on 64-bit

[Samuel Thibault]

We don't need these to be 64bits, 32bit will be completely large enough.

Which issue did you actually encounter?

Samuel

Sergey Bugaev, le dim. 12 févr. 2023 14:10:37 +0300, a ecrit:
> ---
>  hurd/tioctl.defs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hurd/tioctl.defs b/hurd/tioctl.defs
> index 905a4a38..a04f5ae4 100644
> --- a/hurd/tioctl.defs
> +++ b/hurd/tioctl.defs
> @@ -34,9 +34,9 @@ import <hurd/ioctl_types.h>; /* XXX */
>  
>  /* These are the pieces of a struct termios as specified by the
>     definition of _IOT_termios in <termbits.h>. */
> -type modes_t = array[4] of int;
> +type modes_t = array[4] of long;
>  type ccs_t = array[20] of char;
> -type speeds_t = array[2] of int;
> +type speeds_t = array[2] of long;
>  
>  /* This is the arg for a struct winsize as specified by the
>     definition of _IOT_winsize in <sys/ioctl.h>. */
> -- 
> 2.39.1

Re: [RFC PATCH hurd 6/12] hurd: Fix modes_t and speeds_t types on 64-bit

[Sergey Bugaev]

On Sun, Feb 12, 2023 at 6:00 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> We don't need these to be 64bits, 32bit will be completely large enough.
>
> Which issue did you actually encounter?

Well, these MIG definitions need to match the ones in C/glibc. In
hurd/ioctl_types.h (in the Hurd tree) there is

typedef tcflag_t modes_t[4];
typedef speed_t speeds_t[2];

and then in bits/termios.h (in the glibc tree)

/* Type of terminal control flag masks.  */
typedef unsigned long int tcflag_t;

/* Type of control characters.  */
typedef unsigned char cc_t;

/* Type of baud rate specifiers.  */
typedef long int speed_t;

which is why I changed the MIG ones to long. We could do it the other
way around, make our own bits/termios.h and define them as fixed-size
32 bit ints.

Sergey

Re: [RFC PATCH hurd 6/12] hurd: Fix modes_t and speeds_t types on 64-bit

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 18:15:57 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:00 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > We don't need these to be 64bits, 32bit will be completely large enough.
> >
> > Which issue did you actually encounter?
> 
> Well, these MIG definitions need to match the ones in C/glibc. In
> hurd/ioctl_types.h (in the Hurd tree) there is
> 
> typedef tcflag_t modes_t[4];
> typedef speed_t speeds_t[2];
> 
> and then in bits/termios.h (in the glibc tree)
> 
> /* Type of terminal control flag masks.  */
> typedef unsigned long int tcflag_t;
> 
> /* Type of control characters.  */
> typedef unsigned char cc_t;
> 
> /* Type of baud rate specifiers.  */
> typedef long int speed_t;
> 
> which is why I changed the MIG ones to long. We could do it the other
> way around, make our own bits/termios.h and define them as fixed-size
> 32 bit ints.

I'd rather say drop the "long" part, to avoid having to pull the
stdint.h header in. Nowadays' BSD headers just use the int type,
notably.

Samuel

Re: [RFC PATCH hurd 6/12] hurd: Fix modes_t and speeds_t types on 64-bit

[Sergey Bugaev]

On Sun, Feb 12, 2023 at 6:22 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> I'd rather say drop the "long" part, to avoid having to pull the
> stdint.h header in.

That's what I meant, yes.

> Nowadays' BSD headers just use the int type,
> notably.

So given that the Linux port has its own bits/termios.h, is it fine to
just modify the generic one inline, rather than creating a
Hurd-specific version? The patch for that follows.

Sergey

-- >8 --

From 625b774141823a8f504cce8a92b9f45f5e6f050f Mon Sep 17 00:00:00 2001
From: Sergey Bugaev <bugaevc@gmail.com>
Date: Sun, 12 Feb 2023 19:08:57 +0300
Subject: [PATCH] hurd: Fix tcflag_t and speed_t types on 64-bit

These are supposed to stay 32-bit even on 64-bit systems. This matches
BSD and Linux, as well as how these types are already defined in
tioctl.defs

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

diff --git a/bits/termios.h b/bits/termios.h
index 4439c2f1..6a883ceb 100644
--- a/bits/termios.h
+++ b/bits/termios.h
@@ -99,13 +99,13 @@
    `tcflag_t', `cc_t', `speed_t' and the `TC*' constants appropriately.  */

 /* Type of terminal control flag masks.  */
-typedef unsigned long int tcflag_t;
+typedef unsigned int tcflag_t;

 /* Type of control characters.  */
 typedef unsigned char cc_t;

 /* Type of baud rate specifiers.  */
-typedef long int speed_t;
+typedef int speed_t;

 /* Terminal control structure.  */
 struct termios
-- 
2.39.1

Re: [RFC PATCH hurd 6/12] hurd: Fix modes_t and speeds_t types on 64-bit

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 19:13:58 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:22 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > I'd rather say drop the "long" part, to avoid having to pull the
> > stdint.h header in.
> 
> That's what I meant, yes.
> 
> > Nowadays' BSD headers just use the int type,
> > notably.
> 
> So given that the Linux port has its own bits/termios.h, is it fine to
> just modify the generic one inline, rather than creating a
> Hurd-specific version?

Yes.

> The patch for that follows.
> 
> Sergey
> 
> -- >8 --
> 
> From 625b774141823a8f504cce8a92b9f45f5e6f050f Mon Sep 17 00:00:00 2001
> From: Sergey Bugaev <bugaevc@gmail.com>
> Date: Sun, 12 Feb 2023 19:08:57 +0300
> Subject: [PATCH] hurd: Fix tcflag_t and speed_t types on 64-bit
> 
> These are supposed to stay 32-bit even on 64-bit systems. This matches
> BSD and Linux, as well as how these types are already defined in
> tioctl.defs
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  bits/termios.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bits/termios.h b/bits/termios.h
> index 4439c2f1..6a883ceb 100644
> --- a/bits/termios.h
> +++ b/bits/termios.h
> @@ -99,13 +99,13 @@
>     `tcflag_t', `cc_t', `speed_t' and the `TC*' constants appropriately.  */
> 
>  /* Type of terminal control flag masks.  */
> -typedef unsigned long int tcflag_t;
> +typedef unsigned int tcflag_t;
> 
>  /* Type of control characters.  */
>  typedef unsigned char cc_t;
> 
>  /* Type of baud rate specifiers.  */
> -typedef long int speed_t;
> +typedef int speed_t;
> 
>  /* Terminal control structure.  */
>  struct termios
> -- 
> 2.39.1
> 

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

Re: [RFC PATCH hurd 6/12] hurd: Fix modes_t and speeds_t types on 64-bit

[Samuel Thibault]

Applied, thanks!
Sergey Bugaev, le dim. 12 févr. 2023 19:13:58 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:22 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > I'd rather say drop the "long" part, to avoid having to pull the
> > stdint.h header in.
> 
> That's what I meant, yes.
> 
> > Nowadays' BSD headers just use the int type,
> > notably.
> 
> So given that the Linux port has its own bits/termios.h, is it fine to
> just modify the generic one inline, rather than creating a
> Hurd-specific version? The patch for that follows.
> 
> Sergey
> 
> -- >8 --
> 
> From 625b774141823a8f504cce8a92b9f45f5e6f050f Mon Sep 17 00:00:00 2001
> From: Sergey Bugaev <bugaevc@gmail.com>
> Date: Sun, 12 Feb 2023 19:08:57 +0300
> Subject: [PATCH] hurd: Fix tcflag_t and speed_t types on 64-bit
> 
> These are supposed to stay 32-bit even on 64-bit systems. This matches
> BSD and Linux, as well as how these types are already defined in
> tioctl.defs
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  bits/termios.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bits/termios.h b/bits/termios.h
> index 4439c2f1..6a883ceb 100644
> --- a/bits/termios.h
> +++ b/bits/termios.h
> @@ -99,13 +99,13 @@
>     `tcflag_t', `cc_t', `speed_t' and the `TC*' constants appropriately.  */
> 
>  /* Type of terminal control flag masks.  */
> -typedef unsigned long int tcflag_t;
> +typedef unsigned int tcflag_t;
> 
>  /* Type of control characters.  */
>  typedef unsigned char cc_t;
> 
>  /* Type of baud rate specifiers.  */
> -typedef long int speed_t;
> +typedef int speed_t;
> 
>  /* Terminal control structure.  */
>  struct termios
> -- 
> 2.39.1
> 

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

[RFC PATCH mig 7/12] Drop -undef -ansi from cpp flags

[Sergey Bugaev]

Since GNU Mach commit d30481122a5d24ad6b921062f93b9172ef922fc3,
i386/machine_types.defs defines types based on defined(__x86_64__).
Supressing the built-in macro definitions will now result in the wrong
type being silently selected.

-undef was initially introduced in commit
78b6a7665db7b2eae367e17102821cbdca231d19 without much of an explanation.
-ansi was introduced in commit 6940fb91859e46b2e96a331a029f2dc2a0ee51c9
"to avoid -Di386=1 and the like".

Since glibc has been using MIG with CPP set to a custom GCC invocation
which did *not* use either flag, it appears that everything works well
enough even without them. On the other hand, not having __x86_64__
defined most definetely causes issues for anything that does not set a
custom CPP when invoking MIG (i.e., most users). Other built-in
definitions could be used in the future in a similar way (e.g. on other
architectures); it's really more of a coincidence that they have not
been used until now, and things kept working with -undef.
---
 mig.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mig.in b/mig.in
index 63e0269..94fd500 100644
--- a/mig.in
+++ b/mig.in
@@ -38,7 +38,7 @@ migcom=${MIGDIR-$libexecdir}/${MIGCOM-@MIGCOM@}
 # The expansion of TARGET_CC might refer to ${CC}, so make sure it is defined.
 default_cc="@CC@"
 CC="${CC-${default_cc}}"
-default_cpp="@TARGET_CC@ -E -x c -undef -ansi"
+default_cpp="@TARGET_CC@ -E -x c"
 cpp="${CPP-${default_cpp}}"
 
 cppflags=
-- 
2.39.1

Re: [RFC PATCH mig 7/12] Drop -undef -ansi from cpp flags

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 14:10:38 +0300, a ecrit:
> Since GNU Mach commit d30481122a5d24ad6b921062f93b9172ef922fc3,
> i386/machine_types.defs defines types based on defined(__x86_64__).
> Supressing the built-in macro definitions will now result in the wrong
> type being silently selected.
> 
> -undef was initially introduced in commit
> 78b6a7665db7b2eae367e17102821cbdca231d19 without much of an explanation.
> -ansi was introduced in commit 6940fb91859e46b2e96a331a029f2dc2a0ee51c9
> "to avoid -Di386=1 and the like".
> 
> Since glibc has been using MIG with CPP set to a custom GCC invocation
> which did *not* use either flag, it appears that everything works well
> enough even without them. On the other hand, not having __x86_64__
> defined most definetely causes issues for anything that does not set a
> custom CPP when invoking MIG (i.e., most users). Other built-in
> definitions could be used in the future in a similar way (e.g. on other
> architectures); it's really more of a coincidence that they have not
> been used until now, and things kept working with -undef.

That looks alright to me. Flavior, what do you think about it?

> ---
>  mig.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mig.in b/mig.in
> index 63e0269..94fd500 100644
> --- a/mig.in
> +++ b/mig.in
> @@ -38,7 +38,7 @@ migcom=${MIGDIR-$libexecdir}/${MIGCOM-@MIGCOM@}
>  # The expansion of TARGET_CC might refer to ${CC}, so make sure it is defined.
>  default_cc="@CC@"
>  CC="${CC-${default_cc}}"
> -default_cpp="@TARGET_CC@ -E -x c -undef -ansi"
> +default_cpp="@TARGET_CC@ -E -x c"
>  cpp="${CPP-${default_cpp}}"
>  
>  cppflags=
> -- 
> 2.39.1

Re: [RFC PATCH mig 7/12] Drop -undef -ansi from cpp flags

[Flávio Cruz]

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

On Sun, Feb 12, 2023 at 10:02 AM Samuel Thibault <samuel.thibault@gnu.org>
wrote:

> Sergey Bugaev, le dim. 12 févr. 2023 14:10:38 +0300, a ecrit:
> > Since GNU Mach commit d30481122a5d24ad6b921062f93b9172ef922fc3,
> > i386/machine_types.defs defines types based on defined(__x86_64__).
> > Supressing the built-in macro definitions will now result in the wrong
> > type being silently selected.
> >
> > -undef was initially introduced in commit
> > 78b6a7665db7b2eae367e17102821cbdca231d19 without much of an explanation.
> > -ansi was introduced in commit 6940fb91859e46b2e96a331a029f2dc2a0ee51c9
> > "to avoid -Di386=1 and the like".
> >
> > Since glibc has been using MIG with CPP set to a custom GCC invocation
> > which did *not* use either flag, it appears that everything works well
> > enough even without them. On the other hand, not having __x86_64__
> > defined most definetely causes issues for anything that does not set a
> > custom CPP when invoking MIG (i.e., most users). Other built-in
> > definitions could be used in the future in a similar way (e.g. on other
> > architectures); it's really more of a coincidence that they have not
> > been used until now, and things kept working with -undef.
>
> That looks alright to me. Flavior, what do you think about it?
>

Seems fine to me.


>
> > ---
> >  mig.in | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mig.in b/mig.in
> > index 63e0269..94fd500 100644
> > --- a/mig.in
> > +++ b/mig.in
> > @@ -38,7 +38,7 @@ migcom=${MIGDIR-$libexecdir}/${MIGCOM-@MIGCOM@}
> >  # The expansion of TARGET_CC might refer to ${CC}, so make sure it is
> defined.
> >  default_cc="@CC@"
> >  CC="${CC-${default_cc}}"
> > -default_cpp="@TARGET_CC@ -E -x c -undef -ansi"
> > +default_cpp="@TARGET_CC@ -E -x c"
> >  cpp="${CPP-${default_cpp}}"
> >
> >  cppflags=
> > --
> > 2.39.1
>

Re: [RFC PATCH mig 7/12] Drop -undef -ansi from cpp flags

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le dim. 12 févr. 2023 14:10:38 +0300, a ecrit:
> Since GNU Mach commit d30481122a5d24ad6b921062f93b9172ef922fc3,
> i386/machine_types.defs defines types based on defined(__x86_64__).
> Supressing the built-in macro definitions will now result in the wrong
> type being silently selected.
> 
> -undef was initially introduced in commit
> 78b6a7665db7b2eae367e17102821cbdca231d19 without much of an explanation.
> -ansi was introduced in commit 6940fb91859e46b2e96a331a029f2dc2a0ee51c9
> "to avoid -Di386=1 and the like".
> 
> Since glibc has been using MIG with CPP set to a custom GCC invocation
> which did *not* use either flag, it appears that everything works well
> enough even without them. On the other hand, not having __x86_64__
> defined most definetely causes issues for anything that does not set a
> custom CPP when invoking MIG (i.e., most users). Other built-in
> definitions could be used in the future in a similar way (e.g. on other
> architectures); it's really more of a coincidence that they have not
> been used until now, and things kept working with -undef.
> ---
>  mig.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mig.in b/mig.in
> index 63e0269..94fd500 100644
> --- a/mig.in
> +++ b/mig.in
> @@ -38,7 +38,7 @@ migcom=${MIGDIR-$libexecdir}/${MIGCOM-@MIGCOM@}
>  # The expansion of TARGET_CC might refer to ${CC}, so make sure it is defined.
>  default_cc="@CC@"
>  CC="${CC-${default_cc}}"
> -default_cpp="@TARGET_CC@ -E -x c -undef -ansi"
> +default_cpp="@TARGET_CC@ -E -x c"
>  cpp="${CPP-${default_cpp}}"
>  
>  cppflags=
> -- 
> 2.39.1
> 
> 

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

[RFC PATCH mig 8/12] Set max type alignment to sizeof(long)

[Sergey Bugaev]

...not sizeof(int). This appears to be required on x86_64.

PLEASE NOTE: There is now a patch from Flavio Cruz ("Make MIG work for
pure 64 bit kernel and userland") that appears to tackle this same issue
(and more). Most likely this simplistic patch should be dropped in favor
of the one from Flavio.
---
 type.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/type.c b/type.c
index b104c66..7765200 100644
--- a/type.c
+++ b/type.c
@@ -542,7 +542,7 @@ itLongDecl(u_int inname, const_string_t instr, u_int outname,
     it->itOutName = outname;
     it->itOutNameStr = outstr;
     it->itSize = size;
-    it->itAlignment = MIN(word_size, size / 8);
+    it->itAlignment = MIN(sizeof_long, size / 8);
     if (inname == MACH_MSG_TYPE_STRING_C)
     {
 	it->itStruct = false;
-- 
2.39.1

[RFC PATCH glibc 9/12] mach: Look for mach_i386.defs on x86_64 too

[Sergey Bugaev]

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 mach/Makefile             | 3 ++-
 sysdeps/mach/configure    | 6 +++---
 sysdeps/mach/configure.ac | 6 +++---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/mach/Makefile b/mach/Makefile
index 39358fdb..f2fdd7da 100644
--- a/mach/Makefile
+++ b/mach/Makefile
@@ -64,7 +64,8 @@ CFLAGS-RPC_i386_set_ldt.o = $(no-stack-protector)
 CFLAGS-RPC_task_get_special_port.o = $(no-stack-protector)
 \f
 # Translate GNU names for CPUs into the names used in Mach header files.
-mach-machine = $(patsubst powerpc,ppc,$(base-machine))
+mach-machine := $(patsubst powerpc,ppc,$(base-machine))
+mach-machine := $(patsubst x86_64,i386,$(mach-machine))
 
 # Define mach-syscalls and sysno-*.
 ifndef inhibit_mach_syscalls
diff --git a/sysdeps/mach/configure b/sysdeps/mach/configure
index 3f0a9029..8c341d59 100644
--- a/sysdeps/mach/configure
+++ b/sysdeps/mach/configure
@@ -249,7 +249,7 @@ for ifc in mach mach4 gnumach \
 	   clock clock_priv host_priv host_security ledger lock_set \
 	   processor processor_set task task_notify thread_act vm_map \
 	   memory_object memory_object_default default_pager \
-	   i386/mach_i386 \
+	   machine/mach_i386 \
 	   ; do
   as_ac_Header=`$as_echo "ac_cv_header_mach/${ifc}.defs" | $as_tr_sh`
 ac_fn_c_check_header_preproc "$LINENO" "mach/${ifc}.defs" "$as_ac_Header"
@@ -440,7 +440,7 @@ if ${libc_cv_mach_i386_ioports+:} false; then :
 else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
-#include <mach/i386/mach_i386.defs>
+#include <mach/machine/mach_i386.defs>
 
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
@@ -466,7 +466,7 @@ if ${libc_cv_mach_i386_gdt+:} false; then :
 else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
-#include <mach/i386/mach_i386.defs>
+#include <mach/machine/mach_i386.defs>
 
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
diff --git a/sysdeps/mach/configure.ac b/sysdeps/mach/configure.ac
index a57cb259..579c0021 100644
--- a/sysdeps/mach/configure.ac
+++ b/sysdeps/mach/configure.ac
@@ -64,7 +64,7 @@ for ifc in mach mach4 gnumach \
 	   clock clock_priv host_priv host_security ledger lock_set \
 	   processor processor_set task task_notify thread_act vm_map \
 	   memory_object memory_object_default default_pager \
-	   i386/mach_i386 \
+	   machine/mach_i386 \
 	   ; do
   AC_CHECK_HEADER(mach/${ifc}.defs, [dnl
   mach_interface_list="$mach_interface_list $ifc"],, -)
@@ -89,7 +89,7 @@ AC_CHECK_HEADER(machine/ndr_def.h, [dnl
 
 AC_CACHE_CHECK(for i386_io_perm_modify in mach_i386.defs,
 	       libc_cv_mach_i386_ioports, [dnl
-AC_EGREP_HEADER(i386_io_perm_modify, mach/i386/mach_i386.defs,
+AC_EGREP_HEADER(i386_io_perm_modify, mach/machine/mach_i386.defs,
 		libc_cv_mach_i386_ioports=yes,
 		libc_cv_mach_i386_ioports=no)])
 if test $libc_cv_mach_i386_ioports = yes; then
@@ -98,7 +98,7 @@ fi
 
 AC_CACHE_CHECK(for i386_set_gdt in mach_i386.defs,
 	       libc_cv_mach_i386_gdt, [dnl
-AC_EGREP_HEADER(i386_set_gdt, mach/i386/mach_i386.defs,
+AC_EGREP_HEADER(i386_set_gdt, mach/machine/mach_i386.defs,
 		libc_cv_mach_i386_gdt=yes,
 		libc_cv_mach_i386_gdt=no)])
 if test $libc_cv_mach_i386_gdt = yes; then
-- 
2.39.1

Re: [RFC PATCH glibc 9/12] mach: Look for mach_i386.defs on x86_64 too

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 14:10:40 +0300, a ecrit:
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  mach/Makefile             | 3 ++-
>  sysdeps/mach/configure    | 6 +++---
>  sysdeps/mach/configure.ac | 6 +++---
>  3 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/mach/Makefile b/mach/Makefile
> index 39358fdb..f2fdd7da 100644
> --- a/mach/Makefile
> +++ b/mach/Makefile
> @@ -64,7 +64,8 @@ CFLAGS-RPC_i386_set_ldt.o = $(no-stack-protector)
>  CFLAGS-RPC_task_get_special_port.o = $(no-stack-protector)
>  \f
>  # Translate GNU names for CPUs into the names used in Mach header files.
> -mach-machine = $(patsubst powerpc,ppc,$(base-machine))
> +mach-machine := $(patsubst powerpc,ppc,$(base-machine))
> +mach-machine := $(patsubst x86_64,i386,$(mach-machine))

Mmm, no, it's not actually a translation. What exactly does this fix?

To build a 64bit glibc, we'll use headers installed by a 64bit gnumach,
i.e. into /usr/include/x86_64-gnu/

I have applied the rest, thanks!

Samuel

> diff --git a/sysdeps/mach/con b/sysdeps/mach/configure
> index 3f0a9029..8c341d59 100644
> --- a/sysdeps/mach/configure
> +++ b/sysdeps/mach/configure
> @@ -249,7 +249,7 @@ for ifc in mach mach4 gnumach \
>  	   clock clock_priv host_priv host_security ledger lock_set \
>  	   processor processor_set task task_notify thread_act vm_map \
>  	   memory_object memory_object_default default_pager \
> -	   i386/mach_i386 \
> +	   machine/mach_i386 \
>  	   ; do
>    as_ac_Header=`$as_echo "ac_cv_header_mach/${ifc}.defs" | $as_tr_sh`
>  ac_fn_c_check_header_preproc "$LINENO" "mach/${ifc}.defs" "$as_ac_Header"
> @@ -440,7 +440,7 @@ if ${libc_cv_mach_i386_ioports+:} false; then :
>  else
>    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>  /* end confdefs.h.  */
> -#include <mach/i386/mach_i386.defs>
> +#include <mach/machine/mach_i386.defs>
>  
>  _ACEOF
>  if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
> @@ -466,7 +466,7 @@ if ${libc_cv_mach_i386_gdt+:} false; then :
>  else
>    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>  /* end confdefs.h.  */
> -#include <mach/i386/mach_i386.defs>
> +#include <mach/machine/mach_i386.defs>
>  
>  _ACEOF
>  if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
> diff --git a/sysdeps/mach/configure.ac b/sysdeps/mach/configure.ac
> index a57cb259..579c0021 100644
> --- a/sysdeps/mach/configure.ac
> +++ b/sysdeps/mach/configure.ac
> @@ -64,7 +64,7 @@ for ifc in mach mach4 gnumach \
>  	   clock clock_priv host_priv host_security ledger lock_set \
>  	   processor processor_set task task_notify thread_act vm_map \
>  	   memory_object memory_object_default default_pager \
> -	   i386/mach_i386 \
> +	   machine/mach_i386 \
>  	   ; do
>    AC_CHECK_HEADER(mach/${ifc}.defs, [dnl
>    mach_interface_list="$mach_interface_list $ifc"],, -)
> @@ -89,7 +89,7 @@ AC_CHECK_HEADER(machine/ndr_def.h, [dnl
>  
>  AC_CACHE_CHECK(for i386_io_perm_modify in mach_i386.defs,
>  	       libc_cv_mach_i386_ioports, [dnl
> -AC_EGREP_HEADER(i386_io_perm_modify, mach/i386/mach_i386.defs,
> +AC_EGREP_HEADER(i386_io_perm_modify, mach/machine/mach_i386.defs,
>  		libc_cv_mach_i386_ioports=yes,
>  		libc_cv_mach_i386_ioports=no)])
>  if test $libc_cv_mach_i386_ioports = yes; then
> @@ -98,7 +98,7 @@ fi
>  
>  AC_CACHE_CHECK(for i386_set_gdt in mach_i386.defs,
>  	       libc_cv_mach_i386_gdt, [dnl
> -AC_EGREP_HEADER(i386_set_gdt, mach/i386/mach_i386.defs,
> +AC_EGREP_HEADER(i386_set_gdt, mach/machine/mach_i386.defs,
>  		libc_cv_mach_i386_gdt=yes,
>  		libc_cv_mach_i386_gdt=no)])
>  if test $libc_cv_mach_i386_gdt = yes; then
> -- 
> 2.39.1
> 
> 

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

Re: [RFC PATCH glibc 9/12] mach: Look for mach_i386.defs on x86_64 too

[Sergey Bugaev]

> >  # Translate GNU names for CPUs into the names used in Mach header files.
> > -mach-machine = $(patsubst powerpc,ppc,$(base-machine))
> > +mach-machine := $(patsubst powerpc,ppc,$(base-machine))
> > +mach-machine := $(patsubst x86_64,i386,$(mach-machine))
>
> Mmm, no, it's not actually a translation. What exactly does this fix?

The only thing the mach-machine variable is used for (as the comment
above it kind of explains) is for guessing the include guard name in
the machine-specific Mach header. They need this because, uh,

For generating mach-syscalls.mk, the build system passes '#include
<mach/syscall_sw.h>' to the compiler to be macro-expanded.
<mach/syscall_sw.h> contains lines like
kernel_trap(mach_msg_trap,-25,7), and at the very top, this:

/*
 *      The machine-dependent "syscall_sw.h" file should
 *      define a macro for
 *              kernel_trap(trap_name, trap_number, arg_count)
 *      which will expand into assembly code for the
 *      trap.
 *
 *      N.B.: When adding calls, do not put spaces in the macros.
 */
#include <mach/machine/syscall_sw.h>

So when using <mach/syscall_sw.h> normally, those kernel_trap() macros
get expanded into arch-specific assembly definitions, such as on i386:

#define kernel_trap(trap_name,trap_number,number_args) \
ENTRY(trap_name) \
        movl    $ trap_number,%eax; \
        SVC; \
        ret; \
END(trap_name)

Now back to the glibc buildsystem and the generation of
mach-syscalls.mk: the glibc buildsystem preprocesses
<mach/syscall_sw.h> (the one that contains all those kernel_trap ()
"calls") with the -D_MACH_`echo $(mach-machine) | tr a-z
A-Z`_SYSCALL_SW_H_=1 incantation. This expands to something like
-D_MACH_I386_SYSCALL_SW_H_, which "fakes" the include guard of the
<mach/machine/syscall_sw.h>! So those kernel_trap () "calls" do not
get expanded, and the glibc buildsystem then postprocesses them with a
sed invocation to strip those kernel_trap () calls:

sed -n -e 's/^kernel_trap(\(.*\),\([-0-9]*\),\([0-9]*\))$$/\1 \2 \3/p'

And *that* (faking the include guard) is what the mach-machine
variable is used for. As another comment there says, "Go kludges!!!"

Since mach/machine/syscall_sw.h is the i386 version on x86_64 (or --
is it not supposed to be?), the _MACH_I386_SYSCALL_SW_H_ guard is the
one to fake, hence setting mach-machine to i386.

P.S. As you can see, even when the patch is a simple one-line change,
there can be quite a story (and some pulled hairs) behind it :)

> To build a 64bit glibc, we'll use headers installed by a 64bit gnumach,
> i.e. into /usr/include/x86_64-gnu/

Yes, sure, I'm building with the headers from the 64-bit Mach (gnumach
master configured with --host=x86_64-gnu).

Sergey

> I have applied the rest, thanks!
>
> Samuel

Re: [RFC PATCH glibc 9/12] mach: Look for mach_i386.defs on x86_64 too

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 18:38:03 +0300, a ecrit:
> Since mach/machine/syscall_sw.h is the i386 version on x86_64 (or --
> is it not supposed to be?)

Nobody yet decided that the system call interface would be the same on
i386 and on x86_64 :)

Most probably we'll need a different header, to put the trap number of
rax instead of eax, notably. And the systemcall instruction will most
probably not be an lcall.

> the _MACH_I386_SYSCALL_SW_H_ guard is the one to fake, hence setting
> mach-machine to i386.

This can be already fixed by shipping a different file in mach, as we'll
most probably want in the end anyway.

Samuel

Re: [RFC PATCH glibc 9/12] mach: Look for mach_i386.defs on x86_64 too

[Sergey Bugaev]

On Sun, Feb 12, 2023 at 6:46 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Sergey Bugaev, le dim. 12 févr. 2023 18:38:03 +0300, a ecrit:
> > Since mach/machine/syscall_sw.h is the i386 version on x86_64 (or --
> > is it not supposed to be?)
>
> Nobody yet decided that the system call interface would be the same on
> i386 and on x86_64 :)
>
> Most probably we'll need a different header, to put the trap number of
> rax instead of eax, notably. And the systemcall instruction will most
> probably not be an lcall.
>
> > the _MACH_I386_SYSCALL_SW_H_ guard is the one to fake, hence setting
> > mach-machine to i386.
>
> This can be already fixed by shipping a different file in mach, as we'll
> most probably want in the end anyway.

I've kind of assumed that the x86_64 Mach using / installing the i386
headers is intentional, and if some things (like the actual syscall
ABI) are to be different, they'd both be defined in the same file,
predicated on an ifdef. This is what XNU does for sure, see
osfmk/mach/i386/syscall_sw.h

Sergey

Re: [RFC PATCH glibc 9/12] mach: Look for mach_i386.defs on x86_64 too

[Joseph Myers]

This commit broke the glibc build for i686-gnu.  There are errors of the 
form:

In file included from ../hurd/hurd/threadvar.h:23,
                 from ../sysdeps/mach/hurd/mig-reply.c:20:
../sysdeps/mach/hurd/i386/tls.h:104:11: fatal error: mach/i386/mach_i386.h: No such file or directory
  104 | # include <mach/i386/mach_i386.h>
      |           ^~~~~~~~~~~~~~~~~~~~~~~

https://sourceware.org/pipermail/libc-testresults/2023q1/010861.html

-- 
Joseph S. Myers
joseph@codesourcery.com

[RFC PATCH glibc 10/12] hurd: Set up the basic tree for x86_64-gnu

[Sergey Bugaev]

And move pt-setup.c to the generic x86 tree.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/Implies                      | 1 +
 sysdeps/mach/hurd/{i386 => x86}/htl/pt-setup.c | 4 ++--
 sysdeps/mach/hurd/x86_64/Implies               | 2 ++
 sysdeps/mach/hurd/x86_64/htl/Implies           | 3 +++
 sysdeps/mach/x86_64/Implies                    | 1 +
 5 files changed, 9 insertions(+), 2 deletions(-)
 rename sysdeps/mach/hurd/{i386 => x86}/htl/pt-setup.c (97%)
 create mode 100644 sysdeps/mach/hurd/x86_64/Implies
 create mode 100644 sysdeps/mach/hurd/x86_64/htl/Implies
 create mode 100644 sysdeps/mach/x86_64/Implies

diff --git a/sysdeps/mach/hurd/Implies b/sysdeps/mach/hurd/Implies
index d2d5234c..e19dd1fd 100644
--- a/sysdeps/mach/hurd/Implies
+++ b/sysdeps/mach/hurd/Implies
@@ -3,3 +3,4 @@
 gnu
 # The Hurd provides a rough superset of the functionality of 4.4 BSD.
 unix/bsd
+mach/hurd/htl
diff --git a/sysdeps/mach/hurd/i386/htl/pt-setup.c b/sysdeps/mach/hurd/x86/htl/pt-setup.c
similarity index 97%
rename from sysdeps/mach/hurd/i386/htl/pt-setup.c
rename to sysdeps/mach/hurd/x86/htl/pt-setup.c
index 94caed82..3abd92b2 100644
--- a/sysdeps/mach/hurd/i386/htl/pt-setup.c
+++ b/sysdeps/mach/hurd/x86/htl/pt-setup.c
@@ -1,4 +1,4 @@
-/* Setup thread stack.  Hurd/i386 version.
+/* Setup thread stack.  Hurd/x86 version.
    Copyright (C) 2000-2023 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -22,7 +22,7 @@
 
 #include <pt-internal.h>
 
-/* The stack layout used on the i386 is:
+/* The stack layout used on the x86 is:
 
     -----------------
    |  ARG            |
diff --git a/sysdeps/mach/hurd/x86_64/Implies b/sysdeps/mach/hurd/x86_64/Implies
new file mode 100644
index 00000000..6b5e6f47
--- /dev/null
+++ b/sysdeps/mach/hurd/x86_64/Implies
@@ -0,0 +1,2 @@
+mach/hurd/x86
+mach/hurd/x86_64/htl
diff --git a/sysdeps/mach/hurd/x86_64/htl/Implies b/sysdeps/mach/hurd/x86_64/htl/Implies
new file mode 100644
index 00000000..1ae8f16e
--- /dev/null
+++ b/sysdeps/mach/hurd/x86_64/htl/Implies
@@ -0,0 +1,3 @@
+mach/hurd/htl
+mach/hurd/x86/htl
+x86_64/htl
diff --git a/sysdeps/mach/x86_64/Implies b/sysdeps/mach/x86_64/Implies
new file mode 100644
index 00000000..da8291f4
--- /dev/null
+++ b/sysdeps/mach/x86_64/Implies
@@ -0,0 +1 @@
+mach/x86
-- 
2.39.1

Re: [RFC PATCH glibc 10/12] hurd: Set up the basic tree for x86_64-gnu

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 14:10:41 +0300, a ecrit:
> And move pt-setup.c to the generic x86 tree.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/Implies                      | 1 +
>  sysdeps/mach/hurd/{i386 => x86}/htl/pt-setup.c | 4 ++--
>  sysdeps/mach/hurd/x86_64/Implies               | 2 ++
>  sysdeps/mach/hurd/x86_64/htl/Implies           | 3 +++
>  sysdeps/mach/x86_64/Implies                    | 1 +
>  5 files changed, 9 insertions(+), 2 deletions(-)
>  rename sysdeps/mach/hurd/{i386 => x86}/htl/pt-setup.c (97%)
>  create mode 100644 sysdeps/mach/hurd/x86_64/Implies
>  create mode 100644 sysdeps/mach/hurd/x86_64/htl/Implies
>  create mode 100644 sysdeps/mach/x86_64/Implies
> 
> diff --git a/sysdeps/mach/hurd/Implies b/sysdeps/mach/hurd/Implies
> index d2d5234c..e19dd1fd 100644
> --- a/sysdeps/mach/hurd/Implies
> +++ b/sysdeps/mach/hurd/Implies
> @@ -3,3 +3,4 @@
>  gnu
>  # The Hurd provides a rough superset of the functionality of 4.4 BSD.
>  unix/bsd
> +mach/hurd/htl
> diff --git a/sysdeps/mach/hurd/i386/htl/pt-setup.c b/sysdeps/mach/hurd/x86/htl/pt-setup.c
> similarity index 97%
> rename from sysdeps/mach/hurd/i386/htl/pt-setup.c
> rename to sysdeps/mach/hurd/x86/htl/pt-setup.c
> index 94caed82..3abd92b2 100644
> --- a/sysdeps/mach/hurd/i386/htl/pt-setup.c
> +++ b/sysdeps/mach/hurd/x86/htl/pt-setup.c
> @@ -1,4 +1,4 @@
> -/* Setup thread stack.  Hurd/i386 version.
> +/* Setup thread stack.  Hurd/x86 version.
>     Copyright (C) 2000-2023 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> @@ -22,7 +22,7 @@
>  
>  #include <pt-internal.h>
>  
> -/* The stack layout used on the i386 is:
> +/* The stack layout used on the x86 is:
>  
>      -----------------
>     |  ARG            |
> diff --git a/sysdeps/mach/hurd/x86_64/Implies b/sysdeps/mach/hurd/x86_64/Implies
> new file mode 100644
> index 00000000..6b5e6f47
> --- /dev/null
> +++ b/sysdeps/mach/hurd/x86_64/Implies
> @@ -0,0 +1,2 @@
> +mach/hurd/x86
> +mach/hurd/x86_64/htl
> diff --git a/sysdeps/mach/hurd/x86_64/htl/Implies b/sysdeps/mach/hurd/x86_64/htl/Implies
> new file mode 100644
> index 00000000..1ae8f16e
> --- /dev/null
> +++ b/sysdeps/mach/hurd/x86_64/htl/Implies
> @@ -0,0 +1,3 @@
> +mach/hurd/htl
> +mach/hurd/x86/htl
> +x86_64/htl

Always remember to also try the 32bit build, I added this to your
commit:

diff --git a/sysdeps/mach/hurd/i386/htl/Implies b/sysdeps/mach/hurd/i386/htl/Implies
index 7a0f99d772..fe3bd983b8 100644
--- a/sysdeps/mach/hurd/i386/htl/Implies
+++ b/sysdeps/mach/hurd/i386/htl/Implies
@@ -1,2 +1,3 @@
 mach/hurd/htl
+mach/hurd/x86/htl
 i386/htl

Thanks!

> diff --git a/sysdeps/mach/x86_64/Implies b/sysdeps/mach/x86_64/Implies
> new file mode 100644
> index 00000000..da8291f4
> --- /dev/null
> +++ b/sysdeps/mach/x86_64/Implies
> @@ -0,0 +1 @@
> +mach/x86
> -- 
> 2.39.1
> 
> 

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

[RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Sergey Bugaev]

tls.h in particular is very unfinished.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/x86_64/static-start.S     |  27 +++
 sysdeps/mach/hurd/x86_64/tls.h              | 182 ++++++++++++++++++++
 sysdeps/mach/hurd/x86_64/tlsdesc.sym        |  22 +++
 sysdeps/x86_64/htl/bits/pthreadtypes-arch.h |  36 ++++
 sysdeps/x86_64/htl/machine-sp.h             |  29 ++++
 sysdeps/x86_64/htl/pt-machdep.h             |  28 +++
 6 files changed, 324 insertions(+)
 create mode 100644 sysdeps/mach/hurd/x86_64/static-start.S
 create mode 100644 sysdeps/mach/hurd/x86_64/tls.h
 create mode 100644 sysdeps/mach/hurd/x86_64/tlsdesc.sym
 create mode 100644 sysdeps/x86_64/htl/bits/pthreadtypes-arch.h
 create mode 100644 sysdeps/x86_64/htl/machine-sp.h
 create mode 100644 sysdeps/x86_64/htl/pt-machdep.h

diff --git a/sysdeps/mach/hurd/x86_64/static-start.S b/sysdeps/mach/hurd/x86_64/static-start.S
new file mode 100644
index 00000000..982d3d52
--- /dev/null
+++ b/sysdeps/mach/hurd/x86_64/static-start.S
@@ -0,0 +1,27 @@
+/* Startup code for statically linked Hurd/x86_64 binaries.
+   Copyright (C) 1998-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/>.  */
+
+	.text
+	.globl _start
+_start:
+	call _hurd_stack_setup
+	xorq %rdx, %rdx
+	jmp _start1
+
+#define _start _start1
+#include <sysdeps/x86_64/start.S>
diff --git a/sysdeps/mach/hurd/x86_64/tls.h b/sysdeps/mach/hurd/x86_64/tls.h
new file mode 100644
index 00000000..027304d9
--- /dev/null
+++ b/sysdeps/mach/hurd/x86_64/tls.h
@@ -0,0 +1,182 @@
+/* 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>
+
+/* 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");
+
+/* FIXME */
+# define __LIBC_NO_TLS() 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
+
+# define TCB_ALIGNMENT	64
+
+# define TLS_INIT_TP(descr) 0
+
+# if __GNUC_PREREQ (6, 0)
+
+#  define THREAD_SELF							\
+  (*(tcbhead_t * __seg_fs *) offsetof (tcbhead_t, tcb))
+#  define THREAD_GETMEM(descr, member)					\
+  (*(__typeof (descr->member) __seg_fs *) offsetof (tcbhead_t, member))
+#  define THREAD_SETMEM(descr, member, value)				\
+  (*(__typeof (descr->member) __seg_fs *) offsetof (tcbhead_t, member) = value)
+
+# else
+
+#  define THREAD_SELF							\
+  ({ tcbhead_t *__tcb;							\
+     asm ("movq %%fs:%c1,%0" : "=r" (__tcb)				\
+          : "i" (offsetof (tcbhead_t, tcb)));				\
+     __tcb; })
+
+/* Read member of the thread descriptor directly.  */
+# define THREAD_GETMEM(descr, member) \
+  ({ __typeof (descr->member) __value;					\
+     _Static_assert (sizeof (__value) == 1				\
+                     || sizeof (__value) == 4				\
+                     || sizeof (__value) == 8,				\
+                     "size of per-thread data");			\
+     if (sizeof (__value) == 1)						\
+       asm volatile ("movb %%fs:%P2,%b0"				\
+                     : "=q" (__value)					\
+                     : "0" (0), "i" (offsetof (tcbhead_t, member)));	\
+     else if (sizeof (__value) == 4)					\
+       asm volatile ("movl %%fs:%P1,%0"					\
+                     : "=r" (__value)					\
+                     : "i" (offsetof (tcbhead_t, member)));		\
+     else /* 8 */							\
+       asm volatile ("movq %%fs:%P1,%0"					\
+                     : "=r" (__value)					\
+                     : "i" (offsetof (tcbhead_t, member)));		\
+     __value; })
+
+/* Write member of the thread descriptor directly.  */
+#  define THREAD_SETMEM(descr, member, value) \
+  ({									\
+     _Static_assert (sizeof (descr->member) == 1			\
+                     || sizeof (descr->member) == 4			\
+                     || sizeof (descr->member) == 8,			\
+                     "size of per-thread data");			\
+     if (sizeof (descr->member) == 1)					\
+       asm volatile ("movb %b0,%%fs:%P1" :				\
+                     : "iq" (value),					\
+                       "i" (offsetof (tcbhead_t, member)));		\
+     else if (sizeof (descr->member) == 4)				\
+       asm volatile ("movl %0,%%fs:%P1" :				\
+                     : "ir" (value),					\
+                       "i" (offsetof (tcbhead_t, member)));		\
+     else /* 8 */							\
+       asm volatile ("movq %0,%%fs:%P1" :				\
+                     : "ir" (value),					\
+                       "i" (offsetof (tcbhead_t, member)));		\
+     })
+# endif /* __GNUC_PREREQ (6, 0) */
+
+/* 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))
+
+/* 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)
+
+/* 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 */
diff --git a/sysdeps/mach/hurd/x86_64/tlsdesc.sym b/sysdeps/mach/hurd/x86_64/tlsdesc.sym
new file mode 100644
index 00000000..da3b96c1
--- /dev/null
+++ b/sysdeps/mach/hurd/x86_64/tlsdesc.sym
@@ -0,0 +1,22 @@
+#include <stddef.h>
+#include <sysdep.h>
+#include <tls.h>
+#include <link.h>
+#include <dl-tlsdesc.h>
+
+--
+
+-- We have to override sysdeps/x86_64/tlsdesc.sym to adapt to our libpthread.
+
+-- Abuse tls.h macros to derive offsets relative to the thread register.
+
+DTV_OFFSET			offsetof(tcbhead_t, dtv)
+
+TLSDESC_ARG			offsetof(struct tlsdesc, arg)
+
+TLSDESC_GEN_COUNT		offsetof(struct tlsdesc_dynamic_arg, gen_count)
+TLSDESC_MODID			offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_module)
+TLSDESC_MODOFF			offsetof(struct tlsdesc_dynamic_arg, tlsinfo.ti_offset)
+
+TI_MODULE_OFFSET		offsetof(tls_index, ti_module)
+TI_OFFSET_OFFSET		offsetof(tls_index, ti_offset)
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..c3ac991b
--- /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 __attribute__ ((__aligned__(4)))
+#define __ONCE_ALIGNMENT
+
+#endif /* bits/pthreadtypes.h */
diff --git a/sysdeps/x86_64/htl/machine-sp.h b/sysdeps/x86_64/htl/machine-sp.h
new file mode 100644
index 00000000..7ae1c941
--- /dev/null
+++ b/sysdeps/x86_64/htl/machine-sp.h
@@ -0,0 +1,29 @@
+/* Machine-specific function to return the stack pointer.  x86_64 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/>.  */
+
+#ifndef _MACHINE_SP_H
+#define _MACHINE_SP_H
+
+/* Return the current stack pointer.  */
+
+#define __thread_stack_pointer() ({						\
+  register uintptr_t __sp__ asm("rsp");						\
+  __sp__;									\
+})
+
+#endif /* machine-sp.h */
diff --git a/sysdeps/x86_64/htl/pt-machdep.h b/sysdeps/x86_64/htl/pt-machdep.h
new file mode 100644
index 00000000..2e2846fa
--- /dev/null
+++ b/sysdeps/x86_64/htl/pt-machdep.h
@@ -0,0 +1,28 @@
+/* Machine dependent pthreads internal defenitions.  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/>.  */
+
+#ifndef _PT_MACHDEP_H
+#define _PT_MACHDEP_H	1
+
+struct pthread_mcontext
+{
+  void *pc;
+  void *sp;
+};
+
+#endif /* pt-machdep.h */
-- 
2.39.1

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
> It seems that GCC expects TLS on x86_64 to be done relative to %fs, not %gs, so
> that's what I attempted to do in tls.h. The main thing missing there is the
> ability to actually set (and read) the %fs base address of a thread. It is my
> understanding (but note that I have no idea what I'm talking about) that on
> x86_64 the segment descriptors (as in GDT/LDT) are not used for this,

segmentation has somewhat disappeared in x86_64, yes.

> and instead the address can be set by writing to a MSR. Linux exposes
> the arch_prctl (ARCH_[GS]ET_[FG]S) syscall for this; so maybe GNU Mach
> could also have an explicit routine for this, perhaps like this:
> 
> routine i386_set_fgs_base (
> 	target_thread: thread_t;
> 	which: int;
> 	value: rpc_vm_address_t);

Indeed.

> We should not need a getter routine, because one can simply inspect the target
> thread's state (unless, again, I misunderstand things horribly).

For 16bit fs/gs values we could read them from userland yes. But for
fs/gs base, the FSGSBASE instruction is not available on all 64bit
processors. And ATM in THREAD_TCB we want to be able to get the base of
another thread.

> diff --git a/sysdeps/mach/hurd/x86_64/static-start.S b/sysdeps/mach/hurd/x86_64/static-start.S
> new file mode 100644
> index 00000000..982d3d52
> --- /dev/null
> +++ b/sysdeps/mach/hurd/x86_64/static-start.S
> @@ -0,0 +1,27 @@
> +/* 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;

? Isn't that rather feature_1 ?

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

Indeed. Could you perhaps also add them to the i386 tls.h?

> +/* FIXME */
> +# define __LIBC_NO_TLS() 0

We'll want an efficient way to know whether we have configured TLS
indeed. At worse we can make it a global variable.

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

Also copy the comment above TCB_ALIGNMENT.

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

While at it, try to make the i386 version use that too?

Samuel

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Sergey Bugaev]

On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
> > We should not need a getter routine, because one can simply inspect the target
> > thread's state (unless, again, I misunderstand things horribly).
>
> For 16bit fs/gs values we could read them from userland yes. But for
> fs/gs base, the FSGSBASE instruction is not available on all 64bit
> processors. And ATM in THREAD_TCB we want to be able to get the base of
> another thread.

What I've meant is:

__thread_get_state (whatever_thread, &state);
uintptr_t its_fs_base = state->fs_base;

You can't really do the same to *write* [fg]s_base, because doing
thread_set_state on your own thread is bound to end badly.

> > diff --git a/sysdeps/mach/hurd/x86_64/static-start.S b/sysdeps/mach/hurd/x86_64/static-start.S
> > new file mode 100644
> > index 00000000..982d3d52
> > --- /dev/null
> > +++ b/sysdeps/mach/hurd/x86_64/static-start.S
> > @@ -0,0 +1,27 @@
> > +/* 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;
>
> ? Isn't that rather feature_1 ?

sysdeps/mach/hurd/i386/tls.h has 'int private_futex;', which is where
I stole this from. A quick grep confirms that it's never used, so we
might rename both to feature_1, or maybe another instance of
__glibc_padding.

> > +/* 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");
>
> Indeed. Could you perhaps also add them to the i386 tls.h?

> > +/* 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)
>
> While at it, try to make the i386 version use that too?

Yeah, I have not ported the improvements back to the 32-bit version;
maybe I should. Another cool one is doing fs/gs-relative access using
GCC's __seg_fs/__seg_gs when supported.

Sergey

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 19:25:11 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
> > > We should not need a getter routine, because one can simply inspect the target
> > > thread's state (unless, again, I misunderstand things horribly).
> >
> > For 16bit fs/gs values we could read them from userland yes. But for
> > fs/gs base, the FSGSBASE instruction is not available on all 64bit
> > processors. And ATM in THREAD_TCB we want to be able to get the base of
> > another thread.
> 
> What I've meant is:
> 
> __thread_get_state (whatever_thread, &state);
> uintptr_t its_fs_base = state->fs_base;
> 
> You can't really do the same to *write* [fg]s_base, because doing
> thread_set_state on your own thread is bound to end badly.

? Well, sure, just like setting fs/gs through thread state was not done
for i386.

I don't see where you're aiming. Getting fs/gs from __thread_get_state
won't actually give you the base, you'll just read something like 0.

> > > diff --git a/sysdeps/mach/hurd/x86_64/static-start.S b/sysdeps/mach/hurd/x86_64/static-start.S
> > > new file mode 100644
> > > index 00000000..982d3d52
> > > --- /dev/null
> > > +++ b/sysdeps/mach/hurd/x86_64/static-start.S
> > > @@ -0,0 +1,27 @@
> > > +/* 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;
> >
> > ? Isn't that rather feature_1 ?
> 
> sysdeps/mach/hurd/i386/tls.h has 'int private_futex;', which is where
> I stole this from. A quick grep confirms that it's never used,

Yes, this was just to align on the nptl tls.h. But apparently that got
renamed and hurd's tls wasn't updated.

> so we might rename both to feature_1, or maybe another instance of
> __glibc_padding.

Better stay coherent with the nptl version.

> > > +/* 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");
> >
> > Indeed. Could you perhaps also add them to the i386 tls.h?
> 
> > > +/* 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)
> >
> > While at it, try to make the i386 version use that too?
> 
> Yeah, I have not ported the improvements back to the 32-bit version;
> maybe I should.

Better always keep things as coherent as possible. Otherwise another you
will later wonder why in the hell we have differences between the two
versions.

> Another cool one is doing fs/gs-relative access using
> GCC's __seg_fs/__seg_gs when supported.

Yes, that's nice indeed!

Samuel

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Florian Weimer]

* Samuel Thibault via Libc-alpha:

> Sergey Bugaev, le dim. 12 févr. 2023 19:25:11 +0300, a ecrit:
>> On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>> > Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
>> > > We should not need a getter routine, because one can simply inspect the target
>> > > thread's state (unless, again, I misunderstand things horribly).
>> >
>> > For 16bit fs/gs values we could read them from userland yes. But for
>> > fs/gs base, the FSGSBASE instruction is not available on all 64bit
>> > processors. And ATM in THREAD_TCB we want to be able to get the base of
>> > another thread.
>> 
>> What I've meant is:
>> 
>> __thread_get_state (whatever_thread, &state);
>> uintptr_t its_fs_base = state->fs_base;
>> 
>> You can't really do the same to *write* [fg]s_base, because doing
>> thread_set_state on your own thread is bound to end badly.
>
> ? Well, sure, just like setting fs/gs through thread state was not done
> for i386.
>
> I don't see where you're aiming. Getting fs/gs from __thread_get_state
> won't actually give you the base, you'll just read something like 0.

The convention is that the FSBASE address is at %fs:0.  The x86-64 TLS
ABI assumes this, so that it's possible to compute the global (not
thread-specific) address of a TLS variable.

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Samuel Thibault]

Florian Weimer, le dim. 12 févr. 2023 17:40:58 +0100, a ecrit:
> * Samuel Thibault via Libc-alpha:
> 
> > Sergey Bugaev, le dim. 12 févr. 2023 19:25:11 +0300, a ecrit:
> >> On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> >> > Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
> >> > > We should not need a getter routine, because one can simply inspect the target
> >> > > thread's state (unless, again, I misunderstand things horribly).
> >> >
> >> > For 16bit fs/gs values we could read them from userland yes. But for
> >> > fs/gs base, the FSGSBASE instruction is not available on all 64bit
> >> > processors. And ATM in THREAD_TCB we want to be able to get the base of
> >> > another thread.
> >> 
> >> What I've meant is:
> >> 
> >> __thread_get_state (whatever_thread, &state);
> >> uintptr_t its_fs_base = state->fs_base;
> >> 
> >> You can't really do the same to *write* [fg]s_base, because doing
> >> thread_set_state on your own thread is bound to end badly.
> >
> > ? Well, sure, just like setting fs/gs through thread state was not done
> > for i386.
> >
> > I don't see where you're aiming. Getting fs/gs from __thread_get_state
> > won't actually give you the base, you'll just read something like 0.
> 
> The convention is that the FSBASE address is at %fs:0.

Yes, but that works only for reading your own base, not the base of
another thread.

Samuel

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Florian Weimer]

* Samuel Thibault:

> Florian Weimer, le dim. 12 févr. 2023 17:40:58 +0100, a ecrit:
>> * Samuel Thibault via Libc-alpha:
>> 
>> > Sergey Bugaev, le dim. 12 févr. 2023 19:25:11 +0300, a ecrit:
>> >> On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>> >> > Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
>> >> > > We should not need a getter routine, because one can simply inspect the target
>> >> > > thread's state (unless, again, I misunderstand things horribly).
>> >> >
>> >> > For 16bit fs/gs values we could read them from userland yes. But for
>> >> > fs/gs base, the FSGSBASE instruction is not available on all 64bit
>> >> > processors. And ATM in THREAD_TCB we want to be able to get the base of
>> >> > another thread.
>> >> 
>> >> What I've meant is:
>> >> 
>> >> __thread_get_state (whatever_thread, &state);
>> >> uintptr_t its_fs_base = state->fs_base;
>> >> 
>> >> You can't really do the same to *write* [fg]s_base, because doing
>> >> thread_set_state on your own thread is bound to end badly.
>> >
>> > ? Well, sure, just like setting fs/gs through thread state was not done
>> > for i386.
>> >
>> > I don't see where you're aiming. Getting fs/gs from __thread_get_state
>> > won't actually give you the base, you'll just read something like 0.
>> 
>> The convention is that the FSBASE address is at %fs:0.
>
> Yes, but that works only for reading your own base, not the base of
> another thread.

Well, yes, but how do you identify the other thread?  Usually by the
address of its TCB.

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Samuel Thibault]

Florian Weimer, le dim. 12 févr. 2023 20:29:43 +0100, a ecrit:
> * Samuel Thibault:
> 
> > Florian Weimer, le dim. 12 févr. 2023 17:40:58 +0100, a ecrit:
> >> * Samuel Thibault via Libc-alpha:
> >> 
> >> > Sergey Bugaev, le dim. 12 févr. 2023 19:25:11 +0300, a ecrit:
> >> >> On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> >> >> > Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
> >> >> > > We should not need a getter routine, because one can simply inspect the target
> >> >> > > thread's state (unless, again, I misunderstand things horribly).
> >> >> >
> >> >> > For 16bit fs/gs values we could read them from userland yes. But for
> >> >> > fs/gs base, the FSGSBASE instruction is not available on all 64bit
> >> >> > processors. And ATM in THREAD_TCB we want to be able to get the base of
> >> >> > another thread.
> >> >> 
> >> >> What I've meant is:
> >> >> 
> >> >> __thread_get_state (whatever_thread, &state);
> >> >> uintptr_t its_fs_base = state->fs_base;
> >> >> 
> >> >> You can't really do the same to *write* [fg]s_base, because doing
> >> >> thread_set_state on your own thread is bound to end badly.
> >> >
> >> > ? Well, sure, just like setting fs/gs through thread state was not done
> >> > for i386.
> >> >
> >> > I don't see where you're aiming. Getting fs/gs from __thread_get_state
> >> > won't actually give you the base, you'll just read something like 0.
> >> 
> >> The convention is that the FSBASE address is at %fs:0.
> >
> > Yes, but that works only for reading your own base, not the base of
> > another thread.
> 
> Well, yes, but how do you identify the other thread?  Usually by the
> address of its TCB.

Yes, but that's not (yet) the case in htl.

Samuel

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Sergey Bugaev]

On Sun, Feb 12, 2023 at 7:36 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Sergey Bugaev, le dim. 12 févr. 2023 19:25:11 +0300, a ecrit:
> > On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > > Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
> > > > We should not need a getter routine, because one can simply inspect the target
> > > > thread's state (unless, again, I misunderstand things horribly).
> > >
> > > For 16bit fs/gs values we could read them from userland yes. But for
> > > fs/gs base, the FSGSBASE instruction is not available on all 64bit
> > > processors. And ATM in THREAD_TCB we want to be able to get the base of
> > > another thread.
> >
> > What I've meant is:
> >
> > __thread_get_state (whatever_thread, &state);
> > uintptr_t its_fs_base = state->fs_base;
> >
> > You can't really do the same to *write* [fg]s_base, because doing
> > thread_set_state on your own thread is bound to end badly.
>
> ? Well, sure, just like setting fs/gs through thread state was not done
> for i386.
>
> I don't see where you're aiming. Getting fs/gs from __thread_get_state
> won't actually give you the base, you'll just read something like 0.

It is my understanding that the actual values of fs/gs (i.e. the index
of a descriptor) are not useful on x86_64. But fs_base and gs_base are
now things that you have to store in the thread state and save/restore
on every context switch. fs_base and gs_base are like registers in
their own right (well, MSRs are registers). Thus, it should be easy to
read them from the state structure exposed by the kernel.

But again, I really have very little understanding of this, so maybe
I'm talking nonsense.

Sergey

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 19:51:33 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 7:36 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> >
> > Sergey Bugaev, le dim. 12 févr. 2023 19:25:11 +0300, a ecrit:
> > > On Sun, Feb 12, 2023 at 7:11 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > > > Sergey Bugaev, le dim. 12 févr. 2023 14:10:42 +0300, a ecrit:
> > > > > We should not need a getter routine, because one can simply inspect the target
> > > > > thread's state (unless, again, I misunderstand things horribly).
> > > >
> > > > For 16bit fs/gs values we could read them from userland yes. But for
> > > > fs/gs base, the FSGSBASE instruction is not available on all 64bit
> > > > processors. And ATM in THREAD_TCB we want to be able to get the base of
> > > > another thread.
> > >
> > > What I've meant is:
> > >
> > > __thread_get_state (whatever_thread, &state);
> > > uintptr_t its_fs_base = state->fs_base;
> > >
> > > You can't really do the same to *write* [fg]s_base, because doing
> > > thread_set_state on your own thread is bound to end badly.
> >
> > ? Well, sure, just like setting fs/gs through thread state was not done
> > for i386.
> >
> > I don't see where you're aiming. Getting fs/gs from __thread_get_state
> > won't actually give you the base, you'll just read something like 0.
> 
> It is my understanding that the actual values of fs/gs (i.e. the index
> of a descriptor) are not useful on x86_64. But fs_base and gs_base are
> now things that you have to store in the thread state and save/restore
> on every context switch. fs_base and gs_base are like registers in
> their own right (well, MSRs are registers). Thus, it should be easy to
> read them from the state structure exposed by the kernel.

Ah, so you mean adding the fs/gs bases to the thread_state content.

I'd rather make it a separate state content, like we have a separate
i386_DEBUG_STATE content.

(And thus no need for a new RPC).

Samuel

Re: [RFC PATCH glibc 11/12] hurd, htl: Add some x86_64-specific code

[Sergey Bugaev]

On Sun, Feb 12, 2023 at 8:02 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> I'd rather make it a separate state content, like we have a separate
> i386_DEBUG_STATE content.
>
> (And thus no need for a new RPC).

Makes sense to me! 👍

Sergey

[RFC PATCH glibc 12/12] C11 thrd: Downgrade the default alignment of mtx_t

[Sergey Bugaev]

..so that it can match the alignment of pthread_mutex_t on x86_64-gnu.
This likely breaks many other arches (if not all of them), though.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/pthread/threads.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/pthread/threads.h b/sysdeps/pthread/threads.h
index 860d597d..207c1dee 100644
--- a/sysdeps/pthread/threads.h
+++ b/sysdeps/pthread/threads.h
@@ -64,7 +64,7 @@ typedef __once_flag once_flag;
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEX_T];
-  long int __align __LOCK_ALIGNMENT;
+  int __align __LOCK_ALIGNMENT;
 } mtx_t;
 
 typedef union
-- 
2.39.1

Re: [RFC PATCH glibc 12/12] C11 thrd: Downgrade the default alignment of mtx_t

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 14:10:43 +0300, a ecrit:
> ..so that it can match the alignment of pthread_mutex_t on x86_64-gnu.

I'd say rather make pthread_mutex_t aligned on long int, so we can
possibly in the future put some pointers in it without breaking the ABI.

> This likely breaks many other arches (if not all of them), though.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/pthread/threads.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/pthread/threads.h b/sysdeps/pthread/threads.h
> index 860d597d..207c1dee 100644
> --- a/sysdeps/pthread/threads.h
> +++ b/sysdeps/pthread/threads.h
> @@ -64,7 +64,7 @@ typedef __once_flag once_flag;
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEX_T];
> -  long int __align __LOCK_ALIGNMENT;
> +  int __align __LOCK_ALIGNMENT;
>  } mtx_t;
>  
>  typedef union
> -- 
> 2.39.1

Re: [RFC PATCH glibc 12/12] C11 thrd: Downgrade the default alignment of mtx_t

[Sergey Bugaev]

On Sun, Feb 12, 2023 at 6:18 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> I'd say rather make pthread_mutex_t aligned on long int, so we can
> possibly in the future put some pointers in it without breaking the ABI.

I honestly have 0 idea how it is not 8-aligned right now (nor how its
size is 32 and not like 56), given that this is its definition...

no, strike that, I see that there are *two* versions of
struct___pthread_mutex.h, one in sysdeps/mach/hurd/htl/bits and the
other one in sysdeps/htl/bits/types, and the former one wins. The
latter one seems unused then, its only point was to confuse the hell
out of me.

Sergey

Re: [RFC PATCH glibc 12/12] C11 thrd: Downgrade the default alignment of mtx_t

[Samuel Thibault]

Sergey Bugaev, le dim. 12 févr. 2023 18:52:37 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:18 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > I'd say rather make pthread_mutex_t aligned on long int, so we can
> > possibly in the future put some pointers in it without breaking the ABI.
> 
> I honestly have 0 idea how it is not 8-aligned right now (nor how its
> size is 32 and not like 56), given that this is its definition...
> 
> no, strike that, I see that there are *two* versions of
> struct___pthread_mutex.h, one in sysdeps/mach/hurd/htl/bits and the
> other one in sysdeps/htl/bits/types, and the former one wins. The
> latter one seems unused then, its only point was to confuse the hell
> out of me.

History is full of remnants. That's why one just has to take the time to
clean things up and avoid leaving things behind oneself.  It seems the
a99155555c21 cleanup missed this one.

Samuel

Re: [RFC PATCH 0/12] Towards glibc on x86_64-gnu

[Samuel Thibault]

Hello,

Sergey Bugaev via Libc-alpha, le dim. 12 févr. 2023 14:10:31 +0300, a ecrit:
> The main missing pieces seem to be, ordered by scariness increasing:
> - missing x86_64 thread state definition in the kernel headers;
> - TLS things;
> - INTR_MSG_TRAP.

INTR_MSG_TRAP shouldn't be hard. It's the trampoline part which will
be.  Also, sysdeps/mach/hurd/i386/init-first.c. But possibly that part
could be simplified on 32bit first, now that libpthread just takes the
existing stack rather that forcing to use one, see 9cec82de715b which
started to simplify things.

Samuel