Re: [Qemu-devel] [PATCH] exec.c: address_space_translate: handle access to addr 0 of 2^64 sized region

[Paolo Bonzini <pbonzini@redhat.com>]

Il 20/06/2013 16:18, Peter Maydell ha scritto:
> The memory API allows a MemoryRegion's size to be 2^64, as a special
> case (otherwise the size always fits in a 64 bit integer). This meant
> that attempts to access address zero in a 2^64 sized region would
> assert in address_space_translate():
> 
>   #3  0x00007ffff3e4d192 in __GI___assert_fail#(assertion=0x555555a43f32
>     "!a.hi", file=0x555555a43ef0 "include/qemu/int128.h", line=18,
>     function=0x555555a4439f "int128_get64") at assert.c:103
>   #4  0x0000555555877642 in int128_get64 (a=...)
>     at include/qemu/int128.h:18
>   #5  0x00005555558782f2 in address_space_translate (as=0x55555668d140,
>    /addr=0, xlat=0x7fffafac9918, plen=0x7fffafac9920, is_write=false)
>     at exec.c:221
> 
> Fix this by doing the 'min' operation in 128 bit arithmetic
> rather than 64 bit arithmetic (we know the result of the 'min'
> definitely fits in 64 bits because one of the inputs did).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The other possible approach here would be:
>     if (!diff.hi) {
>         *plen = MIN(int128_get64(diff), *plen);
>     }
> (since if diff.hi is nonzero we know plen is smaller) but
> it seems slightly cleaner not to "look inside" the Int128.
> 
>  exec.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/exec.c b/exec.c
> index 5b8b40d..eb200d0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -218,7 +218,7 @@ MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
>      *xlat = addr + section->offset_within_region;
>  
>      diff = int128_sub(section->mr->size, int128_make64(addr));
> -    *plen = MIN(int128_get64(diff), *plen);
> +    *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
>      return section;
>  }
>  #endif
> 

Thanks, applied.

Paolo