[PATCH 0/4] OpenRISC fixes for 2.39

[PATCH 0/4] OpenRISC fixes for 2.39

[Stafford Horne]

I am preparing to get OpenRISC hard-float patches ready for upsteam and took
some time to make sure all the nofpu port tests are clean.  Running the latest
tests I found a few issues.  This series contains a few fixes for these issues.

Also, during reviewing the Linux signal ABI after Linux floating point support
went upstream I found the OpenRISC ucontext definition is missing one field.
This series includes a fix the this ucontext ABI breakage.

Stafford Horne (4):
  or1k: Fix Linux user space signal ABI
  or1k: Update libm test ulps
  or1k: Only define fpu rouding and exceptions with hard-float
  or1k: Add prctl wrapper to unwrap variadic args

 sysdeps/or1k/bits/fenv.h                    | 19 ++++++++++
 sysdeps/or1k/libm-test-ulps                 |  1 +
 sysdeps/unix/sysv/linux/or1k/prctl.c        | 42 +++++++++++++++++++++
 sysdeps/unix/sysv/linux/or1k/sys/ucontext.h |  1 +
 4 files changed, 63 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/or1k/prctl.c

-- 
2.44.0

[PATCH 1/4] or1k: Fix Linux user space signal ABI

[Stafford Horne]

The OpenRISC sigcontext structure has always been defined as:

    struct user_regs_struct {
	    /* GPR R0-R31... */
	    unsigned long gpr[32];
	    unsigned long pc;
	    unsigned long sr;
    };

    struct sigcontext {
	    struct user_regs_struct regs;  /* needs to be first */
	    unsigned long oldmask;	/* unused */
    };

With Linux v6.8 we added FPU support and repurposed the oldmask
to use for the FPCSR (floating point control status register).

    struct sigcontext {
	    struct user_regs_struct regs;  /* needs to be first */
	    union {
		    unsigned long fpcsr;
		    unsigned long oldmask;	/* unused */
	    };
    };

The definition of mcontext_t was always missing the extra space for
oldmask.  This patch adds the field __fpcsr to mcontext_t to fix the ABI
mismatch between glibc and Linux.
---
 sysdeps/unix/sysv/linux/or1k/sys/ucontext.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h b/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h
index b17e919154..1b428592ee 100644
--- a/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h
+++ b/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h
@@ -38,6 +38,7 @@ typedef struct
     unsigned long int __gprs[__NGREG];
     unsigned long int __pc;
     unsigned long int __sr;
+    unsigned long int __fpcsr;
   } mcontext_t;
 
 /* Userlevel context.  */
-- 
2.44.0

[PATCH 2/4] or1k: Update libm test ulps

[Stafford Horne]

To fix test failures:

    FAIL: math/test-float-hypot
    FAIL: math/test-float32-hypot
---
 sysdeps/or1k/libm-test-ulps | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sysdeps/or1k/libm-test-ulps b/sysdeps/or1k/libm-test-ulps
index 94b383669d..785bae70d0 100644
--- a/sysdeps/or1k/libm-test-ulps
+++ b/sysdeps/or1k/libm-test-ulps
@@ -834,6 +834,7 @@ float: 6
 
 Function: "hypot":
 double: 1
+float: 1
 
 Function: "hypot_downward":
 double: 1
-- 
2.44.0

[PATCH 3/4] or1k: Only define fpu rouding and exceptions with hard-float

[Stafford Horne]

This test failure:

  math/test-fenv

If rounding mode and exception macros are defined then the fenv tests
run and always fail.  This patch adds an ifdef using the
__or1k_hard_float__ macro provided by gcc to avoid defining these fenv
macros when they cnnot be used.  This is similar to what is done in csky.

Note, I will post the or1k hard-float support soon. So, I prefer to
leave the hard-float bits here for now.
---
 sysdeps/or1k/bits/fenv.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sysdeps/or1k/bits/fenv.h b/sysdeps/or1k/bits/fenv.h
index 587039ca03..01267805e6 100644
--- a/sysdeps/or1k/bits/fenv.h
+++ b/sysdeps/or1k/bits/fenv.h
@@ -21,6 +21,7 @@
 # error "Never use <bits/fenv.h> directly; include <fenv.h> instead."
 #endif
 
+#ifdef __or1k_hard_float__
 /* Define bits representing exceptions in the FPCSR status word.  */
 enum
   {
@@ -51,6 +52,24 @@ enum
 #define FE_UPWARD     (0x2 << 1)
 #define FE_DOWNWARD   (0x3 << 1)
 
+#else
+
+/* In the soft-float case only rounding to nearest is supported, with
+   no exceptions.  */
+
+enum
+  {
+    __FE_UNDEFINED = -1,
+
+    FE_TONEAREST =
+# define FE_TONEAREST	0x0
+      FE_TONEAREST
+  };
+
+# define FE_ALL_EXCEPT 0
+
+#endif /* __or1k_hard_float__ */
+
 /* Type representing exception flags.  */
 typedef unsigned int fexcept_t;
 
-- 
2.44.0

[PATCH 4/4] or1k: Add prctl wrapper to unwrap variadic args

[Stafford Horne]

On OpenRISC variadic functions and regular functions have different
calling conventions so this wrapper is needed to translate.  This
wrapper is copied from x86_64/x32.  I don't know the build system enough
to find a cleaner way to share the code between x86_64/x32 and or1k
(maybe Implies?), so I went with the straight copy.

This fixes test failures:

  misc/tst-prctl
  nptl/tst-setgetname
---
 sysdeps/unix/sysv/linux/or1k/prctl.c | 42 ++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/or1k/prctl.c

diff --git a/sysdeps/unix/sysv/linux/or1k/prctl.c b/sysdeps/unix/sysv/linux/or1k/prctl.c
new file mode 100644
index 0000000000..1aa707d175
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/or1k/prctl.c
@@ -0,0 +1,42 @@
+/* prctl - Linux specific syscall.  OpenRISC version.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sysdep.h>
+#include <stdarg.h>
+#include <sys/prctl.h>
+
+/* Unconditionally read all potential arguments.  This may pass
+   garbage values to the kernel, but avoids the need for teaching
+   glibc the argument counts of individual options (including ones
+   that are added to the kernel in the future).  */
+
+int
+__prctl (int option, ...)
+{
+  va_list arg;
+  va_start (arg, option);
+  unsigned long int arg2 = va_arg (arg, unsigned long int);
+  unsigned long int arg3 = va_arg (arg, unsigned long int);
+  unsigned long int arg4 = va_arg (arg, unsigned long int);
+  unsigned long int arg5 = va_arg (arg, unsigned long int);
+  va_end (arg);
+  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
+}
+
+libc_hidden_def (__prctl)
+weak_alias (__prctl, prctl)
-- 
2.44.0

Re: [PATCH 1/4] or1k: Fix Linux user space signal ABI

[Adhemerval Zanella Netto]

On 19/03/24 18:42, Stafford Horne wrote:
> The OpenRISC sigcontext structure has always been defined as:
> 
>     struct user_regs_struct {
> 	    /* GPR R0-R31... */
> 	    unsigned long gpr[32];
> 	    unsigned long pc;
> 	    unsigned long sr;
>     };
> 
>     struct sigcontext {
> 	    struct user_regs_struct regs;  /* needs to be first */
> 	    unsigned long oldmask;	/* unused */
>     };
> 
> With Linux v6.8 we added FPU support and repurposed the oldmask
> to use for the FPCSR (floating point control status register).
> 
>     struct sigcontext {
> 	    struct user_regs_struct regs;  /* needs to be first */
> 	    union {
> 		    unsigned long fpcsr;
> 		    unsigned long oldmask;	/* unused */
> 	    };
>     };
> 
> The definition of mcontext_t was always missing the extra space for
> oldmask.  This patch adds the field __fpcsr to mcontext_t to fix the ABI
> mismatch between glibc and Linux.

This is strictly an ABI break, this won't make the swapcontext functions 
to fail (since they are not update to take in consideration the new field),
but it also means that the fpcsr won't be save/restore and the application
can potentially read uninitialized values.

But I take that the fpu support will be a new ABI, so I suggest to fix
when you add it (along with proper support to context functions).

> ---
>  sysdeps/unix/sysv/linux/or1k/sys/ucontext.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h b/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h
> index b17e919154..1b428592ee 100644
> --- a/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h
> +++ b/sysdeps/unix/sysv/linux/or1k/sys/ucontext.h
> @@ -38,6 +38,7 @@ typedef struct
>      unsigned long int __gprs[__NGREG];
>      unsigned long int __pc;
>      unsigned long int __sr;
> +    unsigned long int __fpcsr;
>    } mcontext_t;
>  
>  /* Userlevel context.  */

Re: [PATCH 4/4] or1k: Add prctl wrapper to unwrap variadic args

[Adhemerval Zanella Netto]

On 19/03/24 18:42, Stafford Horne wrote:
> On OpenRISC variadic functions and regular functions have different
> calling conventions so this wrapper is needed to translate.  This
> wrapper is copied from x86_64/x32.  I don't know the build system enough
> to find a cleaner way to share the code between x86_64/x32 and or1k
> (maybe Implies?), so I went with the straight copy.

It looks ok, the old way of cross-reference ABIs implementations only
add complications. 

> 
> This fixes test failures:
> 
>   misc/tst-prctl
>   nptl/tst-setgetname
> ---
>  sysdeps/unix/sysv/linux/or1k/prctl.c | 42 ++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/or1k/prctl.c
> 
> diff --git a/sysdeps/unix/sysv/linux/or1k/prctl.c b/sysdeps/unix/sysv/linux/or1k/prctl.c
> new file mode 100644
> index 0000000000..1aa707d175
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/or1k/prctl.c
> @@ -0,0 +1,42 @@
> +/* prctl - Linux specific syscall.  OpenRISC version.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <sysdep.h>
> +#include <stdarg.h>
> +#include <sys/prctl.h>
> +
> +/* Unconditionally read all potential arguments.  This may pass
> +   garbage values to the kernel, but avoids the need for teaching
> +   glibc the argument counts of individual options (including ones
> +   that are added to the kernel in the future).  */
> +
> +int
> +__prctl (int option, ...)
> +{
> +  va_list arg;
> +  va_start (arg, option);
> +  unsigned long int arg2 = va_arg (arg, unsigned long int);
> +  unsigned long int arg3 = va_arg (arg, unsigned long int);
> +  unsigned long int arg4 = va_arg (arg, unsigned long int);
> +  unsigned long int arg5 = va_arg (arg, unsigned long int);
> +  va_end (arg);
> +  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
> +}
> +
> +libc_hidden_def (__prctl)
> +weak_alias (__prctl, prctl)

Re: [PATCH 1/4] or1k: Fix Linux user space signal ABI

[Stafford Horne]

On Wed, Mar 20, 2024 at 10:24:15AM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 19/03/24 18:42, Stafford Horne wrote:
> > The OpenRISC sigcontext structure has always been defined as:
> > 
> >     struct user_regs_struct {
> > 	    /* GPR R0-R31... */
> > 	    unsigned long gpr[32];
> > 	    unsigned long pc;
> > 	    unsigned long sr;
> >     };
> > 
> >     struct sigcontext {
> > 	    struct user_regs_struct regs;  /* needs to be first */
> > 	    unsigned long oldmask;	/* unused */
> >     };
> > 
> > With Linux v6.8 we added FPU support and repurposed the oldmask
> > to use for the FPCSR (floating point control status register).
> > 
> >     struct sigcontext {
> > 	    struct user_regs_struct regs;  /* needs to be first */
> > 	    union {
> > 		    unsigned long fpcsr;
> > 		    unsigned long oldmask;	/* unused */
> > 	    };
> >     };
> > 
> > The definition of mcontext_t was always missing the extra space for
> > oldmask.  This patch adds the field __fpcsr to mcontext_t to fix the ABI
> > mismatch between glibc and Linux.
> 
> This is strictly an ABI break, this won't make the swapcontext functions 
> to fail (since they are not update to take in consideration the new field),
> but it also means that the fpcsr won't be save/restore and the application
> can potentially read uninitialized values.
> 
> But I take that the fpu support will be a new ABI, so I suggest to fix
> when you add it (along with proper support to context functions).

OK, I got it.  I will post this when the hard float code is added and also fixup
swapcontext etc to populated it correctly with or without hard float.

Note there is broken ABI already, as programs will not be able to access sigmask
if needed due to:

Linux definition:

    struct ucontext {
	    unsigned long     uc_flags;
	    struct ucontext  *uc_link;
	    stack_t           uc_stack;
	    struct sigcontext uc_mcontext;  <-- size differs between glibc and linux
	    sigset_t          uc_sigmask;   <-- won't be able to access if needed
    };

But still I will leave as is for now.  This hasn't cause any issues as far as I
have seen so far.

-Stafford

Re: [PATCH 1/4] or1k: Fix Linux user space signal ABI

[Adhemerval Zanella Netto]

On 20/03/24 11:13, Stafford Horne wrote:
> On Wed, Mar 20, 2024 at 10:24:15AM -0300, Adhemerval Zanella Netto wrote:
>>
>>
>> On 19/03/24 18:42, Stafford Horne wrote:
>>> The OpenRISC sigcontext structure has always been defined as:
>>>
>>>     struct user_regs_struct {
>>> 	    /* GPR R0-R31... */
>>> 	    unsigned long gpr[32];
>>> 	    unsigned long pc;
>>> 	    unsigned long sr;
>>>     };
>>>
>>>     struct sigcontext {
>>> 	    struct user_regs_struct regs;  /* needs to be first */
>>> 	    unsigned long oldmask;	/* unused */
>>>     };
>>>
>>> With Linux v6.8 we added FPU support and repurposed the oldmask
>>> to use for the FPCSR (floating point control status register).
>>>
>>>     struct sigcontext {
>>> 	    struct user_regs_struct regs;  /* needs to be first */
>>> 	    union {
>>> 		    unsigned long fpcsr;
>>> 		    unsigned long oldmask;	/* unused */
>>> 	    };
>>>     };
>>>
>>> The definition of mcontext_t was always missing the extra space for
>>> oldmask.  This patch adds the field __fpcsr to mcontext_t to fix the ABI
>>> mismatch between glibc and Linux.
>>
>> This is strictly an ABI break, this won't make the swapcontext functions 
>> to fail (since they are not update to take in consideration the new field),
>> but it also means that the fpcsr won't be save/restore and the application
>> can potentially read uninitialized values.
>>
>> But I take that the fpu support will be a new ABI, so I suggest to fix
>> when you add it (along with proper support to context functions).
> 
> OK, I got it.  I will post this when the hard float code is added and also fixup
> swapcontext etc to populated it correctly with or without hard float.
> 
> Note there is broken ABI already, as programs will not be able to access sigmask
> if needed due to:
> 
> Linux definition:
> 
>     struct ucontext {
> 	    unsigned long     uc_flags;
> 	    struct ucontext  *uc_link;
> 	    stack_t           uc_stack;
> 	    struct sigcontext uc_mcontext;  <-- size differs between glibc and linux
> 	    sigset_t          uc_sigmask;   <-- won't be able to access if needed
>     };
> > But still I will leave as is for now.  This hasn't cause any issues as far as I
> have seen so far.

But even changing the uc_sigmask offset on ucontext would require versioned
context functions.  Usually to access the signal frame ucontex_t, it is safer
to user the linux UAPI definition instead of the libc one.