[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 for 2.5] cpu-exec: Fix compiler warning (-Wer
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v3 for 2.5] cpu-exec: Fix compiler warning (-Werror=clobbered) |
Date: |
Tue, 27 Oct 2015 19:38:05 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 27/10/2015 19:31, Stefan Weil wrote:
> 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.
Yes, I'll send a pull request next week.
Paolo