[libvirt] [PATCH 4/9] xen: Add coverity[ptr_arith] and [sign_extension] tags

[Eric Blake]

On 01/29/2013 06:41 AM, John Ferlan wrote:

>>>>>              *(pm + (j / 8)) |= cpumap[j] << (8 * (j & 7));

> FWIW:  This path of code only occurs when "(hv_versions.hypervisor <=
> 1)" - IOW really old code.  I checked history and the code was added by
> commit id '86247f2c'.  Also since the target of the "|" operation is a
> 'uint64_t' (e.g. *(pm + (j / 8)), wouldn't the shift from 0->56 be OK
> (e.g. (8 * (j & 7)))?  That is it's not an 'int << (8*4)' it's a
> 'uint64_t << (8*4)'.

Order of operations, broken down by type:

*(uint64_t* + (int / int)) |= unsigned char << (int * (int & int))
*(uint64_t* + int) |= unsigned char << int
*(uint64_t*) |= int << int
uint64_t = uint64_t | int

There really is a sign extension bug, because 'unsigned char << int'
uses 'int' math, but we then widen the int into uint64_t.  If vcpu 31 is
turned on, we end up sign-extending and also enabling vcpus 32-63 at the
same time, which was not the goal.

> 
> When first approaching this I figured I didn't want to introduce a bug
> into code that's been around a long time and that may not have any one
> using it. I agree the line looks ugly and it did take me a bit to think
> about it.

Just because no one has reported the bug or tested it lately doesn't
mean that the bug isn't there - it has been there a LONG time. :)

> 
> Mathematically what you propose with the memcpy() works; however, I come
> from an architecture where a memcpy to an unaligned address causes a
> fault

Such a memcpy() implementation would be flawed, and should be reported
as a bug to that vendor; thankfully, these days, most libc do not have
that flaw.  The C standard guarantees that memcpy() is required to work
on unaligned copies (for a certain definition of work; you may still end
up splicing together unrelated portions of adjacent integers when
reading things back as integers, but byte-for-byte, the operation is
well-defined).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130129/3f2a451b/attachment-0001.sig>