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

[Jan Kiszka <jan.kiszka@siemens.com>]

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 <jan.kiszka@siemens.com>
> ---
> 
>  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.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 26
Corporate Competence Center Embedded Linux