Re: [Qemu-devel] [PATCH v3 for 2.5] cpu-exec: Fix compiler warning (-Werror=clobbered)

[Stefan Weil]

Am 26.09.2015 um 13:23 schrieb Stefan Weil:
> Reloading of local variables after sigsetjmp is only needed for some
> buggy compilers.
> 
> The code which should reload these variables causes compiler warnings
> with gcc 4.7 when compiler optimizations are enabled:
> 
> cpu-exec.c:204:15: error:
>  variable ‘cpu’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> cpu-exec.c:207:15: error:
>  variable ‘cc’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> cpu-exec.c:202:28: error:
>  argument ‘env’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> 
> Now this code is only used for compilers which need it
> (and gcc 4.5.x, x > 0 which does not need it but won't give warnings).
> 
> There were bug reports for clang and gcc 4.5.0, while gcc 4.5.1
> was reported to work fine without the reload code. For clang it
> is not clear which versions are affected, so simply keep the status quo
> for all clang compilations. This can be improved later.
> 
> Signed-off-by: Stefan Weil <address@hidden>
> ---
> 
> v2: Don't remove the code which causes the warnings, but use it
>     only with clang or gcc < 4.6.
> 
> v3: Add assertions for compilers which hopefully don't smash variables
>     (suggested by Peter Maydell).
> 
> I started v1 of this patch two years ago to prepare support for
> builds with compiler option -Wextra.
> 
> See http://patchwork.ozlabs.org/patch/287593/ for the latest
> discussion on this issue.
> 
> 
>  cpu-exec.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 8fd56a6..7dab85a 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -538,15 +538,27 @@ int cpu_exec(CPUState *cpu)
>                     only be set by a memory fault) */
>              } /* for(;;) */
>          } else {
> -            /* Reload env after longjmp - the compiler may have smashed all
> -             * local variables as longjmp is marked 'noreturn'. */
> +#if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6)
> +            /* Some compilers wrongly smash all local variables after
> +             * siglongjmp. There were bug reports for gcc 4.5.0 and clang.
> +             * Reload essential local variables here for those compilers.
> +             * Newer versions of gcc would complain about this code 
> (-Wclobbered). */
>              cpu = current_cpu;
>              cc = CPU_GET_CLASS(cpu);
> -            cpu->can_do_io = 1;
>  #ifdef TARGET_I386
>              x86_cpu = X86_CPU(cpu);
>              env = &x86_cpu->env;
>  #endif
> +#else /* buggy compiler */
> +            /* Assert that the compiler does not smash local variables. */
> +            g_assert(cpu == current_cpu);
> +            g_assert(cc == CPU_GET_CLASS(cpu));
> +#ifdef TARGET_I386
> +            g_assert(x86_cpu == X86_CPU(cpu));
> +            g_assert(env == &x86_cpu->env);
> +#endif
> +#endif /* buggy compiler */
> +            cpu->can_do_io = 1;
>              tb_lock_reset();
>          }
>      } /* for(;;) */
> 


Ping. Is there any chance to get this patch into version 2.5?
I'd be happy to remove this 2 year old issue from my list of
open patches.

Regards,

Stefan