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

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

Commit Message

Sergei Trofimovich via Libc-alpha April 3, 2020, 8:31 p.m.
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

-- 
2.17.1

Comments

Joseph Myers April 3, 2020, 9:22 p.m. | #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?

-- 
Joseph S. Myers
joseph@codesourcery.com
Sergei Trofimovich via Libc-alpha April 7, 2020, 12:47 p.m. | #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;                                                      \
  })
Sergei Trofimovich via Libc-alpha April 7, 2020, 12:54 p.m. | #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?

-- 
H.J.
Sergei Trofimovich via Libc-alpha April 7, 2020, 1:33 p.m. | #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.
Sergei Trofimovich via Libc-alpha April 7, 2020, 1:40 p.m. | #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.
Sergei Trofimovich via Libc-alpha April 7, 2020, 1:41 p.m. | #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))

-- 
H.J.
Sergei Trofimovich via Libc-alpha April 7, 2020, 1:55 p.m. | #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.
Sergei Trofimovich via Libc-alpha April 7, 2020, 1:59 p.m. | #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?

-- 
H.J.
Sergei Trofimovich via Libc-alpha April 7, 2020, 2:04 p.m. | #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
Sergei Trofimovich via Libc-alpha April 7, 2020, 3:45 p.m. | #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?

-- 
H.J.
From c00e3e1c3dac4bc4cdf70029f1fc2a2f08f133cc Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 7 Apr 2020 08:40:56 -0700
Subject: [PATCH 1/2] Add cast_to_unsigned_integer

Cast an integer or a pointer VAL to unsigned integer with proper type.
---
 include/libc-pointer-arith.h | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/libc-pointer-arith.h b/include/libc-pointer-arith.h
index b4d716c727..3468c2f141 100644
--- a/include/libc-pointer-arith.h
+++ b/include/libc-pointer-arith.h
@@ -24,19 +24,31 @@
 /* 1 if 'type' is a pointer type, 0 otherwise.  */
 # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5)
 
-/* intptr_t if P is true, or T if P is false.  */
-# define __integer_if_pointer_type_sub(T, P) \
+/* INTPTR_T if P is true, or T if P is false.  */
+# define __integer_if_pointer_type_sub(T, P, INTPTR_T) \
   __typeof__ (*(0 ? (__typeof__ (0 ? (T *) 0 : (void *) (P))) 0 \
-		  : (__typeof__ (0 ? (intptr_t *) 0 : (void *) (!(P)))) 0))
+		  : (__typeof__ (0 ? (INTPTR_T *) 0 : (void *) (!(P)))) 0))
 
 /* intptr_t if EXPR has a pointer type, or the type of EXPR otherwise.  */
 # define __integer_if_pointer_type(expr) \
   __integer_if_pointer_type_sub(__typeof__ ((__typeof__ (expr)) 0), \
-				__pointer_type (__typeof__ (expr)))
+				__pointer_type (__typeof__ (expr)), \
+				intptr_t)
+
+/* uintptr_t if EXPR has a pointer type, or the type of EXPR otherwise.  */
+# define __unsigned_integer_if_pointer_type(expr) \
+  __integer_if_pointer_type_sub(__typeof__ ((__typeof__ (expr)) 0), \
+				__pointer_type (__typeof__ (expr)), \
+				uintptr_t)
 
 /* Cast an integer or a pointer VAL to integer with proper type.  */
 # define cast_to_integer(val) ((__integer_if_pointer_type (val)) (val))
 
+/* Cast an integer or a pointer VAL to unsigned integer with proper
+   type.  */
+# define cast_to_unsigned_integer(val) \
+  ((__unsigned_integer_if_pointer_type (val)) (val))
+
 /* Align a value by rounding down to closest size.
    e.g. Using size of 4096, we get this behavior:
 	{4095, 4096, 4097} = {0, 4096, 4096}.  */
Sergei Trofimovich via Libc-alpha April 7, 2020, 4:16 p.m. | #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

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