* [PATCH 1/4] or1k: Fix Linux user space signal ABI
2024-03-19 21:42 [PATCH 0/4] OpenRISC fixes for 2.39 Stafford Horne
@ 2024-03-19 21:42 ` Stafford Horne
2024-03-20 13:24 ` Adhemerval Zanella Netto
2024-03-19 21:42 ` [PATCH 2/4] or1k: Update libm test ulps Stafford Horne
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Stafford Horne @ 2024-03-19 21:42 UTC (permalink / raw)
To: GLIBC patches; +Cc: Linux OpenRISC, 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] or1k: Fix Linux user space signal ABI
2024-03-19 21:42 ` [PATCH 1/4] or1k: Fix Linux user space signal ABI Stafford Horne
@ 2024-03-20 13:24 ` Adhemerval Zanella Netto
2024-03-20 14:13 ` Stafford Horne
0 siblings, 1 reply; 9+ messages in thread
From: Adhemerval Zanella Netto @ 2024-03-20 13:24 UTC (permalink / raw)
To: Stafford Horne, GLIBC patches; +Cc: Linux OpenRISC
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. */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] or1k: Fix Linux user space signal ABI
2024-03-20 13:24 ` Adhemerval Zanella Netto
@ 2024-03-20 14:13 ` Stafford Horne
2024-03-20 20:12 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 9+ messages in thread
From: Stafford Horne @ 2024-03-20 14:13 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: GLIBC patches, Linux OpenRISC
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] or1k: Fix Linux user space signal ABI
2024-03-20 14:13 ` Stafford Horne
@ 2024-03-20 20:12 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 9+ messages in thread
From: Adhemerval Zanella Netto @ 2024-03-20 20:12 UTC (permalink / raw)
To: Stafford Horne; +Cc: GLIBC patches, Linux OpenRISC
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] or1k: Update libm test ulps
2024-03-19 21:42 [PATCH 0/4] OpenRISC fixes for 2.39 Stafford Horne
2024-03-19 21:42 ` [PATCH 1/4] or1k: Fix Linux user space signal ABI Stafford Horne
@ 2024-03-19 21:42 ` Stafford Horne
2024-03-19 21:42 ` [PATCH 3/4] or1k: Only define fpu rouding and exceptions with hard-float Stafford Horne
2024-03-19 21:42 ` [PATCH 4/4] or1k: Add prctl wrapper to unwrap variadic args Stafford Horne
3 siblings, 0 replies; 9+ messages in thread
From: Stafford Horne @ 2024-03-19 21:42 UTC (permalink / raw)
To: GLIBC patches; +Cc: Linux OpenRISC, 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] or1k: Only define fpu rouding and exceptions with hard-float
2024-03-19 21:42 [PATCH 0/4] OpenRISC fixes for 2.39 Stafford Horne
2024-03-19 21:42 ` [PATCH 1/4] or1k: Fix Linux user space signal ABI Stafford Horne
2024-03-19 21:42 ` [PATCH 2/4] or1k: Update libm test ulps Stafford Horne
@ 2024-03-19 21:42 ` Stafford Horne
2024-03-19 21:42 ` [PATCH 4/4] or1k: Add prctl wrapper to unwrap variadic args Stafford Horne
3 siblings, 0 replies; 9+ messages in thread
From: Stafford Horne @ 2024-03-19 21:42 UTC (permalink / raw)
To: GLIBC patches; +Cc: Linux OpenRISC, 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] or1k: Add prctl wrapper to unwrap variadic args
2024-03-19 21:42 [PATCH 0/4] OpenRISC fixes for 2.39 Stafford Horne
` (2 preceding siblings ...)
2024-03-19 21:42 ` [PATCH 3/4] or1k: Only define fpu rouding and exceptions with hard-float Stafford Horne
@ 2024-03-19 21:42 ` Stafford Horne
2024-03-20 13:28 ` Adhemerval Zanella Netto
3 siblings, 1 reply; 9+ messages in thread
From: Stafford Horne @ 2024-03-19 21:42 UTC (permalink / raw)
To: GLIBC patches; +Cc: Linux OpenRISC, 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] or1k: Add prctl wrapper to unwrap variadic args
2024-03-19 21:42 ` [PATCH 4/4] or1k: Add prctl wrapper to unwrap variadic args Stafford Horne
@ 2024-03-20 13:28 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 9+ messages in thread
From: Adhemerval Zanella Netto @ 2024-03-20 13:28 UTC (permalink / raw)
To: Stafford Horne, GLIBC patches; +Cc: Linux OpenRISC
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)
^ permalink raw reply [flat|nested] 9+ messages in thread