From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37575) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtFo5-00074e-Mg for qemu-devel@nongnu.org; Mon, 02 Nov 2015 09:09:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtFo4-0004sg-Qz for qemu-devel@nongnu.org; Mon, 02 Nov 2015 09:09:57 -0500 Received: from mail-vk0-x234.google.com ([2607:f8b0:400c:c05::234]:34725) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtFo4-0004sc-Mc for qemu-devel@nongnu.org; Mon, 02 Nov 2015 09:09:56 -0500 Received: by vkgs66 with SMTP id s66so85694162vkg.1 for ; Mon, 02 Nov 2015 06:09:56 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1446473134-4330-1-git-send-email-pbonzini@redhat.com> References: <1446473134-4330-1-git-send-email-pbonzini@redhat.com> From: Peter Maydell Date: Mon, 2 Nov 2015 14:09:36 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Blue Swirl , Mark Cave-Ayland , QEMU Developers On 2 November 2015 at 14:05, Paolo Bonzini wrote: > This is reported by Coverity. The algorithm description at > ftp://ftp.icm.edu.pl/packages/ggi/doc/hw/sparc/Sparc.pdf suggests > that the 32-bit parts of rs2, after the left shift, is treated > as a 64-bit integer. Bits 32 and above are used to do the > saturating truncation. > > Signed-off-by: Paolo Bonzini > --- > target-sparc/vis_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target-sparc/vis_helper.c b/target-sparc/vis_helper.c > index 383cc8b..45fc7db 100644 > --- a/target-sparc/vis_helper.c > +++ b/target-sparc/vis_helper.c > @@ -447,7 +447,7 @@ uint32_t helper_fpackfix(uint64_t gsr, uint64_t rs2) > for (word = 0; word < 2; word++) { > uint32_t val; > int32_t src = rs2 >> (word * 32); > - int64_t scaled = src << scale; > + int64_t scaled = (int64_t)src << scale; > int64_t from_fixed = scaled >> 16; This will now shift left into the sign bit of a signed integer, which is undefined behaviour. thanks -- PMM