Re: [Qemu-devel] [PATCH] cpu-all.h: Don't accidentally sign extend in g2h()

[Peter Maydell]

On 9 March 2012 14:55, Andreas Färber <address@hidden> wrote:
> Am 09.03.2012 15:33, schrieb Peter Maydell:
>> Cast the argument of the g2h() macro to a target_ulong so that
>> it isn't accidentally sign-extended if it is a signed 32 bit
>> type and long is a 64 bit type. In particular, this fixes a
>> bug where it would return the wrong value for 32 bit guests
>> on 64 bit hosts when passed in one of the arg* values from
>> do_syscall() [which are all abi_long and thus signed types].
>> This could result in spurious failure of mlock(), among others.

> So *only* for a 32-bit guest does this cast from signed int to unsigned
> int and then to unsigned long, avoiding the sign extension on 64-bit
> host. For 64-bit guests it remains as broken as before. Commit message
> could be clearer.

The commit message is only claiming to fix a bug "for 32 bit
guests on 64 bit hosts" -- that seemed fairly clear to me
when I wrote it, and indeed it's only the 32-on-64 behaviour
which the patch changes. 64 bit guests on 64 bit hosts remain OK
because the value is in a signed 64 bit integer which is cast to
an unsigned 64 bit integer (twice). 64 bit guests on 32 bit
hosts may or may not be broken for other reasons, but this
change doesn't alter the behaviour of this macro for them either.

> Note that unsigned long would be wrong for Win64 (where we don't
> currently have any user emulation using this macro).
> uintptr_t would be cleaner.

Probably true, but there are a lot of 'unsigned long's lurking in
cpu-all.h, so that would be a separate cleanup patch.

-- PMM