Gitlab CI improvements and fixes

[PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS

Posted by Thomas Huth 5 years, 8 months ago

From: Alex Bennée <alex.bennee@linaro.org>

Newer clangs rightly spot that you can never exceed the full address
space of 64 bit hosts with:

  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.

So lets limit the check to 32 bit hosts only.

Fixes: ee94743034bf
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
[thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS == 32]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 linux-user/elfload.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 01a9323a63..ebc663ea0b 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr,
             exit(EXIT_FAILURE);
         }
     } else {
+#if HOST_LONG_BITS < TARGET_ABI_BITS
         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);
         }
+#endif
     }
 
     /*
-- 
2.18.1

Re: [PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS

Posted by Laurent Vivier 5 years, 8 months ago

Le 25/05/2020 à 15:18, Thomas Huth a écrit :
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> Newer clangs rightly spot that you can never exceed the full address
> space of 64 bit hosts with:
> 
>   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.
> 
> So lets limit the check to 32 bit hosts only.
> 
> Fixes: ee94743034bf
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> [thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS == 32]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  linux-user/elfload.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 01a9323a63..ebc663ea0b 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr,
>              exit(EXIT_FAILURE);
>          }
>      } else {
> +#if HOST_LONG_BITS < TARGET_ABI_BITS
>          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);
>          }
> +#endif
>      }
>  
>      /*
> 

Philippe sent the same patch:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg699796.html

Thanks,
Laurent

Re: [PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS

Posted by Thomas Huth 5 years, 8 months ago

On 27/05/2020 16.44, Laurent Vivier wrote:
> Le 25/05/2020 à 15:18, Thomas Huth a écrit :
>> From: Alex Bennée <alex.bennee@linaro.org>
>>
>> Newer clangs rightly spot that you can never exceed the full address
>> space of 64 bit hosts with:
>>
>>   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.
>>
>> So lets limit the check to 32 bit hosts only.
>>
>> Fixes: ee94743034bf
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> [thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS == 32]
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  linux-user/elfload.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 01a9323a63..ebc663ea0b 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr,
>>              exit(EXIT_FAILURE);
>>          }
>>      } else {
>> +#if HOST_LONG_BITS < TARGET_ABI_BITS
>>          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);
>>          }
>> +#endif
>>      }
>>  
>>      /*
>>
> 
> Philippe sent the same patch:
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg699796.html

Indeed, but looking more closely, he's using slightly different
locations for the #if and #endif ... not sure what's better though...?

 Thomas

Re: [PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS

Posted by Alex Bennée 5 years, 8 months ago

Thomas Huth <thuth@redhat.com> writes:

> On 27/05/2020 16.44, Laurent Vivier wrote:
>> Le 25/05/2020 à 15:18, Thomas Huth a écrit :
>>> From: Alex Bennée <alex.bennee@linaro.org>
>>>
>>> Newer clangs rightly spot that you can never exceed the full address
>>> space of 64 bit hosts with:
>>>
>>>   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.
>>>
>>> So lets limit the check to 32 bit hosts only.
>>>
>>> Fixes: ee94743034bf
>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> [thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS == 32]
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  linux-user/elfload.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>>> index 01a9323a63..ebc663ea0b 100644
>>> --- a/linux-user/elfload.c
>>> +++ b/linux-user/elfload.c
>>> @@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr,
>>>              exit(EXIT_FAILURE);
>>>          }
>>>      } else {
>>> +#if HOST_LONG_BITS < TARGET_ABI_BITS
>>>          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);
>>>          }
>>> +#endif
>>>      }
>>>  
>>>      /*
>>>
>> 
>> Philippe sent the same patch:
>> 
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg699796.html
>
> Indeed, but looking more closely, he's using slightly different
> locations for the #if and #endif ... not sure what's better though...?

Richard was more inclined to suppress the warning:

  Subject: Re: [PATCH v2] linux-user: limit check to HOST_LONG_BITS == 32
  From: Richard Henderson <richard.henderson@linaro.org>
  Message-ID: <3069bc1b-115d-f361-8271-c775bf6957ea@linaro.org>
  Date: Thu, 21 May 2020 20:15:51 -0700

One reason I dropped the f32 patch from my last PR was because this
wasn't the only warning the latest clang picks up.

-- 
Alex Bennée

Re: [PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS

Posted by Thomas Huth 5 years, 8 months ago

On 27/05/2020 18.36, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 27/05/2020 16.44, Laurent Vivier wrote:
>>> Le 25/05/2020 à 15:18, Thomas Huth a écrit :
>>>> From: Alex Bennée <alex.bennee@linaro.org>
>>>>
>>>> Newer clangs rightly spot that you can never exceed the full address
>>>> space of 64 bit hosts with:
>>>>
>>>>   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.
>>>>
>>>> So lets limit the check to 32 bit hosts only.
>>>>
>>>> Fixes: ee94743034bf
>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> [thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS == 32]
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>  linux-user/elfload.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>>>> index 01a9323a63..ebc663ea0b 100644
>>>> --- a/linux-user/elfload.c
>>>> +++ b/linux-user/elfload.c
>>>> @@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr,
>>>>              exit(EXIT_FAILURE);
>>>>          }
>>>>      } else {
>>>> +#if HOST_LONG_BITS < TARGET_ABI_BITS
>>>>          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);
>>>>          }
>>>> +#endif
>>>>      }
>>>>  
>>>>      /*
>>>>
>>>
>>> Philippe sent the same patch:
>>>
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg699796.html
>>
>> Indeed, but looking more closely, he's using slightly different
>> locations for the #if and #endif ... not sure what's better though...?
> 
> Richard was more inclined to suppress the warning:
> 
>   Subject: Re: [PATCH v2] linux-user: limit check to HOST_LONG_BITS == 32
>   From: Richard Henderson <richard.henderson@linaro.org>
>   Message-ID: <3069bc1b-115d-f361-8271-c775bf6957ea@linaro.org>
>   Date: Thu, 21 May 2020 20:15:51 -0700
> 
> One reason I dropped the f32 patch from my last PR was because this
> wasn't the only warning the latest clang picks up.

... but this is currently the only spot that is required to get the
gitlab CI going again, so I think we should include this patch until we
have a final decision whether to disable the warning or not (and we can
still revert this patch after we disabled the warning). Ok?

 Thomas

Re: [PATCH 7/7] linux-user: limit check to HOST_LONG_BITS < TARGET_ABI_BITS

Posted by Alex Bennée 5 years, 8 months ago

Thomas Huth <thuth@redhat.com> writes:

> On 27/05/2020 18.36, Alex Bennée wrote:
>> 
>> Thomas Huth <thuth@redhat.com> writes:
>> 
>>> On 27/05/2020 16.44, Laurent Vivier wrote:
>>>> Le 25/05/2020 à 15:18, Thomas Huth a écrit :
>>>>> From: Alex Bennée <alex.bennee@linaro.org>
>>>>>
>>>>> Newer clangs rightly spot that you can never exceed the full address
>>>>> space of 64 bit hosts with:
>>>>>
>>>>>   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.
>>>>>
>>>>> So lets limit the check to 32 bit hosts only.
>>>>>
>>>>> Fixes: ee94743034bf
>>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>>> [thuth: Use HOST_LONG_BITS < TARGET_ABI_BITS instead of HOST_LONG_BITS == 32]
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>  linux-user/elfload.c | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>>>>> index 01a9323a63..ebc663ea0b 100644
>>>>> --- a/linux-user/elfload.c
>>>>> +++ b/linux-user/elfload.c
>>>>> @@ -2073,12 +2073,14 @@ static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr,
>>>>>              exit(EXIT_FAILURE);
>>>>>          }
>>>>>      } else {
>>>>> +#if HOST_LONG_BITS < TARGET_ABI_BITS
>>>>>          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);
>>>>>          }
>>>>> +#endif
>>>>>      }
>>>>>  
>>>>>      /*
>>>>>
>>>>
>>>> Philippe sent the same patch:
>>>>
>>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg699796.html
>>>
>>> Indeed, but looking more closely, he's using slightly different
>>> locations for the #if and #endif ... not sure what's better though...?
>> 
>> Richard was more inclined to suppress the warning:
>> 
>>   Subject: Re: [PATCH v2] linux-user: limit check to HOST_LONG_BITS == 32
>>   From: Richard Henderson <richard.henderson@linaro.org>
>>   Message-ID: <3069bc1b-115d-f361-8271-c775bf6957ea@linaro.org>
>>   Date: Thu, 21 May 2020 20:15:51 -0700
>> 
>> One reason I dropped the f32 patch from my last PR was because this
>> wasn't the only warning the latest clang picks up.
>
> ... but this is currently the only spot that is required to get the
> gitlab CI going again, so I think we should include this patch until we
> have a final decision whether to disable the warning or not (and we can
> still revert this patch after we disabled the warning). Ok?

I'm certainly happy with that if it gets gitlab working.

My experience with make docker-test-vlang@fedora (with 32) was there
where more things to fix. I guess gitlab didn't trigger them.

-- 
Alex Bennée