Add __noreturn function attribute
PATCH 2/2 Add __noreturn function attribute
Introduce __noreturn attribute and attach it to cpu_loop_exit as well as interrupt/exception helpers for i386. This avoids a bunch of gcc4 warnings.
---
exec-all.h | 5 ++++- target-i386/exec.h | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-)
exec-all.h +4 -1
@@ -20,6 +20,9 @@#ifndef _EXEC_ALL_H_#define _EXEC_ALL_H_ + +#define __noreturn __attribute__ ((__noreturn__)) +/* allow to see translation results - the slowdown should be negligible, so we leave it */ #define DEBUG_DISAS
@@ -82,7 +85,7 @@ TranslationBlock *tb_gen_code(CPUState *env, target_ulong pc, target_ulong cs_base, int flags, int cflags);void cpu_exec_init (CPUState *env); -void cpu_loop_exit (void ); +void __noreturn cpu_loop_exit (void );int page_unprotect (target_ulong address,unsigned long pc,void *puc);void tb_invalidate_phys_page_range (target_phys_addr_t start,target_phys_addr_t end,int is_cpu_write_access);
target-i386/exec.h +4 -4
@@ -67,10 +67,10 @@ void do_interrupt(int intno, int is_int, int error_code, target_ulong next_eip, int is_hw);void do_interrupt_user (int intno,int is_int,int error_code,target_ulong next_eip); -void raise_interrupt (int intno,int is_int,int error_code, -int next_eip_addend); -void raise_exception_err (int exception_index,int error_code); -void raise_exception (int exception_index); +void __noreturn raise_interrupt (int intno,int is_int,int error_code, +int next_eip_addend); +void __noreturn raise_exception_err (int exception_index,int error_code); +void __noreturn raise_exception (int exception_index);void do_smm_enter (void );/* n must be a constant to be efficient */
Quoted 3 lines · Jan Kiszka
Introduce
__noreturnattribute and attach it tocpu_loop_exitas well as interrupt/exception helpers for i386. This avoids a bunch of gcc4 warnings.
ISO/IEC 9899:1990
7.1.3
-- All identifiers that begin with an underscore and
either an uppercase letter or another underscore are always reserved for any use.
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
[..snip..]
Signature · 1 line
mailto:[email protected]
Quoted 6 lines · malc
Quoted 3 lines · Jan Kiszka
Introduce
__noreturnattribute and attach it tocpu_loop_exitas well as interrupt/exception helpers for i386. This avoids a bunch of gcc4 warnings.
Quoted 6 lines
ISO/IEC 9899:1990
7.1.3
-- All identifiers that begin with an underscore and
either an uppercase letter or another underscore are always reserved for any use.
...but commonly used in practice (which is no excuse to do so here, just an explanation). Given that we have __hidden already, I just followed the existing pattern.
Quoted 3 lines
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
I have no problem with calling it 'noreturn' instead.
Jan
----------->
Introduce noreturn attribute and attach it to cpu_loop_exit as well as interrupt/exception helpers for i386. This avoids a bunch of gcc4 warnings.
---
exec-all.h | 5 ++++- target-i386/exec.h | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-)
exec-all.h +4 -1
@@ -20,6 +20,9 @@#ifndef _EXEC_ALL_H_#define _EXEC_ALL_H_ + +#define noreturn __attribute__ ((__noreturn__)) +/* allow to see translation results - the slowdown should be negligible, so we leave it */ #define DEBUG_DISAS
@@ -82,7 +85,7 @@ TranslationBlock *tb_gen_code(CPUState *env, target_ulong pc, target_ulong cs_base, int flags, int cflags);void cpu_exec_init (CPUState *env); -void cpu_loop_exit (void ); +void noreturncpu_loop_exit (void );int page_unprotect (target_ulong address,unsigned long pc,void *puc);void tb_invalidate_phys_page_range (target_phys_addr_t start,target_phys_addr_t end,int is_cpu_write_access);
target-i386/exec.h +4 -4
@@ -67,10 +67,10 @@ void do_interrupt(int intno, int is_int, int error_code, target_ulong next_eip, int is_hw);void do_interrupt_user (int intno,int is_int,int error_code,target_ulong next_eip); -void raise_interrupt (int intno,int is_int,int error_code, -int next_eip_addend); -void raise_exception_err (int exception_index,int error_code); -void raise_exception (int exception_index); +void noreturnraise_interrupt (int intno,int is_int,int error_code, +int next_eip_addend); +void noreturnraise_exception_err (int exception_index,int error_code); +void noreturnraise_exception (int exception_index);void do_smm_enter (void );/* n must be a constant to be efficient */
Quoted 9 lines · Jan Kiszka
Quoted 8 lines · malc
Quoted 3 lines · Jan Kiszka
Introduce
__noreturnattribute and attach it tocpu_loop_exitas well as interrupt/exception helpers for i386. This avoids a bunch of gcc4 warnings.ISO/IEC 9899:1990
Quoted 8 lines
Quoted 4 lines
7.1.3
-- All identifiers that begin with an underscore and
either an uppercase letter or another underscore are always reserved for any use....but commonly used in practice (which is no excuse to do so here, just an explanation). Given that we have
__hiddenalready, I just followed the existing pattern.
A kernel is free to make its own rules about namespaces, but userland applications like Qemu should adhere to the userland standards.
Thiemo
Quoted 6 lines · Jan Kiszka
Quoted 3 lines
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
I have no problem with calling it 'noreturn' instead.
That will break code that wants to use 'noreturn' as a local variable. I think ATTR_NORETURN, while a lot uglier, is safer.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 5 lines · Avi Kivity
Quoted 4 lines · Jan Kiszka
Quoted 3 lines
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
Quoted 8 lines
Quoted 2 lines
I have no problem with calling it 'noreturn' instead.
That will break code that wants to use 'noreturn' as a local variable. I think
ATTR_NORETURN, while a lot uglier, is safer.
Why do we need a new macro instead of just using __attribute__ ((__noreturn__))?
The macro won't save very much writing, needs an include dependency, breaks code which uses the same symbol for other purposes, makes code reusage in other
products more difficult, ...
Stefan
Quoted 3 lines · Stefan Weil
Why do we need a new macro instead of just using
__attribute__((__noreturn__))?
Don't we need it to prevent gcc-ism from being all over the code? Though I wonder how much tcg converted code still depends on being compiled by gcc.
Laurent
Quoted 10 lines · Avi Kivity
Quoted 6 lines · Jan Kiszka
Quoted 3 lines
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
I have no problem with calling it 'noreturn' instead.
That will break code that wants to use 'noreturn' as a local variable. I think
ATTR_NORETURN, while a lot uglier, is safer.
Do you have such code already? Is it exported beyond qemu scope? Then why not going for our own convention "'noreturn' is reserved as function attribute"? (And yes, your macro is ugly :) ).
Jan
Quoted 5 lines · Laurent Desnogues
Quoted 2 lines · Stefan Weil
Why do we need a new macro instead of just using
__attribute__((__noreturn__))?Don't we need it to prevent gcc-ism from being all over the code?
Yep, and that's also why we need wrapping. You can easily define it away if your compiler doesn't support it. We just need to add the required conditions.
Quoted 2 lines
Though I wonder how much tcg converted code still depends on being compiled by gcc.
I guess once the hard dependencies are removed, fixing things like the existing __attributes__ etc. will just be mechanical work.
Jan
Quoted 11 lines · Jan Kiszka
Quoted 5 lines · Laurent Desnogues
Quoted 2 lines · Stefan Weil
Why do we need a new macro instead of just using
__attribute__((__noreturn__))?Don't we need it to prevent gcc-ism from being all over the code?
Yep, and that's also why we need wrapping. You can easily define it away if your compiler doesn't support it. We just need to add the required conditions.
Quoted 8 lines
Quoted 2 lines
Though I wonder how much tcg converted code still depends on being compiled by gcc.
I guess once the hard dependencies are removed, fixing things like the existing
__attributes__etc. will just be mechanical work.Jan
For compilers which don't support __attributes__, a simple
#define __attributes__(dummy) /* dummy */
or an equivalent command line option would eliminate all gcc-isms. I don't think we need wrapping, at least not now.
Stefan
Quoted 9 lines · Jan Kiszka
Quoted 8 lines · Avi Kivity
Quoted 6 lines · Jan Kiszka
Quoted 3 lines
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
I have no problem with calling it 'noreturn' instead.
Quoted 6 lines
Quoted 2 lines
That will break code that wants to use 'noreturn' as a local variable. I think
ATTR_NORETURN, while a lot uglier, is safer.Do you have such code already? Is it exported beyond qemu scope? Then why not going for our own convention "'noreturn' is reserved as function attribute"? (And yes, your macro is ugly :) ).
"Macro names should be in upper case" is also a useful convention.
FWIW, I agree with Stefan, there's currently not much need to isolate gcc-isms.
Thiemo
Am 30.11.2008 um 13:52 schrieb Stefan Weil:
Quoted 12 lines
Quoted 10 lines · Jan Kiszka
Quoted 5 lines · Laurent Desnogues
Quoted 2 lines · Stefan Weil
Why do we need a new macro instead of just using
__attribute__((__noreturn__))?Don't we need it to prevent gcc-ism from being all over the code?
Yep, and that's also why we need wrapping. You can easily define it away if your compiler doesn't support it. We just need to add the required conditions.
Quoted 6 lines
For compilers which don't support
__attributes__, a simple#define
__attributes__(dummy)/* dummy */or an equivalent command line option would eliminate all gcc-isms. I don't think we need wrapping, at least not now.
Aren't there GCCs which support __attributes__ but not all of them? Apple's GCC 4.0.1 comes to mind, which doesn't support visibility attributes.
Andreas
Quoted 8 lines · Thiemo Seufer
Quoted 7 lines · Jan Kiszka
Quoted 6 lines · Avi Kivity
Quoted 5 lines · Jan Kiszka
Quoted 3 lines
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
I have no problem with calling it 'noreturn' instead.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
That will break code that wants to use 'noreturn' as a local variable. I think
ATTR_NORETURN, while a lot uglier, is safer.Do you have such code already? Is it exported beyond qemu scope? Then why not going for our own convention "'noreturn' is reserved as function attribute"? (And yes, your macro is ugly :) ).
"Macro names should be in upper case" is also a useful convention.
Generally yes. But there are exceptions when the macro is used in a context where upper case disturbs the readability instead of improving it. I would argue that this is the case here, but it's always a matter of taste.
Quoted 3 lines
FWIW, I agree with Stefan, there's currently not much need to isolate gcc-isms.
If everyone prefers having __attribute__ in the function prototypes directly -- OK. All I want is to get rid of the warnings without changing the code into the wrong direction.
Jan
In message: <[email protected]>
Stefan Weil <[email protected]> writes:
: Jan Kiszka schrieb:
: > Laurent Desnogues wrote:
: >> On Sun, Nov 30, 2008 at 12:51 PM, Stefan Weil <[email protected]> : >> wrote:
: >>> Why do we need a new macro instead of just using __attribute__ : >>> ((__noreturn__))?
: >> Don't we need it to prevent gcc-ism from being all over the code? : >
: > Yep, and that's also why we need wrapping. You can easily define it away : > if your compiler doesn't support it. We just need to add the required : > conditions.
: >
: >> Though I wonder how much tcg converted code still depends on : >> being compiled by gcc.
: >
: > I guess once the hard dependencies are removed, fixing things like the : > existing __attributes__ etc. will just be mechanical work.
: >
: > Jan
: >
:
:
: For compilers which don't support __attributes__, a simple
:
: #define __attributes__(dummy) /* dummy */
:
: or an equivalent command line option would eliminate all gcc-isms. : I don't think we need wrapping, at least not now.
This doesn't work so well when 'dummy' is __packed__.
The BSD projects have had similar macros for a long long time, and they work out well in practice.
Warner
Quoted 30 lines · Jan Kiszka
Quoted 13 lines · Thiemo Seufer
Quoted 11 lines · Jan Kiszka
Quoted 9 lines · Avi Kivity
Quoted 7 lines · Jan Kiszka
Quoted 4 lines
Breaking the standard is what brings us the joys of recently (re)posted patch for NetBSD and [u]intXX fun.
I have no problem with calling it 'noreturn' instead.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
That will break code that wants to use 'noreturn' as a local variable. I think
ATTR_NORETURN, while a lot uglier, is safer.Do you have such code already? Is it exported beyond qemu scope? Then why not going for our own convention "'noreturn' is reserved as function attribute"? (And yes, your macro is ugly :) ).
"Macro names should be in upper case" is also a useful convention.
Generally yes. But there are exceptions when the macro is used in a context where upper case disturbs the readability instead of improving it. I would argue that this is the case here, but it's always a matter of taste.
Quoted 8 lines
Quoted 3 lines
FWIW, I agree with Stefan, there's currently not much need to isolate gcc-isms.
If everyone prefers having
__attribute__in the function prototypes directly -- OK. All I want is to get rid of the warnings without changing the code into the wrong direction.
Please stick with the #define. It's not about the GCC-ism, it's being able to quickly replace it with something else.
This helps for things like sparse.
Regards,
Anthony Liguori
Quoted 3 lines
Jan
Quoted 36 lines · Anthony Liguori
Quoted 35 lines · Jan Kiszka
Quoted 15 lines · Thiemo Seufer
Quoted 13 lines · Jan Kiszka
Quoted 11 lines · Avi Kivity
Quoted 6 lines · Jan Kiszka
Quoted 4 lines
Breaking the standard is what brings us the joys of recently (re)posted
patch for NetBSD and [u]intXX fun.I have no problem with calling it 'noreturn' instead.
That will break code that wants to use 'noreturn' as a local variable. I think
ATTR_NORETURN, while a lot uglier, is safer.Quoted 7 lines
Quoted 5 lines
Do you have such code already? Is it exported beyond qemu scope? Then why not going for our own convention "'noreturn' is reserved as function
attribute"? (And yes, your macro is ugly :) )."Macro names should be in upper case" is also a useful convention.
Generally yes. But there are exceptions when the macro is used in a context where upper case disturbs the readability instead of improving it. I would argue that this is the case here, but it's always a matter of taste.
Quoted 3 lines
FWIW, I agree with Stefan, there's currently not much need to isolate gcc-isms.
Quoted 7 lines
Quoted 4 lines
If everyone prefers having
__attribute__in the function prototypes directly -- OK. All I want is to get rid of the warnings without changing the code into the wrong direction.Please stick with the #define. It's not about the GCC-ism, it's being able to quickly replace it with something else.
For sure. Err... but which one now? "noreturn" is already available as signed patch.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 26 Corporate Competence Center Embedded Linux
Quoted 39 lines · Jan Kiszka
Quoted 38 lines · Anthony Liguori
Quoted 37 lines · Jan Kiszka
Quoted 15 lines · Thiemo Seufer
Quoted 13 lines · Jan Kiszka
Quoted 11 lines · Avi Kivity
Quoted 6 lines · Jan Kiszka
Quoted 4 lines
Breaking the standard is what brings us the joys of recently (re)posted
patch for NetBSD and [u]intXX fun.I have no problem with calling it 'noreturn' instead.
That will break code that wants to use 'noreturn' as a local variable. I think
ATTR_NORETURN, while a lot uglier, is safer.Quoted 7 lines
Quoted 5 lines
Do you have such code already? Is it exported beyond qemu scope? Then why not going for our own convention "'noreturn' is reserved as function
attribute"? (And yes, your macro is ugly :) )."Macro names should be in upper case" is also a useful convention.
Generally yes. But there are exceptions when the macro is used in a context where upper case disturbs the readability instead of improving it. I would argue that this is the case here, but it's always a matter of taste.
Quoted 3 lines
FWIW, I agree with Stefan, there's currently not much need to isolate gcc-isms.
If everyone prefers having
__attribute__in the function prototypes directly -- OK. All I want is to get rid of the warnings without changing the code into the wrong direction.
Quoted 5 lines
Quoted 2 lines
Please stick with the #define. It's not about the GCC-ism, it's being able to quickly replace it with something else.
For sure. Err... but which one now? "noreturn" is already available as signed patch.
...but that patch in fact has an issue with existing __attribute__((noreturn)) sites. Converting them all to the common macro raises the question where to put the definition. qemu-common.h appears logical on first sight, but due to dyngen-exec.h's redefinition mess we cannot include that header easily.
I'm currently trying to find a workaround that is not too invasive and can quickly be removed once dyngen is gone. The deeper I dig for that, the more weird the dependencies get.
However, please let me know if the preferred macro name is different from "noreturn".
Jan