From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44606) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZnBqf-00044K-Ly for qemu-devel@nongnu.org; Fri, 16 Oct 2015 16:43:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZnBqd-0007As-Es for qemu-devel@nongnu.org; Fri, 16 Oct 2015 16:43:32 -0400 References: <1445017739-14876-1-git-send-email-peter.maydell@linaro.org> From: John Snow Message-ID: <5621616C.8080706@redhat.com> Date: Fri, 16 Oct 2015 16:43:24 -0400 MIME-Version: 1.0 In-Reply-To: <1445017739-14876-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/ide/ahci.c: Fix shift left into sign bit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: qemu-block@nongnu.org, patches@linaro.org On 10/16/2015 01:48 PM, Peter Maydell wrote: > Avoid undefined behaviour from shifting left into the sign bit: > > hw/ide/ahci.c:551:36: runtime error: left shift of 255 by 24 places cannot be represented in type 'int' > > (Unfortunately C's promotion rules mean that in the expression > "some_uint8_t_variable << 24" the LHS gets promoted to signed > int before shifting.) > > Signed-off-by: Peter Maydell > --- > clang's undefined sanitizer produces a lot of copies of this warning during > 'make check'... > > hw/ide/ahci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 796be15..21f76ed 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -548,7 +548,7 @@ static void ahci_init_d2h(AHCIDevice *ad) > ad->init_d2h_sent = true; > /* We're emulating receiving the first Reg H2D Fis from the device; > * Update the SIG register, but otherwise proceed as normal. */ > - pr->sig = (ide_state->hcyl << 24) | > + pr->sig = ((uint32_t)ide_state->hcyl << 24) | > (ide_state->lcyl << 16) | > (ide_state->sector << 8) | > (ide_state->nsector & 0xFF); > Reviewed-by: John Snow Since the "patches" tool seems to still be hiccuping, do you want to just apply this directly? --js