From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1L5ydl-00069E-Lq for qemu-devel@nongnu.org; Fri, 28 Nov 2008 03:23:53 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1L5ydj-00065c-VY for qemu-devel@nongnu.org; Fri, 28 Nov 2008 03:23:53 -0500 Received: from [199.232.76.173] (port=55854 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1L5ydj-00065O-Sc for qemu-devel@nongnu.org; Fri, 28 Nov 2008 03:23:51 -0500 Received: from fmmailgate03.web.de ([217.72.192.234]:36851) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1L5ydj-0002fJ-9c for qemu-devel@nongnu.org; Fri, 28 Nov 2008 03:23:51 -0500 Received: from smtp05.web.de (fmsmtp05.dlan.cinetic.de [172.20.4.166]) by fmmailgate03.web.de (Postfix) with ESMTP id 17A3FF4E5D7D for ; Fri, 28 Nov 2008 09:23:50 +0100 (CET) Received: from [88.64.25.231] (helo=[192.168.1.198]) by smtp05.web.de with asmtp (TLSv1:AES256-SHA:256) (WEB.DE 4.109 #226) id 1L5ydh-0004QC-00 for qemu-devel@nongnu.org; Fri, 28 Nov 2008 09:23:50 +0100 Message-ID: <492FAA91.3010209@web.de> Date: Fri, 28 Nov 2008 09:23:45 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <200811272230.12559.frank.mehnert@sun.com> <492F2984.9050806@web.de> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigBB14EB89AC5077445A6EF521" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: typo in target-i386/ops_sse.h Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigBB14EB89AC5077445A6EF521 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable andrzej zaborowski wrote: > 2008/11/28 Jan Kiszka : >> andrzej zaborowski wrote: >>> Hi, >>> >>> 2008/11/27 Frank Mehnert : >>>> I believe there is a typo in target-i386/ops_sse.h in the macro >>>> SSE_HELPER_F: >>> Ooops, you're right about the typo, but I think it should something l= ike this: >>> --- a/target-i386/ops_sse.h >>> +++ b/target-i386/ops_sse.h >>> @@ -1499,12 +1499,12 @@ void glue(name, SUFFIX) (Reg *d, Reg *s)\ >>> {\ >>> d->elem(0) =3D F(0);\ >>> d->elem(1) =3D F(1);\ >>> - d->elem(2) =3D F(2);\ >>> - d->elem(3) =3D F(3);\ >>> - if (num > 3) {\ >>> - d->elem(4) =3D F(4);\ >>> - d->elem(5) =3D F(5);\ >>> - if (num > 5) {\ >>> + if (num > 2) {\ >>> + d->elem(2) =3D F(2);\ >>> + d->elem(3) =3D F(3);\ >>> + if (num > 4) {\ >>> + d->elem(4) =3D F(4);\ >>> + d->elem(5) =3D F(5);\ >>> d->elem(6) =3D F(6);\ >>> d->elem(7) =3D F(7);\ >>> }\ >>> >>> I'm not sure why this didn't generate warnings. >> It does - with gcc4 (array subscript is above array bounds). I saw the= m >> in kvm-userspace, but there were so many (a lot likely due to >> non-upstream stuff) that I ignored them for now. Now your patch just >> removed 8 upstream warnings. But is this stuff already in use? Should >> cause subtle guest state corruptions if actually executed. >=20 > It is enabled if you specify SSE4.1 support through -cpu, currenlty no > predefined cpu uses it. I think it went unnoticed because I only > tested the first of the 12 instructions using the macro, which wasn't > affected. >=20 >> That reminds me that we should have a "zero new warnings policy" for >> changes. But reality still looks different... >=20 > Well, the subscripts above array bounds here are okay. Similarly Nope, they aren't. These warning are _directly_ related to the incorrect element number checks you fixed. > there are other warnings that generate lots of annoying > false-positives and you would end up working around your compiler, > sometimes sacrificing readability or performance. The only "unfixable" warnings that I currently see with qemu's CFLAGS and my compiler defaults are either related to will-die-soon dyngen fragments or gcc's blindness when it comes to understanding if (condition) var =3D initialized; ... if (condition) use(var); and not warning about potentially uninitialized 'var' (even if 'condition' is stable). The rest is fixable or even pointing to code worth a second look. To give another example for a bug one could have found with the help of the compiler (to be fair: unsupported gcc4): gdbstub.c accesses fpr 32..63 instead of 0..31 on PPC (patch will follow)= =2E And don't underestimate the value of a warning free build, specifically when you have to apply changes with broad impact! Jan --------------enigBB14EB89AC5077445A6EF521 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkkvqpUACgkQniDOoMHTA+nrOQCeIBfkN+7Utvr8rjUE4EXKtlcH D1sAn2GI3S12bFTWKUBDqelBAJMh7Q+D =kGO6 -----END PGP SIGNATURE----- --------------enigBB14EB89AC5077445A6EF521--