It seems like there's no good reason for the compiler to exploit the undefinedness of left shifts. GCC explicitly documents that they do not use at all this possibility and, while they also say this is subject to change, they have been saying this for 10 years (since the wording appeared in the GCC 4.0 manual).
Any workaround for this particular case of undefined behavior uglifies the code. Using unsigned is unsafe (https://github.com/madler/zlib/pull/112 is the proof) because the value becomes positive when extended; -(a << b) works but does not express that the intention is to compute -a * 2^N, especially if "a" is a constant. <rant> The straw that broke the camel back is Clang's latest obnoxious, pointless, unsafe---and did I mention *totally* useless---warning about left shifting a negative integer. It's obnoxious and pointless because the compiler is not using the latitude that the standard gives it, so the warning just adds noise. It is useless and unsafe because it does not catch the widely more common case where the LHS is a variable, and thus gives a false sense of security. The noisy nature of the warning means that it should have never been added to -Wall. The uselessness means that it probably should not have even been added to -Wextra. (It would have made sense to enable the warning for -fsanitize=shift, as the program would always crash if the point is reached. But this was probably too sophisticated to come up with, when you're so busy giving birth to gems such as -Wabsolute-value). </rant> Ubsan also has warnings for undefined behavior of left shifts. Checks for left shift overflow and left shift of negative numbers, unfortunately, cannot be silenced without also silencing the useful ones about out-of-range shift amounts. -fwrapv ought to shut them up, but doesn't yet (https://llvm.org/bugs/show_bug.cgi?id=25552; I am taking care of fixing the same issues in GCC). Luckily ubsan is optional, and the easy workaround is to use -fsanitize-recover. Anyhow, this patch documents our assumptions explicitly, and shuts up the stupid warning. -fwrapv is a bit of a heavy hammer, but it is the safest option and it ought to just work long term as the compilers improve. Thanks to everyone involved in the discussion! Cc: Peter Maydell <peter.mayd...@linaro.org> Cc: Markus Armbruster <arm...@redhat.com> Cc: Laszlo Ersek <ler...@redhat.com> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- HACKING | 6 ++++++ configure | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/HACKING b/HACKING index 12fbc8a..71ad23b 100644 --- a/HACKING +++ b/HACKING @@ -157,3 +157,9 @@ painful. These are: * you may assume that integers are 2s complement representation * you may assume that right shift of a signed integer duplicates the sign bit (ie it is an arithmetic shift, not a logical shift) + +In addition, QEMU assumes that the compiler does not use the latitude +given in C99 and C11 to treat aspects of signed '<<' as undefined, as +documented in the GNU Compiler Collection manual starting at version 4.0. +If a compiler does not respect this when passed the -fwrapv option, +it is not supported for compilation of QEMU. diff --git a/configure b/configure index 6bfa6f5..aa0a6c8 100755 --- a/configure +++ b/configure @@ -380,7 +380,7 @@ 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" +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" @@ -1428,7 +1428,7 @@ fi gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" -gcc_flags="-Wendif-labels $gcc_flags" +gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags" gcc_flags="-Wno-initializer-overrides $gcc_flags" gcc_flags="-Wno-string-plus-int $gcc_flags" # Note that we do not add -Werror to gcc_flags here, because that would -- 2.5.0
