diff mbox series

[1/1] newlib/libc/include/setjmp.h: Add returns_twice attribute to setjmp()

Message ID 20250930144920.1023532-2-joel@rtems.org
State New
Headers
Series Add returns_twice attribute to setjmp() |

Commit Message

Joel Sherrill Sept. 30, 2025, 2:49 p.m. UTC 
  The setjmp() function needs this attribute to help GCC avoid false
positives for the -Wclobbered warning. The -Wclobbered warning is
part of -Wextra.
---
 newlib/libc/include/setjmp.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Torbjörn SVENSSON Oct. 1, 2025, 7:22 a.m. UTC | #1 
Hi Joel,

On 2025-09-30 16:49, Joel Sherrill wrote:
> The setjmp() function needs this attribute to help GCC avoid false
> positives for the -Wclobbered warning. The -Wclobbered warning is
> part of -Wextra.
> ---
>   newlib/libc/include/setjmp.h | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/newlib/libc/include/setjmp.h b/newlib/libc/include/setjmp.h
> index a2830b275..5c16321a4 100644
> --- a/newlib/libc/include/setjmp.h
> +++ b/newlib/libc/include/setjmp.h
> @@ -17,7 +17,12 @@ void	longjmp (jmp_buf __jmpb, int __retval)
>   #else
>   void	longjmp (jmp_buf __jmpb, int __retval);
>   #endif
> -int	setjmp (jmp_buf __jmpb);
> +
> +#ifdef __GNUC__
> +int	setjmp (jmp_buf __jmpb)
> +			__attribute__ ((returns_twice));;

I suppose you only want a single semi-colon here.

> +#else

I think you should also drop the empty "#else" block.

Kind regards,
Torbjörn

> +#endif
>   
>   _END_STD_C
>
Brian Inglis Oct. 1, 2025, 8:59 a.m. UTC | #2 
On 2025-10-01 01:22, Torbjorn SVENSSON wrote:
> On 2025-09-30 16:49, Joel Sherrill wrote:
>> The setjmp() function needs this attribute to help GCC avoid false
>> positives for the -Wclobbered warning. The -Wclobbered warning is
>> part of -Wextra.
>> ---
>>   newlib/libc/include/setjmp.h | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/newlib/libc/include/setjmp.h b/newlib/libc/include/setjmp.h
>> index a2830b275..5c16321a4 100644
>> --- a/newlib/libc/include/setjmp.h
>> +++ b/newlib/libc/include/setjmp.h
>> @@ -17,7 +17,12 @@ void    longjmp (jmp_buf __jmpb, int __retval)
>>   #else
>>   void    longjmp (jmp_buf __jmpb, int __retval);
>>   #endif
>> -int    setjmp (jmp_buf __jmpb);
>> +
>> +#ifdef __GNUC__
>> +int    setjmp (jmp_buf __jmpb)
>> +            __attribute__ ((returns_twice));;
> I suppose you only want a single semi-colon here.
>> +#else
> I think you should also drop the empty "#else" block.
>> +#endif
>>   _END_STD_C

Shouldn't the conditional be around the attribute only?

-int    setjmp (jmp_buf __jmpb);
+int	setjmp (jmp_buf __jmpb)
+#ifdef __GNUC__
+		__attribute__ ((returns_twice))
+#endif
+		;
Joel Sherrill Oct. 2, 2025, 1:54 p.m. UTC | #3 
On Wed, Oct 1, 2025 at 2:23 AM Torbjorn SVENSSON <
torbjorn.svensson@foss.st.com> wrote:

> Hi Joel,
>
> On 2025-09-30 16:49, Joel Sherrill wrote:
> > The setjmp() function needs this attribute to help GCC avoid false
> > positives for the -Wclobbered warning. The -Wclobbered warning is
> > part of -Wextra.
> > ---
> >   newlib/libc/include/setjmp.h | 7 ++++++-
> >   1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/newlib/libc/include/setjmp.h b/newlib/libc/include/setjmp.h
> > index a2830b275..5c16321a4 100644
> > --- a/newlib/libc/include/setjmp.h
> > +++ b/newlib/libc/include/setjmp.h
> > @@ -17,7 +17,12 @@ void       longjmp (jmp_buf __jmpb, int __retval)
> >   #else
> >   void        longjmp (jmp_buf __jmpb, int __retval);
> >   #endif
> > -int  setjmp (jmp_buf __jmpb);
> > +
> > +#ifdef __GNUC__
> > +int  setjmp (jmp_buf __jmpb)
> > +                     __attribute__ ((returns_twice));;
>
> I suppose you only want a single semi-colon here.
>

Yes. Thanks.

>
> > +#else
>
> I think you should also drop the empty "#else" block.
>

It was supposed to have a setjmp() prototype without the attribute. This
is following the pattern above with longjmp() which has it this way.

v2 will fix these.

--joel

>
> Kind regards,
> Torbjörn
>
> > +#endif
> >
> >   _END_STD_C
> >
>
>
Joel Sherrill Oct. 2, 2025, 1:57 p.m. UTC | #4 
On Wed, Oct 1, 2025 at 4:03 AM Brian Inglis <Brian.Inglis@systematicsw.ab.ca>
wrote:

> On 2025-10-01 01:22, Torbjorn SVENSSON wrote:
> > On 2025-09-30 16:49, Joel Sherrill wrote:
> >> The setjmp() function needs this attribute to help GCC avoid false
> >> positives for the -Wclobbered warning. The -Wclobbered warning is
> >> part of -Wextra.
> >> ---
> >>   newlib/libc/include/setjmp.h | 7 ++++++-
> >>   1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/newlib/libc/include/setjmp.h b/newlib/libc/include/setjmp.h
> >> index a2830b275..5c16321a4 100644
> >> --- a/newlib/libc/include/setjmp.h
> >> +++ b/newlib/libc/include/setjmp.h
> >> @@ -17,7 +17,12 @@ void    longjmp (jmp_buf __jmpb, int __retval)
> >>   #else
> >>   void    longjmp (jmp_buf __jmpb, int __retval);
> >>   #endif
> >> -int    setjmp (jmp_buf __jmpb);
> >> +
> >> +#ifdef __GNUC__
> >> +int    setjmp (jmp_buf __jmpb)
> >> +            __attribute__ ((returns_twice));;
> > I suppose you only want a single semi-colon here.
> >> +#else
> > I think you should also drop the empty "#else" block.
> >> +#endif
> >>   _END_STD_C
>
> Shouldn't the conditional be around the attribute only?
>

That's not the way setjmp() was. It has this (tabs lost with copy and
paste):

#ifdef __GNUC__
void longjmp (jmp_buf __jmpb, int __retval)
__attribute__ ((__noreturn__));
#else
void longjmp (jmp_buf __jmpb, int __retval);
#endif

I just copied the style of what was there.


>
> -int    setjmp (jmp_buf __jmpb);
> +int    setjmp (jmp_buf __jmpb)
> +#ifdef __GNUC__
> +               __attribute__ ((returns_twice))
> +#endif
> +               ;
>
>
Do you want longjmp() changed to match this also?

--joel


> --
> 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 à retrancher  but when there is no more to
> cut
>                                  -- Antoine de Saint-Exupéry
>
Richard Earnshaw Oct. 2, 2025, 4:42 p.m. UTC | #5 
On 01/10/2025 08:22, Torbjorn SVENSSON wrote:
> Hi Joel,
> 
> On 2025-09-30 16:49, Joel Sherrill wrote:
>> The setjmp() function needs this attribute to help GCC avoid false
>> positives for the -Wclobbered warning. The -Wclobbered warning is
>> part of -Wextra.
>> ---
>>   newlib/libc/include/setjmp.h | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/newlib/libc/include/setjmp.h b/newlib/libc/include/setjmp.h
>> index a2830b275..5c16321a4 100644
>> --- a/newlib/libc/include/setjmp.h
>> +++ b/newlib/libc/include/setjmp.h
>> @@ -17,7 +17,12 @@ void    longjmp (jmp_buf __jmpb, int __retval)
>>   #else
>>   void    longjmp (jmp_buf __jmpb, int __retval);
>>   #endif
>> -int    setjmp (jmp_buf __jmpb);
>> +
>> +#ifdef __GNUC__
>> +int    setjmp (jmp_buf __jmpb)
>> +            __attribute__ ((returns_twice));;
> 
> I suppose you only want a single semi-colon here.
> 
>> +#else
> 
> I think you should also drop the empty "#else" block.
> 
> Kind regards,
> Torbjörn
> 
>> +#endif
>>     _END_STD_C
>>   
> 

ansidecl.h provides some infrastructure to handle all this.  Wouldn't it be better to follow that model?  In particular you can then arrange for your macro to check the actual GCC version.

R.
Brian Inglis Oct. 2, 2025, 5:21 p.m. UTC | #6 
On 2025-10-02 07:57, Joel Sherrill wrote:
> 
> 
> On Wed, Oct 1, 2025 at 4:03 AM Brian Inglis <Brian.Inglis@systematicsw.ab.ca 
> <mailto:Brian.Inglis@systematicsw.ab.ca>> wrote:
> 
>     On 2025-10-01 01:22, Torbjorn SVENSSON wrote:
>      > On 2025-09-30 16:49, Joel Sherrill wrote:
>      >> The setjmp() function needs this attribute to help GCC avoid false
>      >> positives for the -Wclobbered warning. The -Wclobbered warning is
>      >> part of -Wextra.
>      >> ---
>      >>   newlib/libc/include/setjmp.h | 7 ++++++-
>      >>   1 file changed, 6 insertions(+), 1 deletion(-)
>      >>
>      >> diff --git a/newlib/libc/include/setjmp.h b/newlib/libc/include/setjmp.h
>      >> index a2830b275..5c16321a4 100644
>      >> --- a/newlib/libc/include/setjmp.h
>      >> +++ b/newlib/libc/include/setjmp.h
>      >> @@ -17,7 +17,12 @@ void    longjmp (jmp_buf __jmpb, int __retval)
>      >>   #else
>      >>   void    longjmp (jmp_buf __jmpb, int __retval);
>      >>   #endif
>      >> -int    setjmp (jmp_buf __jmpb);
>      >> +
>      >> +#ifdef __GNUC__
>      >> +int    setjmp (jmp_buf __jmpb)
>      >> +            __attribute__ ((returns_twice));;
>      > I suppose you only want a single semi-colon here.
>      >> +#else
>      > I think you should also drop the empty "#else" block.
>      >> +#endif
>      >>   _END_STD_C
> 
>     Shouldn't the conditional be around the attribute only?
> 
> 
> That's not the way setjmp() was. It has this (tabs lost with copy and paste):
> 
> #ifdef __GNUC__
> void longjmp (jmp_buf __jmpb, int __retval)
> __attribute__ ((__noreturn__));
> #else
> void longjmp (jmp_buf __jmpb, int __retval);
> #endif
> 
> I just copied the style of what was there.
> 
> 
>     -int    setjmp (jmp_buf __jmpb);
>     +int    setjmp (jmp_buf __jmpb)
>     +#ifdef __GNUC__
>     +               __attribute__ ((returns_twice))
>     +#endif
>     +               ;
> 
> 
> Do you want longjmp() changed to match this also?

Your choice - it's just a matter of style: Cygwin tweaks do this a lot, leaving 
as much of the existing code alone as possible, avoiding duplication and having 
to check for changes in two lines if something changes, especially if derived 
from an upstream original.
diff mbox series

Patch

diff --git a/newlib/libc/include/setjmp.h b/newlib/libc/include/setjmp.h
index a2830b275..5c16321a4 100644
--- a/newlib/libc/include/setjmp.h
+++ b/newlib/libc/include/setjmp.h
@@ -17,7 +17,12 @@  void	longjmp (jmp_buf __jmpb, int __retval)
 #else
 void	longjmp (jmp_buf __jmpb, int __retval);
 #endif
-int	setjmp (jmp_buf __jmpb);
+
+#ifdef __GNUC__
+int	setjmp (jmp_buf __jmpb)
+			__attribute__ ((returns_twice));;
+#else
+#endif
 
 _END_STD_C