fix gcc warnings when RESERVED_VA is 0

The current code, while correct, triggers a bunch of gcc warnings when
RESERVED_VA is 0 like so:
linux-user/syscall.c: In function 'do_shmat':
linux-user/syscall.c:3058: warning: comparison of unsigned expression < 0 is always false
linux-user/syscall.c: In function 'open_self_maps':
linux-user/syscall.c:4960: warning: comparison of unsigned expression < 0 is always false
linux-user/syscall.c:4960: warning: comparison of unsigned expression < 0 is always false

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 cpu-all.h |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

On 16 September 2012 01:05, Mike Frysinger <vapier@gentoo.org> wrote:
> The current code, while correct, triggers a bunch of gcc warnings when
> RESERVED_VA is 0 like so:
> linux-user/syscall.c: In function 'do_shmat':
> linux-user/syscall.c:3058: warning: comparison of unsigned expression < 0 is always false
> linux-user/syscall.c: In function 'open_self_maps':
> linux-user/syscall.c:4960: warning: comparison of unsigned expression < 0 is always false
> linux-user/syscall.c:4960: warning: comparison of unsigned expression < 0 is always false
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  cpu-all.h |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/cpu-all.h b/cpu-all.h
> index 5e07d28..0e5dcf0 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -202,10 +202,16 @@ extern unsigned long reserved_va;
>  #if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
>  #define h2g_valid(x) 1
>  #else
> +/* Gcc likes to warn about comparing unsigned longs to < 0, so cpp it away.  */
> +# if RESERVED_VA

This won't do the right thing. In the CONFIG_USE_GUEST_BASE case
RESERVED_VA is #defined to 'reserved_va'. Since that's not a
cpp identifier then (a) cpp should complain since we use -Wundef
and (b) cpp will treat it as zero, meaning that we take the #else
branch regardless, which isn't what we want.

In the longer term it would be nice to add guest-base support to
the SPARC tcg backend, and then we could just unconditionally
enable CONFIG_USE_GUEST_BASE for -user targets.

> +#  define _h2g_reserved_va(x) ((x) < RESERVED_VA)
> +# else
> +#  define _h2g_reserved_va(x) 1
> +# endif
>  #define h2g_valid(x) ({ \
>      unsigned long __guest = (unsigned long)(x) - GUEST_BASE; \
>      (__guest < (1ul << TARGET_VIRT_ADDR_SPACE_BITS)) && \
> -    (!RESERVED_VA || (__guest < RESERVED_VA)); \
> +    _h2g_reserved_va(__guest); \
>  })
>  #endif

-- PMM

On Sun, Sep 16, 2012 at 9:58 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 16 September 2012 01:05, Mike Frysinger <vapier@gentoo.org> wrote:
>> The current code, while correct, triggers a bunch of gcc warnings when
>> RESERVED_VA is 0 like so:
>> linux-user/syscall.c: In function 'do_shmat':
>> linux-user/syscall.c:3058: warning: comparison of unsigned expression < 0 is always false
>> linux-user/syscall.c: In function 'open_self_maps':
>> linux-user/syscall.c:4960: warning: comparison of unsigned expression < 0 is always false
>> linux-user/syscall.c:4960: warning: comparison of unsigned expression < 0 is always false
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>>  cpu-all.h |    8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/cpu-all.h b/cpu-all.h
>> index 5e07d28..0e5dcf0 100644
>> --- a/cpu-all.h
>> +++ b/cpu-all.h
>> @@ -202,10 +202,16 @@ extern unsigned long reserved_va;
>>  #if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
>>  #define h2g_valid(x) 1
>>  #else
>> +/* Gcc likes to warn about comparing unsigned longs to < 0, so cpp it away.  */
>> +# if RESERVED_VA
>
> This won't do the right thing. In the CONFIG_USE_GUEST_BASE case
> RESERVED_VA is #defined to 'reserved_va'. Since that's not a
> cpp identifier then (a) cpp should complain since we use -Wundef
> and (b) cpp will treat it as zero, meaning that we take the #else
> branch regardless, which isn't what we want.
>
> In the longer term it would be nice to add guest-base support to
> the SPARC tcg backend, and then we could just unconditionally
> enable CONFIG_USE_GUEST_BASE for -user targets.

IIRC Richard's Sparc TCG patch set implemented guest base. Any chance
of refreshing that?

>
>> +#  define _h2g_reserved_va(x) ((x) < RESERVED_VA)
>> +# else
>> +#  define _h2g_reserved_va(x) 1
>> +# endif
>>  #define h2g_valid(x) ({ \
>>      unsigned long __guest = (unsigned long)(x) - GUEST_BASE; \
>>      (__guest < (1ul << TARGET_VIRT_ADDR_SPACE_BITS)) && \
>> -    (!RESERVED_VA || (__guest < RESERVED_VA)); \
>> +    _h2g_reserved_va(__guest); \
>>  })
>>  #endif
>
> -- PMM
>

A refreshed version of an old patchset that got mislaid.
I've re-tested it on arm and xtensa system images, and
several linux-user images.


r~


Richard Henderson (13):
  linux-user: Use memcpy in get_user/put_user.
  tcg-sparc: Hack in qemu_ld/st64 for 32-bit.
  tcg-sparc: Fix ADDX opcode.
  tcg-sparc: Assume v9 cpu always, i.e. force v8plus in 32-bit mode.
  tcg-sparc: Fix qemu_ld/st to handle 32-bit host.
  tcg-sparc: Support GUEST_BASE.
  tcg-sparc: Change AREG0 in generated code to %i0.
  tcg-sparc: Clean up cruft stemming from attempts to use global
    registers.
  tcg-sparc: Mask shift immediates to avoid illegal insns.
  tcg-sparc: Use defines for temporaries.
  tcg-sparc: Add %g/%o registers to alloc_order
  tcg-sparc: Fix and enable direct TB chaining.
  tcg: Fix !USE_DIRECT_JUMP

 configure              |  52 +--
 disas.c                |   2 -
 exec-all.h             |   9 +-
 exec.c                 |  12 +-
 linux-user/qemu.h      |  47 +--
 qemu-timer.h           |   8 +-
 tcg/sparc/tcg-target.c | 949 +++++++++++++++++++++++--------------------------
 tcg/sparc/tcg-target.h |  33 +-
 tcg/tcg.c              |   3 +-
 tcg/tcg.h              |   2 +-
 10 files changed, 502 insertions(+), 615 deletions(-)