[PATCH v2 07/18] RISC-V: nptl: update default pthread-offsets.h

[Adhemerval Zanella]

On 08/07/2020 21:14, Maciej W. Rozycki via Libc-alpha wrote:
> On Wed, 3 Jun 2020, Alistair Francis via Libc-alpha wrote:
> 
>> diff --git a/sysdeps/riscv/nptl/pthread-offsets.h b/sysdeps/riscv/nptl/pthread-offsets.h
>> index 31f0587bec..a85c752a1f 100644
>> --- a/sysdeps/riscv/nptl/pthread-offsets.h
>> +++ b/sysdeps/riscv/nptl/pthread-offsets.h
>> @@ -1,3 +1,12 @@
>> -#define __PTHREAD_MUTEX_KIND_OFFSET		16
>> +#if __WORDSIZE == 64
>> +# define __PTHREAD_MUTEX_KIND_OFFSET           16
>> +#else
>> +# define __PTHREAD_MUTEX_KIND_OFFSET           12
>> +#endif
> 
>  OK, we have:
> 
> typedef union
> {
>   struct __pthread_mutex_s __data;
>   char __size[__SIZEOF_PTHREAD_MUTEX_T];
>   long int __align;
> } pthread_mutex_t;
> 
> and then:
> 
> struct __pthread_mutex_s
> {
>   int __lock __LOCK_ALIGNMENT;
>   unsigned int __count;
>   int __owner;
> #if __WORDSIZE == 64
>   unsigned int __nusers;
> #endif
>   int __kind;
> #if __WORDSIZE != 64
>   unsigned int __nusers;
> #endif
> 
> which means the `__nusers' and `__kind' members are swapped between 64-bit 
> and 32-bit hosts.  Which I find kind of weird (what for?), but the offset 
> of `__kind' changes accordingly and the values are correct.

The default __pthread_mutex_s was modeled from how other architectures
has done so we use the common ground as default (and avoid having to
create even more arch specific struct_mutex.h).  I don't exactly why
each port has used this specific layout though.

> 
>> -#define __PTHREAD_RWLOCK_FLAGS_OFFSET		48
>> +
>> +#if __WORDSIZE == 64
>> +# define __PTHREAD_RWLOCK_FLAGS_OFFSET		48
>> +#else
>> +# define __PTHREAD_RWLOCK_FLAGS_OFFSET          24
>> +#endif
> 
>  Likewise (as from 09/18):
> 
> typedef union
> {
>   struct __pthread_rwlock_arch_t __data;
>   char __size[__SIZEOF_PTHREAD_RWLOCK_T];
>   long int __align;
> } pthread_rwlock_t;
> 
> and:
> 
> struct __pthread_rwlock_arch_t
> {
>   unsigned int __readers;
>   unsigned int __writers;
>   unsigned int __wrphase_futex;
>   unsigned int __writers_futex;
>   unsigned int __pad3;
>   unsigned int __pad4;
> #if __riscv_xlen == 64
>   int __cur_writer;
>   int __shared;
>   unsigned long int __pad1;
>   unsigned long int __pad2;
>   unsigned int __flags;
> #else
> # if __BYTE_ORDER == __BIG_ENDIAN
>   unsigned char __pad1;
>   unsigned char __pad2;
>   unsigned char __shared;
>   unsigned char __flags;
> # else
>   unsigned char __flags;
>   unsigned char __shared;
>   unsigned char __pad1;
>   unsigned char __pad2;
> # endif
>   int __cur_writer;
> #endif
> };
> 
> so here we have 48 for RV64, 24 for RV32/LE, and 27 for RV/BE, meaning 
> that your change is wrong.  Please fix that.

This raises a flags because the wrong __PTHREAD_RWLOCK_FLAGS_OFFSET should 
generate a build failure at pthread_rwlock_init.c.

> 
>  Also I think this change makes no sense at this point or indeed on its 
> own as there's been no RV32 support added to <bits/struct_rwlock.h> as at 
> this commit yet, meaning that the offsets would become inconsistent.  
> Please fold it into 09/18.
> 
>   Maciej
>