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

  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(+)
  

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

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
  

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.