From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42476) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdkyK-0007Hk-HW for qemu-devel@nongnu.org; Tue, 05 Nov 2013 13:03:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vdky9-0003Kq-Oh for qemu-devel@nongnu.org; Tue, 05 Nov 2013 13:03:24 -0500 Received: from cantor2.suse.de ([195.135.220.15]:36168 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vdky9-0003Kl-GR for qemu-devel@nongnu.org; Tue, 05 Nov 2013 13:03:13 -0500 Message-ID: <527932DD.3090006@suse.de> Date: Tue, 05 Nov 2013 19:03:09 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1383247916-2660-1-git-send-email-sw@weilnetz.de> <5272B274.8040001@web.de> <52793066.9010403@weilnetz.de> In-Reply-To: <52793066.9010403@weilnetz.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: Stefan Weil , Jan Kiszka , qemu-devel Cc: peter.maydell@linaro.org Am 05.11.2013 18:52, schrieb Stefan Weil: > 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=98longjm= p=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=98longjm= p=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 afte= r >>> + * 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 >=20 > Ping? >=20 > 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. I feel kind of responsable for this as CPU maintainer, but I wasn't CC'ed and have not been following the list so closely lately. ;) It seems the person reporting this for FreeBSD hasn't been CC'ed either? Having applied the previous patch complementing the reload, I'm generally okay with #ifdef'ing it out. But I'd be happier if Jan and/or Peter or anyone else would provide some *-by, including Tested-by. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg