From patchwork Thu Jun 10 19:36:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43824 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E8B25399C00C for ; Thu, 10 Jun 2021 19:37:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E8B25399C00C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1623353871; bh=6haGXzMGV0mzuWBtM1Qb/ej2e/1AgPI8weiC+LwPBME=; 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=DbReFWZNS3wapEiysFghfkRYUDOFnkviqr8vd+ZkJTAUY0zLoQy+sZOCVeIXB48ON dJVdxDRozGJJzitiscTrjheW0dYYKKrVNPVCWQL5w59mArZxUgD2SdqexAmBefJGp+ dT8evnBE2erjTCH4iUkmtmtxDEOBv4gBz7j7UbvA= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) by sourceware.org (Postfix) with ESMTPS id 635B8386FC1A for ; Thu, 10 Jun 2021 19:36:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 635B8386FC1A Received: by mail-qv1-xf36.google.com with SMTP id im10so14471717qvb.3 for ; Thu, 10 Jun 2021 12:36:46 -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=6haGXzMGV0mzuWBtM1Qb/ej2e/1AgPI8weiC+LwPBME=; b=TUwotAiUh/ZGgMdRDxXR4FuX8fdeXoyap2baoBGMEOwTDEdn7NMimIamExAlKBzP+z GZIf1lnKcgSIbq0WV4UkSnTfdT6oXfE/9L4R0lMDMW/2mpqf83WKP+auh4876yCaVjiX Cfn8G0BAgyv2s6vzV/3khsO2irS1yH6Q5AYfwpAksXF8nR4322aGqA9ZMsJxyWox+Y06 HuqHB6tPvrBXYgBDWYETpI5ZfBFnT4/y8Y/WK5UKFvk1RgbNPtdHJ01pXaDlo5c+i9vE SjRB59hMbf9tMMunhTDBmMHW4wG3lbwvn4FezH8FfVizy5CDvertErpj4lGQnJvDsAz5 O/lA== X-Gm-Message-State: AOAM530kl7PvbvNR6AE3f9WK7Fo1kG+pPN2gkYoROZAdAJhQV+f7VRkg pV4i9D6OY/gr/vXHx8moOulIp2edCGM5pw== X-Google-Smtp-Source: ABdhPJy444LHF6fC42/f4xJFgEX+htgo9qCcz7OjSkREqIpclv1MwPV1tRQdxlJ3RrWHPFsSmQeRsQ== X-Received: by 2002:a05:6214:260f:: with SMTP id gu15mr1254798qvb.21.1623353805721; Thu, 10 Jun 2021 12:36:45 -0700 (PDT) Received: from birita.. ([177.194.59.218]) by smtp.googlemail.com with ESMTPSA id i21sm3012583qkl.20.2021.06.10.12.36.44 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 12:36:45 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 1/6] nptl: Fix tst-cancel7 and tst-cancelx7 race condition (BZ #14232) Date: Thu, 10 Jun 2021 16:36:34 -0300 Message-Id: <20210610193639.3650754-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> References: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.3 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 Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" A named semaphore is used to synchronize the pid information on the created file, the semaphore is updated once the file contents is flushed. Checked on x86_64-linux-gnu. --- nptl/Makefile | 2 + nptl/tst-cancel7.c | 96 +++++++++++++++++++++------------------------- 2 files changed, 45 insertions(+), 53 deletions(-) diff --git a/nptl/Makefile b/nptl/Makefile index 3e6cf0c21b..0ad9e24b0f 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -539,6 +539,8 @@ else librt = $(common-objpfx)rt/librt.a endif +$(objpfx)tst-cancel7: $(librt) +$(objpfx)tst-cancelx7: $(librt) $(objpfx)tst-cancel17: $(librt) $(objpfx)tst-cancelx17: $(librt) diff --git a/nptl/tst-cancel7.c b/nptl/tst-cancel7.c index 7a1870ac74..e69fa7e7af 100644 --- a/nptl/tst-cancel7.c +++ b/nptl/tst-cancel7.c @@ -18,44 +18,45 @@ #include #include -#include +#include #include -#include #include -#include -#include -#include +#include +#include +#include +#include +#include +#include #include -const char *command; -const char *pidfile; -char pidfilename[] = "/tmp/tst-cancel7-XXXXXX"; +static const char *command; +static const char *pidfile; +static char *pidfilename; + +static const char sem_name[] = "/tst-cancel7-sem"; +static sem_t *sem; static void * tf (void *arg) { - const char *args = " --direct --pidfile "; - char *cmd = alloca (strlen (command) + strlen (args) - + strlen (pidfilename) + 1); - - strcpy (stpcpy (stpcpy (cmd, command), args), pidfilename); + char *cmd = xasprintf ("%s --direct --pidfile %s", command, pidfilename); system (cmd); /* This call should never return. */ return NULL; } - static void sl (void) { - FILE *f = fopen (pidfile, "w"); - if (f == NULL) - exit (1); + FILE *f = xfopen (pidfile, "w"); fprintf (f, "%lld\n", (long long) getpid ()); fflush (f); + if (sem_post (sem) != 0) + FAIL_EXIT1 ("sem_post: %m"); + struct flock fl = { .l_type = F_WRLCK, @@ -64,7 +65,7 @@ sl (void) .l_len = 1 }; if (fcntl (fileno (f), F_SETLK, &fl) != 0) - exit (1); + FAIL_EXIT1 ("fcntl (F_SETFL): %m"); sigset_t ss; sigfillset (&ss); @@ -76,57 +77,51 @@ sl (void) static void do_prepare (int argc, char *argv[]) { + sem = sem_open (sem_name, O_RDWR | O_CREAT | O_EXCL | O_TRUNC, 0666, 0); + if (sem == SEM_FAILED) + { + if (errno != EEXIST) + FAIL_EXIT1 ("sem_open failed: %m"); + sem = sem_open (sem_name, O_RDWR); + if (sem == SEM_FAILED) + FAIL_EXIT1 ("sem_open failed: %m"); + } + if (command == NULL) command = argv[0]; if (pidfile) sl (); - int fd = mkstemp (pidfilename); + int fd = create_temp_file ("tst-cancel7-pid-", &pidfilename); if (fd == -1) - { - puts ("mkstemp failed"); - exit (1); - } + FAIL_EXIT1 ("create_temp_file failed: %m"); - write (fd, " ", 1); - close (fd); + xwrite (fd, " ", 1); + xclose (fd); } static int do_test (void) { - pthread_t th; - if (pthread_create (&th, NULL, tf, NULL) != 0) - { - puts ("pthread_create failed"); - return 1; - } + pthread_t th = xpthread_create (NULL, tf, NULL); do - sleep (1); + nanosleep (&(struct timespec) { .tv_sec = 0, .tv_nsec = 100000000 }, NULL); while (access (pidfilename, R_OK) != 0); xpthread_cancel (th); void *r = xpthread_join (th); - sleep (1); + if (sem_wait (sem) != 0) + FAIL_EXIT1 ("sem_wait: %m"); - FILE *f = fopen (pidfilename, "r+"); - if (f == NULL) - { - puts ("no pidfile"); - return 1; - } + FILE *f = xfopen (pidfilename, "r+"); long long ll; if (fscanf (f, "%lld\n", &ll) != 1) - { - puts ("could not read pid"); - unlink (pidfilename); - return 1; - } + FAIL_EXIT1 ("fscanf: %m"); struct flock fl = { @@ -136,11 +131,7 @@ do_test (void) .l_len = 1 }; if (fcntl (fileno (f), F_GETLK, &fl) != 0) - { - puts ("F_GETLK failed"); - unlink (pidfilename); - return 1; - } + FAIL_EXIT1 ("fcntl: %m"); if (fl.l_type != F_UNLCK) { @@ -148,13 +139,12 @@ do_test (void) if (fl.l_pid == ll) kill (fl.l_pid, SIGKILL); - unlink (pidfilename); return 1; } - fclose (f); + xfclose (f); - unlink (pidfilename); + sem_unlink (sem_name); return r != PTHREAD_CANCELED; } @@ -181,7 +171,7 @@ do_cleanup (void) fclose (f); } - unlink (pidfilename); + sem_unlink (sem_name); } #define OPT_COMMAND 10000 From patchwork Thu Jun 10 19:36:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43825 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 88F7339A1422 for ; Thu, 10 Jun 2021 19:38:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 88F7339A1422 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1623353923; bh=ApbKBYajyFnS7Zvc1A01I/p+VLgwXFtdG0KI3ooIaEk=; 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=lxAlD7vD7Ierw1NU9qIxPWB3DJczay/LDLmL33w9CNuAsJhR7yDw/9j7EMde8rAW0 GluWX4QSgRoKChfCl/WPOwlOoFvmk7zNAGVZfg+XxrmBnqjDC/954cF++O667q4jcN 3jzG6yO1sd+BZ+gX5apd59EM5kWF4H8hCdEHNBno= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) by sourceware.org (Postfix) with ESMTPS id A63F83938C12 for ; Thu, 10 Jun 2021 19:36:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A63F83938C12 Received: by mail-qv1-xf32.google.com with SMTP id u13so15332903qvt.7 for ; Thu, 10 Jun 2021 12:36:47 -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=ApbKBYajyFnS7Zvc1A01I/p+VLgwXFtdG0KI3ooIaEk=; b=uAODP+b1j9xVy3UXH1pVUcRkvVrT2B/uDHpDoXsoNqfpRS0rUCJtvmEm+yLVu/P8Z1 JLbRaeyQFH1xCzp7JXw/COkWyGVkpYJAGi9xoopha7uZE/gmdajoKEUmyTBN8nHANrvd t/lCEKWHoRL/gbyqR9jbe0imsXAwOwUEsHtiBaF8zVYCan8U601XUvH8rvDsEYn8rpPG RiduJ+5gLz+J/OiDHwZhgWP2EFHUpTGJ3rGnBKlkQm1OKQevkP9UUtzX54KfGYZ60XMw Z2QhCfnHUeTXhWTyj5apucL+35XhLuRbYv427GxWKXPWexRkC3cjTVxP0NIk7rzc2siY BlyA== X-Gm-Message-State: AOAM531pim1ODSZkd/igyudRtiGYsKIDgLtwJU2bosaMx7nqfDzPeSQS Ndz+xOJLkIznv4f+dZqbvRj4CgVR5G5l8A== X-Google-Smtp-Source: ABdhPJyn6PeMT2zJhEoO2DPKLIvfWoivGKBMa4734Tby3j+81fljbtPgFbJkSn2fgLICILUwJYJWzg== X-Received: by 2002:a05:6214:10cc:: with SMTP id r12mr1252773qvs.21.1623353807000; Thu, 10 Jun 2021 12:36:47 -0700 (PDT) Received: from birita.. ([177.194.59.218]) by smtp.googlemail.com with ESMTPSA id i21sm3012583qkl.20.2021.06.10.12.36.46 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 12:36:46 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 2/6] nptl: Set cancellation type and state on pthread_exit Date: Thu, 10 Jun 2021 16:36:35 -0300 Message-Id: <20210610193639.3650754-3-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> References: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.3 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 Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" It is required by POSIX XSH 2.9.5 Thread Cancellation under the heading Thread Cancellation Cleanup Handlers. Checked x86_64-linux-gnu. --- nptl/Makefile | 3 +- nptl/cancellation.c | 7 +- nptl/pthreadP.h | 18 ++++- nptl/pthread_cancel.c | 1 - nptl/pthread_exit.c | 4 +- nptl/pthread_testcancel.c | 1 - nptl/tst-cleanup5.c | 157 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 180 insertions(+), 11 deletions(-) create mode 100644 nptl/tst-cleanup5.c diff --git a/nptl/Makefile b/nptl/Makefile index 0ad9e24b0f..0dd139a53d 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -307,7 +307,8 @@ tests = tst-attr2 tst-attr3 tst-default-attr \ tst-pthread-gdb-attach tst-pthread-gdb-attach-static \ tst-pthread_exit-nothreads \ tst-pthread_exit-nothreads-static \ - tst-thread-setspecific + tst-thread-setspecific \ + tst-cleanup5 tests-nolibpthread = \ tst-pthread_exit-nothreads \ diff --git a/nptl/cancellation.c b/nptl/cancellation.c index 05962784d5..6478c029de 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -42,7 +42,6 @@ __pthread_enable_asynccancel (void) && !(ch & EXITING_BITMASK) && !(ch & TERMINATED_BITMASK)) { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); } @@ -64,3 +63,9 @@ __pthread_disable_asynccancel (int oldtype) self->canceltype = PTHREAD_CANCEL_DEFERRED; } libc_hidden_def (__pthread_disable_asynccancel) + +void +__do_cancel (void) +{ + __exit_thread (PTHREAD_CANCELED); +} diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 675d1de753..ef0b367120 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -268,20 +268,30 @@ extern void __pthread_unregister_cancel (__pthread_unwind_buf_t *__buf) __cleanup_fct_attribute; libc_hidden_proto (__pthread_unregister_cancel) -/* Called when a thread reacts on a cancellation request. */ -static inline void -__attribute ((noreturn, always_inline)) -__do_cancel (void) +static _Noreturn inline void +__exit_thread (void *value) { struct pthread *self = THREAD_SELF; /* Make sure we get no more cancellations. */ THREAD_ATOMIC_BIT_SET (self, cancelhandling, EXITING_BIT); + THREAD_SETMEM (self, result, value); + + /* It is required by POSIX XSH 2.9.5 Thread Cancellation under the heading + Thread Cancellation Cleanup Handlers and also avoid further cancellation + wrapper to act on cancellation. */ + THREAD_SETMEM (self, cancelstate, PTHREAD_CANCEL_DISABLE); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); + __pthread_unwind ((__pthread_unwind_buf_t *) THREAD_GETMEM (self, cleanup_jmp_buf)); } +/* It is a wrapper over __exit_thread (PTHREAD_CANCELED). It is has its own + implementation because it might be called by arch-specific asm code. */ +_Noreturn void __do_cancel (void) attribute_hidden; + /* Internal prototypes. */ diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 0698cd2046..577ff6c746 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -103,7 +103,6 @@ __pthread_cancel (pthread_t th) __libc_multiple_threads = 1; #endif - THREAD_SETMEM (pd, result, PTHREAD_CANCELED); if (pd->cancelstate == PTHREAD_CANCEL_ENABLE && pd->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __do_cancel (); diff --git a/nptl/pthread_exit.c b/nptl/pthread_exit.c index 6abf66463e..978f4042ef 100644 --- a/nptl/pthread_exit.c +++ b/nptl/pthread_exit.c @@ -32,9 +32,7 @@ __pthread_exit (void *value) " must be installed for pthread_exit to work\n"); } - THREAD_SETMEM (THREAD_SELF, result, value); - - __do_cancel (); + __exit_thread (value); } libc_hidden_def (__pthread_exit) weak_alias (__pthread_exit, pthread_exit) diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c index 920374643a..e6adf12580 100644 --- a/nptl/pthread_testcancel.c +++ b/nptl/pthread_testcancel.c @@ -30,7 +30,6 @@ ___pthread_testcancel (void) && !(cancelhandling & EXITING_BITMASK) && !(cancelhandling & TERMINATED_BITMASK)) { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); } } diff --git a/nptl/tst-cleanup5.c b/nptl/tst-cleanup5.c new file mode 100644 index 0000000000..6cc1c141e9 --- /dev/null +++ b/nptl/tst-cleanup5.c @@ -0,0 +1,157 @@ +/* Check if cancellation state and type is correctly set on thread exit. + Copyright (C) 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 +#include +#include + +static int pipefds[2]; +static pthread_barrier_t b; + +static void +clh (void *arg) +{ + /* Although POSIX state setting either the cancellation state or type is + undefined during cleanup handler execution, both calls should be safe + since none has any side-effect (they should not change current state + neither trigger a pending cancellation). */ + + int state; + TEST_VERIFY (pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &state) == 0); + TEST_COMPARE (state, PTHREAD_CANCEL_DISABLE); + + int type; + TEST_VERIFY (pthread_setcanceltype (PTHREAD_CANCEL_DEFERRED, &type) == 0); + TEST_COMPARE (type, PTHREAD_CANCEL_DEFERRED); +} + +/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on + pthread_cleanup_pop sets the correct state and type as pthread_exit. */ +static void * +tf_cancel_deferred (void *arg) +{ + xpthread_barrier_wait (&b); + + pthread_cleanup_push (clh, NULL); + + char c; + read (pipefds[0], &c, 1); + + pthread_cleanup_pop (1); + + return NULL; +} + +/* Check if a thread with PTHREAD_CANCEL_ASYNCHRONOUS cancellation on + blocked read() sets the correct state and type as pthread_exit. */ +static void * +tf_cancel_async (void *arg) +{ + TEST_VERIFY (pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, NULL) + == 0); + + xpthread_barrier_wait (&b); + + pthread_cleanup_push (clh, NULL); + + char c; + read (pipefds[0], &c, 1); + + pthread_cleanup_pop (1); + + return NULL; +} + +/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on + blocked read() sets the correct state and type as pthread_exit. */ +static void * +tf_testcancel (void *arg) +{ + xpthread_barrier_wait (&b); + + pthread_cleanup_push (clh, NULL); + + char c; + read (pipefds[0], &c, 1); + + pthread_testcancel (); + + pthread_cleanup_pop (1); + + return NULL; +} + +#define EXIT_EXPECTED_VALUE ((void *) 42) + +/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on + pthread_exit() sets the correct state and type. */ +static void * +tf_exit (void *arg) +{ + xpthread_barrier_wait (&b); + + pthread_cleanup_push (clh, NULL); + + pthread_exit (EXIT_EXPECTED_VALUE); + + pthread_cleanup_pop (1); + + return NULL; +} + +static int +do_test (void) +{ + xpipe (pipefds); + + xpthread_barrier_init (&b, NULL, 2); + { + pthread_t th = xpthread_create (NULL, tf_cancel_deferred, NULL); + xpthread_barrier_wait (&b); + xpthread_cancel (th); + void *r = xpthread_join (th); + TEST_VERIFY (r == PTHREAD_CANCELED); + } + + { + pthread_t th = xpthread_create (NULL, tf_cancel_async, NULL); + xpthread_barrier_wait (&b); + xpthread_cancel (th); + void *r = xpthread_join (th); + TEST_VERIFY (r == PTHREAD_CANCELED); + } + + { + pthread_t th = xpthread_create (NULL, tf_testcancel, NULL); + xpthread_barrier_wait (&b); + xpthread_cancel (th); + void *r = xpthread_join (th); + TEST_VERIFY (r == PTHREAD_CANCELED); + } + + { + pthread_t th = xpthread_create (NULL, tf_exit, NULL); + xpthread_barrier_wait (&b); + void *r = xpthread_join (th); + TEST_VERIFY (r == EXIT_EXPECTED_VALUE); + } + + return 0; +} + +#include From patchwork Thu Jun 10 19:36:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43826 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4E13239A0008 for ; Thu, 10 Jun 2021 19:39:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4E13239A0008 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1623353968; bh=fVKbDUKCj9j0xjXt1w49A4/bPe3tNnOMV2D7nujHxP8=; 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=kw8h92eO4dJm8b0Pug+RxY26Tg7MMAB4VylAyDvPD4sirC+iiwCqPivJ2eeQ7PPxP DZ2cZYY/KCEvp8ZBhfP4TT22789HqBhILXi9CIlxatO81IFEeS18U6ctZ0djG2O/8q vRBU+HJO9kUAbOtMVAiLXfwFEFf1zD4AVaig3yGg= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) by sourceware.org (Postfix) with ESMTPS id CE828399C89B for ; Thu, 10 Jun 2021 19:36:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CE828399C89B Received: by mail-qt1-x82d.google.com with SMTP id t17so755128qta.11 for ; Thu, 10 Jun 2021 12:36:48 -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=fVKbDUKCj9j0xjXt1w49A4/bPe3tNnOMV2D7nujHxP8=; b=Lpu2YtITSwOIh5N8NXIzN48FxcBbUGIrHChSrdnQYD9EhJTIH+ObUnlLkGiihhjwOP BxslqMNRAI+opX+jmErPmXwNLF0ih2OoKOiHlsR+zSjIHaHi59/5KoB8oD3RhECdG+Xx WxWpkthzcxjlIkd4ZXzrsf4XCxh6gDfzWOm/JJ7i7OaEs4W9s1i2NMhKOufuMm+Ij4/X lECwuH5d1kmDFrjAo7mM0K0EUiOUgobVkd4Kwt8AhRSJeBUzo5sT9RX+iJUAEdSuP0YJ Row554zbe8tngu6my3JJKIruCyTPCRkao5OfByxFQ8+dC5T678XMc0I8rvBJZ+Ghnz/d JKUQ== X-Gm-Message-State: AOAM532kcPRSzI0LvadwtsgwZPZasohjmSrR5fIi0XDItJ9/Ikmflgdw vCnSJv5S3s2Lz8IEW1lxfABMb/inPUafNQ== X-Google-Smtp-Source: ABdhPJzs9zCrfobds6SbYHDlHT6f+aE2XWaoJqjqMyoLFdU14JExKT6kMVoznqFE4BILr7F1Uv3ahA== X-Received: by 2002:ac8:664d:: with SMTP id j13mr447053qtp.84.1623353808250; Thu, 10 Jun 2021 12:36:48 -0700 (PDT) Received: from birita.. ([177.194.59.218]) by smtp.googlemail.com with ESMTPSA id i21sm3012583qkl.20.2021.06.10.12.36.47 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 12:36:48 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 3/6] nptl: Handle robust PI mutexes for !__ASSUME_SET_ROBUST_LIST Date: Thu, 10 Jun 2021 16:36:36 -0300 Message-Id: <20210610193639.3650754-4-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> References: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.3 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 Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" The robust PI mutexes are signaled by setting the LSB bit to 1, so the code requires to take this consideration before access the __pthread_mutex_s. The code is also simplified: the initialization code is not really required, PD->robust_head.list and PD->robust_list.__next are essentially the same regardless of __PTHREAD_MUTEX_HAVE_PREV, the futex wake is optimized to be issued only when required, and the futex shared bit is set only when required. Checked on a build for m68k-linux-gnu. I also checked on x86_64-linux-gnu by removing the check for !__ASSUME_SET_ROBUST_LIST. --- nptl/pthread_create.c | 53 ++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 3f017f1e26..8f1a77b5a8 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -478,35 +478,36 @@ start_thread (void *arg) exit (0); #ifndef __ASSUME_SET_ROBUST_LIST - /* If this thread has any robust mutexes locked, handle them now. */ -# if __PTHREAD_MUTEX_HAVE_PREV - void *robust = pd->robust_head.list; -# else - __pthread_slist_t *robust = pd->robust_list.__next; -# endif - /* We let the kernel do the notification if it is able to do so. - If we have to do it here there for sure are no PI mutexes involved - since the kernel support for them is even more recent. */ - if (!__nptl_set_robust_list_avail - && __builtin_expect (robust != (void *) &pd->robust_head, 0)) + /* We let the kernel do the notification if it is able to do so on the exit + syscall. Otherwise we need to handle before the thread terminates. */ + void **robust; + while ((robust = pd->robust_head.list) + && robust != (void *) &pd->robust_head) { - do + /* Note: robust PI futexes are signaled by setting bit 0. */ + void **robustp = (void **) ((uintptr_t) robust & ~1UL); + + struct __pthread_mutex_s *mtx = (struct __pthread_mutex_s *) + ((char *) robustp - offsetof (struct __pthread_mutex_s, + __list.__next)); + unsigned int nusers = mtx->__nusers; + int shared = mtx->__kind & 128; + + pd->robust_head.list_op_pending = robust; + pd->robust_head.list = *robustp; + /* Although the list will not be changed at this point, it follows the + expected kernel ABI. */ + __asm ("" ::: "memory"); + + int lock = atomic_exchange_relaxed (&mtx->__lock, FUTEX_OWNER_DIED); + /* Wake any users if mutex is acquired with potential users. */ + if (lock > 1 || nusers != 0) { - struct __pthread_mutex_s *this = (struct __pthread_mutex_s *) - ((char *) robust - offsetof (struct __pthread_mutex_s, - __list.__next)); - robust = *((void **) robust); - -# if __PTHREAD_MUTEX_HAVE_PREV - this->__list.__prev = NULL; -# endif - this->__list.__next = NULL; - - atomic_or (&this->__lock, FUTEX_OWNER_DIED); - futex_wake ((unsigned int *) &this->__lock, 1, - /* XYZ */ FUTEX_SHARED); + if ((uintptr_t) robust & 1) + futex_unlock_pi ((unsigned int *) &mtx->__lock, shared); + else + futex_wake ((unsigned int *) &mtx->__lock, 1, shared); } - while (robust != (void *) &pd->robust_head); } #endif From patchwork Thu Jun 10 19:36:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43828 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 48B4939A2408 for ; Thu, 10 Jun 2021 19:40:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 48B4939A2408 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1623354058; bh=ee/cVCl5FcoUKi6VuRxc91WJ2q6uk2nVN1G5oy/vj/M=; 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=x+4X8IU0DpLbIJcRTrHO7l16mOufR4EptcDYn/AVEJYApfdY0wpVrmvvskos5sdsp xoogDsErsRTNXIsSoVbNMeVB2ER5bm8ndNJSFCz/AYvQEzVaabjvvK8OVNgHPB4dpX SRUO/Bu3NaWIXsIz6ZYkpqzN31upxr5BkmQaN0FE= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x82a.google.com (mail-qt1-x82a.google.com [IPv6:2607:f8b0:4864:20::82a]) by sourceware.org (Postfix) with ESMTPS id 6BFBC399C8A4 for ; Thu, 10 Jun 2021 19:36:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6BFBC399C8A4 Received: by mail-qt1-x82a.google.com with SMTP id p21so776250qtw.6 for ; Thu, 10 Jun 2021 12:36:50 -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=ee/cVCl5FcoUKi6VuRxc91WJ2q6uk2nVN1G5oy/vj/M=; b=sAshIBHTdlTKx55mjoGhKwB9IFphL7h1lJuiLdsF6TxHoa0rZvf9UInAOG19EJ2jiY +TnsN0+au2lQ2P19aeNsLTOok9fjHJ+Cc2Am375iBw6cyEQ88KZ3iTY2o8sqYHp9hBnL bKfKXObRAwRhgjBzMjm3bC3kxBATXkVXlahXxQtddRHYoiDNKsWVm+qYwkZ+jQcU5Mcw GDUtOlHn0HTovvfk+itDm1in4SluhHWOeXE4QQXExJAe2x/V6Im6tVZhd3hdqTdaX4ft KqYQkm6HeD9CZWBtrd9AFchTqylu2nPbR92kNZAermcgbLNwqbfotPnnsIi6SJgKfK6A JdkQ== X-Gm-Message-State: AOAM5311P/yHTbVVIkHLUcYi0wuv8gwwdNzmjKUhrXc2fIlACorbsdVP e9LVi5/qJ5+XVEx8g3NA+QkuwQc/lpjc6A== X-Google-Smtp-Source: ABdhPJxwG8J2NDTedIzBdRXezDkP4ZtJpF2QFmCuUCmW71up1/l/eNLrpGChVREKBcqGe4FhR50+AQ== X-Received: by 2002:a05:622a:392:: with SMTP id j18mr377436qtx.283.1623353809610; Thu, 10 Jun 2021 12:36:49 -0700 (PDT) Received: from birita.. ([177.194.59.218]) by smtp.googlemail.com with ESMTPSA id i21sm3012583qkl.20.2021.06.10.12.36.48 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 12:36:49 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 4/6] nptl: Do not use pthread set_tid_address as state synchronization (BZ #19951) Date: Thu, 10 Jun 2021 16:36:37 -0300 Message-Id: <20210610193639.3650754-5-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> References: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.4 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 Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" The use after free described in BZ#19951 is due the use of two different PD fields, 'joinid' and 'cancelhandling', to describe the thread state. And any state change potentially requires to check for both fields atomically to potentially handle partial state (such as pthread_join() with a cancellation handler and a joinstate rollback). This patch uses a different PD member with 4 possible states (JOINABLE, DETACHED, EXITING, and EXITED) to synchronize between pthread_create(), pthread_join(), pthread_detached(), and pthread_exit(): 1. On pthread_create() the inital state is set either to JOINABLE or DETACHED depending of the pthread attribute used. 2. On pthread_detach() call, a CAS is issued on the state. If the CAS fails it means that thread is already detached (DETACHED) or is being terminated (EXITING). For former an EINVAL is returned, while for latter pthread_detach() should be reponsible to join the thread (and deallocate the resources). 3. On pthread_create() exit phase (reached either if the thread function has returned, pthread_exit() has being called, or cancellation handled has been acted upon) we issue a CAS on state to set to EXITING Mode. If the thread is previous on DETACHED mode it will be the thread itself responsible to deallocate any resource, otherwise the threads needs to be joined. 4. The clear_tid_field on 'clone' call is changed to set the new 'state' field on thread exit (EXITED). This state ins only reached on thread termination. 4. The pthread_join() implementation is now simpler: the futex wait is done directly on thread state and there is no need to reset it in case of timeout (since the state is now set either by pthread_detach() or by the kernel on process termination). The race condition on pthread_detach is avoided with only one atomic operation on PD state: once the mode is set to THREAD_STATE_DETACHED it is up to thread itself to deallocate its memory (done on the exit phase at pthread_create()). This change trigger an invalid C11 thread tests: it crates a thread, which detaches itself, and after a timeout the creating thread checks if the join fails. The issue is once thrd_join() is called the thread lifetime has already expired. The test is changed so the slee is done by the thread itself, so the creating thread will try to join a valid thread. Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu, arm-linux-gnueabihf, and powerpc64-linux-gnu. --- nptl/descr.h | 26 +++--- nptl/nptl-stack.h | 2 +- nptl/pthreadP.h | 2 +- nptl/pthread_clockjoin.c | 2 +- nptl/pthread_create.c | 48 ++++++---- nptl/pthread_detach.c | 36 +++----- nptl/pthread_getattr_np.c | 2 +- nptl/pthread_join.c | 2 +- nptl/pthread_join_common.c | 131 ++++++++++------------------ nptl/pthread_timedjoin.c | 2 +- nptl/pthread_tryjoin.c | 18 ++-- sysdeps/nptl/dl-tls_init_tp.c | 4 +- sysdeps/nptl/libc_start_call_main.h | 7 ++ sysdeps/pthread/tst-thrd-detach.c | 16 ++-- 14 files changed, 138 insertions(+), 160 deletions(-) diff --git a/nptl/descr.h b/nptl/descr.h index c85778d449..4b2db6edab 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -124,6 +124,18 @@ struct priority_protection_data }; +/* Define a possible thread state on 'joinstate' field. The value will be + cleared by the kernel when the thread terminates (CLONE_CHILD_CLEARTID), + so THREAD_STATE_EXITED must be 0. */ +enum + { + THREAD_STATE_EXITED = 0, + THREAD_STATE_EXITING, + THREAD_STATE_JOINABLE, + THREAD_STATE_DETACHED, + }; + + /* Thread descriptor data structure. */ struct pthread { @@ -166,8 +178,7 @@ struct pthread GL (dl_stack_user) list. */ list_t list; - /* Thread ID - which is also a 'is this thread descriptor (and - therefore stack) used' flag. */ + /* Thread ID set by the kernel with CLONE_PARENT_SETTID. */ pid_t tid; /* Ununsed. */ @@ -335,15 +346,8 @@ struct pthread hp_timing_t cpuclock_offset_ununsed; #endif - /* If the thread waits to join another one the ID of the latter is - stored here. - - In case a thread is detached this field contains a pointer of the - TCB if the thread itself. This is something which cannot happen - in normal operation. */ - struct pthread *joinid; - /* Check whether a thread is detached. */ -#define IS_DETACHED(pd) ((pd)->joinid == (pd)) + /* The current thread state defined by the THREAD_STATE_* enumeration. */ + unsigned int joinstate; /* The result of the thread function. */ void *result; diff --git a/nptl/nptl-stack.h b/nptl/nptl-stack.h index a6bd8df77f..8f3cfbf71e 100644 --- a/nptl/nptl-stack.h +++ b/nptl/nptl-stack.h @@ -29,7 +29,7 @@ static inline bool __nptl_stack_in_use (struct pthread *pd) { - return pd->tid <= 0; + return atomic_load_relaxed (&pd->joinstate) == THREAD_STATE_EXITED; } /* Remove the stack ELEM from its list. */ diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index ef0b367120..bfb0e40b44 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -536,7 +536,7 @@ libc_hidden_proto (__pthread_setcanceltype) extern void __pthread_testcancel (void); libc_hidden_proto (__pthread_testcancel) extern int __pthread_clockjoin_ex (pthread_t, void **, clockid_t, - const struct __timespec64 *, bool) + const struct __timespec64 *) attribute_hidden; extern int __pthread_sigmask (int, const sigset_t *, sigset_t *); libc_hidden_proto (__pthread_sigmask); diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c index f5007d7831..8798f26fcf 100644 --- a/nptl/pthread_clockjoin.c +++ b/nptl/pthread_clockjoin.c @@ -30,7 +30,7 @@ ___pthread_clockjoin_np64 (pthread_t threadid, void **thread_return, return EINVAL; return __pthread_clockjoin_ex (threadid, thread_return, - clockid, abstime, true); + clockid, abstime); } #if __TIMESIZE == 64 diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 8f1a77b5a8..0c0f2c03e5 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -283,7 +283,8 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr, # define ARCH_CLONE __clone #endif if (__glibc_unlikely (ARCH_CLONE (&start_thread, STACK_VARIABLES_ARGS, - clone_flags, pd, &pd->tid, tp, &pd->tid) + clone_flags, pd, &pd->tid, tp, + &pd->joinstate) == -1)) return errno; @@ -343,7 +344,7 @@ start_thread (void *arg) and free any resource prior return to the pthread_create caller. */ setup_failed = pd->setup_failed == 1; if (setup_failed) - pd->joinid = NULL; + pd->joinstate = THREAD_STATE_JOINABLE; /* And give it up right away. */ lll_unlock (pd->lock, LLL_PRIVATE); @@ -473,9 +474,20 @@ 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); + + /* CONCURRENCY NOTES: + + Concurrent pthread_detach() will either set state to + THREAD_STATE_DETACHED or wait the thread to terminate. The exiting state + set here is set so a pthread_join() wait until all the required cleanup + steps are done. + + The 'joinstate' field will be used to determine who is responsible to + call __nptl_free_tcb below. */ + + unsigned int joinstate = THREAD_STATE_JOINABLE; + atomic_compare_exchange_weak_acquire (&pd->joinstate, &joinstate, + THREAD_STATE_EXITING); #ifndef __ASSUME_SET_ROBUST_LIST /* We let the kernel do the notification if it is able to do so on the exit @@ -511,6 +523,10 @@ start_thread (void *arg) } #endif + if (__glibc_unlikely (atomic_decrement_and_test (&__nptl_nthreads))) + /* This was the last thread. */ + exit (0); + if (!pd->user_stack) advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd, pd->guardsize); @@ -531,9 +547,7 @@ start_thread (void *arg) pd->setxid_futex = 0; } - /* If the thread is detached free the TCB. */ - if (IS_DETACHED (pd)) - /* Free the TCB. */ + if (joinstate == THREAD_STATE_DETACHED) __nptl_free_tcb (pd); out: @@ -541,7 +555,7 @@ out: The 'exit' implementation in the kernel will signal when the process is really dead since 'clone' got passed the CLONE_CHILD_CLEARTID - flag. The 'tid' field in the TCB will be set to zero. + flag. The 'joinstate' field in the TCB will be set to zero. The exit code is zero since in case all threads exit by calling 'pthread_exit' the exit status must be 0 (zero). */ @@ -635,10 +649,9 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, pd->flags = ((iattr->flags & ~(ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET)) | (self->flags & (ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET))); - /* Initialize the field for the ID of the thread which is waiting - for us. This is a self-reference in case the thread is created - detached. */ - pd->joinid = iattr->flags & ATTR_FLAG_DETACHSTATE ? pd : NULL; + pd->joinstate = iattr->flags & ATTR_FLAG_DETACHSTATE + ? THREAD_STATE_DETACHED + : THREAD_STATE_JOINABLE; /* The debug events are inherited from the parent. */ pd->eventbuf = self->eventbuf; @@ -797,10 +810,11 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, /* Similar to pthread_join, but since thread creation has failed at startup there is no need to handle all the steps. */ - pid_t tid; - while ((tid = atomic_load_acquire (&pd->tid)) != 0) - __futex_abstimed_wait_cancelable64 ((unsigned int *) &pd->tid, - tid, 0, NULL, LLL_SHARED); + unsigned int state; + while ((state = atomic_load_acquire (&pd->joinstate)) + != THREAD_STATE_EXITED) + __futex_abstimed_wait_cancelable64 (&pd->joinstate, state, 0, + NULL, LLL_SHARED); } /* State (c) or (d) and we have ownership of PD (see CONCURRENCY diff --git a/nptl/pthread_detach.c b/nptl/pthread_detach.c index ac50db9b0e..97d292cab8 100644 --- a/nptl/pthread_detach.c +++ b/nptl/pthread_detach.c @@ -26,32 +26,24 @@ ___pthread_detach (pthread_t th) { struct pthread *pd = (struct pthread *) th; - /* Make sure the descriptor is valid. */ - if (INVALID_NOT_TERMINATED_TD_P (pd)) - /* Not a valid thread handle. */ - return ESRCH; + /* CONCURRENCY NOTES: - int result = 0; + Concurrent pthread_detach() will return EINVAL for the case the thread + is already detached (THREAD_STATE_DETACHED). POSIX states it is + undefined to call pthread_detach if TH refers to a non joinable thread. - /* Mark the thread as detached. */ - if (atomic_compare_and_exchange_bool_acq (&pd->joinid, pd, NULL)) + For the case the thread is being terminated (THREAD_STATE_EXITING), + pthread_detach() will responsible to clean up the stack. */ + + int curstate = THREAD_STATE_JOINABLE; + if (!atomic_compare_exchange_weak_acquire (&pd->joinstate, &curstate, + THREAD_STATE_DETACHED)) { - /* There are two possibilities here. First, the thread might - already be detached. In this case we return EINVAL. - Otherwise there might already be a waiter. The standard does - not mention what happens in this case. */ - if (IS_DETACHED (pd)) - result = EINVAL; + if (curstate == THREAD_STATE_DETACHED) + return EINVAL; + return __pthread_join (th, 0); } - else - /* Check whether the thread terminated meanwhile. In this case we - will just free the TCB. */ - if ((pd->cancelhandling & EXITING_BITMASK) != 0) - /* Note that the code in __free_tcb makes sure each thread - control block is freed only once. */ - __nptl_free_tcb (pd); - - return result; + return 0; } versioned_symbol (libc, ___pthread_detach, pthread_detach, GLIBC_2_34); libc_hidden_ver (___pthread_detach, __pthread_detach) diff --git a/nptl/pthread_getattr_np.c b/nptl/pthread_getattr_np.c index 25807cb529..560ba9ab98 100644 --- a/nptl/pthread_getattr_np.c +++ b/nptl/pthread_getattr_np.c @@ -53,7 +53,7 @@ __pthread_getattr_np (pthread_t thread_id, pthread_attr_t *attr) iattr->flags = thread->flags; /* The thread might be detached by now. */ - if (IS_DETACHED (thread)) + if (atomic_load_acquire (&thread->joinstate) == THREAD_STATE_DETACHED) iattr->flags |= ATTR_FLAG_DETACHSTATE; /* This is the guardsize after adjusting it. */ diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c index d2b33de73d..195a537029 100644 --- a/nptl/pthread_join.c +++ b/nptl/pthread_join.c @@ -23,7 +23,7 @@ int ___pthread_join (pthread_t threadid, void **thread_return) { return __pthread_clockjoin_ex (threadid, thread_return, 0 /* Ignored */, - NULL, true); + NULL); } versioned_symbol (libc, ___pthread_join, pthread_join, GLIBC_2_34); libc_hidden_ver (___pthread_join, __pthread_join) diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index 7303069316..0870f36ecb 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -22,113 +22,74 @@ #include #include -static void -cleanup (void *arg) +/* Check for a possible deadlock situation where 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 structures but it is not + necessary. In the unlikely case that two threads are really caught in this + situation they will deadlock. It is the programmer's problem to figure + this out. */ +static inline bool +check_for_deadlock (int state, struct pthread *pd) { - /* If we already changed the waiter ID, reset it. The call cannot - fail for any reason but the thread not having done that yet so - there is no reason for a loop. */ struct pthread *self = THREAD_SELF; - atomic_compare_exchange_weak_acquire (&arg, &self, NULL); + return ((pd == self + || (atomic_load_acquire (&self->joinstate) == THREAD_STATE_DETACHED + && (pd->cancelhandling + & (CANCELED_BITMASK | EXITING_BITMASK + | TERMINATED_BITMASK)) == 0)) + && !(self->cancelstate == PTHREAD_CANCEL_ENABLE + && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK + | TERMINATED_BITMASK)) + == CANCELED_BITMASK)); } int __pthread_clockjoin_ex (pthread_t threadid, void **thread_return, clockid_t clockid, - const struct __timespec64 *abstime, bool block) + const struct __timespec64 *abstime) { struct pthread *pd = (struct pthread *) threadid; - /* Make sure the descriptor is valid. */ - if (INVALID_NOT_TERMINATED_TD_P (pd)) - /* Not a valid thread handle. */ - return ESRCH; - - /* Is the thread joinable?. */ - if (IS_DETACHED (pd)) - /* We cannot wait for the thread. */ - return EINVAL; - - struct pthread *self = THREAD_SELF; - int result = 0; - LIBC_PROBE (pthread_join, 1, threadid); - if ((pd == self - || (self->joinid == pd - && (pd->cancelhandling - & (CANCELED_BITMASK | EXITING_BITMASK - | TERMINATED_BITMASK)) == 0)) - && !(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 - structures but it is not necessary. In the unlikely case that - two threads are really caught in this situation they will - deadlock. It is the programmer's problem to figure this - out. */ - return EDEADLK; - - /* Wait for the thread to finish. If it is already locked something - is wrong. There can only be one waiter. */ - else if (__glibc_unlikely (atomic_compare_exchange_weak_acquire (&pd->joinid, - &self, - NULL))) - /* There is already somebody waiting for the thread. */ - return EINVAL; - - /* BLOCK waits either indefinitely or based on an absolute time. POSIX also - states a cancellation point shall occur for pthread_join, and we use the - same rationale for posix_timedjoin_np. Both clockwait_tid and the futex - call use the cancellable variant. */ - if (block) + int result = 0; + unsigned int state; + while ((state = atomic_load_acquire (&pd->joinstate)) + != THREAD_STATE_EXITED) { - /* During the wait we change to asynchronous cancellation. If we - are cancelled the thread we are waiting for must be marked as - un-wait-ed for again. */ - pthread_cleanup_push (cleanup, &pd->joinid); - - /* We need acquire MO here so that we synchronize with the - kernel's store to 0 when the clone terminates. (see above) */ - pid_t tid; - while ((tid = atomic_load_acquire (&pd->tid)) != 0) - { - /* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via - futex wake-up when the clone terminates. The memory location - contains the thread ID while the clone is running and is reset to - zero by the kernel afterwards. The kernel up to version 3.16.3 - does not use the private futex operations for futex wake-up when - the clone terminates. */ - int ret = __futex_abstimed_wait_cancelable64 ( - (unsigned int *) &pd->tid, tid, clockid, abstime, LLL_SHARED); - if (ret == ETIMEDOUT || ret == EOVERFLOW) - { - result = ret; - break; - } + if (check_for_deadlock (state, pd)) + return EDEADLK; + + /* POSIX states calling pthread_join() on a non joinable thread is + undefined. However, if PD is still in the cache we can still warn + the caller. */ + if (state == THREAD_STATE_DETACHED) + return EINVAL; + + /* pthread_join() is a cancellation entrypoint and we use the same + rationale for pthread_timedjoin_np(). + + The kernel notifies a process which uses CLONE_CHILD_CLEARTID via + a memory zerouing and futex wake up when the process terminates. + The futex operation is not private. */ + int ret = __futex_abstimed_wait_cancelable64 (&pd->joinstate, state, + clockid, abstime, + LLL_SHARED); + if (ret == ETIMEDOUT || ret == EOVERFLOW) + { + result = ret; + break; } - - pthread_cleanup_pop (0); } void *pd_result = pd->result; - if (__glibc_likely (result == 0)) + if (result == 0) { - /* We mark the thread as terminated and as joined. */ - pd->tid = -1; - - /* Store the return value if the caller is interested. */ + pd->tid = 0; if (thread_return != NULL) *thread_return = pd_result; - - /* Free the TCB. */ __nptl_free_tcb (pd); } - else - pd->joinid = NULL; LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd_result); diff --git a/nptl/pthread_timedjoin.c b/nptl/pthread_timedjoin.c index ebc31f935a..b35c8f4279 100644 --- a/nptl/pthread_timedjoin.c +++ b/nptl/pthread_timedjoin.c @@ -25,7 +25,7 @@ ___pthread_timedjoin_np64 (pthread_t threadid, void **thread_return, const struct __timespec64 *abstime) { return __pthread_clockjoin_ex (threadid, thread_return, - CLOCK_REALTIME, abstime, true); + CLOCK_REALTIME, abstime); } #if __TIMESIZE == 64 diff --git a/nptl/pthread_tryjoin.c b/nptl/pthread_tryjoin.c index fd938e8780..726f2abc68 100644 --- a/nptl/pthread_tryjoin.c +++ b/nptl/pthread_tryjoin.c @@ -22,15 +22,17 @@ int __pthread_tryjoin_np (pthread_t threadid, void **thread_return) { - /* Return right away if the thread hasn't terminated yet. */ - struct pthread *pd = (struct pthread *) threadid; - if (pd->tid != 0) - return EBUSY; + /* The joinable state (THREAD_STATE_JOINABLE) is straigthforward since the + thread hasn't finished yet and trying to join might block. + Both detached (THREAD_STATE_DETACHED) and exiting (THREAD_STATE_EXITING) + might also result in a possible blocking call: a detached thread might + change its state to exiting and a exiting thread my take some time to + exit (and thus let the kernel set the state to THREAD_STATE_EXITED). */ - /* If pd->tid == 0 then lll_wait_tid will not block on futex - operation. */ - return __pthread_clockjoin_ex (threadid, thread_return, 0 /* Ignored */, - NULL, false); + struct pthread *pd = (struct pthread *) threadid; + return atomic_load_acquire (&pd->joinstate) != THREAD_STATE_EXITED + ? EBUSY + : __pthread_clockjoin_ex (threadid, thread_return, 0, NULL); } versioned_symbol (libc, __pthread_tryjoin_np, pthread_tryjoin_np, GLIBC_2_34); diff --git a/sysdeps/nptl/dl-tls_init_tp.c b/sysdeps/nptl/dl-tls_init_tp.c index b7b3bb1cdb..32a9acd8f7 100644 --- a/sysdeps/nptl/dl-tls_init_tp.c +++ b/sysdeps/nptl/dl-tls_init_tp.c @@ -62,7 +62,7 @@ __tls_init_tp (void) /* Early initialization of the TCB. */ struct pthread *pd = THREAD_SELF; - pd->tid = INTERNAL_SYSCALL_CALL (set_tid_address, &pd->tid); + pd->tid = INTERNAL_SYSCALL_CALL (set_tid_address, &pd->joinstate); THREAD_SETMEM (pd, specific[0], &pd->specific_1stblock[0]); THREAD_SETMEM (pd, user_stack, true); @@ -97,4 +97,6 @@ __tls_init_tp (void) THREAD_SETMEM (pd, cancelstate, PTHREAD_CANCEL_ENABLE); THREAD_SETMEM (pd, canceltype, PTHREAD_CANCEL_DEFERRED); + + THREAD_SETMEM (pd, joinstate, THREAD_STATE_JOINABLE); } diff --git a/sysdeps/nptl/libc_start_call_main.h b/sysdeps/nptl/libc_start_call_main.h index b56bf34325..592326e977 100644 --- a/sysdeps/nptl/libc_start_call_main.h +++ b/sysdeps/nptl/libc_start_call_main.h @@ -17,6 +17,7 @@ . */ #include +#include #include _Noreturn static void @@ -65,6 +66,12 @@ __libc_start_call_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), /* One less thread. Decrement the counter. If it is zero we terminate the entire process. */ result = 0; + + /* For the case a thread is waiting for the main thread to finish. */ + struct pthread *self = THREAD_SELF; + atomic_store_release (&self->joinstate, THREAD_STATE_EXITED); + futex_wake (&self->joinstate, 1, FUTEX_SHARED); + if (! atomic_decrement_and_test (&__nptl_nthreads)) /* Not much left to do but to exit the thread, not the process. */ while (1) diff --git a/sysdeps/pthread/tst-thrd-detach.c b/sysdeps/pthread/tst-thrd-detach.c index c844767748..e1906a0e10 100644 --- a/sysdeps/pthread/tst-thrd-detach.c +++ b/sysdeps/pthread/tst-thrd-detach.c @@ -20,14 +20,14 @@ #include #include #include - +#include #include static int detach_thrd (void *arg) { - if (thrd_detach (thrd_current ()) != thrd_success) - FAIL_EXIT1 ("thrd_detach failed"); + thrd_sleep (&(struct timespec) { .tv_sec = INT_MAX }, NULL); + thrd_exit (thrd_success); } @@ -36,15 +36,11 @@ do_test (void) { thrd_t id; - /* Create new thread. */ - if (thrd_create (&id, detach_thrd, NULL) != thrd_success) - FAIL_EXIT1 ("thrd_create failed"); + TEST_COMPARE (thrd_create (&id, detach_thrd, NULL), thrd_success); - /* Give some time so the thread can finish. */ - thrd_sleep (&(struct timespec) {.tv_sec = 2}, NULL); + TEST_COMPARE (thrd_detach (id), thrd_success); - if (thrd_join (id, NULL) == thrd_success) - FAIL_EXIT1 ("thrd_join succeed where it should fail"); + TEST_COMPARE (thrd_join (id, NULL), thrd_error); return 0; } From patchwork Thu Jun 10 19:36:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43827 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5475F39A0468 for ; Thu, 10 Jun 2021 19:40:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5475F39A0468 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1623354013; bh=6U6dosfU9/k2S2NPw6SEJ/Bv4ImeeLTNOS5G8I3JpNU=; 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=bJEFU53XpnCJVooLBK9s8Zvp6+ueJxQiZNIKOTa0M4xt+2RpxBhYMn0QU4Sl3fPAn jpZPGf0//AkvDayrNt6xEN3+XyK7IQkaDjeKBRlOapkBdwIp77WYfY86XbuyDuLHQt jyeTkZqCU9u9uDgkr41xvpVlJ2WTcfoIkCGioh7k= 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 89223399E048 for ; Thu, 10 Jun 2021 19:36:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 89223399E048 Received: by mail-qk1-x731.google.com with SMTP id c138so15704742qkg.5 for ; Thu, 10 Jun 2021 12:36:51 -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=6U6dosfU9/k2S2NPw6SEJ/Bv4ImeeLTNOS5G8I3JpNU=; b=pyIPrMCg+0dENDb/pef7l4yrcaP+FHCsWiPQVPJrghhf23XtjLWWstWbYog8x6kylM BIpNqdwS7V+8MkZXwxPozhdL95CL1zDCORYbeGWDNXFO4LG5lzsR0KiJPy05W5p+j3bu HOSyh20IYoHTjsFmeNYi4rf0z6cS0OL+l79n/WXR9/WUzpKT9uSFHOYYU0Bo/22Pyl+1 rz+nIeaL3iQH4f2ochw2E3IGPqir9P7TT7avBeJ0Bp+Fk1FBZ23/M+C4+PCC+y8Igmwc S5nVCkKi/FV8Ulxu31Po+svq5wJF82TK+80THt/l4PLHBaRmiNVrNvCPWiDfexgJQ312 rO5g== X-Gm-Message-State: AOAM5332MaNYzmr6atHVyr4LsvEoHchINxblvBrOH27e+2BoIn0U8UAH gSwPtIRke+rDNuFz7949MTWT8OXzjUERkA== X-Google-Smtp-Source: ABdhPJx2vJMJNJFOKQLw7/jULg1C9Y9dsi6L3lBaF1MR5TfBoa4D4SwfvIq1TOxnk/Und/FYTN6qFw== X-Received: by 2002:a37:a417:: with SMTP id n23mr201985qke.265.1623353810886; Thu, 10 Jun 2021 12:36:50 -0700 (PDT) Received: from birita.. ([177.194.59.218]) by smtp.googlemail.com with ESMTPSA id i21sm3012583qkl.20.2021.06.10.12.36.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 12:36:50 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 5/6] nptl: Move setxid flag out of cancelhandling Date: Thu, 10 Jun 2021 16:36:38 -0300 Message-Id: <20210610193639.3650754-6-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> References: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.4 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 Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Now that the thread state is tracked by PD->joinstate, there is no need to keep the setxid flag within the PD->cancelhandling. It simplifies the atomic code to set and reset it (since there is no need to handle the thread state concurrent update). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/allocatestack.c | 3 +++ nptl/descr.h | 5 ++--- nptl/nptl_setxid.c | 49 ++++++++++--------------------------------- nptl/pthread_create.c | 4 ++-- 4 files changed, 18 insertions(+), 43 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 9be6c42894..aae2281517 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -140,6 +140,7 @@ get_cached_stack (size_t *sizep, void **memp) } /* Don't allow setxid until cloned. */ + result->setxid_flag = 0; result->setxid_futex = -1; /* Dequeue the entry. */ @@ -342,6 +343,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, #endif /* Don't allow setxid until cloned. */ + pd->setxid_flag = 0; pd->setxid_futex = -1; /* Allocate the DTV for this thread. */ @@ -463,6 +465,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, #endif /* Don't allow setxid until cloned. */ + pd->setxid_flag = 0; pd->setxid_futex = -1; /* Allocate the DTV for this thread. */ diff --git a/nptl/descr.h b/nptl/descr.h index 4b2db6edab..563b152bff 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -297,9 +297,6 @@ struct pthread /* Bit set if thread terminated and TCB is freed. */ #define TERMINATED_BIT 5 #define TERMINATED_BITMASK (0x01 << TERMINATED_BIT) - /* Bit set if thread is supposed to change XID. */ -#define SETXID_BIT 6 -#define SETXID_BITMASK (0x01 << SETXID_BIT) /* Flags. Including those copied from the thread attribute. */ int flags; @@ -339,6 +336,8 @@ struct pthread /* Lock to synchronize access to the descriptor. */ int lock; + /* Indicate whether thread is supposed to change XID. */ + int setxid_flag; /* Lock for synchronizing setxid calls. */ unsigned int setxid_futex; diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c index 2f35772411..c337fa8577 100644 --- a/nptl/nptl_setxid.c +++ b/nptl/nptl_setxid.c @@ -76,14 +76,7 @@ __nptl_setxid_sighandler (int sig, siginfo_t *si, void *ctx) /* Reset the SETXID flag. */ struct pthread *self = THREAD_SELF; - int flags, newval; - do - { - flags = THREAD_GETMEM (self, cancelhandling); - newval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - flags & ~SETXID_BITMASK, flags); - } - while (flags != newval); + atomic_store_release (&self->setxid_flag, 0); /* And release the futex. */ self->setxid_futex = 1; @@ -97,8 +90,6 @@ libc_hidden_def (__nptl_setxid_sighandler) static void setxid_mark_thread (struct xid_command *cmdp, struct pthread *t) { - int ch; - /* Wait until this thread is cloned. */ if (t->setxid_futex == -1 && ! atomic_compare_and_exchange_bool_acq (&t->setxid_futex, -2, -1)) @@ -109,41 +100,23 @@ setxid_mark_thread (struct xid_command *cmdp, struct pthread *t) /* Don't let the thread exit before the setxid handler runs. */ t->setxid_futex = 0; - do - { - ch = t->cancelhandling; + /* If thread is exiting right now, ignore it. */ + if (atomic_load_acquire (&t->joinstate) == THREAD_STATE_EXITING) + return; - /* If the thread is exiting right now, ignore it. */ - if ((ch & EXITING_BITMASK) != 0) - { - /* Release the futex if there is no other setxid in - progress. */ - if ((ch & SETXID_BITMASK) == 0) - { - t->setxid_futex = 1; - futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE); - } - return; - } + /* Release the futex if there is no other setxid in progress. */ + if (atomic_exchange_acquire (&t->setxid_flag, 1) == 0) + { + t->setxid_futex = 1; + futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE); } - while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling, - ch | SETXID_BITMASK, ch)); } static void setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t) { - int ch; - - do - { - ch = t->cancelhandling; - if ((ch & SETXID_BITMASK) == 0) - return; - } - while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling, - ch & ~SETXID_BITMASK, ch)); + atomic_exchange_release (&t->setxid_flag, 0); /* Release the futex just in case. */ t->setxid_futex = 1; @@ -154,7 +127,7 @@ setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t) static int setxid_signal_thread (struct xid_command *cmdp, struct pthread *t) { - if ((t->cancelhandling & SETXID_BITMASK) == 0) + if (atomic_load_relaxed (&t->setxid_flag) == 0) return 0; int val; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 0c0f2c03e5..0b1c67f6d2 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -531,7 +531,7 @@ start_thread (void *arg) advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd, pd->guardsize); - if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK)) + if (__glibc_unlikely (atomic_load_relaxed (&pd->setxid_flag) == 1)) { /* Some other thread might call any of the setXid functions and expect us to reply. In this case wait until we did that. */ @@ -541,7 +541,7 @@ start_thread (void *arg) condition used in the surrounding loop (cancelhandling). We need to check and document why this is correct. */ futex_wait_simple (&pd->setxid_futex, 0, FUTEX_PRIVATE); - while (pd->cancelhandling & SETXID_BITMASK); + while (atomic_load_relaxed (&pd->setxid_flag) == 1); /* Reset the value so that the stack can be reused. */ pd->setxid_futex = 0; From patchwork Thu Jun 10 19:36:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43829 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BB72539A1420 for ; Thu, 10 Jun 2021 19:41:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BB72539A1420 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1623354109; bh=NcK03V9uS7PmlKwYcMtZB2YfPGEEoKxA6/r60tO+HtA=; 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=VW+E6gtpBPjDrnsaSQNX9V+kYDWvLG9UzN9zcUC+DCftjGrvDV3MzN6Lu0QRHfMpm QuV6VA01ZiamUI9/B74moWaDbj63K1/A36fBWCTSSbUjuEjvcgvZb02ph3xLHcdsMP 40mNIBxiiPVjbNJKjSXBpnJNkzrE9uUkWIn22r9Y= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x72a.google.com (mail-qk1-x72a.google.com [IPv6:2607:f8b0:4864:20::72a]) by sourceware.org (Postfix) with ESMTPS id E3A09399C8B5 for ; Thu, 10 Jun 2021 19:36:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E3A09399C8B5 Received: by mail-qk1-x72a.google.com with SMTP id k11so26950436qkk.1 for ; Thu, 10 Jun 2021 12:36:52 -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=NcK03V9uS7PmlKwYcMtZB2YfPGEEoKxA6/r60tO+HtA=; b=V4xn4Q9fhIfmIBifSlMtFfesNjuhkRTYs93OdSHzRulc0khLevCoo1q87L/WQE2RbG qrsNodLskoxWQdq7eu3dfLCGIV6OJcN4bsG+s3FCvnCP2IG/vMwCblNXcC2vY30GLT2R A6B2TcsbGUNqZArS+GegBaWnnML0GBk5oUrqMTQNe/tub/0rQH7m44s2SvDwHaNECSG0 1dLat1zUzD+Ymmg7SMaknd3L/SjQ4fvzp+2TlRTCt7xMOOojGpS3CJNPLxsrXD/jiscr EzoZYjpwORuFuAeihhHhxyhDGaUlF8q/CpOxLGz704JjZbWDnpOqzg0meZ4SSLoCQEse CYEA== X-Gm-Message-State: AOAM532/lEToChkUBVZeO8AWYHu73vj+0CrFDWKRpX3tA/4JZk8cIcXK aaZD886PaU6ltY0lQSQ7Yl+mI7ds5ZLhXA== X-Google-Smtp-Source: ABdhPJzBwebnowhZ5AEy5RYQLaDcAkR1EJyJ4iaO+yg1jkUE57wPBRGtF3qo6/3rb1uEgLshpWFpzQ== X-Received: by 2002:a05:620a:6c9:: with SMTP id 9mr197292qky.303.1623353812190; Thu, 10 Jun 2021 12:36:52 -0700 (PDT) Received: from birita.. ([177.194.59.218]) by smtp.googlemail.com with ESMTPSA id i21sm3012583qkl.20.2021.06.10.12.36.51 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 12:36:51 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 6/6] nptl: Replace struct thread cancelhandling field Date: Thu, 10 Jun 2021 16:36:39 -0300 Message-Id: <20210610193639.3650754-7-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> References: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.4 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 Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Now that both thread state and setxid is being tracked by two different PD members (joinstate and setxid_flag), there is no need to keep track of exiting (EXITING_BIT) and terminated (TERMINATED_BIT) state in 'cancelhandling'. The 'cancelhandling' field is renamed to 'cancelstatus' and now only signal whether the thread has been cancelled (set by pthread_cancel()). It allows simplify the atomic operation required. There is no need to check the thread state on __pthread_enable_asynccancel() nor on pthread_testcancel () anymore now that cancellation is explicit disabled when thread start the exit code (__do_cancel()). On __nptl_free_tcp(), the PD->joinstate now defines whether is the creating thread or created thread that calls it. So there is no concurrent call of the function and thus no need to set the TERMINATED_BIT. For SIGCANCEL handler, sigcancel_handler(), PD->joinstate is used instead (pthread_cancel() might still be called concurrenty in assynchronous mode). Finally the nptl_db is adjusted to check the PD->joinstate information instead of cancelhandling to check for the thread state. Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/allocatestack.c | 2 +- nptl/cancellation.c | 10 ++-------- nptl/descr.h | 13 ++----------- nptl/nptl_free_tcb.c | 22 ++++++---------------- nptl/pthreadP.h | 3 --- nptl/pthread_cancel.c | 10 ++++------ nptl/pthread_create.c | 10 ++-------- nptl/pthread_join_common.c | 10 +++------- nptl/pthread_testcancel.c | 10 ++-------- nptl_db/structs.def | 2 +- nptl_db/td_thr_get_info.c | 16 +++++++--------- nptl_db/td_thr_getfpregs.c | 9 +++++---- nptl_db/td_thr_getgregs.c | 9 +++++---- nptl_db/td_thr_setfpregs.c | 9 +++++---- nptl_db/td_thr_setgregs.c | 9 +++++---- sysdeps/hppa/nptl/tcb-offsets.sym | 1 - sysdeps/i386/nptl/tcb-offsets.sym | 1 - sysdeps/sh/nptl/tcb-offsets.sym | 1 - sysdeps/x86_64/nptl/tcb-offsets.sym | 4 ---- 19 files changed, 50 insertions(+), 101 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index aae2281517..f9829e2dae 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -160,7 +160,7 @@ get_cached_stack (size_t *sizep, void **memp) *memp = result->stackblock; /* Cancellation handling is back to the default. */ - result->cancelhandling = 0; + result->cancelstatus = 0; result->cancelstate = PTHREAD_CANCEL_ENABLE; result->canceltype = PTHREAD_CANCEL_DEFERRED; result->cleanup = NULL; diff --git a/nptl/cancellation.c b/nptl/cancellation.c index 6478c029de..a244b3eeea 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -35,15 +35,9 @@ __pthread_enable_asynccancel (void) int oldval = THREAD_GETMEM (self, canceltype); THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_ASYNCHRONOUS); - int ch = THREAD_GETMEM (self, cancelhandling); - if (self->cancelstate == PTHREAD_CANCEL_ENABLE - && (ch & CANCELED_BITMASK) - && !(ch & EXITING_BITMASK) - && !(ch & TERMINATED_BITMASK)) - { - __do_cancel (); - } + && self->cancelstatus == 1) + __do_cancel (); return oldval; } diff --git a/nptl/descr.h b/nptl/descr.h index 563b152bff..1bfa2b9b52 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -286,17 +286,8 @@ struct pthread struct pthread_unwind_buf *cleanup_jmp_buf; #define HAVE_CLEANUP_JMP_BUF - /* Flags determining processing of cancellation. */ - int cancelhandling; - /* Bit set if canceled. */ -#define CANCELED_BIT 3 -#define CANCELED_BITMASK (0x01 << CANCELED_BIT) - /* Bit set if thread is exiting. */ -#define EXITING_BIT 4 -#define EXITING_BITMASK (0x01 << EXITING_BIT) - /* Bit set if thread terminated and TCB is freed. */ -#define TERMINATED_BIT 5 -#define TERMINATED_BITMASK (0x01 << TERMINATED_BIT) + /* Flag to determine whether the thread is signaled to be cancelled. */ + int cancelstatus; /* Flags. Including those copied from the thread attribute. */ int flags; diff --git a/nptl/nptl_free_tcb.c b/nptl/nptl_free_tcb.c index cbf3580f59..15e1a18562 100644 --- a/nptl/nptl_free_tcb.c +++ b/nptl/nptl_free_tcb.c @@ -24,22 +24,12 @@ void __nptl_free_tcb (struct pthread *pd) { - /* The thread is exiting now. */ - if (atomic_bit_test_set (&pd->cancelhandling, TERMINATED_BIT) == 0) - { - /* Free TPP data. */ - if (pd->tpp != NULL) - { - struct priority_protection_data *tpp = pd->tpp; + free (pd->tpp); + pd->tpp = NULL; - pd->tpp = NULL; - free (tpp); - } - - /* Queue the stack memory block for reuse and exit the process. The - kernel will signal via writing to the address returned by - QUEUE-STACK when the stack is available. */ - __nptl_deallocate_stack (pd); - } + /* Queue the stack memory block for reuse and exit the process. The kernel + will signal via writing to the address returned by QUEUE-STACK when the + stack is available. */ + __nptl_deallocate_stack (pd); } libc_hidden_def (__nptl_free_tcb) diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index bfb0e40b44..18b7cfbd79 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -273,9 +273,6 @@ __exit_thread (void *value) { struct pthread *self = THREAD_SELF; - /* Make sure we get no more cancellations. */ - THREAD_ATOMIC_BIT_SET (self, cancelhandling, EXITING_BIT); - THREAD_SETMEM (self, result, value); /* It is required by POSIX XSH 2.9.5 Thread Cancellation under the heading diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 577ff6c746..c3d07258b6 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -43,11 +43,8 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) struct pthread *self = THREAD_SELF; - int ch = atomic_load_relaxed (&self->cancelhandling); - /* Cancelation not enabled, not cancelled, or already exitting. */ if (self->cancelstate == PTHREAD_CANCEL_DISABLE - || (ch & CANCELED_BITMASK) == 0 - || (ch & EXITING_BITMASK) != 0) + || atomic_load_relaxed (&self->joinstate) == THREAD_STATE_EXITING) return; /* Set the return value. */ @@ -88,8 +85,9 @@ __pthread_cancel (pthread_t th) } #endif - int oldch = atomic_fetch_or_acquire (&pd->cancelhandling, CANCELED_BITMASK); - if ((oldch & CANCELED_BITMASK) != 0) + /* If already cancelled just return (cancellation will be acted upon in next + cancellation entrypoint). */ + if (atomic_exchange_acquire (&pd->cancelstatus, 1) == 1) return 0; if (pd == THREAD_SELF) diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 0b1c67f6d2..f34fbff096 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -469,12 +469,6 @@ start_thread (void *arg) } } - /* The thread is exiting now. Don't set this bit until after we've hit - the event-reporting breakpoint, so that td_thr_get_info on us while at - the breakpoint reports TD_THR_RUN state rather than TD_THR_ZOMBIE. */ - atomic_bit_set (&pd->cancelhandling, EXITING_BIT); - - /* CONCURRENCY NOTES: Concurrent pthread_detach() will either set state to @@ -538,8 +532,8 @@ start_thread (void *arg) do /* XXX This differs from the typical futex_wait_simple pattern in that the futex_wait condition (setxid_futex) is different from the - condition used in the surrounding loop (cancelhandling). We need - to check and document why this is correct. */ + condition used in the surrounding loop. We need to check and + document why this is correct. */ futex_wait_simple (&pd->setxid_futex, 0, FUTEX_PRIVATE); while (atomic_load_relaxed (&pd->setxid_flag) == 1); diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index 0870f36ecb..a131efba3b 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -33,14 +33,10 @@ check_for_deadlock (int state, struct pthread *pd) { struct pthread *self = THREAD_SELF; return ((pd == self - || (atomic_load_acquire (&self->joinstate) == THREAD_STATE_DETACHED - && (pd->cancelhandling - & (CANCELED_BITMASK | EXITING_BITMASK - | TERMINATED_BITMASK)) == 0)) + || (atomic_load_acquire (&self->joinstate) + == THREAD_STATE_DETACHED)) && !(self->cancelstate == PTHREAD_CANCEL_ENABLE - && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK - | TERMINATED_BITMASK)) - == CANCELED_BITMASK)); + && atomic_load_relaxed (&self->cancelstatus) == 1)); } int diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c index e6adf12580..b06327ebe3 100644 --- a/nptl/pthread_testcancel.c +++ b/nptl/pthread_testcancel.c @@ -24,14 +24,8 @@ void ___pthread_testcancel (void) { 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)) - { - __do_cancel (); - } + if (self->cancelstate == PTHREAD_CANCEL_ENABLE && self->cancelstatus == 1) + __do_cancel (); } versioned_symbol (libc, ___pthread_testcancel, pthread_testcancel, GLIBC_2_34); versioned_symbol (libc, ___pthread_testcancel, __pthread_testcancel, diff --git a/nptl_db/structs.def b/nptl_db/structs.def index 6a726f207e..26f9a41f1a 100644 --- a/nptl_db/structs.def +++ b/nptl_db/structs.def @@ -57,7 +57,7 @@ DB_STRUCT_FIELD (pthread, list) DB_STRUCT_FIELD (pthread, report_events) DB_STRUCT_FIELD (pthread, tid) DB_STRUCT_FIELD (pthread, start_routine) -DB_STRUCT_FIELD (pthread, cancelhandling) +DB_STRUCT_FIELD (pthread, joinstate) DB_STRUCT_FIELD (pthread, schedpolicy) DB_STRUCT_FIELD (pthread, schedparam_sched_priority) DB_STRUCT_FIELD (pthread, specific) diff --git a/nptl_db/td_thr_get_info.c b/nptl_db/td_thr_get_info.c index 01af021d2a..82f1f2667c 100644 --- a/nptl_db/td_thr_get_info.c +++ b/nptl_db/td_thr_get_info.c @@ -28,7 +28,7 @@ td_thr_get_info (const td_thrhandle_t *th, td_thrinfo_t *infop) { td_err_e err; void *copy; - psaddr_t tls, schedpolicy, schedprio, cancelhandling, tid, report_events; + psaddr_t tls, schedpolicy, schedprio, joinstate, tid, report_events; LOG ("td_thr_get_info"); @@ -37,7 +37,7 @@ td_thr_get_info (const td_thrhandle_t *th, td_thrinfo_t *infop) /* Special case for the main thread before initialization. */ copy = NULL; tls = 0; - cancelhandling = 0; + joinstate = 0; schedpolicy = SCHED_OTHER; schedprio = 0; tid = 0; @@ -76,8 +76,8 @@ td_thr_get_info (const td_thrhandle_t *th, td_thrinfo_t *infop) err = DB_GET_FIELD_LOCAL (tid, th->th_ta_p, copy, pthread, tid, 0); if (err != TD_OK) return err; - err = DB_GET_FIELD_LOCAL (cancelhandling, th->th_ta_p, copy, pthread, - cancelhandling, 0); + err = DB_GET_FIELD_LOCAL (joinstate, th->th_ta_p, copy, pthread, + joinstate, 0); if (err != TD_OK) return err; err = DB_GET_FIELD_LOCAL (report_events, th->th_ta_p, copy, pthread, @@ -96,13 +96,11 @@ td_thr_get_info (const td_thrhandle_t *th, td_thrinfo_t *infop) ? 0 : (uintptr_t) schedprio); infop->ti_type = TD_THR_USER; - if ((((int) (uintptr_t) cancelhandling) & EXITING_BITMASK) == 0) - /* XXX For now there is no way to get more information. */ + int js = (int) (uintptr_t) joinstate; + if (js == THREAD_STATE_JOINABLE || js == THREAD_STATE_DETACHED) infop->ti_state = TD_THR_ACTIVE; - else if ((((int) (uintptr_t) cancelhandling) & TERMINATED_BITMASK) == 0) - infop->ti_state = TD_THR_ZOMBIE; else - infop->ti_state = TD_THR_UNKNOWN; + infop->ti_state = TD_THR_ZOMBIE; /* Initialization which are the same in both cases. */ infop->ti_ta_p = th->th_ta_p; diff --git a/nptl_db/td_thr_getfpregs.c b/nptl_db/td_thr_getfpregs.c index 3d08aa3f60..23a8a215c2 100644 --- a/nptl_db/td_thr_getfpregs.c +++ b/nptl_db/td_thr_getfpregs.c @@ -23,7 +23,7 @@ td_err_e td_thr_getfpregs (const td_thrhandle_t *th, prfpregset_t *regset) { - psaddr_t cancelhandling, tid; + psaddr_t joinstate, tid; td_err_e err; LOG ("td_thr_getfpregs"); @@ -34,13 +34,14 @@ td_thr_getfpregs (const td_thrhandle_t *th, prfpregset_t *regset) regset) != PS_OK ? TD_ERR : TD_OK; /* We have to get the state and the PID for this thread. */ - err = DB_GET_FIELD (cancelhandling, th->th_ta_p, th->th_unique, pthread, - cancelhandling, 0); + err = DB_GET_FIELD (joinstate, th->th_ta_p, th->th_unique, pthread, + joinstate, 0); if (err != TD_OK) return err; /* If the thread already terminated we return all zeroes. */ - if (((int) (uintptr_t) cancelhandling) & TERMINATED_BITMASK) + int js = (int) (uintptr_t) joinstate; + if (js == THREAD_STATE_EXITING || js == THREAD_STATE_EXITED) memset (regset, '\0', sizeof (*regset)); /* Otherwise get the register content through the callback. */ else diff --git a/nptl_db/td_thr_getgregs.c b/nptl_db/td_thr_getgregs.c index 8f9fab096f..b92402670f 100644 --- a/nptl_db/td_thr_getgregs.c +++ b/nptl_db/td_thr_getgregs.c @@ -23,7 +23,7 @@ td_err_e td_thr_getgregs (const td_thrhandle_t *th, prgregset_t regset) { - psaddr_t cancelhandling, tid; + psaddr_t joinstate, tid; td_err_e err; LOG ("td_thr_getgregs"); @@ -34,13 +34,14 @@ td_thr_getgregs (const td_thrhandle_t *th, prgregset_t regset) regset) != PS_OK ? TD_ERR : TD_OK; /* We have to get the state and the PID for this thread. */ - err = DB_GET_FIELD (cancelhandling, th->th_ta_p, th->th_unique, pthread, - cancelhandling, 0); + err = DB_GET_FIELD (joinstate, th->th_ta_p, th->th_unique, pthread, + joinstate, 0); if (err != TD_OK) return err; /* If the thread already terminated we return all zeroes. */ - if (((int) (uintptr_t) cancelhandling) & TERMINATED_BITMASK) + int js = (int) (uintptr_t) joinstate; + if (js == THREAD_STATE_EXITING || js == THREAD_STATE_EXITED) memset (regset, '\0', sizeof (*regset)); /* Otherwise get the register content through the callback. */ else diff --git a/nptl_db/td_thr_setfpregs.c b/nptl_db/td_thr_setfpregs.c index bddb0359a8..73ab675ce6 100644 --- a/nptl_db/td_thr_setfpregs.c +++ b/nptl_db/td_thr_setfpregs.c @@ -23,7 +23,7 @@ td_err_e td_thr_setfpregs (const td_thrhandle_t *th, const prfpregset_t *fpregs) { - psaddr_t cancelhandling, tid; + psaddr_t joinstate, tid; td_err_e err; LOG ("td_thr_setfpregs"); @@ -34,13 +34,14 @@ td_thr_setfpregs (const td_thrhandle_t *th, const prfpregset_t *fpregs) fpregs) != PS_OK ? TD_ERR : TD_OK; /* We have to get the state and the PID for this thread. */ - err = DB_GET_FIELD (cancelhandling, th->th_ta_p, th->th_unique, pthread, - cancelhandling, 0); + err = DB_GET_FIELD (joinstate, th->th_ta_p, th->th_unique, pthread, + joinstate, 0); if (err != TD_OK) return err; /* Only set the registers if the thread hasn't yet terminated. */ - if ((((int) (uintptr_t) cancelhandling) & TERMINATED_BITMASK) == 0) + int js = (int) (uintptr_t) joinstate; + if (js != THREAD_STATE_EXITING || js != THREAD_STATE_EXITED) { err = DB_GET_FIELD (tid, th->th_ta_p, th->th_unique, pthread, tid, 0); if (err != TD_OK) diff --git a/nptl_db/td_thr_setgregs.c b/nptl_db/td_thr_setgregs.c index 2a76a10754..186df94cbc 100644 --- a/nptl_db/td_thr_setgregs.c +++ b/nptl_db/td_thr_setgregs.c @@ -23,7 +23,7 @@ td_err_e td_thr_setgregs (const td_thrhandle_t *th, prgregset_t gregs) { - psaddr_t cancelhandling, tid; + psaddr_t joinstate, tid; td_err_e err; LOG ("td_thr_setgregs"); @@ -34,13 +34,14 @@ td_thr_setgregs (const td_thrhandle_t *th, prgregset_t gregs) gregs) != PS_OK ? TD_ERR : TD_OK; /* We have to get the state and the PID for this thread. */ - err = DB_GET_FIELD (cancelhandling, th->th_ta_p, th->th_unique, pthread, - cancelhandling, 0); + err = DB_GET_FIELD (joinstate, th->th_ta_p, th->th_unique, pthread, + joinstate, 0); if (err != TD_OK) return err; /* Only set the registers if the thread hasn't yet terminated. */ - if ((((int) (uintptr_t) cancelhandling) & TERMINATED_BITMASK) == 0) + int js = (int) (uintptr_t) joinstate; + if (js != THREAD_STATE_EXITING || js != THREAD_STATE_EXITED) { err = DB_GET_FIELD (tid, th->th_ta_p, th->th_unique, pthread, tid, 0); if (err != TD_OK) diff --git a/sysdeps/hppa/nptl/tcb-offsets.sym b/sysdeps/hppa/nptl/tcb-offsets.sym index 6e852f35b1..8937f1ec21 100644 --- a/sysdeps/hppa/nptl/tcb-offsets.sym +++ b/sysdeps/hppa/nptl/tcb-offsets.sym @@ -3,7 +3,6 @@ RESULT offsetof (struct pthread, result) TID offsetof (struct pthread, tid) -CANCELHANDLING offsetof (struct pthread, cancelhandling) CLEANUP_JMP_BUF offsetof (struct pthread, cleanup_jmp_buf) MULTIPLE_THREADS_OFFSET offsetof (struct pthread, header.multiple_threads) TLS_PRE_TCB_SIZE sizeof (struct pthread) diff --git a/sysdeps/i386/nptl/tcb-offsets.sym b/sysdeps/i386/nptl/tcb-offsets.sym index 2ec9e787c1..85a27dc29f 100644 --- a/sysdeps/i386/nptl/tcb-offsets.sym +++ b/sysdeps/i386/nptl/tcb-offsets.sym @@ -4,7 +4,6 @@ RESULT offsetof (struct pthread, result) TID offsetof (struct pthread, tid) -CANCELHANDLING offsetof (struct pthread, cancelhandling) CLEANUP_JMP_BUF offsetof (struct pthread, cleanup_jmp_buf) MULTIPLE_THREADS_OFFSET offsetof (tcbhead_t, multiple_threads) SYSINFO_OFFSET offsetof (tcbhead_t, sysinfo) diff --git a/sysdeps/sh/nptl/tcb-offsets.sym b/sysdeps/sh/nptl/tcb-offsets.sym index 234207779d..60c9e40b72 100644 --- a/sysdeps/sh/nptl/tcb-offsets.sym +++ b/sysdeps/sh/nptl/tcb-offsets.sym @@ -4,7 +4,6 @@ RESULT offsetof (struct pthread, result) TID offsetof (struct pthread, tid) -CANCELHANDLING offsetof (struct pthread, cancelhandling) CLEANUP_JMP_BUF offsetof (struct pthread, cleanup_jmp_buf) MULTIPLE_THREADS_OFFSET offsetof (struct pthread, header.multiple_threads) TLS_PRE_TCB_SIZE sizeof (struct pthread) diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym index 2bbd563a6c..6cc845f7ed 100644 --- a/sysdeps/x86_64/nptl/tcb-offsets.sym +++ b/sysdeps/x86_64/nptl/tcb-offsets.sym @@ -4,7 +4,6 @@ RESULT offsetof (struct pthread, result) TID offsetof (struct pthread, tid) -CANCELHANDLING offsetof (struct pthread, cancelhandling) CLEANUP_JMP_BUF offsetof (struct pthread, cleanup_jmp_buf) CLEANUP offsetof (struct pthread, cleanup) CLEANUP_PREV offsetof (struct _pthread_cleanup_buffer, __prev) @@ -13,6 +12,3 @@ MULTIPLE_THREADS_OFFSET offsetof (tcbhead_t, multiple_threads) POINTER_GUARD offsetof (tcbhead_t, pointer_guard) FEATURE_1_OFFSET offsetof (tcbhead_t, feature_1) SSP_BASE_OFFSET offsetof (tcbhead_t, ssp_base) - --- Not strictly offsets, but these values are also used in the TCB. -TCB_CANCELED_BITMASK CANCELED_BITMASK