From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55156) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhNvZ-0004ny-Oo for qemu-devel@nongnu.org; Wed, 30 Sep 2015 16:24:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZhNvU-0004fH-R2 for qemu-devel@nongnu.org; Wed, 30 Sep 2015 16:24:37 -0400 Received: from mail-pa0-x22b.google.com ([2607:f8b0:400e:c03::22b]:36166) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhNvU-0004fD-Lr for qemu-devel@nongnu.org; Wed, 30 Sep 2015 16:24:32 -0400 Received: by pablk4 with SMTP id lk4so49733635pab.3 for ; Wed, 30 Sep 2015 13:24:32 -0700 (PDT) Sender: Richard Henderson References: <1443558863-26132-1-git-send-email-ehabkost@redhat.com> <1443558863-26132-2-git-send-email-ehabkost@redhat.com> <560BE355.60901@redhat.com> From: Richard Henderson Message-ID: <560C44F4.6030200@twiddle.net> Date: Thu, 1 Oct 2015 06:24:20 +1000 MIME-Version: 1.0 In-Reply-To: <560BE355.60901@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Eduardo Habkost , qemu-devel@nongnu.org On 09/30/2015 11:27 PM, Paolo Bonzini wrote: > > > On 29/09/2015 22:34, Eduardo Habkost wrote: >> Fix undefined behavior detected by clang runtime check: >> >> qemu/target-i386/cpu.c:1494:15: runtime error: >> left shift of 1 by 31 places cannot be represented in type 'int' >> >> While doing that, add extra parenthesis for clarity. >> >> Reported-by: Peter Maydell >> Signed-off-by: Eduardo Habkost >> --- >> target-i386/cpu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index 2b914b2..6af6db9 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -1491,7 +1491,7 @@ static void report_unavailable_features(FeatureWord w, uint32_t mask) >> int i; >> >> for (i = 0; i < 32; ++i) { >> - if (1 << i & mask) { >> + if ((1UL << i) & mask) { > > 1U is enough. > > Paolo > > ps: Ego ceterum censeo that these warnings are useless and uglify the > code unnecessarily. But it looks like I'm in a minority so the patch is > okay. I totally agree. There are no ones-compliment machines anymore, and so the whole point of that "undefined" in the C standard is moot. Let's all accept that shifts of signed quantities do exactly what we expect. Without looking, I don't suppose either compiler has a switch to disable just the shift part of ubsan? r~