From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vdko4-0002cw-VI for qemu-devel@nongnu.org; Tue, 05 Nov 2013 12:52:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vdkny-00087S-Tq for qemu-devel@nongnu.org; Tue, 05 Nov 2013 12:52:48 -0500 Received: from v220110690675601.yourvserver.net ([37.221.199.173]:52508) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vdkny-00087M-KM for qemu-devel@nongnu.org; Tue, 05 Nov 2013 12:52:42 -0500 Message-ID: <52793066.9010403@weilnetz.de> Date: Tue, 05 Nov 2013 18:52:38 +0100 From: Stefan Weil MIME-Version: 1.0 References: <1383247916-2660-1-git-send-email-sw@weilnetz.de> <5272B274.8040001@web.de> In-Reply-To: <5272B274.8040001@web.de> Content-Type: multipart/alternative; boundary="------------050609070507090505090609" Subject: Re: [Qemu-devel] [PATCH v2] cpu-exec: Fix compiler warning (-Werror=clobbered) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka , qemu-devel Cc: peter.maydell@linaro.org This is a multi-part message in MIME format. --------------050609070507090505090609 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Am 31.10.2013 20:41, schrieb Jan Kiszka: > On 2013-10-31 20:31, Stefan Weil wrote: >> 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 =E2=80=98cpu=E2=80=99 might be clobbered by =E2=80=98longjmp= =E2=80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered] >> cpu-exec.c:207:15: error: >> variable =E2=80=98cc=E2=80=99 might be clobbered by =E2=80=98longjmp=E2= =80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered] >> cpu-exec.c:202:28: error: >> argument =E2=80=98env=E2=80=99 might be clobbered by =E2=80=98longjmp= =E2=80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered] >> >> 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. >> >> Signed-off-by: Stefan Weil >> --- >> >> v2: Don't remove the code which causes the warnings, but use it >> only with clang or gcc < 4.6. >> >> cpu-exec.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/cpu-exec.c b/cpu-exec.c >> index 30cfa2a..fec20c3 100644 >> --- a/cpu-exec.c >> +++ b/cpu-exec.c >> @@ -677,14 +677,18 @@ int cpu_exec(CPUArchState *env) >> 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. >> + * gcc 4.7 would complain about this code (-Wclobbered). = */ >> cpu =3D current_cpu; >> env =3D cpu->env_ptr; >> #if !(defined(CONFIG_USER_ONLY) && \ >> (defined(TARGET_M68K) || defined(TARGET_PPC) || defined(TARGET_S390X))) >> cc =3D CPU_GET_CLASS(cpu); >> #endif >> +#endif /* __clang__ or old gcc */ >> } >> } /* for(;;) */ >>=20 >> > > Are all clang versions affected? Then this looks reasonable. > > Jan Ping? As cpu-exec.c has no explicit maintainer, I'd add this patch to my next pull request, if nobody minds, but I'd appreciate more comments or a Reviewed-by of course. Stefan --------------050609070507090505090609 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Am 31.10.2013 20:41, schrieb Jan Kiszka:
> On 2013-10-31 20:31, Stefan Weil wrote:
>> 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:
>>=C2=A0 variable =E2=80=98cpu=E2=80=99 might be clobbered by= =E2=80=98longjmp=E2=80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered]
>> cpu-exec.c:207:15: error:
>>=C2=A0 variable =E2=80=98cc=E2=80=99 might be clobbered by = =E2=80=98longjmp=E2=80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered]
>> cpu-exec.c:202:28: error:
>>=C2=A0 argument =E2=80=98env=E2=80=99 might be clobbered by= =E2=80=98longjmp=E2=80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered]
>>
>> 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.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> v2: Don't remove the code which causes the warnings, but use it
>>=C2=A0=C2=A0=C2=A0=C2=A0 only with clang or gcc < 4.6. >>
>>=C2=A0 cpu-exec.c |=C2=A0=C2=A0=C2=A0 8 ++++++--
>>=C2=A0 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 30cfa2a..fec20c3 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -677,14 +677,18 @@ int cpu_exec(CPUArchState *env)
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 only be set = by a memory fault) */
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 } /* for(;;) */
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } el= se {
>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 /* Reload env after longjmp - the compiler may have smashed all
>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 * local variables as longjmp is marked 'noreturn'. */
>> +#if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6)
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 /* Some compilers wrongly smash all local variables after
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 * siglongjmp. There were bug reports for gcc 4.5.0 and clang.
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 * Reload essential local variables here for those compilers.
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 * gcc 4.7 would complain about this code (-Wclobbered). */
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 cpu =3D current_cpu;
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 env =3D cpu->env_ptr;
>>=C2=A0 #if !(defined(CONFIG_USER_ONLY) && \
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (defined(TARGET_= M68K) || defined(TARGET_PPC) || defined(TARGET_S390X)))
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 cc =3D CPU_GET_CLASS(cpu);
>>=C2=A0 #endif
>> +#endif /* __clang__ or old gcc */
>>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } /* for(;;) */
>>=C2=A0
>>
>
> Are all clang versions affected? Then this looks reasonable. >
> Jan

Ping?

As cpu-exec.c has no explicit maintainer, I'd add this patch to my next pull request, if nobody minds, but I'd appreciate more comments or a Reviewed-by of course.

Stefan


--------------050609070507090505090609--