Re: [RFC PATCH 2/2] linux-user/mmap: Fix Clang 'type-limit-compare' warning

[Richard Henderson <richard.henderson@linaro.org>]

On 5/3/20 5:49 AM, Aleksandar Markovic wrote:
> нед, 3. мај 2020. у 13:33 Philippe Mathieu-Daudé <f4bug@amsat.org> је
> написао/ла:
>>
>> When building with Clang 10 on Fedora 32, we get:
>>
>>     CC      linux-user/mmap.o
>>   linux-user/mmap.c:720:49: error: result of comparison 'unsigned long' > 18446744073709551615 is always false [-Werror,-Wtautological-type-limit-compare]
>>           if ((unsigned long)host_addr + new_size > (abi_ulong)-1) {
>>               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
>>
>> Fix by restricting the check for when target sizeof(abi_ulong) is
>> smaller than target sizeof(unsigned long).
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  linux-user/mmap.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index e378033797..b14652d894 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -714,6 +714,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
>>              errno = ENOMEM;
>>              host_addr = MAP_FAILED;
>>          }
>> +#if TARGET_ABI_BITS < TARGET_LONG_BITS
>>          /* Check if address fits target address space */
>>          if ((unsigned long)host_addr + new_size > (abi_ulong)-1) {
>>              /* Revert mremap() changes */
>> @@ -721,6 +722,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
>>              errno = ENOMEM;
>>              host_addr = MAP_FAILED;
>>          }
>> +#endif /* TARGET_ABI_BITS < TARGET_LONG_BITS */
> 
> Hm, Philippe, this will silence the clang error, but is this the right
> thing to do?
> 
> Why do you think the case:
> 
> TARGET_ABI_BITS < TARGET_LONG_BITS
> 
> doesn't need this check?

I think that's quite obvious from the clang warning -- the test is always false
due to type limits.

That said, this is at minimum the second occurrence of having to add ifdefs to
work around this particular new warning, because there does not seem to be any
other way to suppress the warning, and I'm not keen on that.

I would prefer that we disable the warning on the compiler command line in
configure.


r~