Message ID | 20200403203201.7494-5-adhemerval.zanella@linaro.org |
---|---|
State | Superseded |
Headers |
Return-Path: <adhemerval.zanella@linaro.org> X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x743.google.com (mail-qk1-x743.google.com [IPv6:2607:f8b0:4864:20::743]) by sourceware.org (Postfix) with ESMTPS id 2142F385DC32 for <libc-alpha@sourceware.org>; Fri, 3 Apr 2020 20:32:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2142F385DC32 Received: by mail-qk1-x743.google.com with SMTP id o18so6696243qko.12 for <libc-alpha@sourceware.org>; Fri, 03 Apr 2020 13:32:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=ppZzckekZfNYt2NfRARHaNZ1L2FJ0Cwf9d/NoX2RkRI=; b=XLSLUq7rKiUc6FLy7ObUssWSRSNDPZQZg7PAftasTrX7jszavhCsE/KNj8YmtUzFtq x5niisBu0QNHXU4J4EbkHdpXnLdCbTJjpZ0cg24seDzDqFiRCIRgTpjjr7U0dxAW/oPa gL5lxas1VnLxsZRDRJICUtcpMhioxKBGnHNW7r0P7SmN/SslNe3P7AVcGxNmtaqBGnKc CguesKkis2n25WRKsxxyI1HEQK5BC5QaGB2+QQmBlPlANHwfptuUfJOv/re9g11FoKRX bixPTBDfZ2jfEC0m53qHLLkbqfj2eSI69qoDHxIW8b3Oje0LIlMuhBA4UZw+ThVWGmz+ VQYA== X-Gm-Message-State: AGi0PuZUS0f4CyA/tBhKjTOyrdrrZXhIGGTotD49ZgkVRdaUyKqGFOlU W1qI6zI7MczHVvpvcMXk31qXBPlvjjc= X-Google-Smtp-Source: APiQypKNU+QB6dI22hQfDbKqhKmHNagyCOq+Li+y7BRmfMnzJH++Ydr+RiALlmREFCEglk/2Q8A4GQ== X-Received: by 2002:a05:620a:127c:: with SMTP id b28mr10992470qkl.317.1585945934475; Fri, 03 Apr 2020 13:32:14 -0700 (PDT) Received: from localhost.localdomain ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id d141sm7063535qke.68.2020.04.03.13.32.13 for <libc-alpha@sourceware.org> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Apr 2020 13:32:13 -0700 (PDT) From: Adhemerval Zanella <adhemerval.zanella@linaro.org> To: libc-alpha@sourceware.org Subject: [PATCH v4 04/21] nptl: x32: Fix Race conditions in pthread cancellation [BZ#12683] Date: Fri, 3 Apr 2020 17:31:44 -0300 Message-Id: <20200403203201.7494-5-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200403203201.7494-1-adhemerval.zanella@linaro.org> References: <20200403203201.7494-1-adhemerval.zanella@linaro.org> X-Spam-Status: No, score=-26.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <http://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <http://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> X-List-Received-Date: Fri, 03 Apr 2020 20:32:16 -0000 |
Series |
nptl: Fix Race conditions in pthread cancellation [BZ#12683]
|
|
Commit Message
Adhemerval Zanella
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
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?
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; \ })
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?
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.
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?
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))
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.
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?
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
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?
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 --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