From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35040) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZujI9-0005qN-3H for qemu-devel@nongnu.org; Fri, 06 Nov 2015 10:51:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZujI4-0007DG-9f for qemu-devel@nongnu.org; Fri, 06 Nov 2015 10:51:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38324) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZujI4-0007D4-4N for qemu-devel@nongnu.org; Fri, 06 Nov 2015 10:51:00 -0500 From: Paolo Bonzini Date: Fri, 6 Nov 2015 16:50:56 +0100 Message-Id: <1446825056-7989-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH] target-sparc: fix 32-bit truncation in fpackfix, avoid undefined behavior List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Blue Swirl , peter.maydell@linaro.org, Mark Cave-Ayland , armbru@redhat.com, rth@twiddle.net 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. Use a cast to unsigned in order to placate ubsan for negative inputs, and do so in the other fpack* instructions too. Signed-off-by: Paolo Bonzini --- target-sparc/vis_helper.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/target-sparc/vis_helper.c b/target-sparc/vis_helper.c index 383cc8b..a1c3343 100644 --- a/target-sparc/vis_helper.c +++ b/target-sparc/vis_helper.c @@ -404,7 +404,8 @@ uint32_t helper_fpack16(uint64_t gsr, uint64_t rs2) for (byte = 0; byte < 4; byte++) { uint32_t val; int16_t src = rs2 >> (byte * 16); - int32_t scaled = src << scale; + /* The cast to unsigned avoids undefined behavior for negative src. */ + int32_t scaled = (uint32_t)src << scale; int32_t from_fixed = scaled >> 7; val = (from_fixed < 0 ? 0 : @@ -426,7 +427,8 @@ uint64_t helper_fpack32(uint64_t gsr, uint64_t rs1, uint64_t rs2) for (word = 0; word < 2; word++) { uint64_t val; int32_t src = rs2 >> (word * 32); - int64_t scaled = (int64_t)src << scale; + /* The cast to unsigned avoids undefined behavior for negative src. */ + int64_t scaled = (uint64_t)src << scale; int64_t from_fixed = scaled >> 23; val = (from_fixed < 0 ? 0 : @@ -447,7 +449,8 @@ 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; + /* The cast to unsigned avoids undefined behavior for negative src. */ + int64_t scaled = (uint64_t)src << scale; int64_t from_fixed = scaled >> 16; val = (from_fixed < -32768 ? -32768 : -- 2.5.0