From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39893) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QewTZ-0007D3-Ka for qemu-devel@nongnu.org; Thu, 07 Jul 2011 17:51:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QewTY-0001oR-Aq for qemu-devel@nongnu.org; Thu, 07 Jul 2011 17:51:13 -0400 Received: from mail-pv0-f173.google.com ([74.125.83.173]:34302) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QewTY-0001oD-22 for qemu-devel@nongnu.org; Thu, 07 Jul 2011 17:51:12 -0400 Received: by pvg3 with SMTP id 3so930614pvg.4 for ; Thu, 07 Jul 2011 14:51:10 -0700 (PDT) Date: Fri, 8 Jul 2011 03:21:01 +0530 From: Raghavendra D Prabhu Message-ID: <20110707215101.GA17982@Xye> References: <20110705153615.GB4463@Xye> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vtzGhvizbBRQ85DL" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 2/3] Add fno-strict-overflow List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Peter Maydell , qemu-devel@nongnu.org, kvm@vger.kernel.org --vtzGhvizbBRQ85DL Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable * On Tue, Jul 05, 2011 at 09:30:44PM +0100, Stefan Hajnoczi wrote: >On Tue, Jul 5, 2011 at 4:36 PM, Raghavendra D Prabhu > wrote: >> * On Mon, Jul 04, 2011 at 11:38:30PM +0100, Peter Maydell >> wrote: >>> On 4 July 2011 23:00, Raghavendra D Prabhu >>> wrote: >>>> This is to avoid gcc optimizating out the comparison in assert, >>>> due to assumption of signed overflow being undefined by default >>>> (-Werror=3Dstrict-overflow). >>>> --- a/Makefile.hw >>>> +++ b/Makefile.hw >>>> @@ -9,7 +9,7 @@ include $(SRC_PATH)/rules.mak >>>> $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw) >>>> -QEMU_CFLAGS+=3D-I.. -I$(SRC_PATH)/fpu >>>> +QEMU_CFLAGS+=3D-I.. -I$(SRC_PATH)/fpu -fno-strict-overflow >>> Can you give a more detailed description of the problem this is trying >>> to solve? I think it would be nicer if we could remove the assumptions >>> about signed overflows instead, if that's practical. >> Following line in pcie.c:pcie_add_capability:505 >> =C2=A0 =C2=A0assert(offset < offset + size); >> is what the compiler was warning about. The compiler optimizes out that >> comparison without fno-strict-overflow flag. More information about it >> is here - =C2=A0http://www.airs.com/blog/archives/120 -- as already ment= ioned by >> Stefan. >>> (Also, if we do want to add this compiler flag then it ought to be >>> done in configure I think, as we do for -fno-strict-aliasing.) >> Globally adding that flag can limits the optimizations of gcc since in >> other places (loops) the undefined behavior can be advantageous, hence >> added only to Makefile.hw. > >Doing this on a per-subsystem or per-file basis does not make sense to >me. This is a general C coding issue that needs to be settled for the >entire codebase. We will not catch instances of overflow slipping in >during patch review, so limiting the scope of -fno-strict-overflow is >not feasible. > >I suggest we cover all of QEMU with -fwrapv instead of worrying about >-fno-strict-overflow. That way we can get some optimizations and it >reflects the model that we are all assuming: >"This option instructs the compiler to assume that signed arithmetic >overflow of addition, subtraction and multiplication wraps around >using twos-complement representation. This flag enables some >optimizations and disables others. This option is enabled by default >for the Java front-end, as required by the Java language >specification." >http://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Code-Gen-Options.html > >Stefan > I have removed that option from Makefile; instead replaced it with another assert which shouldn't be affected by overflow. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D diff --git a/Makefile.hw b/Makefile.hw index 23dac45..b9181ab 100644 --- a/Makefile.hw +++ b/Makefile.hw @@ -9,7 +9,7 @@ include $(SRC_PATH)/rules.mak $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw) -QEMU_CFLAGS+=3D-I.. -I$(SRC_PATH)/fpu -fno-strict-overflow +QEMU_CFLAGS+=3D-I.. -I$(SRC_PATH)/fpu include $(SRC_PATH)/Makefile.objs diff --git a/hw/pcie.c b/hw/pcie.c index 39607bf..cfb11fe 100644 --- a/hw/pcie.c +++ b/hw/pcie.c @@ -502,7 +502,7 @@ void pcie_add_capability(PCIDevice *dev, uint16_t next; assert(offset >=3D PCI_CONFIG_SPACE_SIZE); - assert(offset < offset + size); + assert(UINT_MAX - size > offset); assert(offset + size < PCIE_CONFIG_SPACE_SIZE); assert(size >=3D 8); assert(pci_is_express(dev)); -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net --vtzGhvizbBRQ85DL Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (GNU/Linux) iQEcBAEBAgAGBQJOFipFAAoJEKYW3KHXK+l313wH/3qfSNFAZn6/tucQHgkxEPwN w5Z60WcmY6pBmXlNfUa8UtqcxWLY+TpNMhxT2hVRfGvvHxn5iFzT1F5MUlmNZ7Ul WGNUH8ASbzRFf1/n6wFeSLN5dU+MDBCmNeO8/YBKzUdIOiG+I9HnWl9D7+Lu0zx9 DaR3kLIATacQWNy/FbSqVGYBAXEvz83zO/MQ3l3B3I3JJXPPsPd17ZRl/eHRm3xB eigsmn13QYHdRDmBwRhI1NFkrLc3zJJivzz6CHXxsRGjVf3N7r9H8/Ky4ybDtpsY FOZ6PsEZvenSrxsK695/jtJka+Gi4OOLo9uRQEY3d6sUsyIDedTErQ8WyTRAIEI= =+cdm -----END PGP SIGNATURE----- --vtzGhvizbBRQ85DL--