From patchwork Wed Nov 25 17:19:09 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 548656 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 90811140306 for ; Thu, 26 Nov 2015 04:22:09 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=HbMYmTkI; dkim-atps=neutral Received: from localhost ([::1]:46770 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1dlf-00086W-Lq for incoming@patchwork.ozlabs.org; Wed, 25 Nov 2015 12:22:07 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46475) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1dj5-0003Rh-Dz for qemu-devel@nongnu.org; Wed, 25 Nov 2015 12:19:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1dj1-0007VN-1T for qemu-devel@nongnu.org; Wed, 25 Nov 2015 12:19:27 -0500 Received: from mail-wm0-x22e.google.com ([2a00:1450:400c:c09::22e]:36948) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1dj0-0007VG-Od for qemu-devel@nongnu.org; Wed, 25 Nov 2015 12:19:22 -0500 Received: by wmww144 with SMTP id w144so78319147wmw.0 for ; Wed, 25 Nov 2015 09:19:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=N6cZi3f/2UMNQSCKpj2m4RxJ0P7YFd4ejWWKIJAyWoI=; b=HbMYmTkID3V0uiiv2rSg0CZWTfQDchLfBMWwKRfnzgnBUJXKNrk0A+r0x3ZxAR9slV XlmHgdPwG+4rvMgX2dPHVpqvLNh9wiD1rihHc6uQIuh6w2O5/Z4wHDCxlaB05X/6LBnp knuNPsQUD/ARbuFbVuyG3rrQ5JQB6ObUAREjKhWEJOIqV+t+n0oKdItcI59/0PeHDt8a YuBw9wjzAqexZSKho8ftT4dWM26gG1RMECOaqpgxAxXO2CFCJu5l05f2pIZIWCjcrJlJ KTiDPuNGnilkNEIQXVkuskUg9XNZxyw19nCp0f7+L8lplPCNvRLERY7NYFS24GAbKcjJ 6rsQ== X-Received: by 10.28.226.11 with SMTP id z11mr6159236wmg.56.1448471962110; Wed, 25 Nov 2015 09:19:22 -0800 (PST) Received: from 640k.lan (94-39-152-11.adsl-ull.clienti.tiscali.it. [94.39.152.11]) by smtp.gmail.com with ESMTPSA id b84sm4444274wmh.15.2015.11.25.09.19.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Nov 2015 09:19:21 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Wed, 25 Nov 2015 18:19:09 +0100 Message-Id: <1448471956-66873-3-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1448471956-66873-1-git-send-email-pbonzini@redhat.com> References: <1448471956-66873-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c09::22e Cc: Peter Maydell Subject: [Qemu-devel] [PULL 2/9] QEMU does not care about left shifts of signed negative values X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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. 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). 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. Note that -fstrict-overflow does not silence ubsan's overflow warnings, hence it's reasonable to assume that it won't silence the left shift warnings either. QEMU doesn't rely on pointer overflow anyway, and that's the other major difference between -fwrapv (which only cares about integer overflow) and -fstrict-overflow. Thanks to everyone involved in the discussion! Cc: Peter Maydell Reviewed-by: Markus Armbruster Grudgingly-reviewed-by: Laszlo Ersek Signed-off-by: Paolo Bonzini --- 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 71d6cbc..5bb8187 100755 --- a/configure +++ b/configure @@ -413,7 +413,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" @@ -1461,7 +1461,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