public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] sparc: Improve setjmp()
@ 2023-10-06  5:31 Sebastian Huber
  2023-10-12  9:39 ` Sebastian Huber
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Huber @ 2023-10-06  5:31 UTC (permalink / raw)
  To: newlib

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sparc: Improve setjmp()
  2023-10-06  5:31 [PATCH] sparc: Improve setjmp() Sebastian Huber
@ 2023-10-12  9:39 ` Sebastian Huber
  2023-10-12 14:50   ` Jeff Johnston
  2023-10-12 15:19   ` Brian Inglis
  0 siblings, 2 replies; 6+ messages in thread
From: Sebastian Huber @ 2023-10-12  9:39 UTC (permalink / raw)
  To: newlib

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sparc: Improve setjmp()
  2023-10-12  9:39 ` Sebastian Huber
@ 2023-10-12 14:50   ` Jeff Johnston
  2023-10-12 14:55     ` Sebastian Huber
  2023-10-12 15:19   ` Brian Inglis
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff Johnston @ 2023-10-12 14:50 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: newlib

[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sparc: Improve setjmp()
  2023-10-12 14:50   ` Jeff Johnston
@ 2023-10-12 14:55     ` Sebastian Huber
  2023-10-12 15:33       ` Jeff Johnston
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Huber @ 2023-10-12 14:55 UTC (permalink / raw)
  To: Jeff Johnston; +Cc: newlib

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sparc: Improve setjmp()
  2023-10-12  9:39 ` Sebastian Huber
  2023-10-12 14:50   ` Jeff Johnston
@ 2023-10-12 15:19   ` Brian Inglis
  1 sibling, 0 replies; 6+ messages in thread
From: Brian Inglis @ 2023-10-12 15:19 UTC (permalink / raw)
  To: newlib

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?

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sparc: Improve setjmp()
  2023-10-12 14:55     ` Sebastian Huber
@ 2023-10-12 15:33       ` Jeff Johnston
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff Johnston @ 2023-10-12 15:33 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: newlib

[-- Attachment #1: Type: text/plain, Size: 3810 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-12 15:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-06  5:31 [PATCH] sparc: Improve setjmp() Sebastian Huber
2023-10-12  9:39 ` Sebastian Huber
2023-10-12 14:50   ` Jeff Johnston
2023-10-12 14:55     ` Sebastian Huber
2023-10-12 15:33       ` Jeff Johnston
2023-10-12 15:19   ` Brian Inglis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).