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

Message ID 20230212111044.610942-10-bugaevc@gmail.com
State Committed, archived
Headers
Series Towards glibc on x86_64-gnu |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Sergey Bugaev Feb. 12, 2023, 11:10 a.m. UTC
  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(-)
  

Comments

Samuel Thibault Feb. 12, 2023, 3:07 p.m. UTC | #1
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)
>  
>  # 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
> 
>
  
Sergey Bugaev Feb. 12, 2023, 3:38 p.m. UTC | #2
> >  # 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
  
Samuel Thibault Feb. 12, 2023, 3:46 p.m. UTC | #3
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
  
Sergey Bugaev Feb. 12, 2023, 4:01 p.m. UTC | #4
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
  
Joseph Myers Feb. 16, 2023, 8:22 p.m. UTC | #5
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
  

Patch

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