[PATCH v4 04/21] nptl: x32: Fix Race conditions in pthread cancellation [BZ#12683]

[Adhemerval Zanella]

On 07/04/2020 10:59, H.J. Lu wrote:
> On Tue, Apr 7, 2020 at 6:55 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 07/04/2020 10:41, H.J. Lu wrote:
>>> On Tue, Apr 7, 2020 at 6:40 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>
>>>> On Tue, Apr 7, 2020 at 6:33 AM Adhemerval Zanella
>>>> <adhemerval.zanella@linaro.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 07/04/2020 09:54, H.J. Lu wrote:
>>>>>> On Tue, Apr 7, 2020 at 5:47 AM Adhemerval Zanella via Libc-alpha
>>>>>> <libc-alpha@sourceware.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 03/04/2020 18:22, Joseph Myers wrote:
>>>>>>>> On Fri, 3 Apr 2020, Adhemerval Zanella via Libc-alpha wrote:
>>>>>>>>
>>>>>>>>> This patches adds the x32 modification required for the BZ#12683.
>>>>>>>>> It follows the x86_64-x32 ABI and pointers are zero-extended.
>>>>>>>>> However, compiler may not see such cases and accuse a cast from pointer
>>>>>>>>> to integer of different size and for such cases the warning is
>>>>>>>>> explict disabled.
>>>>>>>>
>>>>>>>> MIPS n32 uses an intermediate cast to (__typeof__ ((X) - (X))), so that a
>>>>>>>> conversion to a different size is never directly from a pointer type.
>>>>>>>> Does something like that help here to avoid the warning without needing to
>>>>>>>> use diagnostic pragmas?
>>>>>>>
>>>>>>> The intermediate cast to (__typeof__ ((X) - (X))) is not suffice for x32
>>>>>>> (the resulting argumetn it will passed as function argument instead of
>>>>>>> asm input).  I have replaced with:
>>>>>>>
>>>>>>>   #define __SSC(__x)                                              \
>>>>>>>   ({                                                            \
>>>>>>>     __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8       \
>>>>>>>       ? (unsigned long int) (uintptr_t)(__x)                    \
>>>>>>>       : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x);   \
>>>>>>>     __arg;                                                      \
>>>>>>>   })
>>>>>>>
>>>>>>
>>>>>> Have you looked at libc-pointer-arith.h?
>>>>>
>>>>> That was my first approach, by using cast_to_integer macro. I tried to
>>>>> change its internals to zero extend pointers correctly, but I couldn't
>>>>> find a easier way without also adding the cast point suppression
>>>>> warning in this original patch.
>>>>
>>>> Have you looked at sysdeps/unix/sysv/linux/x86_64/sysdep.h?
>>>>
>>>
>>> /* Create a variable 'name' based on type 'X' to avoid explicit types.
>>>    This is mainly used set use 64-bits arguments in x32.   */
>>> #define TYPEFY(X, name) __typeof__ ((X) - (X)) name
>>> /* Explicit cast the argument to avoid integer from pointer warning on
>>>    x32.  */
>>> #define ARGIFY(X) ((__typeof__ ((X) - (X))) (X))
>>>
>>
>> Yes, I was the one that actually added them (78ca091cdd2). But for this
>> case, the issue is it requires another implicit cast on the
>> __syscall_cancel call itself.  The difference here is the call is not
>> done through an asm input anymore.
> 
> Do you have a git branch I can try?
> 

Yes, azanella/bz12683 [1]. I just has it rebased against master.

[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/bz12683