From patchwork Thu May 27 17:28:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484739 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=rTYImJvZ; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZY46v7hz9sWc for ; Fri, 28 May 2021 03:28:40 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5B3333838028; Thu, 27 May 2021 17:28:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5B3333838028 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136512; bh=QrWL8+Jha5S+hivK/rQ/vEGUvn9Axia3707mTCCDctI=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=rTYImJvZiBK4muTYs/PUgMubSqAE4FEFs5eQdxNmoYFCxYTtGjzBTWn8xzMmgVjsL 4X3VnxiI1GaBm/pW8zXrks++efAxFqxUxjohWBg5bqX23npDep25F1+jJbA5WWGfyn OT9PX5bB4g95dXuWqw2cMD9yZX4a5pVBgJOACG5Y= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x731.google.com (mail-qk1-x731.google.com [IPv6:2607:f8b0:4864:20::731]) by sourceware.org (Postfix) with ESMTPS id C23443839C56 for ; Thu, 27 May 2021 17:28:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C23443839C56 Received: by mail-qk1-x731.google.com with SMTP id o27so1406068qkj.9 for ; Thu, 27 May 2021 10:28:29 -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:mime-version:content-transfer-encoding; bh=QrWL8+Jha5S+hivK/rQ/vEGUvn9Axia3707mTCCDctI=; b=ERObR1nllloRO2T4oFntIGii5QajOdGPfwxTjYNUTUL3Fu5op/sTjTVJmMmTUfU1hQ Q/ZFArawWuCkSq/fWJsFchORN3jIqBapf6J6uYlr6LMOwUhm3QHWNsKTL8J9TDVkj5uu 1pQHKKNq3g7emk+cmP+KAmN+tRRetxt0m/TI1FQ+rINWLB7SFin73h47LJxT4b3PdgUB A/Fxt6cimIeErwnXsqnTKbPsB/wU07i4Mn+Up0mTHG99+J0ZdGgre7mx50FBJW+jD/kE xCE7I1QaAWzybh4kPgsRMAPw2g6AVKbPZeqQGBa/1dPnwGDYvI1qXFDjDnlyOYjXeqHZ /aLw== X-Gm-Message-State: AOAM530ABi+7w5rPg5WuzC9bEIO6ueXLbfGGpqE8XIzLslNJz23EcVNc djH6jtqih6clzL0N2rOgfWnwIUT/6+FTgg== X-Google-Smtp-Source: ABdhPJwYY1+NhOUtDTj+bznRf470+aTgoWIUkxQMjp1HiAk1HmCUz57saMmL8rbU2GmJtkEUmX4QEA== X-Received: by 2002:a05:620a:1667:: with SMTP id d7mr4676258qko.462.1622136509121; Thu, 27 May 2021 10:28:29 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.28 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:28 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 1/9] nptl: Remove exit-thread.h Date: Thu, 27 May 2021 14:28:15 -0300 Message-Id: <20210527172823.3461314-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" No function change. The code is used only for Linux, besides being included in generic code. --- csu/libc-start.c | 1 - nptl/pthread_create.c | 6 +++-- sysdeps/generic/exit-thread.h | 28 -------------------- sysdeps/nptl/libc_start_call_main.h | 5 +++- sysdeps/unix/sysv/linux/exit-thread.h | 37 --------------------------- 5 files changed, 8 insertions(+), 69 deletions(-) delete mode 100644 sysdeps/generic/exit-thread.h delete mode 100644 sysdeps/unix/sysv/linux/exit-thread.h diff --git a/csu/libc-start.c b/csu/libc-start.c index 8688cba76d..5b5913e7bf 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 2d2535b07d..aacbba1cac 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -575,7 +574,10 @@ start_thread (void *arg) The exit code is zero since in case all threads exit by calling 'pthread_exit' the exit status must be 0 (zero). */ - __exit_thread (); + while (1) + { + INTERNAL_SYSCALL_CALL (exit, 0); + } /* NOTREACHED */ } diff --git a/sysdeps/generic/exit-thread.h b/sysdeps/generic/exit-thread.h deleted file mode 100644 index 676ef553a5..0000000000 --- a/sysdeps/generic/exit-thread.h +++ /dev/null @@ -1,28 +0,0 @@ -/* Call to terminate the current thread. Stub version. - Copyright (C) 2014-2021 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 - . */ - -/* This causes the current thread to exit, without affecting other - threads in the process if there are any. If there are no other - threads left, then this has the effect of _exit (0). */ - -static inline void __attribute__ ((noreturn, always_inline, unused)) -__exit_thread (void) -{ - while (1) - asm ("write me!"); -} diff --git a/sysdeps/nptl/libc_start_call_main.h b/sysdeps/nptl/libc_start_call_main.h index c579c65f78..08b1800e83 100644 --- a/sysdeps/nptl/libc_start_call_main.h +++ b/sysdeps/nptl/libc_start_call_main.h @@ -67,7 +67,10 @@ __libc_start_call_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), result = 0; if (! atomic_decrement_and_test (&__nptl_nthreads)) /* Not much left to do but to exit the thread, not the process. */ - __exit_thread (); + while (1) + { + INTERNAL_SYSCALL_CALL (exit, 0); + } } exit (result); diff --git a/sysdeps/unix/sysv/linux/exit-thread.h b/sysdeps/unix/sysv/linux/exit-thread.h deleted file mode 100644 index 9e1b7f3752..0000000000 --- a/sysdeps/unix/sysv/linux/exit-thread.h +++ /dev/null @@ -1,37 +0,0 @@ -/* Call to terminate the current thread. Linux version. - Copyright (C) 2014-2021 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 - . */ - -#include - -/* This causes the current thread to exit, without affecting other - threads in the process if there are any. If there are no other - threads left, then this has the effect of _exit (0). */ - -static inline void __attribute__ ((noreturn, always_inline, unused)) -__exit_thread (void) -{ - /* Doing this in a loop is mostly just to satisfy the compiler that the - function really qualifies as noreturn. It also means that in some - pathological situation where the system call does not get made or does - not work, the thread will simply spin rather than running off the end - of the caller and doing unexpectedly strange things. */ - while (1) - { - INTERNAL_SYSCALL_CALL (exit, 0); - } -} From patchwork Thu May 27 17:28:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484740 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=qDM3WG1m; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZY93LtXz9sW7 for ; Fri, 28 May 2021 03:28:45 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6EE743959E52; Thu, 27 May 2021 17:28:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6EE743959E52 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136514; bh=nEbzpQuPbu3mv3pYXtWNQ9pLfOMpsj9Uoi0vcQek6ZQ=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=qDM3WG1m63U2zLcgRZKHW8lnUzQucCiN3DYYggDVkzX3CwNxBJkjTvlPNsyyoHuCs EEmQy/wgHPRayD81NBMSOKBLNszfN/c60StBXJCLLQuhQCKfd5soiLO79yzJFQYLDs MG81K5iTn13jZLDIOmD7tbKMb5d2IXXrSjtT/VbE= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) by sourceware.org (Postfix) with ESMTPS id 338053844007 for ; Thu, 27 May 2021 17:28:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 338053844007 Received: by mail-qk1-x72b.google.com with SMTP id o3so1421047qke.7 for ; Thu, 27 May 2021 10:28:31 -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:mime-version:content-transfer-encoding; bh=nEbzpQuPbu3mv3pYXtWNQ9pLfOMpsj9Uoi0vcQek6ZQ=; b=XgQgKzQWhJAGAkGznV1qHwPN9UNPVeY4JF/DJRaGmG6uvJqyMcSoa0b/HRgb8KE6iS sZegDGccTlC74he7K0itx/u3gNXeeGMNfA+mU9QyJiT8cEFu5qsNNWXvxDcIsErb0xYG NU/Uc2jpmOVrTlkXYnMT1q3BDaSsfPQjD/9iznyQ39KjzjSoDxIlR6Kwp6uvPTHqCa9y r6yJMQI7/Pkh97QttgHnRdkMMODcj4FtqiRiXogl+LL5stjiwGyT7lqoqY6Wk9SfL8He BBB3VZOIEQUAp9j7mEYu544SMG1MnYwlYldbpPLf3IC9jTiYik+oaOzWrEf7sTj2Kg0k 7QPA== X-Gm-Message-State: AOAM530e7z3i1M9kdVKWmBUHcpM8SSaCwBhx5RfUsyJqUkg89hPzv4IO N0FkGlqsoXZ+qAHe6vT2L4nMzwQjQVXWCw== X-Google-Smtp-Source: ABdhPJzKH9SNnyvKyZyUoX12DS5xC1Z2+fqkp+wRXatKtFy6ohA94dLGDfh7F5okJnQEUdH3UFySiA== X-Received: by 2002:a37:7c02:: with SMTP id x2mr4619750qkc.483.1622136510358; Thu, 27 May 2021 10:28:30 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.29 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:30 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 2/9] nptl: Deallocate the thread stack on setup failure (BZ #19511) Date: Thu, 27 May 2021 14:28:16 -0300 Message-Id: <20210527172823.3461314-3-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" To setup either the thread scheduling parameters or affinity, pthread_create enforce synchronization on created thread wait until its parent either unlock the 'struct pthread' or send a cancellation signal if a failure occurs. However, cancelling the thread does not deallocate the newly created stack since cancellation expects that a pthread_join to deallocate any allocated thread resouces (threads stack or TLS). I used a different strategy than the one proposed on BZ#19511 comment #4. Making the parent responsible for the cleanup requires additional synchronization similar to what pthread_join does. Instead, this patch reassigns an unused 'struct pthread' member, parent_cancelhandling_unsed, to indicate whether the setup has failed and set the thread itself to deallocate the allocated resouces (similar to what detached mode does). This strategy also simplifies by not using thread cancellation and thus not running libgcc_so load in the signal handler (which is avoided in thread cancellation since 'pthread_cancel' is the one responsible to dlopen libgcc_s). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/allocatestack.c | 1 + nptl/descr.h | 5 ++-- nptl/pthread_create.c | 70 ++++++++++++++++++++----------------------- 3 files changed, 36 insertions(+), 40 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index dc81a2ca73..2114bd2e27 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -161,6 +161,7 @@ get_cached_stack (size_t *sizep, void **memp) /* Cancellation handling is back to the default. */ result->cancelhandling = 0; result->cleanup = NULL; + result->setup_failed = 0; /* No pending event. */ result->nextevent = NULL; diff --git a/nptl/descr.h b/nptl/descr.h index 3de9535449..9d8297b45f 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -340,8 +340,9 @@ struct pthread /* True if thread must stop at startup time. */ bool stopped_start; - /* Formerly used for dealing with cancellation. */ - int parent_cancelhandling_unsed; + /* Indicate that a thread creation setup has failed (for instance the + scheduler or affinity). */ + int setup_failed; /* Lock to synchronize access to the descriptor. */ int lock; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index aacbba1cac..018af30c85 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -169,17 +169,15 @@ late_init (void) (c) If the detached thread setup failed and THREAD_RAN is true, then the creating thread releases ownership to the new thread by - sending a cancellation signal. All threads set THREAD_RAN to - true as quickly as possible after returning from the OS kernel's - thread creation routine. + setting the PD->setup_failed 1 and releasing PD->lock. All threads + set THREAD_RAN to true as quickly as possible after returning from + the OS kernel's thread creation routine. (d) If the joinable thread setup failed and THREAD_RAN is true, then then the creating thread retains ownership of PD and must cleanup state. Ownership cannot be released to the process via the return of pthread_create since a non-zero result entails PD is undefined and therefore cannot be joined to free the resources. - We privately call pthread_join on the thread to finish handling - the resource shutdown (Or at least we should, see bug 19511). (e) If the thread creation failed and THREAD_RAN is false, then the creating thread retains ownership of PD and must cleanup state. @@ -239,8 +237,8 @@ late_init (void) The return value is zero for success or an errno code for failure. If the return value is ENOMEM, that will be translated to EAGAIN, so create_thread need not do that. On failure, *THREAD_RAN should - be set to true iff the thread actually started up and then got - canceled before calling user code (*PD->start_routine). */ + be set to true iff the thread actually started up but before calling + the user code (*PD->start_routine). */ static int _Noreturn start_thread (void *arg); @@ -308,35 +306,23 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr, == -1)) return errno; - /* It's started now, so if we fail below, we'll have to cancel it - and let it clean itself up. */ + /* It's started now, so if we fail below, we'll have to let it clean itself + up. */ *thread_ran = true; /* Now we have the possibility to set scheduling parameters etc. */ if (attr != NULL) { - int res; - /* Set the affinity mask if necessary. */ if (need_setaffinity) { assert (*stopped_start); - res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, - attr->extension->cpusetsize, - attr->extension->cpuset); - + int res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, + attr->extension->cpusetsize, + attr->extension->cpuset); if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - err_out: - { - /* The operation failed. We have to kill the thread. - We let the normal cancellation mechanism do the work. */ - - pid_t pid = __getpid (); - INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); - - return INTERNAL_SYSCALL_ERRNO (res); - } + return INTERNAL_SYSCALL_ERRNO (res); } /* Set the scheduling parameters. */ @@ -344,11 +330,10 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr, { assert (*stopped_start); - res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, - pd->schedpolicy, &pd->schedparam); - + int res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, + pd->schedpolicy, &pd->schedparam); if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - goto err_out; + return INTERNAL_SYSCALL_ERRNO (res); } } @@ -424,17 +409,22 @@ start_thread (void *arg) have ownership (see CONCURRENCY NOTES above). */ if (__glibc_unlikely (pd->stopped_start)) { - int oldtype = LIBC_CANCEL_ASYNC (); - /* Get the lock the parent locked to force synchronization. */ lll_lock (pd->lock, LLL_PRIVATE); /* We have ownership of PD now. */ + if (pd->setup_failed == 1) + { + /* In the case of a failed setup, the created thread will be + responsible to free up the allocated resources. The + detached mode ensure it won't be joined and it will trigger + the required cleanup. */ + pd->joinid = pd; + __do_cancel (); + } /* And give it up right away. */ lll_unlock (pd->lock, LLL_PRIVATE); - - LIBC_CANCEL_RESET (oldtype); } LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg); @@ -561,7 +551,7 @@ start_thread (void *arg) pd->setxid_futex = 0; } - /* If the thread is detached free the TCB. */ + /* If the thread is detached or an setup error occurred, free the TCB. */ if (IS_DETACHED (pd)) /* Free the TCB. */ __nptl_free_tcb (pd); @@ -761,7 +751,6 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, signal mask of this thread, so save it in the startup information. */ pd->sigmask = original_sigmask; - /* Reset the cancellation signal mask in case this thread is running cancellation. */ __sigdelset (&pd->sigmask, SIGCANCEL); @@ -820,9 +809,14 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, CONCURRENCY NOTES above). We can assert that STOPPED_START must have been true because thread creation didn't fail, but thread attribute setting did. */ - /* See bug 19511 which explains why doing nothing here is a - resource leak for a joinable thread. */ - assert (stopped_start); + { + assert (stopped_start); + /* State (a), we own PD. The thread blocked on this lock will + finish due setup_failed being set. The thread will be + responsible to cleanup any allocated resource. */ + pd->setup_failed = 1; + lll_unlock (pd->lock, LLL_PRIVATE); + } else { /* State (e) and we have ownership of PD (see CONCURRENCY From patchwork Thu May 27 17:28:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484741 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=vZC3nN6+; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZYF6pfwz9sRf for ; Fri, 28 May 2021 03:28:49 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 000C73959CB5; Thu, 27 May 2021 17:28:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 000C73959CB5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136517; bh=1rDskCFxJOY6TQeexJoMdFMOBPZaft3uvIXkXagLwhA=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=vZC3nN6+vj2GcFdZj1qUbzCvYmFUZE1TmfDzWR1VY70e7Fl95oNuGIyoXSkhXyqsG oWc8n43YgYZ+p1QQUKRnmUN/cAhs+FLnLGeT7NccQ8TZWF4NV0eR8BOVNx9mFb/pF2 SGFv+ZzLwHXPepV5l+iBslIIeULD+oSnGgCdSdgw= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x82f.google.com (mail-qt1-x82f.google.com [IPv6:2607:f8b0:4864:20::82f]) by sourceware.org (Postfix) with ESMTPS id 5E9D43959CB3 for ; Thu, 27 May 2021 17:28:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5E9D43959CB3 Received: by mail-qt1-x82f.google.com with SMTP id m13so754060qtk.13 for ; Thu, 27 May 2021 10:28:32 -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:mime-version:content-transfer-encoding; bh=1rDskCFxJOY6TQeexJoMdFMOBPZaft3uvIXkXagLwhA=; b=dtwdiB0Ku37aVu+ADQs1q3QlNoS3CkPKsYVaUGGWu0IMZDGXm+USwf6KnG3Im8o/R5 nJ5Jj6+9/FzKchdsbow/Ivrvkb4pQi9Kfg6nDI+B+F74YS0skMOcNZ/E+AxGyJwuUwa5 M1IBSBkMT5N6Kd4TtpXxCaYkcVa0EMEKaiKcfbLaIAiClV6+x1rYweMEKFjUeQiFH3ul 7TpITyi+V+k3A5/BM0ktW5Zi1VV/n+lNXQD4GKbLklZRiaqVfvrbCqkNMDBikcCkA52z dBxD1sH4+pl9zt0LhPGMwNIPQZrUsRCAiKTzdFjYDkxZUMolrKrXhS0dJxopv7XDZLpW sAig== X-Gm-Message-State: AOAM5312v4yFSKyzanVI+IUpcBN79WzXzEiy9a9rtj2C3i3sjBdTVOTH HlPDgT/9NqYtLTFUq4maql3ksHDkEVuJhA== X-Google-Smtp-Source: ABdhPJx3igy44UBJqDbeRIJyej5maMgIITLs9NCaUeUN3qBwM8BVD7TOseABGOVOg8MN7tj/gYzN2Q== X-Received: by 2002:ac8:72d8:: with SMTP id o24mr4071686qtp.321.1622136511692; Thu, 27 May 2021 10:28:31 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.30 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:31 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 3/9] nptl: Install cancellation handler on pthread_cancel Date: Thu, 27 May 2021 14:28:17 -0300 Message-Id: <20210527172823.3461314-4-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that cancellation is not used anymore to handle thread setup creation failure, the sighandle can be installed only when pthread_cancel is actually used. Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/Versions | 3 +-- nptl/pthreadP.h | 6 ------ nptl/pthread_cancel.c | 49 ++++++++++++++++++++++++------------------- nptl/pthread_create.c | 15 ------------- 4 files changed, 28 insertions(+), 45 deletions(-) diff --git a/nptl/Versions b/nptl/Versions index af62a47cca..590761e730 100644 --- a/nptl/Versions +++ b/nptl/Versions @@ -395,7 +395,6 @@ libc { __nptl_free_tcb; __nptl_nthreads; __nptl_setxid_sighandler; - __nptl_sigcancel_handler; __nptl_stack_list_add; __nptl_stack_list_del; __pthread_attr_copy; @@ -514,4 +513,4 @@ ld { __nptl_initial_report_events; __nptl_set_robust_list_avail; } -} \ No newline at end of file +} diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 05f2bae521..48d48e7008 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -569,12 +569,6 @@ libc_hidden_proto (__pthread_attr_setsigmask_internal) extern __typeof (pthread_attr_getsigmask_np) __pthread_attr_getsigmask_np; libc_hidden_proto (__pthread_attr_getsigmask_np) -/* The cancellation signal handler defined alongside with - pthread_cancel. This is included in statically linked binaries - only if pthread_cancel is linked in. */ -void __nptl_sigcancel_handler (int sig, siginfo_t *si, void *ctx); -libc_hidden_proto (__nptl_sigcancel_handler) - /* Special versions which use non-exported functions. */ extern void __pthread_cleanup_push (struct _pthread_cleanup_buffer *buffer, void (*routine) (void *), void *arg); diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 802c691874..deb404600c 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -28,11 +28,19 @@ #include #include -/* For asynchronous cancellation we use a signal. This is the core - logic of the signal handler. */ +/* For asynchronous cancellation we use a signal. */ static void -sigcancel_handler (void) +sigcancel_handler (int sig, siginfo_t *si, void *ctx) { + /* Safety check. It would be possible to call this function for + other signals and send a signal from another process. This is not + correct and might even be a security problem. Try to catch as + many incorrect invocations as possible. */ + if (sig != SIGCANCEL + || si->si_pid != __getpid() + || si->si_code != SI_TKILL) + return; + struct pthread *self = THREAD_SELF; int oldval = THREAD_GETMEM (self, cancelhandling); @@ -66,24 +74,6 @@ sigcancel_handler (void) } } -/* This is the actually installed SIGCANCEL handler. It adds some - safety checks before performing the cancellation. */ -void -__nptl_sigcancel_handler (int sig, siginfo_t *si, void *ctx) -{ - /* Safety check. It would be possible to call this function for - other signals and send a signal from another process. This is not - correct and might even be a security problem. Try to catch as - many incorrect invocations as possible. */ - if (sig != SIGCANCEL - || si->si_pid != __getpid() - || si->si_code != SI_TKILL) - return; - - sigcancel_handler (); -} -libc_hidden_def (__nptl_sigcancel_handler) - int __pthread_cancel (pthread_t th) { @@ -94,6 +84,17 @@ __pthread_cancel (pthread_t th) /* Not a valid thread handle. */ return ESRCH; + static int init_sigcancel = 0; + if (atomic_load_relaxed (&init_sigcancel) == 0) + { + struct sigaction sa; + sa.sa_sigaction = sigcancel_handler; + sa.sa_flags = SA_SIGINFO; + __sigemptyset (&sa.sa_mask); + __libc_sigaction (SIGCANCEL, &sa, NULL); + atomic_store_relaxed (&init_sigcancel, 1); + } + #ifdef SHARED /* Trigger an error if libgcc_s cannot be loaded. */ { @@ -134,7 +135,11 @@ __pthread_cancel (pthread_t th) call pthread_cancel (pthread_self ()) without calling pthread_create, so the signal handler may not have been set up for a self-cancel. */ - sigcancel_handler (); + { + THREAD_SETMEM (pd, result, PTHREAD_CANCELED); + if ((newval & CANCELTYPE_BITMASK) != 0) + __do_cancel (); + } else { /* The cancellation handler will take care of marking the diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 018af30c85..15ce5ad4a1 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -67,21 +67,6 @@ late_init (void) struct sigaction sa; __sigemptyset (&sa.sa_mask); - /* Install the cancellation signal handler (in static builds only if - pthread_cancel has been linked in). If for some reason we cannot - install the handler we do not abort. Maybe we should, but it is - only asynchronous cancellation which is affected. */ -#ifndef SHARED - extern __typeof (__nptl_sigcancel_handler) __nptl_sigcancel_handler - __attribute__ ((weak)); - if (__nptl_sigcancel_handler != NULL) -#endif - { - sa.sa_sigaction = __nptl_sigcancel_handler; - sa.sa_flags = SA_SIGINFO; - (void) __libc_sigaction (SIGCANCEL, &sa, NULL); - } - /* Install the handle to change the threads' uid/gid. Use SA_ONSTACK because the signal may be sent to threads that are running with custom stacks. (This is less likely for From patchwork Thu May 27 17:28:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484742 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=QqQFPxRg; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZYL0DQDz9sRf for ; Fri, 28 May 2021 03:28:54 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 849213959E5C; Thu, 27 May 2021 17:28:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 849213959E5C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136518; bh=4RjSdfkkXWNuS2wPNfzNcl1g5C/zMfBG8GrbBUz8y0A=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=QqQFPxRgosK5prSYSRoJUjwkfUSzTTX65da+uayZYSH/uKco0LMoNkmQxH+V4jO9M j+MTP8bkrfvDNqu6TEoI0i70VYcnO67Xbk3dtiPWx4yBm+nfDSlXyyqkiw718oqWTc UC0hYgbqcVSzZVWbpVyMU7jnyw0uCLyrphYrP7mQ= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf2f.google.com (mail-qv1-xf2f.google.com [IPv6:2607:f8b0:4864:20::f2f]) by sourceware.org (Postfix) with ESMTPS id A0FCD3844007 for ; Thu, 27 May 2021 17:28:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A0FCD3844007 Received: by mail-qv1-xf2f.google.com with SMTP id u33so496247qvf.9 for ; Thu, 27 May 2021 10:28:33 -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:mime-version:content-transfer-encoding; bh=4RjSdfkkXWNuS2wPNfzNcl1g5C/zMfBG8GrbBUz8y0A=; b=IjwaoXgKGMXkjsBKYymk/M/i9Oqz2ddjkd4rgiMNDv0LMOMkJAUKhxrpSWMlxs48Vg aZtVOWY20Aevyv6cMhkdNpxU0B6QRmvdqzZzdTmu7cJlxvqppeS7hPzOkNeVuC0lIwF0 C3yH2Whr1m4Pqmv7QY0FEYMPx1cTI5461UMuGsHXYgY34w2gNTOjdESUEBB8MtiKN/Ge vaaBHLjffmLpx6V2Mr8Xch0iVM+a9g4BIy/XjUQBT16nVA3sLfMk9T3Rj+ShJe3W2Yu6 vmWFpNNzbPmo7ZyeFMyqHsb2Rremjt9XtwQHfHhrfRpgkqF9rKpWKyLObmN+J0+oGrt+ yoFA== X-Gm-Message-State: AOAM530R7Xo5e3SsLxtq6m8REse2SKWVlcu13WGlZMsU+Y5DMZ8zi5/1 bnd4R0zGqOsiTKITE1ZNQjVCwS6p3s7RZg== X-Google-Smtp-Source: ABdhPJzzd0xCwa810QVx/GBckbutvyj9iGSx5TzWbLzZ9odWZxRqf3XY7bepbMi9rvaeUMLrwmjYag== X-Received: by 2002:a05:6214:7c2:: with SMTP id bb2mr5091218qvb.51.1622136512981; Thu, 27 May 2021 10:28:32 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.31 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:32 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 4/9] nptl: Remove CANCELING_BITMASK Date: Thu, 27 May 2021 14:28:18 -0300 Message-Id: <20210527172823.3461314-5-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" The CANCELING_BITMASK is used as an optimization to avoid sending the signal when pthread_cancel is called in a concurrent manner. This requires then to put both the cancellation state and type on a shared state (cancelhandling), since 'pthread_cancel' checks whether cancellation is enabled and asynchrnous to either cancel itself of sending the signal. It also requires handle the CANCELING_BITMASK on __pthread_disable_asynccancel, however this is incurs in the same issues described on BZ#12683: the cancellation is acting even *after* the syscalls returns with user visible side-effects. This patch removes this optimization and simplifies the pthread cancellation implementation: pthread_cancel now first check if cancellation is already pending and if not always send a signal if the target is not itself. The SIGCANCEL handler is also simpified since there is not need to setup a CAS loop. It also alows to move both the cancellation state and mode out of 'cancelhadling' (it is done in subsequent patches). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/cancellation.c | 12 ---- nptl/descr.h | 3 - nptl/pthread_cancel.c | 124 ++++++++++--------------------------- nptl/pthread_join_common.c | 2 +- 4 files changed, 35 insertions(+), 106 deletions(-) diff --git a/nptl/cancellation.c b/nptl/cancellation.c index c20845adc0..b15f25d8f6 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -88,17 +88,5 @@ __pthread_disable_asynccancel (int oldtype) /* Prepare the next round. */ oldval = curval; } - - /* We cannot return when we are being canceled. Upon return the - thread might be things which would have to be undone. The - following loop should loop until the cancellation signal is - delivered. */ - while (__builtin_expect ((newval & (CANCELING_BITMASK | CANCELED_BITMASK)) - == CANCELING_BITMASK, 0)) - { - futex_wait_simple ((unsigned int *) &self->cancelhandling, newval, - FUTEX_PRIVATE); - newval = THREAD_GETMEM (self, cancelhandling); - } } libc_hidden_def (__pthread_disable_asynccancel) diff --git a/nptl/descr.h b/nptl/descr.h index 9d8297b45f..a120365f88 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -283,9 +283,6 @@ struct pthread /* Bit set if asynchronous cancellation mode is selected. */ #define CANCELTYPE_BIT 1 #define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) - /* Bit set if canceling has been initiated. */ -#define CANCELING_BIT 2 -#define CANCELING_BITMASK (0x01 << CANCELING_BIT) /* Bit set if canceled. */ #define CANCELED_BIT 3 #define CANCELED_BITMASK (0x01 << CANCELED_BIT) diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index deb404600c..8dfbcff8c3 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -43,35 +43,18 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - /* We are canceled now. When canceled by another thread this flag - is already set but if the signal is directly send (internally or - from another process) is has to be done here. */ - int newval = oldval | CANCELING_BITMASK | CANCELED_BITMASK; - - if (oldval == newval || (oldval & EXITING_BITMASK) != 0) - /* Already canceled or exiting. */ - break; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (curval == oldval) - { - /* Set the return value. */ - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - - /* Make sure asynchronous cancellation is still enabled. */ - if ((newval & CANCELTYPE_BITMASK) != 0) - /* Run the registered destructors and terminate the thread. */ - __do_cancel (); - - break; - } - - oldval = curval; - } + int ch = atomic_load_relaxed (&self->cancelhandling); + /* Cancelation not enabled, not cancelled, or already exitting. */ + if ((ch & CANCELSTATE_BITMASK) != 0 + || (ch & CANCELED_BITMASK) == 0 + || (ch & EXITING_BITMASK) != 0) + return; + + /* Set the return value. */ + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + /* Make sure asynchronous cancellation is still enabled. */ + if ((ch & CANCELTYPE_BITMASK) != 0) + __do_cancel (); } int @@ -104,72 +87,33 @@ __pthread_cancel (pthread_t th) " must be installed for pthread_cancel to work\n"); } #endif - int result = 0; - int oldval; - int newval; - do + + int oldch = atomic_fetch_or_acquire (&pd->cancelhandling, CANCELED_BITMASK); + if ((oldch & CANCELED_BITMASK) != 0) + return 0; + + if (pd == THREAD_SELF) { - again: - oldval = pd->cancelhandling; - newval = oldval | CANCELING_BITMASK | CANCELED_BITMASK; - - /* Avoid doing unnecessary work. The atomic operation can - potentially be expensive if the bug has to be locked and - remote cache lines have to be invalidated. */ - if (oldval == newval) - break; - - /* If the cancellation is handled asynchronously just send a - signal. We avoid this if possible since it's more - expensive. */ - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) - { - /* Mark the cancellation as "in progress". */ - if (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling, - oldval | CANCELING_BITMASK, - oldval)) - goto again; - - if (pd == THREAD_SELF) - /* This is not merely an optimization: An application may - call pthread_cancel (pthread_self ()) without calling - pthread_create, so the signal handler may not have been - set up for a self-cancel. */ - { - THREAD_SETMEM (pd, result, PTHREAD_CANCELED); - if ((newval & CANCELTYPE_BITMASK) != 0) - __do_cancel (); - } - else - { - /* The cancellation handler will take care of marking the - thread as canceled. */ - pid_t pid = __getpid (); - - int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, - SIGCANCEL); - if (INTERNAL_SYSCALL_ERROR_P (val)) - result = INTERNAL_SYSCALL_ERRNO (val); - } - - break; - } - - /* A single-threaded process should be able to kill itself, since - there is nothing in the POSIX specification that says that it - cannot. So we set multiple_threads to true so that cancellation - points get executed. */ - THREAD_SETMEM (THREAD_SELF, header.multiple_threads, 1); + /* A single-threaded process should be able to kill itself, since there + is nothing in the POSIX specification that says that it cannot. So + we set multiple_threads to true so that cancellation points get + executed. */ + THREAD_SETMEM (THREAD_SELF, header.multiple_threads, 1); #ifndef TLS_MULTIPLE_THREADS_IN_TCB - __libc_multiple_threads = 1; + __libc_multiple_threads = 1; #endif + + THREAD_SETMEM (pd, result, PTHREAD_CANCELED); + if ((oldch & CANCELTYPE_BITMASK) != 0) + __do_cancel (); + return 0; } - /* Mark the thread as canceled. This has to be done - atomically since other bits could be modified as well. */ - while (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling, newval, - oldval)); - return result; + pid_t pid = __getpid (); + int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); + return INTERNAL_SYSCALL_ERROR_P (val) + ? INTERNAL_SYSCALL_ERRNO (val) + : 0; } versioned_symbol (libc, __pthread_cancel, pthread_cancel, GLIBC_2_34); diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index e87801b5a3..f842c91a08 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -57,7 +57,7 @@ __pthread_clockjoin_ex (pthread_t threadid, void **thread_return, if ((pd == self || (self->joinid == pd && (pd->cancelhandling - & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK + & (CANCELED_BITMASK | EXITING_BITMASK | TERMINATED_BITMASK)) == 0)) && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling)) /* This is a deadlock situation. The threads are waiting for each From patchwork Thu May 27 17:28:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484743 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=TPX+vVmI; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZYQ139Zz9sRf for ; Fri, 28 May 2021 03:28:58 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 12CF9395A832; Thu, 27 May 2021 17:28:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 12CF9395A832 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136519; bh=ltyWsRI36K7a+59qwoQh4cS/504OpUvYokKmTLRihZY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=TPX+vVmI0DK5so3bqKSfoefuq/OKtqHLa47ZNao/pg8lFpZU+k5UPZGidXJtaHAvj N95RXPsNoNExm1DaWxIo7gSZVHsif01xXZWUyqzmI6scT9NqOfoinx5rxoli2N1FII 1zdZo9qh2qNaib+0oSfX/Zjv6kCeeC/Mm1Z78v3I= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) by sourceware.org (Postfix) with ESMTPS id 0B6E43959E5C for ; Thu, 27 May 2021 17:28:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0B6E43959E5C Received: by mail-qk1-x72b.google.com with SMTP id j189so1432376qkf.2 for ; Thu, 27 May 2021 10:28:35 -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:mime-version:content-transfer-encoding; bh=ltyWsRI36K7a+59qwoQh4cS/504OpUvYokKmTLRihZY=; b=i1aeI3thtOd++UfxNb52APgHOxutEfZjcOjMENicqimCnKZ1Vhn037IMeXY+m7D2ba a1lYtF4XTHFrTcwTWXOw1R2JSjX3RcKCmdjgxOEEultLoKHo835hD8NDnt20YH5mTRSJ WJt4OSVCyyzYOqS/UmrIkJtb2UWe95XoK3mvUIGKtgqYwuoYj7NPEQgiWCthgpcsdR2O EgEikrPAyzIWdXA5y47I8ILAQEsBknxzijkYcEtxwLKJg29ITUvLDxawxaynK+5/T61R zkpmOoTHJupaPd1kVpwZoBhtK1ULM0LsQDJr82aZAY+uMdRYkErlT6zxWpyLZ57xDMA7 Vczw== X-Gm-Message-State: AOAM530nGoQbbR+FJPQqavIxqajjup9y7Jix5gNa5SPLS2ZHW5EUfG1P niIK9rVGqBz/pbh6I/iSHX7SQf6919+q/Q== X-Google-Smtp-Source: ABdhPJzRjkkbsWBZb2tmyARpSr6alem7hD4I6UqqbTWTXdRo9VVcowtZ2yREibVW0JFfYaXSTe+4Jw== X-Received: by 2002:a05:620a:1675:: with SMTP id d21mr4465501qko.390.1622136514266; Thu, 27 May 2021 10:28:34 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.33 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:33 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 5/9] nptl: Move cancel state out of cancelhandling Date: Thu, 27 May 2021 14:28:19 -0300 Message-Id: <20210527172823.3461314-6-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that thread cancellation state is not accessed concurrently anymore, it is possible to move it out the 'cancelhandling'. The code is also simplified: the CANCELLATION_P is replaced with a internal pthread_testcancel call and the CANCELSTATE_BIT{MASK} is removed. The second part of this patchset also keeps the pthread_setcanceltype as cancellation entrypoint by calling pthread_testcancel if the new type is PTHREAD_CANCEL_ASYNCHRONOUS. With this behavior pthread_setcancelstate does not require to act on cancellation if cancel type is asynchronous (is already handled either by pthread_setcanceltype or by the signal handler). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- manual/pattern.texi | 1 - manual/process.texi | 3 +-- nptl/allocatestack.c | 1 + nptl/cancellation.c | 3 ++- nptl/cleanup_defer.c | 2 +- nptl/descr.h | 14 ++++++-------- nptl/libc-cleanup.c | 2 +- nptl/pthreadP.h | 12 ------------ nptl/pthread_cancel.c | 2 +- nptl/pthread_join_common.c | 5 ++++- nptl/pthread_setcancelstate.c | 36 +++-------------------------------- nptl/pthread_setcanceltype.c | 3 ++- nptl/pthread_testcancel.c | 11 ++++++++++- sysdeps/nptl/dl-tls_init_tp.c | 2 ++ 14 files changed, 34 insertions(+), 63 deletions(-) diff --git a/manual/pattern.texi b/manual/pattern.texi index 39ae97a3c4..4fa4c25525 100644 --- a/manual/pattern.texi +++ b/manual/pattern.texi @@ -1820,7 +1820,6 @@ the beginning of the vector. @c (disable cancellation around exec_comm; it may do_cancel the @c second time, if async cancel is enabled) @c THREAD_ATOMIC_CMPXCHG_VAL dup ok -@c CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS dup ok @c do_cancel @ascuplugin @ascuheap @acsmem @c THREAD_ATOMIC_BIT_SET dup ok @c pthread_unwind @ascuplugin @ascuheap @acsmem diff --git a/manual/process.texi b/manual/process.texi index 54e65f76c6..134d5c6143 100644 --- a/manual/process.texi +++ b/manual/process.texi @@ -68,7 +68,7 @@ until the subprogram terminates before you can do anything else. @c CLEANUP_HANDLER @ascuplugin @ascuheap @acsmem @c libc_cleanup_region_start @ascuplugin @ascuheap @acsmem @c pthread_cleanup_push_defer @ascuplugin @ascuheap @acsmem -@c CANCELLATION_P @ascuplugin @ascuheap @acsmem +@c __pthread_testcancel @ascuplugin @ascuheap @acsmem @c CANCEL_ENABLED_AND_CANCELED ok @c do_cancel @ascuplugin @ascuheap @acsmem @c cancel_handler ok @@ -86,7 +86,6 @@ until the subprogram terminates before you can do anything else. @c SINGLE_THREAD_P ok @c LIBC_CANCEL_ASYNC @ascuplugin @ascuheap @acsmem @c libc_enable_asynccancel @ascuplugin @ascuheap @acsmem -@c CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS dup ok @c do_cancel dup @ascuplugin @ascuheap @acsmem @c LIBC_CANCEL_RESET ok @c libc_disable_asynccancel ok diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 2114bd2e27..54e95baad7 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -160,6 +160,7 @@ get_cached_stack (size_t *sizep, void **memp) /* Cancellation handling is back to the default. */ result->cancelhandling = 0; + result->cancelstate = PTHREAD_CANCEL_ENABLE; result->cleanup = NULL; result->setup_failed = 0; diff --git a/nptl/cancellation.c b/nptl/cancellation.c index b15f25d8f6..ce00603109 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -44,7 +44,8 @@ __pthread_enable_asynccancel (void) oldval); if (__glibc_likely (curval == oldval)) { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) { THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c index 6d85359118..873ab007fd 100644 --- a/nptl/cleanup_defer.c +++ b/nptl/cleanup_defer.c @@ -92,7 +92,7 @@ ___pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf) cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } } versioned_symbol (libc, ___pthread_unregister_cancel_restore, diff --git a/nptl/descr.h b/nptl/descr.h index a120365f88..35f5330e7f 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -277,9 +277,6 @@ struct pthread /* Flags determining processing of cancellation. */ int cancelhandling; - /* Bit set if cancellation is disabled. */ -#define CANCELSTATE_BIT 0 -#define CANCELSTATE_BITMASK (0x01 << CANCELSTATE_BIT) /* Bit set if asynchronous cancellation mode is selected. */ #define CANCELTYPE_BIT 1 #define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) @@ -298,11 +295,8 @@ struct pthread /* Mask for the rest. Helps the compiler to optimize. */ #define CANCEL_RESTMASK 0xffffff80 -#define CANCEL_ENABLED_AND_CANCELED(value) \ - (((value) & (CANCELSTATE_BITMASK | CANCELED_BITMASK | EXITING_BITMASK \ - | CANCEL_RESTMASK | TERMINATED_BITMASK)) == CANCELED_BITMASK) -#define CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS(value) \ - (((value) & (CANCELSTATE_BITMASK | CANCELTYPE_BITMASK | CANCELED_BITMASK \ +#define CANCEL_CANCELED_AND_ASYNCHRONOUS(value) \ + (((value) & (CANCELTYPE_BITMASK | CANCELED_BITMASK \ | EXITING_BITMASK | CANCEL_RESTMASK | TERMINATED_BITMASK)) \ == (CANCELTYPE_BITMASK | CANCELED_BITMASK)) @@ -404,6 +398,10 @@ struct pthread /* Indicates whether is a C11 thread created by thrd_creat. */ bool c11; + /* Thread cancel state (PTHREAD_CANCEL_ENABLE or + PTHREAD_CANCEL_DISABLE). */ + unsigned char cancelstate; + /* Used on strsignal. */ struct tls_internal_t tls_state; diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c index 14ccfe9285..6286b8b525 100644 --- a/nptl/libc-cleanup.c +++ b/nptl/libc-cleanup.c @@ -79,7 +79,7 @@ __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer) cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } } libc_hidden_def (__libc_cleanup_pop_restore) diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 48d48e7008..3e7a4f52ab 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -245,18 +245,6 @@ libc_hidden_proto (__pthread_current_priority) #define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0) #define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0) -/* Cancellation test. */ -#define CANCELLATION_P(self) \ - do { \ - int cancelhandling = THREAD_GETMEM (self, cancelhandling); \ - if (CANCEL_ENABLED_AND_CANCELED (cancelhandling)) \ - { \ - THREAD_SETMEM (self, result, PTHREAD_CANCELED); \ - __do_cancel (); \ - } \ - } while (0) - - extern void __pthread_unwind (__pthread_unwind_buf_t *__buf) __cleanup_fct_attribute __attribute ((__noreturn__)) #if !defined SHARED && !IS_IN (libpthread) diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 8dfbcff8c3..69b57c8808 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -45,7 +45,7 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) int ch = atomic_load_relaxed (&self->cancelhandling); /* Cancelation not enabled, not cancelled, or already exitting. */ - if ((ch & CANCELSTATE_BITMASK) != 0 + if (self->cancelstate == PTHREAD_CANCEL_DISABLE || (ch & CANCELED_BITMASK) == 0 || (ch & EXITING_BITMASK) != 0) return; diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index f842c91a08..7303069316 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -59,7 +59,10 @@ __pthread_clockjoin_ex (pthread_t threadid, void **thread_return, && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK | TERMINATED_BITMASK)) == 0)) - && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling)) + && !(self->cancelstate == PTHREAD_CANCEL_ENABLE + && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK + | TERMINATED_BITMASK)) + == CANCELED_BITMASK)) /* This is a deadlock situation. The threads are waiting for each other to finish. Note that this is a "may" error. To be 100% sure we catch this error we would have to lock the data diff --git a/nptl/pthread_setcancelstate.c b/nptl/pthread_setcancelstate.c index e3696ca348..7e2b6e4974 100644 --- a/nptl/pthread_setcancelstate.c +++ b/nptl/pthread_setcancelstate.c @@ -31,39 +31,9 @@ __pthread_setcancelstate (int state, int *oldstate) self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = (state == PTHREAD_CANCEL_DISABLE - ? oldval | CANCELSTATE_BITMASK - : oldval & ~CANCELSTATE_BITMASK); - - /* Store the old value. */ - if (oldstate != NULL) - *oldstate = ((oldval & CANCELSTATE_BITMASK) - ? PTHREAD_CANCEL_DISABLE : PTHREAD_CANCEL_ENABLE); - - /* Avoid doing unnecessary work. The atomic operation can - potentially be expensive if the memory has to be locked and - remote cache lines have to be invalidated. */ - if (oldval == newval) - break; - - /* Update the cancel handling word. This has to be done - atomically since other bits could be modified as well. */ - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) - __do_cancel (); - - break; - } - - /* Prepare for the next round. */ - oldval = curval; - } + if (oldstate != NULL) + *oldstate = self->cancelstate; + self->cancelstate = state; return 0; } diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c index 5f061d512b..ae5df1d591 100644 --- a/nptl/pthread_setcanceltype.c +++ b/nptl/pthread_setcanceltype.c @@ -53,7 +53,8 @@ __pthread_setcanceltype (int type, int *oldtype) oldval); if (__glibc_likely (curval == oldval)) { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) { THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c index a9e941ddb7..920374643a 100644 --- a/nptl/pthread_testcancel.c +++ b/nptl/pthread_testcancel.c @@ -23,7 +23,16 @@ void ___pthread_testcancel (void) { - CANCELLATION_P (THREAD_SELF); + struct pthread *self = THREAD_SELF; + int cancelhandling = THREAD_GETMEM (self, cancelhandling); + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && (cancelhandling & CANCELED_BITMASK) + && !(cancelhandling & EXITING_BITMASK) + && !(cancelhandling & TERMINATED_BITMASK)) + { + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + __do_cancel (); + } } versioned_symbol (libc, ___pthread_testcancel, pthread_testcancel, GLIBC_2_34); versioned_symbol (libc, ___pthread_testcancel, __pthread_testcancel, diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c index 1f7790297f..378b26a2f5 100644 --- a/sysdeps/nptl/dl-tls_init_tp.c +++ b/sysdeps/nptl/dl-tls_init_tp.c @@ -94,4 +94,6 @@ __tls_init_tp (void) It will be bigger than it actually is, but for unwind.c/pt-longjmp.c purposes this is good enough. */ THREAD_SETMEM (pd, stackblock_size, (size_t) __libc_stack_end); + + THREAD_SETMEM (pd, cancelstate, PTHREAD_CANCEL_ENABLE); } From patchwork Thu May 27 17:28:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484745 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=mMMCt/Za; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZYZ5vwVz9sT6 for ; Fri, 28 May 2021 03:29:06 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A1CB63959E51; Thu, 27 May 2021 17:28:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A1CB63959E51 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136521; bh=rNsA27gX/Dihomfg8JnF2KDQih+KbVkRjDaSFhdvTQA=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=mMMCt/ZanAs43BOD1dxjolsuUEGKKn267W3iusy04jekwziYt4/VZdzVOalneyOS0 1zd8W4jXD7JtCiUPt6pVlnhTb4FAsxMmokwxgmlkpFkKylOFMbE+meteqhr47psRGt RCsyQ+ioa8Fal6kDH6lulT/kldHKQgbUf+O8DRnk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x82b.google.com (mail-qt1-x82b.google.com [IPv6:2607:f8b0:4864:20::82b]) by sourceware.org (Postfix) with ESMTPS id 1D3B3383800A for ; Thu, 27 May 2021 17:28:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1D3B3383800A Received: by mail-qt1-x82b.google.com with SMTP id a15so826567qta.0 for ; Thu, 27 May 2021 10:28:37 -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:mime-version:content-transfer-encoding; bh=rNsA27gX/Dihomfg8JnF2KDQih+KbVkRjDaSFhdvTQA=; b=Hw/a751Glx6+qtTT7fedBP6LcIc/kDEExK+fwXvUyir03D8hzkruNbKf0SyFkoWCNw vsQI6LQs2fOk+YJOnAFDUujz5zDIA+x+nUqb6H6jVUxY9LTI6+urhOMiC/Wwaej92rNs s1ptYCMD/tLF2ThStcTcKNevA2Jri60J6eCp74BSpZt3PtJPhSZdCp76MIQCXrsOB3Lm mFZ/n5s8kS1xuV1JSeS5GgJmzsS89LML9MfwVCdD2egK5z5TP8Yq8a/YAStDTK1HyfkR g6dp6c67+PqqZnGplwwG3w4/maGE6oWn5s1h9HPtr+v/TBljrMgzB9P/8JU9hBA+HiVY hMOA== X-Gm-Message-State: AOAM532VYGkLDXTSVKB49EFmGepI7vK+CPtj5vmiUmwLw8qboAckKCZj pIoSV4DNO32Vb8+lZIjpFyojfhI1uecTPg== X-Google-Smtp-Source: ABdhPJwS7kpfFL+Cdlq7NdK4pj1CQmujs1bd0gt6x2GRBDCJWIZ8cVWEFXXBpI9s26Nmhb7MfDFdWA== X-Received: by 2002:ac8:1342:: with SMTP id f2mr4180659qtj.148.1622136515552; Thu, 27 May 2021 10:28:35 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.34 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:35 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 6/9] nptl: Move cancel type out of cancelhandling Date: Thu, 27 May 2021 14:28:20 -0300 Message-Id: <20210527172823.3461314-7-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that the thread cancellation type is not accessed concurrently anymore, it is possible to move it out the cancelhandling. By removing the cancel state out of the internal thread cancel handling state there is no need to check if cancelled bit was set in CAS operation. It allows simplifing the cancellation wrappers and the CANCEL_CANCELED_AND_ASYNCHRONOUS is removed. Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/allocatestack.c | 1 + nptl/cancellation.c | 51 +++++++++-------------------------- nptl/cleanup_defer.c | 46 ++++--------------------------- nptl/descr.h | 14 +++------- nptl/libc-cleanup.c | 44 +++--------------------------- nptl/pthread_cancel.c | 4 +-- nptl/pthread_setcanceltype.c | 42 ++++------------------------- sysdeps/nptl/dl-tls_init_tp.c | 1 + 8 files changed, 34 insertions(+), 169 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 54e95baad7..9be6c42894 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -161,6 +161,7 @@ get_cached_stack (size_t *sizep, void **memp) /* Cancellation handling is back to the default. */ result->cancelhandling = 0; result->cancelstate = PTHREAD_CANCEL_ENABLE; + result->canceltype = PTHREAD_CANCEL_DEFERRED; result->cleanup = NULL; result->setup_failed = 0; diff --git a/nptl/cancellation.c b/nptl/cancellation.c index ce00603109..05962784d5 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -31,31 +31,19 @@ int __pthread_enable_asynccancel (void) { struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = oldval | CANCELTYPE_BITMASK; - - if (newval == oldval) - break; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (self->cancelstate == PTHREAD_CANCEL_ENABLE - && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) - { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - __do_cancel (); - } + int oldval = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_ASYNCHRONOUS); - break; - } + int ch = THREAD_GETMEM (self, cancelhandling); - /* Prepare the next round. */ - oldval = curval; + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && (ch & CANCELED_BITMASK) + && !(ch & EXITING_BITMASK) + && !(ch & TERMINATED_BITMASK)) + { + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + __do_cancel (); } return oldval; @@ -69,25 +57,10 @@ __pthread_disable_asynccancel (int oldtype) { /* If asynchronous cancellation was enabled before we do not have anything to do. */ - if (oldtype & CANCELTYPE_BITMASK) + if (oldtype == PTHREAD_CANCEL_ASYNCHRONOUS) return; struct pthread *self = THREAD_SELF; - int newval; - - int oldval = THREAD_GETMEM (self, cancelhandling); - - while (1) - { - newval = oldval & ~CANCELTYPE_BITMASK; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - break; - - /* Prepare the next round. */ - oldval = curval; - } + self->canceltype = PTHREAD_CANCEL_DEFERRED; } libc_hidden_def (__pthread_disable_asynccancel) diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c index 873ab007fd..7e858d0df0 100644 --- a/nptl/cleanup_defer.c +++ b/nptl/cleanup_defer.c @@ -31,27 +31,9 @@ ___pthread_register_cancel_defer (__pthread_unwind_buf_t *buf) ibuf->priv.data.prev = THREAD_GETMEM (self, cleanup_jmp_buf); ibuf->priv.data.cleanup = THREAD_GETMEM (self, cleanup); - int cancelhandling = THREAD_GETMEM (self, cancelhandling); - /* Disable asynchronous cancellation for now. */ - if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK)) - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - & ~CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - ibuf->priv.data.canceltype = (cancelhandling & CANCELTYPE_BITMASK - ? PTHREAD_CANCEL_ASYNCHRONOUS - : PTHREAD_CANCEL_DEFERRED); + ibuf->priv.data.canceltype = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); /* Store the new cleanup handler info. */ THREAD_SETMEM (self, cleanup_jmp_buf, (struct pthread_unwind_buf *) buf); @@ -73,27 +55,9 @@ ___pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf) THREAD_SETMEM (self, cleanup_jmp_buf, ibuf->priv.data.prev); - int cancelhandling; - if (ibuf->priv.data.canceltype != PTHREAD_CANCEL_DEFERRED - && ((cancelhandling = THREAD_GETMEM (self, cancelhandling)) - & CANCELTYPE_BITMASK) == 0) - { - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - | CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - __pthread_testcancel (); - } + THREAD_SETMEM (self, canceltype, ibuf->priv.data.canceltype); + if (ibuf->priv.data.canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) + __pthread_testcancel (); } versioned_symbol (libc, ___pthread_unregister_cancel_restore, __pthread_unregister_cancel_restore, GLIBC_2_34); diff --git a/nptl/descr.h b/nptl/descr.h index 35f5330e7f..c85778d449 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -277,9 +277,6 @@ struct pthread /* Flags determining processing of cancellation. */ int cancelhandling; - /* Bit set if asynchronous cancellation mode is selected. */ -#define CANCELTYPE_BIT 1 -#define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) /* Bit set if canceled. */ #define CANCELED_BIT 3 #define CANCELED_BITMASK (0x01 << CANCELED_BIT) @@ -292,13 +289,6 @@ struct pthread /* Bit set if thread is supposed to change XID. */ #define SETXID_BIT 6 #define SETXID_BITMASK (0x01 << SETXID_BIT) - /* Mask for the rest. Helps the compiler to optimize. */ -#define CANCEL_RESTMASK 0xffffff80 - -#define CANCEL_CANCELED_AND_ASYNCHRONOUS(value) \ - (((value) & (CANCELTYPE_BITMASK | CANCELED_BITMASK \ - | EXITING_BITMASK | CANCEL_RESTMASK | TERMINATED_BITMASK)) \ - == (CANCELTYPE_BITMASK | CANCELED_BITMASK)) /* Flags. Including those copied from the thread attribute. */ int flags; @@ -402,6 +392,10 @@ struct pthread PTHREAD_CANCEL_DISABLE). */ unsigned char cancelstate; + /* Thread cancel type (PTHREAD_CANCEL_DEFERRED or + PTHREAD_CANCEL_ASYNCHRONOUS). */ + unsigned char canceltype; + /* Used on strsignal. */ struct tls_internal_t tls_state; diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c index 6286b8b525..180d15bc9e 100644 --- a/nptl/libc-cleanup.c +++ b/nptl/libc-cleanup.c @@ -27,27 +27,9 @@ __libc_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer) buffer->__prev = THREAD_GETMEM (self, cleanup); - int cancelhandling = THREAD_GETMEM (self, cancelhandling); - /* Disable asynchronous cancellation for now. */ - if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK)) - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - & ~CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - buffer->__canceltype = (cancelhandling & CANCELTYPE_BITMASK - ? PTHREAD_CANCEL_ASYNCHRONOUS - : PTHREAD_CANCEL_DEFERRED); + buffer->__canceltype = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); THREAD_SETMEM (self, cleanup, buffer); } @@ -60,26 +42,8 @@ __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer) THREAD_SETMEM (self, cleanup, buffer->__prev); - int cancelhandling; - if (__builtin_expect (buffer->__canceltype != PTHREAD_CANCEL_DEFERRED, 0) - && ((cancelhandling = THREAD_GETMEM (self, cancelhandling)) - & CANCELTYPE_BITMASK) == 0) - { - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - | CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - + THREAD_SETMEM (self, canceltype, buffer->__canceltype); + if (buffer->__canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __pthread_testcancel (); - } } libc_hidden_def (__libc_cleanup_pop_restore) diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 69b57c8808..c6086b6c3c 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -53,7 +53,7 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) /* Set the return value. */ THREAD_SETMEM (self, result, PTHREAD_CANCELED); /* Make sure asynchronous cancellation is still enabled. */ - if ((ch & CANCELTYPE_BITMASK) != 0) + if (self->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __do_cancel (); } @@ -104,7 +104,7 @@ __pthread_cancel (pthread_t th) #endif THREAD_SETMEM (pd, result, PTHREAD_CANCELED); - if ((oldch & CANCELTYPE_BITMASK) != 0) + if (pd->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __do_cancel (); return 0; } diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c index ae5df1d591..e7b24ae733 100644 --- a/nptl/pthread_setcanceltype.c +++ b/nptl/pthread_setcanceltype.c @@ -29,43 +29,11 @@ __pthread_setcanceltype (int type, int *oldtype) volatile struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = (type == PTHREAD_CANCEL_ASYNCHRONOUS - ? oldval | CANCELTYPE_BITMASK - : oldval & ~CANCELTYPE_BITMASK); - - /* Store the old value. */ - if (oldtype != NULL) - *oldtype = ((oldval & CANCELTYPE_BITMASK) - ? PTHREAD_CANCEL_ASYNCHRONOUS : PTHREAD_CANCEL_DEFERRED); - - /* Avoid doing unnecessary work. The atomic operation can - potentially be expensive if the memory has to be locked and - remote cache lines have to be invalidated. */ - if (oldval == newval) - break; - - /* Update the cancel handling word. This has to be done - atomically since other bits could be modified as well. */ - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (self->cancelstate == PTHREAD_CANCEL_ENABLE - && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) - { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - __do_cancel (); - } - - break; - } - - /* Prepare for the next round. */ - oldval = curval; - } + if (oldtype != NULL) + *oldtype = self->canceltype; + self->canceltype = type; + if (type == PTHREAD_CANCEL_ASYNCHRONOUS) + __pthread_testcancel (); return 0; } diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c index 378b26a2f5..b7b3bb1cdb 100644 --- a/sysdeps/nptl/dl-tls_init_tp.c +++ b/sysdeps/nptl/dl-tls_init_tp.c @@ -96,4 +96,5 @@ __tls_init_tp (void) THREAD_SETMEM (pd, stackblock_size, (size_t) __libc_stack_end); THREAD_SETMEM (pd, cancelstate, PTHREAD_CANCEL_ENABLE); + THREAD_SETMEM (pd, canceltype, PTHREAD_CANCEL_DEFERRED); } From patchwork Thu May 27 17:28:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484744 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=e9b2truq; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZYV2YK8z9sRf for ; Fri, 28 May 2021 03:29:02 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1E502395B46E; Thu, 27 May 2021 17:28:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1E502395B46E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136521; bh=AXDBXj2vGE9aiPEfxAByhjhcE+bWnsVcNW9mDvSxc50=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=e9b2truqJnCroK/6Ln+OZHz5qtH3J20UrCQ5B7YsK8fYt8PcZi8sm3R9COaIu9ri5 r0vSCQltPqIR/LNZuF5/nZklNvgHoncAb5X6By+XSGFZZNybLJEvICULspXhaGGcbl ItyI83G8COJI8JUkkLRYm8j9bj+ntTAl+cFrXYgc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x732.google.com (mail-qk1-x732.google.com [IPv6:2607:f8b0:4864:20::732]) by sourceware.org (Postfix) with ESMTPS id AF1E7395ACC9 for ; Thu, 27 May 2021 17:28:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AF1E7395ACC9 Received: by mail-qk1-x732.google.com with SMTP id k4so1465195qkd.0 for ; Thu, 27 May 2021 10:28:37 -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:mime-version:content-transfer-encoding; bh=AXDBXj2vGE9aiPEfxAByhjhcE+bWnsVcNW9mDvSxc50=; b=QpQVFTyT7rUZOan1IWF3T6cmT6KJNIUhrkuDu6hFGDRy4t2YAdUvM1DI7S1+66uTHu FMy4v1nfS0zlbZHnmBGgTwojY/s4x+Gmicor2UH9z/HcAs4cuF/oEun10ISfnhutYRQJ eZ/GBFSoEkGCxrPU7GVHEletUFMArpr7CY37SNdkYz8Q/XU7GKQUAyZXcU14DGxAc/3M hTS8bjXUe3rxVOMTjuVLti6v3QP3gweLjPK/65t4oOepJAk62sr7v2dTCqUF/WWcIqT8 iwVdhEsPZMjapevYLLyqy/GbpinFw66uYL5cawdUR4EXyS+zFaOM8+9PCFCBGG6V6Y7b o/vA== X-Gm-Message-State: AOAM53284MAZ4J3D8p6N6rS2XOGdCoWBGS/ttYqjLC+ODulUHrHFMNms O6MLi4ap6aso64kG+4HZ3/8YSO41zJiT6g== X-Google-Smtp-Source: ABdhPJxgy8m9r30+8TSjJIl4xPwZVx7gq9SZizrG2ehuRkRZyGDp/G/MDAptoxG7nnaz5suqqNHaFA== X-Received: by 2002:a05:620a:29c4:: with SMTP id s4mr4619205qkp.22.1622136516862; Thu, 27 May 2021 10:28:36 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.35 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:36 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 7/9] nptl: Implement raise in terms of pthread_kill Date: Thu, 27 May 2021 14:28:21 -0300 Message-Id: <20210527172823.3461314-8-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that pthread_kill is provided by libc.so it is possible to implement the generic POSIX implementation as 'pthread_kill(pthread_self(), sig)'. For Linux implementation, pthread_kill read the targeting TID from the TCB. For raise, this it not possible because it would make raise fail when issue after vfork (where creates the resulting process has a different TID from the parent, but its TCB is not updated as for pthread_create). To make raise use pthread_kill, it is make usable from vfork by getting the target thread id through gettid syscall. Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- include/pthread.h | 5 ++++ nptl/pthreadP.h | 4 ++- nptl/pthread_kill.c | 40 ++++++++++++++++--------- nptl/pthread_self.c | 4 ++- sysdeps/htl/pthreadP.h | 2 -- sysdeps/posix/raise.c | 11 +++++-- sysdeps/unix/sysv/linux/raise.c | 52 --------------------------------- 7 files changed, 47 insertions(+), 71 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/raise.c diff --git a/include/pthread.h b/include/pthread.h index a3e1cf51b0..1158919247 100644 --- a/include/pthread.h +++ b/include/pthread.h @@ -16,4 +16,9 @@ extern int __pthread_barrier_wait (pthread_barrier_t *__barrier) /* This function is called to initialize the pthread library. */ extern void __pthread_initialize (void) __attribute__ ((weak)); + +extern int __pthread_kill (pthread_t threadid, int signo); + +extern pthread_t __pthread_self (void); + #endif diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 3e7a4f52ab..618922f47a 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -508,11 +508,13 @@ extern int __pthread_once (pthread_once_t *once_control, libc_hidden_proto (__pthread_once) extern int __pthread_atfork (void (*prepare) (void), void (*parent) (void), void (*child) (void)); -extern pthread_t __pthread_self (void); +libc_hidden_proto (__pthread_self) extern int __pthread_equal (pthread_t thread1, pthread_t thread2); extern int __pthread_detach (pthread_t th); libc_hidden_proto (__pthread_detach) extern int __pthread_kill (pthread_t threadid, int signo); +libc_hidden_proto (__pthread_kill) +extern int __pthread_cancel (pthread_t th); extern void __pthread_exit (void *value) __attribute__ ((__noreturn__)); libc_hidden_proto (__pthread_exit) extern int __pthread_join (pthread_t threadid, void **thread_return); diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index ad7e011779..0392c3f9e5 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -28,24 +28,38 @@ __pthread_kill (pthread_t threadid, int signo) if (__is_internal_signal (signo)) return EINVAL; - /* Force load of pd->tid into local variable or register. Otherwise - if a thread exits between ESRCH test and tgkill, we might return - EINVAL, because pd->tid would be cleared by the kernel. */ + pid_t tid; struct pthread *pd = (struct pthread *) threadid; - pid_t tid = atomic_forced_read (pd->tid); - if (__glibc_unlikely (tid <= 0)) - /* Not a valid thread handle. */ - return ESRCH; - /* We have a special syscall to do the work. */ - pid_t pid = __getpid (); + if (pd == THREAD_SELF) + tid = INLINE_SYSCALL_CALL (gettid); + else + /* Force load of pd->tid into local variable or register. Otherwise + if a thread exits between ESRCH test and tgkill, we might return + EINVAL, because pd->tid would be cleared by the kernel. */ + tid = atomic_forced_read (pd->tid); - int val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo); - return (INTERNAL_SYSCALL_ERROR_P (val) - ? INTERNAL_SYSCALL_ERRNO (val) : 0); + int val; + if (__glibc_likely (tid > 0)) + { + pid_t pid = __getpid (); + + val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo); + val = (INTERNAL_SYSCALL_ERROR_P (val) + ? INTERNAL_SYSCALL_ERRNO (val) : 0); + } + else + val = ESRCH; + + return val; } +/* Some architectures (for instance arm) might pull raise through libgcc, so + avoid the symbol version if it ends up being used on ld.so. */ +#if !IS_IN(rtld) +libc_hidden_def (__pthread_kill) versioned_symbol (libc, __pthread_kill, pthread_kill, GLIBC_2_34); -#if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_34) +# if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_34) compat_symbol (libc, __pthread_kill, pthread_kill, GLIBC_2_0); +# endif #endif diff --git a/nptl/pthread_self.c b/nptl/pthread_self.c index f877a2e6bd..196d93fb8e 100644 --- a/nptl/pthread_self.c +++ b/nptl/pthread_self.c @@ -20,7 +20,9 @@ #include pthread_t -pthread_self (void) +__pthread_self (void) { return (pthread_t) THREAD_SELF; } +libc_hidden_def (__pthread_self) +weak_alias (__pthread_self, pthread_self) diff --git a/sysdeps/htl/pthreadP.h b/sysdeps/htl/pthreadP.h index 8e2cf2ce65..3b357b7bdc 100644 --- a/sysdeps/htl/pthreadP.h +++ b/sysdeps/htl/pthreadP.h @@ -31,8 +31,6 @@ extern void __pthread_init_static_tls (struct link_map *) attribute_hidden; /* These represent the interface used by glibc itself. */ -extern pthread_t __pthread_self (void); -extern int __pthread_kill (pthread_t threadid, int signo); extern struct __pthread **__pthread_threads; extern int __pthread_mutex_init (pthread_mutex_t *__mutex, const pthread_mutexattr_t *__attr); diff --git a/sysdeps/posix/raise.c b/sysdeps/posix/raise.c index 9fdb4d538b..4806c0cc63 100644 --- a/sysdeps/posix/raise.c +++ b/sysdeps/posix/raise.c @@ -16,13 +16,20 @@ . */ #include -#include +#include +#include /* Raise the signal SIG. */ int raise (int sig) { - return __kill (__getpid (), sig); + int ret = __pthread_kill (__pthread_self (), sig); + if (ret != 0) + { + __set_errno (ret); + ret = -1; + } + return ret; } libc_hidden_def (raise) weak_alias (raise, gsignal) diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c deleted file mode 100644 index 9be3b37f53..0000000000 --- a/sysdeps/unix/sysv/linux/raise.c +++ /dev/null @@ -1,52 +0,0 @@ -/* Copyright (C) 2002-2021 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Ulrich Drepper , 2002. - - 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 - . */ - -#include -#include -#include -#include -#include -#include - -int -raise (int sig) -{ - /* rt_sigprocmask may fail if: - - 1. sigsetsize != sizeof (sigset_t) (EINVAL) - 2. a failure in copy from/to user space (EFAULT) - 3. an invalid 'how' operation (EINVAL) - - The first case is already handle in glibc syscall call by using the arch - defined _NSIG. Second case is handled by using a stack allocated mask. - The last one should be handled by the block/unblock functions. */ - - sigset_t set; - __libc_signal_block_app (&set); - - pid_t pid = INTERNAL_SYSCALL_CALL (getpid); - pid_t tid = INTERNAL_SYSCALL_CALL (gettid); - - int ret = INLINE_SYSCALL_CALL (tgkill, pid, tid, sig); - - __libc_signal_restore_set (&set); - - return ret; -} -libc_hidden_def (raise) -weak_alias (raise, gsignal) From patchwork Thu May 27 17:28:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484746 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=ANgxolDW; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZYg0WRzz9sRf for ; Fri, 28 May 2021 03:29:11 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3002E395A00A; Thu, 27 May 2021 17:28:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3002E395A00A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136522; bh=1DeyDCWUQ+3Y4HyRrOjhBvzTZqYpLVc25zluV7cBJhE=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ANgxolDWwELo2Zr+o2/Lazf8sGBjyg8mKq0GuXFOooqoKT+hfwKqP502Q4pbs8PVy TkrMlkVpumyEZVnE8aWJhtAYOs7fkhuAlpJBlzw2S9VlNVWf5jML6a01KjK4Av8TtL S1/G3Br6Wazm+gAlPAeTAJIpVmTV0snCfG3F6yfY= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x82f.google.com (mail-qt1-x82f.google.com [IPv6:2607:f8b0:4864:20::82f]) by sourceware.org (Postfix) with ESMTPS id CA586395B41A for ; Thu, 27 May 2021 17:28:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CA586395B41A Received: by mail-qt1-x82f.google.com with SMTP id k19so800422qta.2 for ; Thu, 27 May 2021 10:28:38 -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:mime-version:content-transfer-encoding; bh=1DeyDCWUQ+3Y4HyRrOjhBvzTZqYpLVc25zluV7cBJhE=; b=ENHo59hEO1YFlm7nnh7Ief091PM0ZutAgOCxegDu3vwrTxbAe5ICY3czKCTn7kL3+9 ZfDvbfntlfQ5S9/K7tTmKNKbcUKOvd13/7xaMAy755Cq74GEzniL7LgsU4lYSR0YGJBA wEr7zMP5WM9hRg007DSntUjktyFqbgxFVBYnoISW3h7ANmzaCWFnRPJalqfWr+AbGlYk hOqVOPoIDvEsgjlT+fa4LgmqX4EFjQs+NNepj/Xv0hO5KtV+ZTIU+t1SKXpqFDYm7G94 3Be8D6Jvf0vFSlF9bCZ8CDgHwkl86swXgy3zkwIUMOhCfu223mVaPql/HPrJhNPMi1cc Fl/A== X-Gm-Message-State: AOAM533ehBGy38saHKR2gEqJtY2kcamjs8S7UrGBbgnTG8qS/ZDo/Phx iePy4pue+j1ol6lmlG79m2Z2CYweWV2Hxg== X-Google-Smtp-Source: ABdhPJxzqeS9wkBXEOgL34P+uDEc7uIE23H1G3wxTrw1DXIDmHQmJTSrwKObDWyljdYoLrtAxF7AJA== X-Received: by 2002:ac8:65d8:: with SMTP id t24mr4210026qto.332.1622136518205; Thu, 27 May 2021 10:28:38 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.37 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:37 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 8/9] nptl: Use pthread_kill on pthread_cancel Date: Thu, 27 May 2021 14:28:22 -0300 Message-Id: <20210527172823.3461314-9-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" It consolidates the tgkill call and it is the first step of making pthread_cancel async-signal-safe. It also fix a possible issue where the 'struct pthread' tid is not read atomically, which might send an invalid cancellation signal (similar to what db988e50a87f613cb6b9e98a2fc66a4848bc3546 fixed for pthread_join). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/pthreadP.h | 2 ++ nptl/pthread_cancel.c | 6 +----- nptl/pthread_kill.c | 18 ++++++++++++------ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 618922f47a..675d1de753 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -515,6 +515,8 @@ libc_hidden_proto (__pthread_detach) extern int __pthread_kill (pthread_t threadid, int signo); libc_hidden_proto (__pthread_kill) extern int __pthread_cancel (pthread_t th); +extern int __pthread_kill_internal (pthread_t threadid, int signo) + attribute_hidden; extern void __pthread_exit (void *value) __attribute__ ((__noreturn__)); libc_hidden_proto (__pthread_exit) extern int __pthread_join (pthread_t threadid, void **thread_return); diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index c6086b6c3c..5fae763861 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -109,11 +109,7 @@ __pthread_cancel (pthread_t th) return 0; } - pid_t pid = __getpid (); - int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); - return INTERNAL_SYSCALL_ERROR_P (val) - ? INTERNAL_SYSCALL_ERRNO (val) - : 0; + return __pthread_kill_internal (th, SIGCANCEL); } versioned_symbol (libc, __pthread_cancel, pthread_cancel, GLIBC_2_34); diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index 0392c3f9e5..93387dd60a 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -21,13 +21,8 @@ #include int -__pthread_kill (pthread_t threadid, int signo) +__pthread_kill_internal (pthread_t threadid, int signo) { - /* Disallow sending the signal we use for cancellation, timers, - for the setxid implementation. */ - if (__is_internal_signal (signo)) - return EINVAL; - pid_t tid; struct pthread *pd = (struct pthread *) threadid; @@ -53,6 +48,17 @@ __pthread_kill (pthread_t threadid, int signo) return val; } + +int +__pthread_kill (pthread_t threadid, int signo) +{ + /* Disallow sending the signal we use for cancellation, timers, + for the setxid implementation. */ + if (__is_internal_signal (signo)) + return EINVAL; + + return __pthread_kill_internal (threadid, signo); +} /* Some architectures (for instance arm) might pull raise through libgcc, so avoid the symbol version if it ends up being used on ld.so. */ #if !IS_IN(rtld) From patchwork Thu May 27 17:28:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1484747 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=YLbI0YKO; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FrZYl3P44z9sT6 for ; Fri, 28 May 2021 03:29:15 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2D105395B839; Thu, 27 May 2021 17:28:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2D105395B839 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622136523; bh=o2eS4AfUTQYAL1vRALavhsT4l3P6Hdm4Ms1WTR5ehoY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=YLbI0YKO1KUSUEcauaL5WuUEuSWhzkRzeaifpWBaDrUHRKMPp3qYnZ2JL1H3MNe6L XuGap4M3nfO1kQnd+wWo59EverSffLrJfoXAmdjsf80F0mL+enIeZDQzviAeZffFPX uLlKFR6RYbRZktNay/6B20DW6Sw6B2WBjyqE/0wo= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x736.google.com (mail-qk1-x736.google.com [IPv6:2607:f8b0:4864:20::736]) by sourceware.org (Postfix) with ESMTPS id 1305B395A00B for ; Thu, 27 May 2021 17:28:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1305B395A00B Received: by mail-qk1-x736.google.com with SMTP id q10so1425896qkc.5 for ; Thu, 27 May 2021 10:28:40 -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:mime-version:content-transfer-encoding; bh=o2eS4AfUTQYAL1vRALavhsT4l3P6Hdm4Ms1WTR5ehoY=; b=ugVijWDs5xtz3CMJ6jRT2TbyoFfnGio/szw35Xn3PKwHdL8fXyIMS/lmgmtO35DTgj yKcRZ4p06ia9a0+BXrS5vZCduAHHPKWSL9xqFQA+mza1IQ+gDWwyXoyAng2CTjbjRVrC YdVnfl8tAjTFZXhzSfm22oPbpN2c2iB0NortUx2H3jFPi808kohRu2xJUKme8wVerXra IuGU4e1f8r5Yn/0QpzH/fgX1S5jA7UZ6+uBzAq+KzKJ9DyS1qeIApUfJY0m1JATx0S7d dcx0LrUFgotZ6Zpp3Jq+quyKz72IQYz2vic5q8HOGwGQlegr9GRZSE7/Y1hDTCYvh5d3 t/mQ== X-Gm-Message-State: AOAM533S5gcKA7+F6AtFzW5ssTV9sgsDc6Q40eN4yTF9ZsYlyWMou638 h9spFkerzaTjKb7CgDsJzZ/N5KJH6t6PEg== X-Google-Smtp-Source: ABdhPJzSIOhObBso/1Gxvo6Y+6VCOqFC7kO8uF/2sEjj17C2pRUbJnETS0DGVR/yCGC7o6lhDOSGNg== X-Received: by 2002:a05:620a:a48:: with SMTP id j8mr4684628qka.135.1622136519537; Thu, 27 May 2021 10:28:39 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.38 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:39 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 9/9] nptl: Avoid async cancellation to wrongly update __nptl_nthreads (BZ #19366) Date: Thu, 27 May 2021 14:28:23 -0300 Message-Id: <20210527172823.3461314-10-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" The testcase provided on BZ#19366 may update __nptl_nthreads in a wrong order, triggering an early process exit because the thread decrement the value twice. The issue is once the thread exits without acting on cancellation, it decreaments '__nptl_nthreads' and then atomically set 'cancelhandling' with EXITING_BIT (thus preventing further cancellation handler to act). The issue happens if a SIGCANCEL is received between checking '__ntpl_nthreads' and setting EXITING_BIT. To avoid it, the '__nptl_nthreads' decrement is moved after EXITING_BIT. It does not fully follow the POSIX XSH 2.9.5 Thread Cancellation under the heading Thread Cancellation Cleanup Handlers that states that when a cancellation request is acted upon, or when a thread calls pthread_exit(), the thread first disables cancellation by setting its cancelability state to PTHREAD_CANCEL_DISABLE and its cancelability type to PTHREAD_CANCEL_DEFERRED. The issue is '__pthread_enable_asynccancel' explicit enabled assynchrnous cancellation, so an interrupted syscall within the cancellation cleanup handlers might see an invalid cancelling type (a possible fix might be possible with my proposed solution to BZ#12683). Trying to come up with a test is quite hard since it requires to mimic the timing issue described below, however I see that the buz report reproducer does not early exit anymore. Checked on x86_64-linux-gnu. --- nptl/pthread_create.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 15ce5ad4a1..79af3412cf 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -442,13 +442,6 @@ start_thread (void *arg) /* Clean up any state libc stored in thread-local variables. */ __libc_thread_freeres (); - /* If this is the last thread we terminate the process now. We - do not notify the debugger, it might just irritate it if there - is no thread left. */ - if (__glibc_unlikely (atomic_decrement_and_test (&__nptl_nthreads))) - /* This was the last thread. */ - exit (0); - /* Report the death of the thread if this is wanted. */ if (__glibc_unlikely (pd->report_events)) { @@ -483,6 +476,10 @@ start_thread (void *arg) the breakpoint reports TD_THR_RUN state rather than TD_THR_ZOMBIE. */ atomic_bit_set (&pd->cancelhandling, EXITING_BIT); + if (__glibc_unlikely (atomic_decrement_and_test (&__nptl_nthreads))) + /* This was the last thread. */ + exit (0); + #ifndef __ASSUME_SET_ROBUST_LIST /* If this thread has any robust mutexes locked, handle them now. */ # if __PTHREAD_MUTEX_HAVE_PREV