diff mbox series

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

Message ID 20200403203201.7494-5-adhemerval.zanella@linaro.org
State Superseded
Headers
Series nptl: Fix Race conditions in pthread cancellation [BZ#12683] |

Commit Message

Adhemerval Zanella Netto April 3, 2020, 8:31 p.m. UTC 
  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.

Checked on x86_64-linux-gnu-x32.
---
 .../sysv/linux/x86_64/x32/syscall_types.h     | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/syscall_types.h

Comments

Joseph Myers April 3, 2020, 9:22 p.m. UTC | #1 
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?
Adhemerval Zanella Netto April 7, 2020, 12:47 p.m. UTC | #2 
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;                                                      \
  })
H.J. Lu April 7, 2020, 12:54 p.m. UTC | #3 
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?
Adhemerval Zanella Netto April 7, 2020, 1:33 p.m. UTC | #4 
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.
H.J. Lu April 7, 2020, 1:40 p.m. UTC | #5 
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?
H.J. Lu April 7, 2020, 1:41 p.m. UTC | #6 
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))
Adhemerval Zanella Netto April 7, 2020, 1:55 p.m. UTC | #7 
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.
H.J. Lu April 7, 2020, 1:59 p.m. UTC | #8 
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?
Adhemerval Zanella Netto April 7, 2020, 2:04 p.m. UTC | #9 
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
H.J. Lu April 7, 2020, 3:45 p.m. UTC | #10 
On Tue, Apr 7, 2020 at 7:04 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> 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

How about these 2 patches?
Adhemerval Zanella Netto April 7, 2020, 4:16 p.m. UTC | #11 
On 07/04/2020 12:45, H.J. Lu wrote:
> On Tue, Apr 7, 2020 at 7:04 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> 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
> 
> How about these 2 patches?
> 

OK, I would assume libc-pointer-arith.h is a preferable solution.
Coincidently it is essentially the same strategy I used sometime
ago [1], but I don't recall exactly why I have abandoned it.

I have used you suggestion, thanks.

[1] https://sourceware.org/legacy-ml/libc-alpha/2017-12/msg00315.html
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/syscall_types.h b/sysdeps/unix/sysv/linux/x86_64/x32/syscall_types.h
new file mode 100644
index 0000000000..25b10a0b28
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/syscall_types.h
@@ -0,0 +1,40 @@ 
+/* Types and macros used for syscall issuing.  x86_64/x32 version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SYSCALL_TYPES_H
+#define _SYSCALL_TYPES_H
+
+#include <libc-diag.h>
+
+typedef long long int __syscall_arg_t;
+
+/* Syscall arguments for x32 follows x86_64 ABI, however pointers are 32 bits
+   should be zero extended.  However compiler may not see such cases and
+   accuse a cast from pointer to integer of different size.  */
+#define __SSC(__x)						\
+  ({								\
+    DIAG_PUSH_NEEDS_COMMENT;					\
+    DIAG_IGNORE_NEEDS_COMMENT (5, "-Wpointer-to-int-cast");	\
+    __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8	\
+			    ? (unsigned long int) (__x) 	\
+			    : (long long int) ((__x));		\
+    DIAG_POP_NEEDS_COMMENT;					\
+    __arg;							\
+  })
+
+#endif