RE: [PATCH] scsi: aic7xxx: Fix unintended sign extension issue

From: David Laight
Date: Thu Oct 25 2018 - 11:54:40 EST

From: James Bottomley
> Sent: 25 October 2018 16:33
> On Thu, 2018-10-25 at 16:13 +0100, Colin King wrote:
> > From: Colin Ian King <colin.king@xxxxxxxxxxxxx>
> >
> > In the expression "ahc_inb(ahc, port+3) << 24", the initial value is
> > a u8, but is promoted to a signed int, then sign-extended to
> > uint64_t.
> Why is this, that's highly non intuitive? The compiler is supposed to
> promote to the biggest type, which is uint64_t and then do the
> calculation

Do not doubt the wisdom on the ANSI C committee that decided to do
'value preserving' integer promotions instead of the 'sign preserving'
ones of K&R C.

So 'unsigned char' is promoted to 'int' almost everywhere it is used
(unless they are both the same size - which is allowed).
This means that ahc_inb() << 24 is actually undefined (signed integer
overflow can do anything it likes).

By far the best fix is to change the return type of ahc_inb() to
be 'unsigned int'.
On systems without byte sized registers (about everything except x86)
this will almost certainly generate better code.


Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)