[v1] linux-user: Replace __u64 with uint64_t

[PATCH] linux-user: Replace __u64 with uint64_t

Posted by Khem Raj 2 years, 6 months ago

uint64_t is available in all userspaces via compiler include stdint.h
therefore use it instead of __u64 which is linux internal type, it fixes
build on some platforms eg. aarch64 systems using musl C library

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 linux-user/host/aarch64/hostdep.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
index a8d41a21ad..34d934f665 100644
--- a/linux-user/host/aarch64/hostdep.h
+++ b/linux-user/host/aarch64/hostdep.h
@@ -25,7 +25,7 @@ extern char safe_syscall_end[];
 static inline void rewind_if_in_safe_syscall(void *puc)
 {
     ucontext_t *uc = puc;
-    __u64 *pcreg = &uc->uc_mcontext.pc;
+    uint64_t *pcreg = &uc->uc_mcontext.pc;
 
     if (*pcreg > (uintptr_t)safe_syscall_start
         && *pcreg < (uintptr_t)safe_syscall_end) {
-- 
2.33.1

Re: [PATCH] linux-user: Replace __u64 with uint64_t

Posted by Philippe Mathieu-Daudé 2 years, 6 months ago

On 11/8/21 20:42, Khem Raj wrote:
> uint64_t is available in all userspaces via compiler include stdint.h
> therefore use it instead of __u64 which is linux internal type, it fixes
> build on some platforms eg. aarch64 systems using musl C library
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  linux-user/host/aarch64/hostdep.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
> index a8d41a21ad..34d934f665 100644
> --- a/linux-user/host/aarch64/hostdep.h
> +++ b/linux-user/host/aarch64/hostdep.h
> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
>  static inline void rewind_if_in_safe_syscall(void *puc)
>  {
>      ucontext_t *uc = puc;
> -    __u64 *pcreg = &uc->uc_mcontext.pc;
> +    uint64_t *pcreg = &uc->uc_mcontext.pc;
>  
>      if (*pcreg > (uintptr_t)safe_syscall_start
>          && *pcreg < (uintptr_t)safe_syscall_end) {
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH] linux-user: Replace __u64 with uint64_t

Posted by Warner Losh 2 years, 6 months ago


> On Nov 8, 2021, at 12:42 PM, Khem Raj <raj.khem@gmail.com> wrote:
> 
> uint64_t is available in all userspaces via compiler include stdint.h
> therefore use it instead of __u64 which is linux internal type, it fixes
> build on some platforms eg. aarch64 systems using musl C library
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
> linux-user/host/aarch64/hostdep.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
> index a8d41a21ad..34d934f665 100644
> --- a/linux-user/host/aarch64/hostdep.h
> +++ b/linux-user/host/aarch64/hostdep.h
> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
> static inline void rewind_if_in_safe_syscall(void *puc)
> {
>     ucontext_t *uc = puc;
> -    __u64 *pcreg = &uc->uc_mcontext.pc;
> +    uint64_t *pcreg = &uc->uc_mcontext.pc;
> 
>     if (*pcreg > (uintptr_t)safe_syscall_start
>         && *pcreg < (uintptr_t)safe_syscall_end) {

Reviewed-by: Warner Losh <imp@bsdimp.com>

I wonder why we don’t have a typedef like bed’s register_t though…

Re: [PATCH] linux-user: Replace __u64 with uint64_t

Posted by Richard Henderson 2 years, 6 months ago

On 11/8/21 8:42 PM, Khem Raj wrote:
> uint64_t is available in all userspaces via compiler include stdint.h
> therefore use it instead of __u64 which is linux internal type, it fixes
> build on some platforms eg. aarch64 systems using musl C library
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>   linux-user/host/aarch64/hostdep.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
> index a8d41a21ad..34d934f665 100644
> --- a/linux-user/host/aarch64/hostdep.h
> +++ b/linux-user/host/aarch64/hostdep.h
> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
>   static inline void rewind_if_in_safe_syscall(void *puc)
>   {
>       ucontext_t *uc = puc;
> -    __u64 *pcreg = &uc->uc_mcontext.pc;
> +    uint64_t *pcreg = &uc->uc_mcontext.pc;
>   
>       if (*pcreg > (uintptr_t)safe_syscall_start
>           && *pcreg < (uintptr_t)safe_syscall_end) {

Queued to tcg-next, since that's where the original patch came from.

r~

Re: [PATCH] linux-user: Replace __u64 with uint64_t

Posted by Richard Henderson 2 years, 6 months ago

On 11/10/21 10:25 AM, Richard Henderson wrote:
> On 11/8/21 8:42 PM, Khem Raj wrote:
>> uint64_t is available in all userspaces via compiler include stdint.h
>> therefore use it instead of __u64 which is linux internal type, it fixes
>> build on some platforms eg. aarch64 systems using musl C library
>>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> ---
>>   linux-user/host/aarch64/hostdep.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
>> index a8d41a21ad..34d934f665 100644
>> --- a/linux-user/host/aarch64/hostdep.h
>> +++ b/linux-user/host/aarch64/hostdep.h
>> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
>>   static inline void rewind_if_in_safe_syscall(void *puc)
>>   {
>>       ucontext_t *uc = puc;
>> -    __u64 *pcreg = &uc->uc_mcontext.pc;
>> +    uint64_t *pcreg = &uc->uc_mcontext.pc;
>>       if (*pcreg > (uintptr_t)safe_syscall_start
>>           && *pcreg < (uintptr_t)safe_syscall_end) {
> 
> Queued to tcg-next, since that's where the original patch came from.

Dequeued.  We need something more, because on debian we now get:

../linux-user/host/aarch64/hostdep.h:28:23: error: initialization of 'uint64_t *' {aka 
'long unsigned int *'} from incompatible pointer type 'long long unsigned int *' 
[-Werror=incompatible-pointer-types]
      uint64_t *pcreg = &uc->uc_mcontext.pc;
                        ^
cc1: all warnings being treated as errors

Perhaps we need to use __typeof() to get the correct pointer type?  Oh for c++11 auto 
declarations... :-)


r~

Re: [PATCH] linux-user: Replace __u64 with uint64_t

Posted by Richard Henderson 2 years, 6 months ago

On 11/10/21 4:12 PM, Richard Henderson wrote:
> On 11/10/21 10:25 AM, Richard Henderson wrote:
>> On 11/8/21 8:42 PM, Khem Raj wrote:
>>> uint64_t is available in all userspaces via compiler include stdint.h
>>> therefore use it instead of __u64 which is linux internal type, it fixes
>>> build on some platforms eg. aarch64 systems using musl C library
>>>
>>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>>> ---
>>>   linux-user/host/aarch64/hostdep.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
>>> index a8d41a21ad..34d934f665 100644
>>> --- a/linux-user/host/aarch64/hostdep.h
>>> +++ b/linux-user/host/aarch64/hostdep.h
>>> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
>>>   static inline void rewind_if_in_safe_syscall(void *puc)
>>>   {
>>>       ucontext_t *uc = puc;
>>> -    __u64 *pcreg = &uc->uc_mcontext.pc;
>>> +    uint64_t *pcreg = &uc->uc_mcontext.pc;
>>>       if (*pcreg > (uintptr_t)safe_syscall_start
>>>           && *pcreg < (uintptr_t)safe_syscall_end) {
>>
>> Queued to tcg-next, since that's where the original patch came from.
> 
> Dequeued.  We need something more, because on debian we now get:
> 
> ../linux-user/host/aarch64/hostdep.h:28:23: error: initialization of 'uint64_t *' {aka 
> 'long unsigned int *'} from incompatible pointer type 'long long unsigned int *' 
> [-Werror=incompatible-pointer-types]
>       uint64_t *pcreg = &uc->uc_mcontext.pc;
>                         ^
> cc1: all warnings being treated as errors
> 
> Perhaps we need to use __typeof() to get the correct pointer type?  Oh for c++11 auto 
> declarations... :-)

No, we need to not use a pointer at all and grab Warner's

   https://patchew.org/QEMU/20211108023738.42125-1-imp@bsdimp.com/

Even grabbing the first two patches of that series for 6.2 would fix this.


r~

Re: [PATCH] linux-user: Replace __u64 with uint64_t

Posted by Warner Losh 2 years, 6 months ago

On Wed, Nov 10, 2021 at 8:19 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 11/10/21 4:12 PM, Richard Henderson wrote:
> > On 11/10/21 10:25 AM, Richard Henderson wrote:
> >> On 11/8/21 8:42 PM, Khem Raj wrote:
> >>> uint64_t is available in all userspaces via compiler include stdint.h
> >>> therefore use it instead of __u64 which is linux internal type, it
> fixes
> >>> build on some platforms eg. aarch64 systems using musl C library
> >>>
> >>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> >>> ---
> >>>   linux-user/host/aarch64/hostdep.h | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/linux-user/host/aarch64/hostdep.h
> b/linux-user/host/aarch64/hostdep.h
> >>> index a8d41a21ad..34d934f665 100644
> >>> --- a/linux-user/host/aarch64/hostdep.h
> >>> +++ b/linux-user/host/aarch64/hostdep.h
> >>> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
> >>>   static inline void rewind_if_in_safe_syscall(void *puc)
> >>>   {
> >>>       ucontext_t *uc = puc;
> >>> -    __u64 *pcreg = &uc->uc_mcontext.pc;
> >>> +    uint64_t *pcreg = &uc->uc_mcontext.pc;
> >>>       if (*pcreg > (uintptr_t)safe_syscall_start
> >>>           && *pcreg < (uintptr_t)safe_syscall_end) {
> >>
> >> Queued to tcg-next, since that's where the original patch came from.
> >
> > Dequeued.  We need something more, because on debian we now get:
> >
> > ../linux-user/host/aarch64/hostdep.h:28:23: error: initialization of
> 'uint64_t *' {aka
> > 'long unsigned int *'} from incompatible pointer type 'long long
> unsigned int *'
> > [-Werror=incompatible-pointer-types]
> >       uint64_t *pcreg = &uc->uc_mcontext.pc;
> >                         ^
> > cc1: all warnings being treated as errors
> >
> > Perhaps we need to use __typeof() to get the correct pointer type?  Oh
> for c++11 auto
> > declarations... :-)
>
> No, we need to not use a pointer at all and grab Warner's
>
>    https://patchew.org/QEMU/20211108023738.42125-1-imp@bsdimp.com/
>
> Even grabbing the first two patches of that series for 6.2 would fix this.
>

I tried to do a pointer for this stuff, only to find several places where
there
were mismatches like's reported there.

I also have an improvement to that series I was going to send later today
that fixes the check-patch warning by moving some externs around.  I was
going to do that after looking through my inbox to see if there'd been more
comments or not. I'll make sure the relevant fixes are first for this issue.

Warner

Re: [PATCH] linux-user: Replace __u64 with uint64_t

Posted by Khem Raj 2 years, 6 months ago

On Wed, Nov 10, 2021 at 8:03 AM Warner Losh <imp@bsdimp.com> wrote:
>
>
>
> On Wed, Nov 10, 2021 at 8:19 AM Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> On 11/10/21 4:12 PM, Richard Henderson wrote:
>> > On 11/10/21 10:25 AM, Richard Henderson wrote:
>> >> On 11/8/21 8:42 PM, Khem Raj wrote:
>> >>> uint64_t is available in all userspaces via compiler include stdint.h
>> >>> therefore use it instead of __u64 which is linux internal type, it fixes
>> >>> build on some platforms eg. aarch64 systems using musl C library
>> >>>
>> >>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> >>> ---
>> >>>   linux-user/host/aarch64/hostdep.h | 2 +-
>> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
>> >>> index a8d41a21ad..34d934f665 100644
>> >>> --- a/linux-user/host/aarch64/hostdep.h
>> >>> +++ b/linux-user/host/aarch64/hostdep.h
>> >>> @@ -25,7 +25,7 @@ extern char safe_syscall_end[];
>> >>>   static inline void rewind_if_in_safe_syscall(void *puc)
>> >>>   {
>> >>>       ucontext_t *uc = puc;
>> >>> -    __u64 *pcreg = &uc->uc_mcontext.pc;
>> >>> +    uint64_t *pcreg = &uc->uc_mcontext.pc;
>> >>>       if (*pcreg > (uintptr_t)safe_syscall_start
>> >>>           && *pcreg < (uintptr_t)safe_syscall_end) {
>> >>
>> >> Queued to tcg-next, since that's where the original patch came from.
>> >
>> > Dequeued.  We need something more, because on debian we now get:
>> >
>> > ../linux-user/host/aarch64/hostdep.h:28:23: error: initialization of 'uint64_t *' {aka
>> > 'long unsigned int *'} from incompatible pointer type 'long long unsigned int *'
>> > [-Werror=incompatible-pointer-types]
>> >       uint64_t *pcreg = &uc->uc_mcontext.pc;
>> >                         ^
>> > cc1: all warnings being treated as errors
>> >
>> > Perhaps we need to use __typeof() to get the correct pointer type?  Oh for c++11 auto
>> > declarations... :-)
>>
>> No, we need to not use a pointer at all and grab Warner's
>>
>>    https://patchew.org/QEMU/20211108023738.42125-1-imp@bsdimp.com/
>>
>> Even grabbing the first two patches of that series for 6.2 would fix this.
>
>
> I tried to do a pointer for this stuff, only to find several places where there
> were mismatches like's reported there.
>
> I also have an improvement to that series I was going to send later today
> that fixes the check-patch warning by moving some externs around.  I was
> going to do that after looking through my inbox to see if there'd been more
> comments or not. I'll make sure the relevant fixes are first for this issue.

Thanks Warner, if you are fixing it in your series, then we should
drop this.  Should we
use uintptr_t or something

>
> Warner