| Message ID | 1473685808-9629-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
Peter Maydell <peter.maydell@linaro.org> writes: > QEMU's code relies on left shifts of signed integers always > being defined behaviour with the obvious 2s-complement > semantics. The only way to tell the compiler (and any > associated undefined-behaviour sanitizer) that we require a > C dialect with these semantics is to use the -fwrapv option. > This is a bit of a heavy hammer for the job as it also gives > us guaranteed semantics on integer arithmetic overflow which > in theory we don't require. > > In an ideal world this would allow us to drop the warning > flag -Wno-shift-negative-value, but we must retain this to > avoid spurious warnings on clang versions predating the > fix to https://llvm.org/bugs/show_bug.cgi?id=25552. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > We agreed before 2.7 release that this was the best long term > approach to our shift issues, since it's now clear that both > clang and gcc do agree that -fwrapv provides the semantics we > want. > > configure | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index 331c36f..14efce3 100755 > --- a/configure > +++ b/configure > @@ -389,7 +389,11 @@ sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" > ARFLAGS="${ARFLAGS-rv}" > > # default flags for all hosts > -QEMU_CFLAGS="-fno-strict-aliasing -fno-common $QEMU_CFLAGS" > +# We use -fwrapv to tell the compiler that we require a C dialect where > +# left shift of signed integers is well defined and has the expected > +# 2s-complement style results. (Both clang and gcc agree that it > +# provides these semantics.) > +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS" > QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" > QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" > QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" If I remember correctly, we discussed -fno-strict-overflow (which the kernel uses), but in the end opted for the more stringent -fwrapv. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 12 September 2016 at 16:04, Markus Armbruster <armbru@redhat.com> wrote: > If I remember correctly, we discussed -fno-strict-overflow (which the > kernel uses), but in the end opted for the more stringent -fwrapv. Yep. -fno-strict-overflow just says "don't do bad things on integer overflow (but it's still a bug in the program if it happens)", so overflows are still program bugs and the compiler/sanitizer will still complain about them. You need -fwrapv to say "overflows aren't bugs at all". thanks -- PMM
On 12/09/2016 15:10, Peter Maydell wrote: > QEMU's code relies on left shifts of signed integers always > being defined behaviour with the obvious 2s-complement > semantics. The only way to tell the compiler (and any > associated undefined-behaviour sanitizer) that we require a > C dialect with these semantics is to use the -fwrapv option. > This is a bit of a heavy hammer for the job as it also gives > us guaranteed semantics on integer arithmetic overflow which > in theory we don't require. > > In an ideal world this would allow us to drop the warning > flag -Wno-shift-negative-value, but we must retain this to > avoid spurious warnings on clang versions predating the > fix to https://llvm.org/bugs/show_bug.cgi?id=25552. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > We agreed before 2.7 release that this was the best long term > approach to our shift issues, since it's now clear that both > clang and gcc do agree that -fwrapv provides the semantics we > want. > > configure | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index 331c36f..14efce3 100755 > --- a/configure > +++ b/configure > @@ -389,7 +389,11 @@ sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" > ARFLAGS="${ARFLAGS-rv}" > > # default flags for all hosts > -QEMU_CFLAGS="-fno-strict-aliasing -fno-common $QEMU_CFLAGS" > +# We use -fwrapv to tell the compiler that we require a C dialect where > +# left shift of signed integers is well defined and has the expected > +# 2s-complement style results. (Both clang and gcc agree that it > +# provides these semantics.) > +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS" > QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" > QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" > QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
On 12 September 2016 at 18:06, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 12/09/2016 15:10, Peter Maydell wrote: >> QEMU's code relies on left shifts of signed integers always >> being defined behaviour with the obvious 2s-complement >> semantics. The only way to tell the compiler (and any >> associated undefined-behaviour sanitizer) that we require a >> C dialect with these semantics is to use the -fwrapv option. >> This is a bit of a heavy hammer for the job as it also gives >> us guaranteed semantics on integer arithmetic overflow which >> in theory we don't require. >> >> In an ideal world this would allow us to drop the warning >> flag -Wno-shift-negative-value, but we must retain this to >> avoid spurious warnings on clang versions predating the >> fix to https://llvm.org/bugs/show_bug.cgi?id=25552. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> We agreed before 2.7 release that this was the best long term >> approach to our shift issues, since it's now clear that both >> clang and gcc do agree that -fwrapv provides the semantics we >> want. >> >> configure | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/configure b/configure >> index 331c36f..14efce3 100755 >> --- a/configure >> +++ b/configure >> @@ -389,7 +389,11 @@ sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" >> ARFLAGS="${ARFLAGS-rv}" >> >> # default flags for all hosts >> -QEMU_CFLAGS="-fno-strict-aliasing -fno-common $QEMU_CFLAGS" >> +# We use -fwrapv to tell the compiler that we require a C dialect where >> +# left shift of signed integers is well defined and has the expected >> +# 2s-complement style results. (Both clang and gcc agree that it >> +# provides these semantics.) >> +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS" >> QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" >> QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" >> QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" >> > > Acked-by: Paolo Bonzini <pbonzini@redhat.com> Applied to master, thanks. -- PMM
diff --git a/configure b/configure index 331c36f..14efce3 100755 --- a/configure +++ b/configure @@ -389,7 +389,11 @@ sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" ARFLAGS="${ARFLAGS-rv}" # default flags for all hosts -QEMU_CFLAGS="-fno-strict-aliasing -fno-common $QEMU_CFLAGS" +# We use -fwrapv to tell the compiler that we require a C dialect where +# left shift of signed integers is well defined and has the expected +# 2s-complement style results. (Both clang and gcc agree that it +# provides these semantics.) +QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS" QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
QEMU's code relies on left shifts of signed integers always being defined behaviour with the obvious 2s-complement semantics. The only way to tell the compiler (and any associated undefined-behaviour sanitizer) that we require a C dialect with these semantics is to use the -fwrapv option. This is a bit of a heavy hammer for the job as it also gives us guaranteed semantics on integer arithmetic overflow which in theory we don't require. In an ideal world this would allow us to drop the warning flag -Wno-shift-negative-value, but we must retain this to avoid spurious warnings on clang versions predating the fix to https://llvm.org/bugs/show_bug.cgi?id=25552. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- We agreed before 2.7 release that this was the best long term approach to our shift issues, since it's now clear that both clang and gcc do agree that -fwrapv provides the semantics we want. configure | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)