[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] Add fno-strict-overflow
From: |
Raghavendra D Prabhu |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] Add fno-strict-overflow |
Date: |
Fri, 8 Jul 2011 03:21:01 +0530 |
User-agent: |
Mutt/1.5.21 (2010-12-30) |
* On Tue, Jul 05, 2011 at 09:30:44PM +0100, Stefan Hajnoczi <address@hidden>
wrote:
On Tue, Jul 5, 2011 at 4:36 PM, Raghavendra D Prabhu
<address@hidden> wrote:
* On Mon, Jul 04, 2011 at 11:38:30PM +0100, Peter Maydell
<address@hidden> wrote:
On 4 July 2011 23:00, Raghavendra D Prabhu <address@hidden>
wrote:
This is to avoid gcc optimizating out the comparison in assert,
due to assumption of signed overflow being undefined by default
(-Werror=strict-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+=-I.. -I$(SRC_PATH)/fpu
+QEMU_CFLAGS+=-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
assert(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 - http://www.airs.com/blog/archives/120 -- as already mentioned 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.
=======================================
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+=-I.. -I$(SRC_PATH)/fpu -fno-strict-overflow
+QEMU_CFLAGS+=-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 >= PCI_CONFIG_SPACE_SIZE);
- assert(offset < offset + size);
+ assert(UINT_MAX - size > offset);
assert(offset + size < PCIE_CONFIG_SPACE_SIZE);
assert(size >= 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
pgp2NEqxJ4FGh.pgp
Description: PGP signature
[Qemu-devel] [PATCH 3/3] Avoid Wunsed-but-set warnings (or errors in case of Werror), Raghavendra D Prabhu, 2011/07/04