[PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[philomath]

Since the _Noreturn identifier is not in the user's namespace, we can use it
directly.  in case it's not a keyword (i.e. pre-C11), it is defined as
__attribute__((__noreturn__)) on GCC (where it is implemented since version
2.95), and empty elsewhere.

There are some more interfaces and functions that could make use of this
specifier, here are only those specified in the C11 standard.

---
 include/setjmp.h      | 10 ++++--
 include/stdlib.h      | 15 ++++++---
 src/exit/_Exit.c      | 10 +++++-
 src/exit/abort.c      | 10 +++++-
 src/exit/exit.c       | 10 +++++-
 src/exit/quick_exit.c | 10 +++++-
 6 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/include/setjmp.h b/include/setjmp.h
index 7dc7276..6db4786 100644
--- a/include/setjmp.h
+++ b/include/setjmp.h
@@ -27,9 +27,15 @@ int _setjmp (jmp_buf);
 void _longjmp (jmp_buf, int);
 #endif
 
-
+#if __STDC_VERSION__ < 201112L
+# if __GNUC__
+#  define _Noreturn __attribute__((__noreturn__))
+# else
+#  define _Noreturn
+# endif
+#endif
 int setjmp (jmp_buf);
-void longjmp (jmp_buf, int);
+_Noreturn void longjmp (jmp_buf, int);
 
 #define setjmp setjmp
 #define longjmp longjmp
diff --git a/include/stdlib.h b/include/stdlib.h
index 1749cb3..5e28cd2 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -40,12 +40,19 @@ void *realloc (void *, size_t);
 void free (void *);
 void *aligned_alloc(size_t alignment, size_t size);
 
-void abort (void);
+#if __STDC_VERSION__ < 201112L
+# if __GNUC__
+#  define _Noreturn __attribute__((__noreturn__))
+# else
+#  define _Noreturn
+# endif
+#endif
+_Noreturn void abort (void);
 int atexit (void (*) (void));
-void exit (int);
-void _Exit (int);
+_Noreturn void exit (int);
+_Noreturn void _Exit (int);
 int at_quick_exit (void (*) (void));
-void quick_exit (int);
+_Noreturn void quick_exit (int);
 
 char *getenv (const char *);
 
diff --git a/src/exit/_Exit.c b/src/exit/_Exit.c
index 6ceb143..ea85e4a 100644
--- a/src/exit/_Exit.c
+++ b/src/exit/_Exit.c
@@ -1,7 +1,15 @@
 #include <stdlib.h>
 #include "syscall.h"
 
-void _Exit(int ec)
+#if __STDC_VERSION__ < 201112L
+# if __GNUC__
+#  define _Noreturn __attribute__((__noreturn__))
+# else
+#  define _Noreturn
+# endif
+#endif
+
+_Noreturn void _Exit(int ec)
 {
 	__syscall(SYS_exit_group, ec);
 	__syscall(SYS_exit, ec);
diff --git a/src/exit/abort.c b/src/exit/abort.c
index c5b9e52..a3fa436 100644
--- a/src/exit/abort.c
+++ b/src/exit/abort.c
@@ -2,7 +2,15 @@
 #include <signal.h>
 #include "syscall.h"
 
-void abort(void)
+#if __STDC_VERSION__ < 201112L
+# if __GNUC__
+#  define _Noreturn __attribute__((__noreturn__))
+# else
+#  define _Noreturn
+# endif
+#endif
+
+_Noreturn void abort(void)
 {
 	raise(SIGABRT);
 	raise(SIGKILL);
diff --git a/src/exit/exit.c b/src/exit/exit.c
index e4aeaf1..0a1c9aa 100644
--- a/src/exit/exit.c
+++ b/src/exit/exit.c
@@ -14,7 +14,15 @@ weak_alias(dummy, __funcs_on_exit);
 weak_alias(dummy, __flush_on_exit);
 weak_alias(dummy, __seek_on_exit);
 
-void exit(int code)
+#if __STDC_VERSION__ < 201112L
+# if __GNUC__
+#  define _Noreturn __attribute__((__noreturn__))
+# else
+#  define _Noreturn
+# endif
+#endif
+
+_Noreturn void exit(int code)
 {
 	static int lock;
 
diff --git a/src/exit/quick_exit.c b/src/exit/quick_exit.c
index 18d5288..936cd7f 100644
--- a/src/exit/quick_exit.c
+++ b/src/exit/quick_exit.c
@@ -3,10 +3,18 @@
 #include "atomic.h"
 #include "libc.h"
 
+#if __STDC_VERSION__ < 201112L
+# if __GNUC__
+#  define _Noreturn __attribute__((__noreturn__))
+# else
+#  define _Noreturn
+# endif
+#endif
+
 static void dummy() { }
 weak_alias(dummy, __funcs_on_quick_exit);
 
-void quick_exit(int code)
+_Noreturn void quick_exit(int code)
 {
 	static int lock;
 	while (a_swap(&lock, 1)) __syscall(SYS_pause);
-- 
1.7.12

Re: [PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[Rich Felker]

On Wed, Sep 05, 2012 at 01:53:11PM +0200, philomath wrote:
> Since the _Noreturn identifier is not in the user's namespace, we can use it
> directly.  in case it's not a keyword (i.e. pre-C11), it is defined as
> __attribute__((__noreturn__)) on GCC (where it is implemented since version
> 2.95), and empty elsewhere.
> 
> There are some more interfaces and functions that could make use of this
> specifier, here are only those specified in the C11 standard.
> 
> ---
>  include/setjmp.h      | 10 ++++--
>  include/stdlib.h      | 15 ++++++---
>  src/exit/_Exit.c      | 10 +++++-
>  src/exit/abort.c      | 10 +++++-
>  src/exit/exit.c       | 10 +++++-
>  src/exit/quick_exit.c | 10 +++++-
>  6 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/include/setjmp.h b/include/setjmp.h
> index 7dc7276..6db4786 100644
> --- a/include/setjmp.h
> +++ b/include/setjmp.h
> @@ -27,9 +27,15 @@ int _setjmp (jmp_buf);
>  void _longjmp (jmp_buf, int);
>  #endif
>  
> -
> +#if __STDC_VERSION__ < 201112L
> +# if __GNUC__
> +#  define _Noreturn __attribute__((__noreturn__))
> +# else
> +#  define _Noreturn
> +# endif
> +#endif

Is there a way you can make the logic work without nested #ifs? If
not, please at least use consistent style (no spaces after #) with the
rest of the headers, but I'd prefer if we could nesting altogether.
Perhaps..

#if __STDC_VERSION__ >= 201112L
#elif defined(__GNUC__)
#define _Noreturn __attribute__((__noreturn__))
#else
#define _Noreturn
#endif

> diff --git a/src/exit/_Exit.c b/src/exit/_Exit.c
> index 6ceb143..ea85e4a 100644
> --- a/src/exit/_Exit.c
> +++ b/src/exit/_Exit.c
> @@ -1,7 +1,15 @@
>  #include <stdlib.h>
>  #include "syscall.h"
>  
> -void _Exit(int ec)
> +#if __STDC_VERSION__ < 201112L
> +# if __GNUC__
> +#  define _Noreturn __attribute__((__noreturn__))
> +# else
> +#  define _Noreturn
> +# endif
> +#endif

Why is this duplicated in the source files? The headers already
handled it, so _Noreturn is already defined if needed; it can just be
used as if this were always C11 code.

BTW, this brings up the point that we should be checking for support
for -std=c11 or -std=c1x and using those if available, and only
falling back to -std=c99 if not.

Rich

Re: [PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[philomath]

Hi,

Here is an updated version, addressing the issues rich mentioned.

Should I add _Noreturn to other functions too (such as the various a_crash
versions, *err*, etc)?

---
Since the _Noreturn identifier is not in the user's namespace, we can use it
directly.  in case it's not a keyword (i.e. pre-C11), it is defined as
__attribute__((__noreturn__)) on GCC (where it is implemented since version
2.95), and empty elsewhere.

---
 include/setjmp.h      |  9 ++++++++-
 include/stdlib.h      | 15 +++++++++++----
 src/exit/_Exit.c      |  2 +-
 src/exit/abort.c      |  2 +-
 src/exit/exit.c       |  2 +-
 src/exit/quick_exit.c |  2 +-
 6 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/include/setjmp.h b/include/setjmp.h
index 7dc7276..321d859 100644
--- a/include/setjmp.h
+++ b/include/setjmp.h
@@ -28,8 +28,15 @@ void _longjmp (jmp_buf, int);
 #endif
 
 
+#if __STDC_VERSION__ >= 201112L
+#elif defined(__GNUC__)
+#define _Noreturn __attribute__((__noreturn__))
+#else
+#define _Noreturn
+#endif
+
 int setjmp (jmp_buf);
-void longjmp (jmp_buf, int);
+_Noreturn void longjmp (jmp_buf, int);
 
 #define setjmp setjmp
 #define longjmp longjmp
diff --git a/include/stdlib.h b/include/stdlib.h
index 1749cb3..03dee19 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -40,12 +40,19 @@ void *realloc (void *, size_t);
 void free (void *);
 void *aligned_alloc(size_t alignment, size_t size);
 
-void abort (void);
+#if __STDC_VERSION__ >= 201112L
+#elif defined(__GNUC__)
+#define _Noreturn __attribute__((__noreturn__))
+#else
+#define _Noreturn
+#endif
+
+_Noreturn void abort (void);
 int atexit (void (*) (void));
-void exit (int);
-void _Exit (int);
+_Noreturn void exit (int);
+_Noreturn void _Exit (int);
 int at_quick_exit (void (*) (void));
-void quick_exit (int);
+_Noreturn void quick_exit (int);
 
 char *getenv (const char *);
 
diff --git a/src/exit/_Exit.c b/src/exit/_Exit.c
index 6ceb143..c00a2ff 100644
--- a/src/exit/_Exit.c
+++ b/src/exit/_Exit.c
@@ -1,7 +1,7 @@
 #include <stdlib.h>
 #include "syscall.h"
 
-void _Exit(int ec)
+_Noreturn void _Exit(int ec)
 {
 	__syscall(SYS_exit_group, ec);
 	__syscall(SYS_exit, ec);
diff --git a/src/exit/abort.c b/src/exit/abort.c
index c5b9e52..203dd35 100644
--- a/src/exit/abort.c
+++ b/src/exit/abort.c
@@ -2,7 +2,7 @@
 #include <signal.h>
 #include "syscall.h"
 
-void abort(void)
+_Noreturn void abort(void)
 {
 	raise(SIGABRT);
 	raise(SIGKILL);
diff --git a/src/exit/exit.c b/src/exit/exit.c
index e4aeaf1..e4932b5 100644
--- a/src/exit/exit.c
+++ b/src/exit/exit.c
@@ -14,7 +14,7 @@ weak_alias(dummy, __funcs_on_exit);
 weak_alias(dummy, __flush_on_exit);
 weak_alias(dummy, __seek_on_exit);
 
-void exit(int code)
+_Noreturn void exit(int code)
 {
 	static int lock;
 
diff --git a/src/exit/quick_exit.c b/src/exit/quick_exit.c
index 18d5288..1175d80 100644
--- a/src/exit/quick_exit.c
+++ b/src/exit/quick_exit.c
@@ -6,7 +6,7 @@
 static void dummy() { }
 weak_alias(dummy, __funcs_on_quick_exit);
 
-void quick_exit(int code)
+_Noreturn void quick_exit(int code)
 {
 	static int lock;
 	while (a_swap(&lock, 1)) __syscall(SYS_pause);
-- 
1.7.12

Re: Re: [PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[Rich Felker]

On Wed, Sep 05, 2012 at 08:02:35PM +0200, philomath wrote:
> Hi,
> 
> Here is an updated version, addressing the issues rich mentioned.
> 
> Should I add _Noreturn to other functions too (such as the various a_crash
> versions, *err*, etc)?

I don't really care about the err.h functions; these are legacy junk
and should not be used in modern programs. I'm pretty much indifferent
to whether they get _Noreturn, but if so, let's save it for a separate
patch/commit.

As for a_crash, I'd like having it there in principle, but I don't
like duplicating the #if logic in each platform's atomic.h. Since this
code is internal and already depends on __asm__, it probably would not
hurt to just use __attribute__((__noreturn__)), perhaps with #ifdef
__GNUC__ around it. It's the public headers that need to be compatible
with an arbitrary C99/C11 compiler; the internals of musl depend on
"minimal GNU C" in the form of inline asm and some related things.
This can also be a separate patch/commit since it's unrelated to
making the headers conform to C11.

Rich

Re: Re: [PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[philomath]

Hi,

On Wed, 5 Sep 2012 23:12:36 -0400
Rich Felker <dalias@aerifal.cx> wrote:

> On Wed, Sep 05, 2012 at 08:02:35PM +0200, philomath wrote:
> > Hi,
> > 
> > Here is an updated version, addressing the issues rich mentioned.
> > 
> > Should I add _Noreturn to other functions too (such as the various a_crash
> > versions, *err*, etc)?
> 
> I don't really care about the err.h functions; these are legacy junk
> and should not be used in modern programs. I'm pretty much indifferent
> to whether they get _Noreturn, but if so, let's save it for a separate
> patch/commit.

Right, they shouldn't be used. but we might as well add this as we pass along,
if you don't object.

> As for a_crash, I'd like having it there in principle, but I don't
> like duplicating the #if logic in each platform's atomic.h. Since this
> code is internal and already depends on __asm__, it probably would not
> hurt to just use __attribute__((__noreturn__)), perhaps with #ifdef
> __GNUC__ around it. It's the public headers that need to be compatible
> with an arbitrary C99/C11 compiler; the internals of musl depend on
> "minimal GNU C" in the form of inline asm and some related things.
> This can also be a separate patch/commit since it's unrelated to
> making the headers conform to C11.

Good, will do.
Other then thrd_exit (which is not yet implemented on musl), the patch I
sent adds _Noreturn to all such C11 functions.  is the patch in good shape now?

Some ther functions that can use _Noreturn:

pthread_exit, _exit, siglongjmp, _longjmp.  these are not specified in POSIX
with _Noreturn, neither will they be in the upcoming TC1. but that's not a
problem, since we are just conveying to the compiler what the standard does
say, right?

_start (the C version), __stack_chk_fail, __assert_fail, cleanup_fromsig.
non-standard functions.

Thanks.

Re: Re: [PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[Rich Felker]

On Thu, Sep 06, 2012 at 04:02:50PM +0200, philomath wrote:
> Good, will do.
> Other then thrd_exit (which is not yet implemented on musl), the patch I
> sent adds _Noreturn to all such C11 functions.  is the patch in good shape now?

I think so.

> Some ther functions that can use _Noreturn:
> 
> pthread_exit, _exit, siglongjmp, _longjmp.

Agree.

> these are not specified in POSIX
> with _Noreturn, neither will they be in the upcoming TC1. but that's not a

That's because POSIX is aligned with C99 not C11, and C99 has no such
thing as _Noreturn. Anyway, whether or not the function is declared
with noreturn should not cause problems.

> _start (the C version), __stack_chk_fail, __assert_fail, cleanup_fromsig.
> non-standard functions.

By _start do you mean the ldso function? That is not callable from C
so there's no reason for it to be declared _Noreturn. Also, I think it
would be wrong since the interface contract is NOT _Noreturn; it just
happens that the stub implementation does not return. But the stub
will never be used anyway.

cleanup_fromsig has no use for _Noreturn, and in fact I'm not sure
_Noreturn would be legal because pthread_cleanup_push does not take a
pointer to a _Noreturn function.

__stack_chk_fail is never called explicitly, so I don't think there's
any use in _Noreturn on it.

__assert_fail could definitely benefit from _Noreturn; it should give
less bloat in code that uses assert.

Rich

Re: Re: [PATCH] Add _Noreturn specifier to functions specified as such by ISO C11

[Anthony J. Bentley]

philomath writes:
> Hi,
> 
> On Wed, 5 Sep 2012 23:12:36 -0400
> Rich Felker <dalias@aerifal.cx> wrote:
> 
> > On Wed, Sep 05, 2012 at 08:02:35PM +0200, philomath wrote:
> > > Hi,
> > > 
> > > Here is an updated version, addressing the issues rich mentioned.
> > > 
> > > Should I add _Noreturn to other functions too (such as the various a_cras
> h
> > > versions, *err*, etc)?
> > 
> > I don't really care about the err.h functions; these are legacy junk
> > and should not be used in modern programs.

err.h functions are not considered legacy by the BSDs, and are still
recommended in their style guides.