sparc: Improve setjmp()

  Flush the windows in setjmp().  This helps if the stack is changed after
the setjmp() and we want to jump back to the original stack using
longjmp().
---
 newlib/libc/machine/sparc/setjmp.S | 2 ++
 1 file changed, 2 insertions(+)
  

On 06.10.23 07:31, Sebastian Huber wrote:
> Flush the windows in setjmp().  This helps if the stack is changed after
> the setjmp() and we want to jump back to the original stack using
> longjmp().
> ---
>   newlib/libc/machine/sparc/setjmp.S | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/newlib/libc/machine/sparc/setjmp.S b/newlib/libc/machine/sparc/setjmp.S
> index 613df2ba2..d7185be4c 100644
> --- a/newlib/libc/machine/sparc/setjmp.S
> +++ b/newlib/libc/machine/sparc/setjmp.S
> @@ -110,6 +110,8 @@
>   
>   ENTRY(setjmp)
>   ENTRY(_setjmp)
> +        ta      0x03            /* Flush registers, just in case another stack
> +                                   is used after the setjmp().  */
>           st      %sp, [%o0]      /* caller's stack pointer */
>           st      %i7, [%o0+4]    /* caller's return pc */
>           st      %fp, [%o0+8]    /* store caller's frame pointer */

I am not sure if there is anyone left being able to review this change.
  

Hi Sebastian,

I am not familiar with sparc to comment, but Corinna has performed
maintenance on the setjmp.S file if
you want to wait for her to look at the change.  Otherwise, if you have a
test case that
verifies the change, feel free to merge and you can confirm with Corinna
when she is back.

-- Jeff J.


On Thu, Oct 12, 2023 at 5:39 AM Sebastian Huber <
sebastian.huber@embedded-brains.de> wrote:

> On 06.10.23 07:31, Sebastian Huber wrote:
> > Flush the windows in setjmp().  This helps if the stack is changed after
> > the setjmp() and we want to jump back to the original stack using
> > longjmp().
> > ---
> >   newlib/libc/machine/sparc/setjmp.S | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/newlib/libc/machine/sparc/setjmp.S
> b/newlib/libc/machine/sparc/setjmp.S
> > index 613df2ba2..d7185be4c 100644
> > --- a/newlib/libc/machine/sparc/setjmp.S
> > +++ b/newlib/libc/machine/sparc/setjmp.S
> > @@ -110,6 +110,8 @@
> >
> >   ENTRY(setjmp)
> >   ENTRY(_setjmp)
> > +        ta      0x03            /* Flush registers, just in case
> another stack
> > +                                   is used after the setjmp().  */
> >           st      %sp, [%o0]      /* caller's stack pointer */
> >           st      %i7, [%o0+4]    /* caller's return pc */
> >           st      %fp, [%o0+8]    /* store caller's frame pointer */
>
> I am not sure if there is anyone left being able to review this change.
>
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.huber@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
>
>
  

Hello Jeff,

On 12.10.23 16:50, Jeff Johnston wrote:
> Hi Sebastian,
> 
> I am not familiar with sparc to comment, but Corinna has performed 
> maintenance on the setjmp.S file if
> you want to wait for her to look at the change.  Otherwise, if you have 
> a test case that
> verifies the change, feel free to merge and you can confirm with Corinna 
> when she is back.

the glibc version does also the window flush. I noticed the issue by 
testing compiler builtins which on some systems result in a trap, for 
example:

     volatile int64_t n;
     volatile int64_t d;

     n = INT64_C( 0 );
     d = INT64_C( 0 );
     do_longjmp = true;

     if ( setjmp( exception_return_context ) == 0 ) {
       n = n % d;
     }

In RTEMS, we use a processor-specific stack to handle synchronous 
exceptions. If you want to jump back to the thread from the exception 
handler, then you have to change back to the original thread stack. This 
works only if you flush the windows in the setjmp().

> 
> -- Jeff J.
> 
> 
> On Thu, Oct 12, 2023 at 5:39 AM Sebastian Huber 
> <sebastian.huber@embedded-brains.de 
> <mailto:sebastian.huber@embedded-brains.de>> wrote:
> 
>     On 06.10.23 07:31, Sebastian Huber wrote:
>      > Flush the windows in setjmp().  This helps if the stack is
>     changed after
>      > the setjmp() and we want to jump back to the original stack using
>      > longjmp().
>      > ---
>      >   newlib/libc/machine/sparc/setjmp.S | 2 ++
>      >   1 file changed, 2 insertions(+)
>      >
>      > diff --git a/newlib/libc/machine/sparc/setjmp.S
>     b/newlib/libc/machine/sparc/setjmp.S
>      > index 613df2ba2..d7185be4c 100644
>      > --- a/newlib/libc/machine/sparc/setjmp.S
>      > +++ b/newlib/libc/machine/sparc/setjmp.S
>      > @@ -110,6 +110,8 @@
>      >
>      >   ENTRY(setjmp)
>      >   ENTRY(_setjmp)
>      > +        ta      0x03            /* Flush registers, just in case
>     another stack
>      > +                                   is used after the setjmp().  */
>      >           st      %sp, [%o0]      /* caller's stack pointer */
>      >           st      %i7, [%o0+4]    /* caller's return pc */
>      >           st      %fp, [%o0+8]    /* store caller's frame pointer */
> 
>     I am not sure if there is anyone left being able to review this change.
> 
>     -- 
>     embedded brains GmbH
>     Herr Sebastian HUBER
>     Dornierstr. 4
>     82178 Puchheim
>     Germany
>     email: sebastian.huber@embedded-brains.de
>     <mailto:sebastian.huber@embedded-brains.de>
>     phone: +49-89-18 94 741 - 16
>     fax:   +49-89-18 94 741 - 08
> 
>     Registergericht: Amtsgericht München
>     Registernummer: HRB 157899
>     Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
>     Unsere Datenschutzerklärung finden Sie hier:
>     https://embedded-brains.de/datenschutzerklaerung/
>     <https://embedded-brains.de/datenschutzerklaerung/>
>
  

On 2023-10-12 03:39, Sebastian Huber wrote:
> On 06.10.23 07:31, Sebastian Huber wrote:
>> Flush the windows in setjmp().  This helps if the stack is changed after
>> the setjmp() and we want to jump back to the original stack using
>> longjmp().
>> ---
>>   newlib/libc/machine/sparc/setjmp.S | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/newlib/libc/machine/sparc/setjmp.S 
>> b/newlib/libc/machine/sparc/setjmp.S
>> index 613df2ba2..d7185be4c 100644
>> --- a/newlib/libc/machine/sparc/setjmp.S
>> +++ b/newlib/libc/machine/sparc/setjmp.S
>> @@ -110,6 +110,8 @@
>>   ENTRY(setjmp)
>>   ENTRY(_setjmp)
>> +        ta      0x03            /* Flush registers, just in case another stack
>> +                                   is used after the setjmp().  */
>>           st      %sp, [%o0]      /* caller's stack pointer */
>>           st      %i7, [%o0+4]    /* caller's return pc */
>>           st      %fp, [%o0+8]    /* store caller's frame pointer */
> 
> I am not sure if there is anyone left being able to review this change.

Hopefully someone is, but as a former SunOS/Sparc guy (sysadmin/board swapper 
but not assembler there) I got interested, and found one related discussion on 
the sparclinux list:

https://lore.kernel.org/sparclinux/20111007232209.GA11892@wooyd.org/t/#u

which seems to indicate that you are DTRT; also for comparison:

	https://gcc.gnu.org/onlinedocs/gcc/Nonlocal-Gotos.html
and
	$ info gcc 'nonlocal gotos'

although:

https://stackoverflow.com/questions/72711501/special-treatment-of-setjmp-longjmp-by-compilers
	https://gcc.gnu.org/legacy-ml/gcc/2018-03/msg00032.html
	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83368

and some threads about LLVM enhancing and dropping Sparc __builtin_set/longjmp.

But as Knuth said about some code: "I have proved it correct, but not tested 
it.", which is the proof of the pudding: issue before patch; no issues after patch?
  

On Thu, Oct 12, 2023 at 10:55 AM Sebastian Huber <
sebastian.huber@embedded-brains.de> wrote:

> Hello Jeff,
>
> On 12.10.23 16:50, Jeff Johnston wrote:
> > Hi Sebastian,
> >
> > I am not familiar with sparc to comment, but Corinna has performed
> > maintenance on the setjmp.S file if
> > you want to wait for her to look at the change.  Otherwise, if you have
> > a test case that
> > verifies the change, feel free to merge and you can confirm with Corinna
> > when she is back.
>
> the glibc version does also the window flush. I noticed the issue by
> testing compiler builtins which on some systems result in a trap, for
> example:
>

Then by all means go ahead and check it in.

-- Jeff J.

>
>      volatile int64_t n;
>      volatile int64_t d;
>
>      n = INT64_C( 0 );
>      d = INT64_C( 0 );
>      do_longjmp = true;
>
>      if ( setjmp( exception_return_context ) == 0 ) {
>        n = n % d;
>      }
>
> In RTEMS, we use a processor-specific stack to handle synchronous
> exceptions. If you want to jump back to the thread from the exception
> handler, then you have to change back to the original thread stack. This
> works only if you flush the windows in the setjmp().
>
> >
> > -- Jeff J.
> >
> >
> > On Thu, Oct 12, 2023 at 5:39 AM Sebastian Huber
> > <sebastian.huber@embedded-brains.de
> > <mailto:sebastian.huber@embedded-brains.de>> wrote:
> >
> >     On 06.10.23 07:31, Sebastian Huber wrote:
> >      > Flush the windows in setjmp().  This helps if the stack is
> >     changed after
> >      > the setjmp() and we want to jump back to the original stack using
> >      > longjmp().
> >      > ---
> >      >   newlib/libc/machine/sparc/setjmp.S | 2 ++
> >      >   1 file changed, 2 insertions(+)
> >      >
> >      > diff --git a/newlib/libc/machine/sparc/setjmp.S
> >     b/newlib/libc/machine/sparc/setjmp.S
> >      > index 613df2ba2..d7185be4c 100644
> >      > --- a/newlib/libc/machine/sparc/setjmp.S
> >      > +++ b/newlib/libc/machine/sparc/setjmp.S
> >      > @@ -110,6 +110,8 @@
> >      >
> >      >   ENTRY(setjmp)
> >      >   ENTRY(_setjmp)
> >      > +        ta      0x03            /* Flush registers, just in case
> >     another stack
> >      > +                                   is used after the setjmp().
> */
> >      >           st      %sp, [%o0]      /* caller's stack pointer */
> >      >           st      %i7, [%o0+4]    /* caller's return pc */
> >      >           st      %fp, [%o0+8]    /* store caller's frame pointer
> */
> >
> >     I am not sure if there is anyone left being able to review this
> change.
> >
> >     --
> >     embedded brains GmbH
> >     Herr Sebastian HUBER
> >     Dornierstr. 4
> >     82178 Puchheim
> >     Germany
> >     email: sebastian.huber@embedded-brains.de
> >     <mailto:sebastian.huber@embedded-brains.de>
> >     phone: +49-89-18 94 741 - 16
> >     fax:   +49-89-18 94 741 - 08
> >
> >     Registergericht: Amtsgericht München
> >     Registernummer: HRB 157899
> >     Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas
> Dörfler
> >     Unsere Datenschutzerklärung finden Sie hier:
> >     https://embedded-brains.de/datenschutzerklaerung/
> >     <https://embedded-brains.de/datenschutzerklaerung/>
> >
>
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.huber@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
>
>