Re: [Qemu-devel] Re: [PATCH] Add noreturn function attribute

[Blue Swirl]

On 1/14/09, Jan Kiszka <address@hidden> wrote:
> Jan Kiszka wrote:
>  > 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.
>  >
>  > [ Note that this patch comes with a workaround to include qemu-common.h
>  > even in cases where is currently causes conflicts with dyngen-exec.h.
>  > I've been told that these conflicts will get resolved in the future
>  > (/me will try to have a look as well - as time permits). ]
>  >
>  > Signed-off-by: Jan Kiszka <address@hidden>
>  > ---
>  >
>  >  cpu-all.h               |    7 ++++---
>  >  darwin-user/signal.c    |    3 ++-
>  >  exec-all.h              |    5 ++++-
>  >  linux-user/signal.c     |    3 ++-
>  >  qemu-common.h           |   14 +++++++++++---
>  >  qemu-img.c              |    2 +-
>  >  target-i386/exec.h      |    5 +++--
>  >  target-i386/op_helper.c |    4 ++--
>  >  8 files changed, 29 insertions(+), 14 deletions(-)
>  >
>  > diff --git a/cpu-all.h b/cpu-all.h
>  > index 648264c..329d26d 100644
>  > --- a/cpu-all.h
>  > +++ b/cpu-all.h
>  > @@ -20,6 +20,8 @@
>  >  #ifndef CPU_ALL_H
>  >  #define CPU_ALL_H
>  >
>  > +#include "qemu-common.h"
>  > +
>  >  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) || 
> defined(__hppa__)
>  >  #define WORDS_ALIGNED
>  >  #endif
>  > @@ -751,9 +753,8 @@ void cpu_dump_statistics (CPUState *env, FILE *f,
>  >                            int (*cpu_fprintf)(FILE *f, const char *fmt, 
> ...),
>  >                            int flags);
>  >
>  > -void cpu_abort(CPUState *env, const char *fmt, ...)
>  > -    __attribute__ ((__format__ (__printf__, 2, 3)))
>  > -    __attribute__ ((__noreturn__));
>  > +void noreturn cpu_abort(CPUState *env, const char *fmt, ...)
>  > +    __attribute__ ((__format__ (__printf__, 2, 3)));
>  >  extern CPUState *first_cpu;
>  >  extern CPUState *cpu_single_env;
>  >  extern int64_t qemu_icount;
>  > diff --git a/darwin-user/signal.c b/darwin-user/signal.c
>  > index f412b36..98d0894 100644
>  > --- a/darwin-user/signal.c
>  > +++ b/darwin-user/signal.c
>  > @@ -36,6 +36,7 @@
>  >  #include <signal.h>
>  >
>  >  #include "qemu.h"
>  > +#include "qemu-common.h"
>  >
>  >  #define DEBUG_SIGNAL
>  >
>  > @@ -131,7 +132,7 @@ static inline void free_sigqueue(struct sigqueue *q)
>  >  }
>  >
>  >  /* abort execution with signal */
>  > -void __attribute((noreturn)) force_sig(int sig)
>  > +void noreturn force_sig(int sig)
>  >  {
>  >      int host_sig;
>  >      host_sig = target_to_host_signal(sig);
>  > diff --git a/exec-all.h b/exec-all.h
>  > index fd96adf..03c174c 100644
>  > --- a/exec-all.h
>  > +++ b/exec-all.h
>  > @@ -20,6 +20,9 @@
>  >
>  >  #ifndef _EXEC_ALL_H_
>  >  #define _EXEC_ALL_H_
>  > +
>  > +#include "qemu-common.h"
>  > +
>  >  /* 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);
>  > diff --git a/linux-user/signal.c b/linux-user/signal.c
>  > index e0f6aaf..9d4946d 100644
>  > --- a/linux-user/signal.c
>  > +++ b/linux-user/signal.c
>  > @@ -27,6 +27,7 @@
>  >  #include <sys/ucontext.h>
>  >
>  >  #include "qemu.h"
>  > +#include "qemu-common.h"
>  >  #include "target_signal.h"
>  >
>  >  //#define DEBUG_SIGNAL
>  > @@ -325,7 +326,7 @@ static inline void free_sigqueue(CPUState *env, struct 
> sigqueue *q)
>  >  }
>  >
>  >  /* abort execution with signal */
>  > -static void __attribute((noreturn)) force_sig(int sig)
>  > +static void noreturn force_sig(int sig)
>  >  {
>  >      int host_sig;
>  >      host_sig = target_to_host_signal(sig);
>  > diff --git a/qemu-common.h b/qemu-common.h
>  > index d3df63e..6ee31e0 100644
>  > --- a/qemu-common.h
>  > +++ b/qemu-common.h
>  > @@ -2,6 +2,13 @@
>  >  #ifndef QEMU_COMMON_H
>  >  #define QEMU_COMMON_H
>  >
>  > +#define noreturn __attribute__ ((__noreturn__))
>  > +
>  > +/* Hack around the mess dyngen-exec.h causes: We need noreturn in files 
> that
>  > +   cannot include the following headers without conflicts. This condition 
> has
>  > +   to be removed once dyngen is gone. */
>  > +#ifndef __DYNGEN_EXEC_H__
>  > +
>  >  /* we put basic includes here to avoid repeating them in device drivers */
>  >  #include <stdlib.h>
>  >  #include <stdio.h>
>  > @@ -134,9 +141,8 @@ void *get_mmap_addr(unsigned long size);
>  >
>  >  /* Error handling.  */
>  >
>  > -void hw_error(const char *fmt, ...)
>  > -    __attribute__ ((__format__ (__printf__, 1, 2)))
>  > -    __attribute__ ((__noreturn__));
>  > +void noreturn hw_error(const char *fmt, ...)
>  > +    __attribute__ ((__format__ (__printf__, 1, 2)));
>  >
>  >  /* IO callbacks.  */
>  >  typedef void IOReadHandler(void *opaque, const uint8_t *buf, int size);
>  > @@ -179,4 +185,6 @@ int cpu_load(QEMUFile *f, void *opaque, int 
> version_id);
>  >  /* Force QEMU to stop what it's doing and service IO */
>  >  void qemu_service_io(void);
>  >
>  > +#endif /* dyngen-exec.h hack */
>  > +
>  >  #endif
>  > diff --git a/qemu-img.c b/qemu-img.c
>  > index 207535f..d72dd71 100644
>  > --- a/qemu-img.c
>  > +++ b/qemu-img.c
>  > @@ -33,7 +33,7 @@
>  >  /* Default to cache=writeback as data integrity is not important for 
> qemu-tcg. */
>  >  #define BRDV_O_FLAGS BDRV_O_CACHE_WB
>  >
>  > -static void __attribute__((noreturn)) error(const char *fmt, ...)
>  > +static void noreturn error(const char *fmt, ...)
>  >  {
>  >      va_list ap;
>  >      va_start(ap, fmt);
>  > diff --git a/target-i386/exec.h b/target-i386/exec.h
>  > index 4d97a1b..48d6102 100644
>  > --- a/target-i386/exec.h
>  > +++ b/target-i386/exec.h
>  > @@ -31,6 +31,7 @@
>  >
>  >  register struct CPUX86State *env asm(AREG0);
>  >
>  > +#include "qemu-common.h"
>  >  #include "qemu-log.h"
>  >
>  >  #define EAX (env->regs[R_EAX])
>  > @@ -62,8 +63,8 @@ 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_exception_err(int exception_index, int error_code);
>  > -void raise_exception(int exception_index);
>  > +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 */
>  > diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
>  > index 52fee3d..d50a9c0 100644
>  > --- a/target-i386/op_helper.c
>  > +++ b/target-i386/op_helper.c
>  > @@ -1284,8 +1284,8 @@ static int check_exception(int intno, int 
> *error_code)
>  >   * EIP value AFTER the interrupt instruction. It is only relevant if
>  >   * is_int is TRUE.
>  >   */
>  > -static void raise_interrupt(int intno, int is_int, int error_code,
>  > -                            int next_eip_addend)
>  > +static void noreturn raise_interrupt(int intno, int is_int, int 
> error_code,
>  > +                                     int next_eip_addend)
>  >  {
>  >      if (!is_int) {
>  >          helper_svm_check_intercept_param(SVM_EXIT_EXCP_BASE + intno, 
> error_code);
>  >
>  >
>  >
>
>
> Any comment on this? It still applies - and it still kills a lot of
>  compiler warnings.

Thanks, applied as r6303.