Re: [PATCH v1 04/10] linux-user: completely re-write init_guest_space

[Richard Henderson]

On 5/21/20 1:21 AM, Alex Bennée wrote:
> 
> Thomas Huth <address@hidden> writes:
> 
>> On 13/05/2020 19.51, Alex Bennée wrote:
>>> First we ensure all guest space initialisation logic comes through
>>> probe_guest_base once we understand the nature of the binary we are
>>> loading. The convoluted init_guest_space routine is removed and
>>> replaced with a number of pgb_* helpers which are called depending on
>>> what requirements we have when loading the binary.
>>>
>>> We first try to do what is requested by the host. Failing that we try
>>> and satisfy the guest requested base address. If all those options
>>> fail we fall back to finding a space in the memory map using our
>>> recently written read_self_maps() helper.
>>>
>>> There are some additional complications we try and take into account
>>> when looking for holes in the address space. We try not to go directly
>>> after the system brk() space so there is space for a little growth. We
>>> also don't want to have to use negative offsets which would result in
>>> slightly less efficient code on x86 when it's unable to use the
>>> segment offset register.
>>>
>>> Less mind-binding gotos and hopefully clearer logic throughout.
>>>
>>> Signed-off-by: Alex Bennée <address@hidden>
>>> Acked-by: Laurent Vivier <address@hidden>
> <snip>
>>> +    } else {
>>> +        if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
>>> +            error_report("%s: requires more virtual address space "
>>> +                         "than the host can provide (0x%" PRIx64 ")",
>>> +                         image_name, (uint64_t)guest_hiaddr - guest_base);
>>> +            exit(EXIT_FAILURE);
>>>          }
>>
>>  Hi Alex,
>>
>> this causes an error with newer versions of Clang:
>>
>> linux-user/elfload.c:2076:41: error: result of comparison 'unsigned
>> long' > 18446744073709551615 is always false
>> [-Werror,-Wtautological-type-limit-compare]
>> 4685         if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
>> 4686             ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
>> 4687 1 error generated.
>>
>> Any ideas how to fix this?
> 
> I guess this check only makes sense when abi_ulong > uintptr_t or IOW 64
> bit guests running on 32 bit hosts. We could just wrap that check in:
> 
> #if HOST_LONG_BITS == 32
> 
> #endif

As I've suggested elsewhere, I think we should disable this warning on the
command-line.


r~