From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45306) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXAyP-0005P7-2J for qemu-devel@nongnu.org; Mon, 07 Apr 2014 10:56:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WXAyI-00061f-Vn for qemu-devel@nongnu.org; Mon, 07 Apr 2014 10:56:33 -0400 Received: from mail-ee0-f48.google.com ([74.125.83.48]:36971) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXAyI-00061I-QE for qemu-devel@nongnu.org; Mon, 07 Apr 2014 10:56:26 -0400 Received: by mail-ee0-f48.google.com with SMTP id b57so737519eek.35 for ; Mon, 07 Apr 2014 07:56:25 -0700 (PDT) Message-ID: <5342BC96.6010204@cloudius-systems.com> Date: Mon, 07 Apr 2014 17:56:22 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1396019577-2013-1-git-send-email-peter.maydell@linaro.org> <1396019577-2013-3-git-send-email-peter.maydell@linaro.org> <5340FDA3.1080902@msgid.tls.msk.ru> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/3] int128.h: Avoid undefined behaviours involving signed arithmetic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , Michael Tokarev Cc: QEMU Trivial , Patch Tracking , QEMU Developers , Richard Henderson On 04/06/2014 01:18 PM, Peter Maydell wrote: > On 6 April 2014 08:09, Michael Tokarev wrote: >> 28.03.2014 19:12, Peter Maydell wrote: >>> Add casts when we're performing arithmetic on the .hi parts of an >>> Int128, to avoid undefined behaviour. >> [] >>> static inline Int128 int128_sub(Int128 a, Int128 b) >>> { >>> - return (Int128){ a.lo - b.lo, a.hi - b.hi - (a.lo < b.lo) }; >>> + return (Int128){ a.lo - b.lo, (uint64_t)a.hi - b.hi - (a.lo < b.lo) }; >> What was wrong with this one? I don't think casting to unsigned here is >> a good idea. > This patch is fixing these three clang sanitizer warnings: > /home/petmay01/linaro/qemu-from-laptop/qemu/include/qemu/int128.h:81:40: > runtime error: signed integer overflow: 0 - -9223372036854775808 > cannot be represented in type 'long' > /home/petmay01/linaro/qemu-from-laptop/qemu/include/qemu/int128.h:81:47: > runtime error: signed integer overflow: -9223372036854775808 - 1 > cannot be represented in type 'long' > /home/petmay01/linaro/qemu-from-laptop/qemu/include/qemu/int128.h:56:47: > runtime error: left shift of negative value -9223372036854775807 > > of which the first two are in this function. > > Note that int128_add() already has a cast. > > The alternative would be to say that Int128 should have > undefined behaviour on underflow/overflow and the test > code is wrong, but that doesn't seem very useful to me. > > Isn't the test broken here? It is trying to add (or shift) -2^127 and something else, and the result truly overflows. A better behaviour would be to abort when this happens. Int128 was designed to avoid silent overflows, not to silently cause breakage. Not that I think it is necessary, there is no way for the guest to trigger an overflow.