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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list