From patchwork Thu Jul 31 15:00:45 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 117338 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 BF887385DC06 for ; Thu, 31 Jul 2025 15:05:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BF887385DC06 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=qsiCZNKi X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pf1-x431.google.com (mail-pf1-x431.google.com [IPv6:2607:f8b0:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 5BE193858436 for ; Thu, 31 Jul 2025 15:01:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5BE193858436 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5BE193858436 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::431 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753974099; cv=none; b=MuKcBB/wzHMHbT2iS26ryXB7B/Pdx9ev4HHbBQkGwED34i/7FHjrw4NIxSbYsRUGcEhq+J4GahQkzY6TUsnlPJuAJrOl341I1Tt4Jo9Uvg6u7kQFxsug02HTx55eDMfHU2bQ/48FFpoEBpdlbZa/2RUE0G4sy0cQ1fgUv8/DE7E= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753974099; c=relaxed/simple; bh=JiXf/goTymwchom6s6+QUCM+Q0+iYeWHtMi5HCbIOEo=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=q8TdnxaJJMWzM1m1UoVYq/FHz1PLhQGJxBiD31S6Vva3eOOsclG7y9bfkPFF/Ltwr2SMWxAQPYVYYwN/DvfbxFXC59KP33PzpiUCLhdDtLq1IhVhPzgyrtSQA2/t5s+/+fx6mtZQwJkf34+dK9f7PYOs4ABcMW6MRQ3h2KpTnlk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x431.google.com with SMTP id d2e1a72fcca58-7698e914cd2so645685b3a.3 for ; Thu, 31 Jul 2025 08:01:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1753974097; x=1754578897; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=quh5hzBW6btY0pzAKHoQOoViS+M4XBXiuNDAaltQncU=; b=qsiCZNKiphEOT/HvyhVkDcf4HJ48ArI+cgo0zc3f0XO3XWvQkwrwJCNnMTXfGJfVJd f+lkiKmbI0pyc8F9o3MrEluApJKGSmo4Qq5+2CMnLXhU3ACV4LsysnS6YuUNq8SGKn6F 3JqugIm2r1Hway34zGzv5t6w+PZh/DAgFwLrSWcTQnDhFQFyZq2z6G0BTQJccB8G9l86 zHKdM/uxE9MKtFs7zQjvV22VzxyW6IN/13uqE/OxRUaDTj5HpOjhyhpFFoumvUU3w1Zb cNfR6MFePXNb0h4264/YQSn9C6uPaKmAJ1Q3PYPaKUZgDCEPcFWa5eJaSXPLR7SEwn4q A4eA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753974097; x=1754578897; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=quh5hzBW6btY0pzAKHoQOoViS+M4XBXiuNDAaltQncU=; b=Xz9F37rsM6CFpmCZMLgJsCkKScwT4cFkuzbjPHkNUSDwRd/bkASc5WfRL7kPZJPKBl 4hDXktU9ns7kMF2JQ27cILRWmK3qpHDqP5WCOdtk6C6K5oAkIZIRBkHMPRdELMzJUr1f eCUkOVENxlvQkPPjiNbHu284KSEAfc3AS/c3BPcPaUOX3rEsAb6fcYhnkp7uDE6rylKW /XfPfdr5p2tku3eZtPmDmvuRKNVx5v+jfkOUcHpf+40cu4NCP3FY2W4KuwHwbBtzJYD7 Z7hDMKlrtPzPs/IPWwaE1j/8ZFwpW0Zn7KSIn851uaCkIPSVQ6wbMpts2Tr78fz+2qdW hu5w== X-Gm-Message-State: AOJu0YxLpCqRYPQ3y63qINVOH01Fkv0cReUzkFLIwSh8zLmiQhGUv/D+ IRokjqqQW9Vsd5rPL/jzrqs7Y+GyLn1tjBT/8zOheG6Z7zP4HFP94QIXU5otZvJ8nNCGLXWYJta QQiGa X-Gm-Gg: ASbGncsI/lRMV6RyIR3QcT+NSd/RlYH7GtzFQ8kgIgnrAxWurUyj0JdKjDw2ylm2fdw H0Cq14ypoUYzJwxPH+2MAQYxp9u/I41puF9zrbd0NHBCBKMAZOvsQqp76O7AGJkHbQXxYR6huZa KKJWGbDtxSFkiUmtoZ3qzERaoa1h8vAMN09cT8QL+PMj+K/oHON7bXM4/U4o6D9ZlevFKZEPrLs aNw/9xWLXe1CXt6stJ5VbYTY3fltBWEF8QjEVKc48KU/bOB4c/JCpSqVtjp+2ljwf43Msljlute wx9E8miSikCyD7k32Aj/hKsnVh0rKKRUIMnUCmmpaDf/YkGx0vB0gag2W1IdUssFH7/IGtp6Ayy 1U+wip/mdNsZhUU0VmAAMGDkKfDfduslx5Q== X-Google-Smtp-Source: AGHT+IE6s9j1WTcOO2dCB6BbdfZ5F/q49AaKQ2q09YaXnqeJAcMdTL+APWzJ3bhHT4RE8tHauMMYFw== X-Received: by 2002:a05:6a21:33a7:b0:23d:33ce:bc7a with SMTP id adf61e73a8af0-23dc0e92014mr11275461637.23.1753974096868; Thu, 31 Jul 2025 08:01:36 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c3:54f:77a1:2d85:2067:5448]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-76bccfd73a2sm1871793b3a.108.2025.07.31.08.01.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 31 Jul 2025 08:01:36 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Florian Weimer , Simon Griebel Subject: [PATCH v4 1/3] nptl: Set cancellation type and state on pthread_exit (BZ #28267) Date: Thu, 31 Jul 2025 12:00:45 -0300 Message-ID: <20250731150130.2970892-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250731150130.2970892-1-adhemerval.zanella@linaro.org> References: <20250731150130.2970892-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.0 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~patchwork=sourceware.org@sourceware.org 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 | 1 + nptl/tst-cleanup5.c | 133 ++++++++++++++++++++++++++++++++++++++++ sysdeps/nptl/pthreadP.h | 15 ++++- 3 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 nptl/tst-cleanup5.c diff --git a/nptl/Makefile b/nptl/Makefile index e6481d5694..f324570861 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -282,6 +282,7 @@ tests = \ tst-cancel17 \ tst-cancel24 \ tst-cancel31 \ + tst-cleanup5 \ tst-cond26 \ tst-context1 \ tst-default-attr \ diff --git a/nptl/tst-cleanup5.c b/nptl/tst-cleanup5.c new file mode 100644 index 0000000000..0df1ddeb68 --- /dev/null +++ b/nptl/tst-cleanup5.c @@ -0,0 +1,133 @@ +/* Check if cancellation state and type is correctly set on thread exit. + Copyright (C) 2025 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 +#include + +static int pipefds[2]; +static pthread_barrier_t b; + +static void +clh (void *arg) +{ + /* Although POSIX states that setting either the cancellation state or type + is undefined during cleanup handler execution, both calls should be safe + since neither has any side-effect (they should not change the current + state, nor 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; + xread (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; + xread (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); + { + printf ("info: checking PTHREAD_CANCEL_DEFERRED\n"); + 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); + } + + { + printf ("info: checking PTHREAD_CANCEL_DEFERRED with pthread_testcancel\n"); + 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); + } + + { + printf ("info: checking PTHREAD_CANCEL_DEFERRED with pthread_exit\n"); + 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 diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h index 8f256967e2..4a3881e945 100644 --- a/sysdeps/nptl/pthreadP.h +++ b/sysdeps/nptl/pthreadP.h @@ -268,7 +268,20 @@ __do_cancel (void *result) self->result = result; /* Make sure we get no more cancellations. */ - atomic_fetch_or_relaxed (&self->cancelhandling, EXITING_BITMASK); + int oldval = atomic_load_relaxed (&self->cancelhandling); + int newval; + do + { + /* 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. */ + newval = oldval | CANCELSTATE_BITMASK | EXITING_BITMASK; + newval = newval & ~CANCELTYPE_BITMASK; + if (oldval == newval) + break; + } + while (!atomic_compare_exchange_weak_acquire (&self->cancelhandling, + &oldval, newval)); __pthread_unwind ((__pthread_unwind_buf_t *) THREAD_GETMEM (self, cleanup_jmp_buf)); From patchwork Thu Jul 31 15:00:46 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 117337 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 8C68F385624B for ; Thu, 31 Jul 2025 15:05:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8C68F385624B Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=ttDzouPN X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pf1-x432.google.com (mail-pf1-x432.google.com [IPv6:2607:f8b0:4864:20::432]) by sourceware.org (Postfix) with ESMTPS id 151D43857C6E for ; Thu, 31 Jul 2025 15:01:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 151D43857C6E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 151D43857C6E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::432 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753974101; cv=none; b=Lrkjoyqis4FFxpnwSAkthmAjkMxUs0CDbtwBGiQ2OVpcA4KB4uNpBNev6RjrIoFu63uLIfPyFVz6sFFRNnR4XTHCKBv5KC7U3WKmsukPY96dunniRAstsZeQ2epBlSMUn3XH/66k+/TpbcUPxW6kFd/nEhxUZEfdIRSB35AHGSo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753974101; c=relaxed/simple; bh=aSJV8Z+KOBieMpMwWsAzxlQkIsBsLjz3tZzYbg8OCCM=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=DQOElklcm5WsJ0rGDGwgk8D7ku4b6TeJnOWaKBm6nv0Bno17EBmNTXYBDkuO8TWD0l47tXFJ8LNZ0Juh1YP/Hb5t8GHctsSks+0/7T8kRJGK9whFCldU+213Fu4JstkTATu1exF3U/CxyZMXSZlrf7o3+hcxQX3yAcoxi5Plxh4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x432.google.com with SMTP id d2e1a72fcca58-748d982e97cso504234b3a.1 for ; Thu, 31 Jul 2025 08:01:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1753974099; x=1754578899; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=gMoJCnFXnN70TwQODMlsFyatvWN7Ccu06ohZj1iukGE=; b=ttDzouPNz5rzrHtl3f8+oFdsoxMbTnTQgcR1DzrJ4oXLMxlNkj4Ex0WH/gtN2RYuwe YFC6mmp29cSa/IyEgfLhG0eCHzdE4Ln+859UsROJke0sEs3fO6/TxoYcsit8Lnnh3PCK aplxInMN+kU7xGXvVuylBpQqJg6SHikvibyzE2vCKWaWc+RTnUVISHvip8/n1Upv2/6G vh58P4ksbOpflijhwLIKlngG2ujypcH5zgeUn5F2RB2jQE8y4IRRXvk0k/VtN3xXDOvq x9wJK+0nqpd3y/6gnjSMQnrKFVYBG/hK2RIT6vWh+eL5HIyWLOzVOZxxN5NgxkxHUaBc o15Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753974099; x=1754578899; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gMoJCnFXnN70TwQODMlsFyatvWN7Ccu06ohZj1iukGE=; b=EIc/DRcHlOuHHIS+6Zlvabpq9bl36Cve+0TyoV+drhnJGNrvo0iptAoL9+gGgd/4Uh Tep3YLX4TW8S401sXB9rnukuH89i/y6TK2E2ztNzcW/Q/xbd1bcMmwidMxMncrYrKCro EpU0/WEs6KgCZ0TK8Dao+3CHJWtAqohYhf/J73AxGrIyDqalVdb1N7hyg/jLLwSW3Bqc O9wBJR9H/BFo5GVVxi9YK58yu46MMpOmvu60kJDMKMGIzYc8TVsJTeEJ/YOYm4OS/K0i MyMxHf6WhOdMB+FqBTkiYQeTUVwUx6xy/TcPKqdQJTPUKMtQ3oyYV7mvMwJmtXtRAQWW ZBTA== X-Gm-Message-State: AOJu0Yx9FdWaYlICtGA+esh3URUW5AIFq7/RowNJkLnfLJVD1TyobLya M751mGfOwUh0c2jCXmIDK0totbgME9V0JnnNgddalKVOXP6q4z+pW0uk+Klld0Dairi471BLMO1 aSegE X-Gm-Gg: ASbGncuIQW1y2sSoInNk+NqWy4HWc7m4qUGiME7OHvFGyqNMCWy614uStw2bplnBdfG 40W3ps7+mmRh1T7rlHpZvjGBLG1o+im5hzN/K7BFUeh317HriUeOrmC6IKgkfBOjKIk+X5+9rMq Zz/TFM2CSNHs6cQeOs6ISMySe5FQ9r4cyPMtP3nLVDcGNqZmtdy4Rw4/V8PYL/y+DiD5oUZSJqs XkJqse6Cyz33pVG1h5GBy9rUt9tBEp52OFhI01j5aJenzpdbDS9KAxl/EVAbbxJV0mVDWu0QoXG 1GRKzoog95tefoSw4tTEh+XLKrsG1UVP52UPNAigPaBVv7hR7p1xLCwN7N0Pa5ifZQQ5eTCUYIq iOY84lWCt7T4lnRJ54tLtkPuK1B74wb5u7Q== X-Google-Smtp-Source: AGHT+IG/dGxjUiY7shDiLYqwmgkW3N1Emk8V+KawhLUPK2W1GT/dxSv/ThGT9T3dqw0U4sMBFOci1Q== X-Received: by 2002:a05:6a20:1590:b0:230:f847:6586 with SMTP id adf61e73a8af0-23dc0e92791mr11848480637.29.1753974098784; Thu, 31 Jul 2025 08:01:38 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c3:54f:77a1:2d85:2067:5448]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-76bccfd73a2sm1871793b3a.108.2025.07.31.08.01.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 31 Jul 2025 08:01:38 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Florian Weimer , Simon Griebel Subject: [PATCH v4 2/3] nptl: Do not use pthread set_tid_address as state synchronization (BZ #19951) Date: Thu, 31 Jul 2025 12:00:46 -0300 Message-ID: <20250731150130.2970892-3-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250731150130.2970892-1-adhemerval.zanella@linaro.org> References: <20250731150130.2970892-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, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~patchwork=sourceware.org@sourceware.org The use-after-free described in BZ#19951 is due to the use of two different PD fields, 'joinid' and 'cancelhandling', to describe the thread state and to synchronize the calls of pthread_join, pthread_detach, pthread_exit, and normal thread exit. Any state change potentially requires checking for both field atomically to handle partial state (such as pthread_join() with a cancellation handler to issue a 'joinstate' field rollback). This patch uses a different PD member with 4 possible states (JOINABLE, DETACHED, EXITING, and EXITED) instead of pthread 'tid' field, with the following logic: 1. On pthread_create, the initial state is set either to JOINABLE or DETACHED depending on the pthread attribute used. 2. On pthread_detach, a CAS is issued on the state. If the CAS fails, it means that the thread is already detached (DETACHED) or is being terminated (EXITING). For the former, an EINVAL is returned, while for the latter, pthread_detach should be responsible for joining the thread (and deallocate any internal resources). 3. In the exit phase of the wrapper function for the thread start routine (reached either if the thread function has returned, pthread_exit has been called, or cancellation handled has been acted upon), we issue a CAS on state to set it to the EXITING mode. If the thread is previously on DETACHED mode, the thread itself is responsible for arranging the deallocation of any resource; otherwise, the thread needs to be joined (detached threads cannot immediately deallocate themselves). 4. The clear_tid_field on 'clone' call is changed to set the new 'state' field on thread exit (EXITED). This state is only reached at thread termination. 5. 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 the thread itself to deallocate its memory (done on the exit phase at pthread_create()). Also, the INVALID_NOT_TERMINATED_TD_P is removed since a negative tid is not possible, and the macro is not used anywhere. This change triggers an invalid C11 thread test: it creates 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 is not defined. 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/pthread_cancel.c | 3 +- nptl/pthread_clockjoin.c | 2 +- nptl/pthread_create.c | 42 +++++++--- nptl/pthread_detach.c | 40 +++++----- nptl/pthread_getattr_np.c | 2 +- nptl/pthread_join.c | 2 +- nptl/pthread_join_common.c | 118 ++++++++++------------------ nptl/pthread_timedjoin.c | 2 +- nptl/pthread_tryjoin.c | 19 +++-- sysdeps/nptl/dl-tls_init_tp.c | 3 +- sysdeps/nptl/libc_start_call_main.h | 6 ++ sysdeps/nptl/pthreadP.h | 3 +- sysdeps/pthread/tst-thrd-detach.c | 6 +- 15 files changed, 137 insertions(+), 139 deletions(-) diff --git a/nptl/descr.h b/nptl/descr.h index ada6867a19..396b2aa008 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -132,6 +132,18 @@ enum allocate_stack_mode_t ALLOCATE_GUARD_USER = 2, }; +/* Possible values for the 'joinstate' field. The field will be cleared + (set to THREAD_STATE_EXITED) atomically by the kernel when thread + terminated. */ +enum thread_state_t +{ + THREAD_STATE_EXITED = 0, + THREAD_STATE_EXITING, + THREAD_STATE_JOINABLE, + THREAD_STATE_DETACHED, +}; + + /* Thread descriptor data structure. */ struct pthread { @@ -174,8 +186,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; /* List of robust mutexes the thread is holding. */ @@ -345,15 +356,8 @@ struct pthread /* Lock for synchronizing setxid calls. */ unsigned int setxid_futex; - /* 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 ac672e16cf..ff62f3ed43 100644 --- a/nptl/nptl-stack.h +++ b/nptl/nptl-stack.h @@ -34,7 +34,7 @@ extern int32_t __nptl_stack_hugetlb; 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/pthread_cancel.c b/nptl/pthread_cancel.c index b838273881..575f29f068 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -60,7 +60,8 @@ __pthread_cancel (pthread_t th) { volatile struct pthread *pd = (volatile struct pthread *) th; - if (pd->tid == 0) + int state = atomic_load_relaxed (&pd->joinstate); + if (state == THREAD_STATE_EXITED) /* The thread has already exited on the kernel side. Its outcome (regular exit, other cancelation) has already been determined. */ diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c index 53944fbda0..fcb0cf8f6d 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 e1033d4ee6..6c567ccccb 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -290,7 +290,7 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr, .flags = clone_flags, .pidfd = (uintptr_t) &pd->tid, .parent_tid = (uintptr_t) &pd->tid, - .child_tid = (uintptr_t) &pd->tid, + .child_tid = (uintptr_t) &pd->joinstate, .stack = (uintptr_t) stackaddr, .stack_size = stacksize, .tls = (uintptr_t) tp, @@ -355,12 +355,14 @@ 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); if (setup_failed) + /* No need to clear the tid here, pthread_create() will join the + thread prior returning to caller. */ goto out; } @@ -492,6 +494,22 @@ start_thread (void *arg) the breakpoint reports TD_THR_RUN state rather than TD_THR_ZOMBIE. */ atomic_fetch_or_relaxed (&pd->cancelhandling, EXITING_BITMASK); + /* CONCURRENCY NOTES: + + Concurrent pthread_detach() will either set state to + THREAD_STATE_DETACHED or wait for 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 'prevstate' field will be used to determine who is responsible to + call __nptl_free_tcb below. */ + + unsigned int prevstate; + do + prevstate = atomic_load_relaxed (&pd->joinstate); + while (!atomic_compare_exchange_weak_acquire (&pd->joinstate, &prevstate, + THREAD_STATE_EXITING)); + if (__glibc_unlikely (atomic_fetch_add_relaxed (&__nptl_nthreads, -1) == 1)) /* This was the last thread. */ exit (0); @@ -574,20 +592,21 @@ start_thread (void *arg) pd->setxid_futex = 0; } - /* If the thread is detached free the TCB. */ - if (IS_DETACHED (pd)) + if (prevstate == THREAD_STATE_DETACHED) /* Free the TCB. */ __nptl_free_tcb (pd); /* Remove the associated name from the thread stack. */ name_stack_maps (pd, false); + pd->tid = 0; + out: /* We cannot call '_exit' here. '_exit' will terminate the process. 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. rseq TLS is still registered at this point. Rely on implicit unregistration performed by the kernel on thread teardown. This is not a @@ -702,7 +721,9 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, /* 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; @@ -861,10 +882,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 3bbc037bdc..55bbf07c37 100644 --- a/nptl/pthread_detach.c +++ b/nptl/pthread_detach.c @@ -25,32 +25,28 @@ ___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. */ + + unsigned int prevstate = atomic_load_relaxed (&pd->joinstate); + do { - /* 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 (prevstate != THREAD_STATE_JOINABLE) + { + if (prevstate == 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; + while (!atomic_compare_exchange_weak_acquire (&pd->joinstate, &prevstate, + THREAD_STATE_DETACHED)); + 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 43dd16d59c..f6d7526041 100644 --- a/nptl/pthread_getattr_np.c +++ b/nptl/pthread_getattr_np.c @@ -52,7 +52,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 cf8e56c8cf..c0ff2e9e77 100644 --- a/nptl/pthread_join.c +++ b/nptl/pthread_join.c @@ -22,7 +22,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 ff389bf599..2db240fc7d 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -22,116 +22,78 @@ #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 (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 + & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK + | TERMINATED_BITMASK)) == 0)) + && !cancel_enabled_and_canceled (self->cancelhandling)); } 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; - /* Make sure the clock and time specified are valid. */ if (abstime && __glibc_unlikely (!futex_abstimed_supported_clockid (clockid) || ! valid_nanoseconds (abstime->tv_nsec))) return EINVAL; - struct pthread *self = THREAD_SELF; - int result = 0; - LIBC_PROBE (pthread_join, 1, threadid); - if ((pd == self - || (self->joinid == pd - && (pd->cancelhandling - & (CANCELING_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 - 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); + if (check_for_deadlock (pd)) + return EDEADLK; - /* 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; - } + /* POSIX states calling pthread_join on a non joinable thread is + undefined. However, if PD is still in the cache we can 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 zeroing 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)) { - /* We mark the thread as terminated and as joined. */ - pd->tid = -1; - - /* Store the return value if the caller is interested. */ 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 691b69a89e..3bd17a7922 100644 --- a/nptl/pthread_timedjoin.c +++ b/nptl/pthread_timedjoin.c @@ -24,7 +24,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 54b528fd19..e49644d464 100644 --- a/nptl/pthread_tryjoin.c +++ b/nptl/pthread_tryjoin.c @@ -21,15 +21,18 @@ 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. - /* 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); + The exiting thread (THREAD_STATE_EXITING) also migth result in a 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). */ + + 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 47566dce4f..1d7442fc23 100644 --- a/sysdeps/nptl/dl-tls_init_tp.c +++ b/sysdeps/nptl/dl-tls_init_tp.c @@ -73,9 +73,10 @@ __tls_init_tp (void) list_add (&pd->list, &GL (dl_stack_user)); /* Early initialization of the TCB. */ - 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, stack_mode, ALLOCATE_GUARD_USER); + THREAD_SETMEM (pd, joinstate, THREAD_STATE_JOINABLE); /* Before initializing GL (dl_stack_user), the debugger could not find us and had to set __nptl_initial_report_events. Propagate diff --git a/sysdeps/nptl/libc_start_call_main.h b/sysdeps/nptl/libc_start_call_main.h index ca0436d27a..c1ce90e2f1 100644 --- a/sysdeps/nptl/libc_start_call_main.h +++ b/sysdeps/nptl/libc_start_call_main.h @@ -18,6 +18,7 @@ #include #include +#include _Noreturn static void __libc_start_call_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), @@ -65,6 +66,11 @@ __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_fetch_add_relaxed (&__nptl_nthreads, -1) != 1) /* Not much left to do but to exit the thread, not the process. */ while (1) diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h index 4a3881e945..9012bcc09b 100644 --- a/sysdeps/nptl/pthreadP.h +++ b/sysdeps/nptl/pthreadP.h @@ -233,7 +233,6 @@ libc_hidden_proto (__pthread_current_priority) nothing. And if the test triggers the thread descriptor is guaranteed to be invalid. */ #define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0) -#define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0) extern void __pthread_unwind (__pthread_unwind_buf_t *__buf) __cleanup_fct_attribute __attribute ((__noreturn__)) @@ -534,7 +533,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/sysdeps/pthread/tst-thrd-detach.c b/sysdeps/pthread/tst-thrd-detach.c index 966e7c1289..fa8d4181f3 100644 --- a/sysdeps/pthread/tst-thrd-detach.c +++ b/sysdeps/pthread/tst-thrd-detach.c @@ -28,7 +28,10 @@ detach_thrd (void *arg) { if (thrd_detach (thrd_current ()) != thrd_success) FAIL_EXIT1 ("thrd_detach failed"); - thrd_exit (thrd_success); + + pause (); + + return 0; } static int @@ -43,6 +46,7 @@ do_test (void) /* Give some time so the thread can finish. */ thrd_sleep (&(struct timespec) {.tv_sec = 2}, NULL); + /* Calling thrd_join on a detached thread is UB... */ if (thrd_join (id, NULL) == thrd_success) FAIL_EXIT1 ("thrd_join succeed where it should fail"); From patchwork Thu Jul 31 15:00:47 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 117339 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 3B770385DC06 for ; Thu, 31 Jul 2025 15:07:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3B770385DC06 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=eDMej6MC X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pf1-x432.google.com (mail-pf1-x432.google.com [IPv6:2607:f8b0:4864:20::432]) by sourceware.org (Postfix) with ESMTPS id C4A923857011 for ; Thu, 31 Jul 2025 15:01:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C4A923857011 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C4A923857011 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::432 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753974104; cv=none; b=PkULsuSvbw9tsi3NOieYAqK4KHvRVU3TwIcGRqwaKOysOeztQ3YwYIC4GC3SD0TOqRNrlJUQhrTGmpB8LKwbiCD2o4xwarg0wU+Jl2NSBpBSpQoZYvYsrxzSVjqgFpAwOAJhsB4QeLXMP+3tyZvQ8ocM8tSu2J8dqHIqyfHLK+k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1753974104; c=relaxed/simple; bh=fNqdkJJ4c0Gj9Ba06MOBzYqXOvxR5XZfcW0T3Na+ab4=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=J1XjkrKprDoHIETJdNnvsY8Z+36rJIdtF39XbYKwdk1mogZJdr55NEeOlCYnWXi/gy7etPa6k0qojTKo5ZLl62ukqIGrIZTEH/JZI2HnOJihyLE2LpVkJ3DpXPL82wx9aySOlgaJ6Vn4yTdFp+5i4aMntQbVol7vB2Ik1NxP0Ro= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C4A923857011 Received: by mail-pf1-x432.google.com with SMTP id d2e1a72fcca58-7682560a2f2so1101702b3a.1 for ; Thu, 31 Jul 2025 08:01:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1753974101; x=1754578901; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=9igzuEE8aW/EesjIgfVykt7k8eMu0HgqapAqHGy28I4=; b=eDMej6MCI/23rgaTufdNskEvempuKwMYLxvLeWypA7XxI76t+8uPFgMRvpDAYXHo56 RgTPwDAgs6aJ8CgGvU8tgRD167mspP3u2c2bAdnScBSDcgwCg2fOuWohnqkJIMdBUDVH QklCH4yZNyGAZQTd0+VFnbJacFr9zaNFmX6q7zR756kzDgL6UfhINN5eNBpOEOcHt79Z WpqzEd35L4rdQnEf7A/2ArxKl/6OG+3ley4VZnKQ9iaw4//CoVp9HYJZ6J2cQK5AqHNI 8RflPkSBJAuMGIpj3in40dg7zFOH7Tx9llGVYtjK6WyfHBpRAjlXZF/EUxv6HWMhYVsM 2cOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753974101; x=1754578901; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9igzuEE8aW/EesjIgfVykt7k8eMu0HgqapAqHGy28I4=; b=MOhz9idhvOXbYegAk8UpdDUDDXyVMJ8V2CAHozUsEXzuJdxDnBH8j6v9MiqJZD8ekI R3PqzOTYmHx2/kBmGgO3/qsFpWB/xjxlLvfDgoGn7gvwLtBaPvTacWKVt3aNFDqJwbjK s/OgZOqxga9504C2l/NjZK0fNdIv43myCYQ/BCRgfmGrcx052rN7bKImg5E++Y+oLGA/ vapdXNzeG3TlmnvHiiAr58tZGM8OoSPvsfhIklHUEjF8elkTpF6kcYRg7RIQ5kyySLHT N2Re5Y6CkRyTEXyEiaiC5RLoS925/KJ2FN7ACuI7ZPcPThvV2+1C2DiSims2qIv1h7vN b2tQ== X-Gm-Message-State: AOJu0Yz3ebUsyVj/jWWkzzaNTM9ruCy9+sxk9gpZsHBK++5s8vaReck/ xy72LYl9Wpz4maaSuCsEF/Pp2IuMiljpbcxmIBQkENZNcroprxxV8soKuvegKozC/vtQVLUsXug 7/C8P X-Gm-Gg: ASbGnctjPdfFKoI4ubiE2wQgC/66GXOYMwGX48vevlToUjQ0qL/jd8DG07rgXV4e/aF 6/64m2PGICic3dIluL8puwyzy+XHQqGGJS874DQHpY9gLMNnbqJbS/xN0LcLZmBaL0CEEzULTFX LeQc6tGzALJqUXUQ6wval1TMNw1JUIWtEbRcBCnvAYAxInLNbTR7mGR+lD6kA9gAJnXGl5v+L/F rSEgxPKbi18lBD7VDOmDj4j29cLwIbI9YkVlaSe8VNR6TVi71+AvgPtEDsFjWXkrRByc2wTHhhI iVezOv/LhvzUZ7Up5jY56Tajo7BBT7bl7QN/W0D6ZWmwoGuMT7KCZefzZkgX9Mbm4DYOy+bpeCW EhiPOT9y0lfK3m5xcGTepFheOpYhHmse5gA== X-Google-Smtp-Source: AGHT+IFZOs2y3t8TlZXCTjv4LiklWP0QtNplZnbAxXzrIq50Eo+23UOhjsmhFjqNN9xoEB6uxiqxSw== X-Received: by 2002:a05:6a00:6f54:b0:736:b400:b58f with SMTP id d2e1a72fcca58-76bccf03421mr3721725b3a.0.1753974100853; Thu, 31 Jul 2025 08:01:40 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c3:54f:77a1:2d85:2067:5448]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-76bccfd73a2sm1871793b3a.108.2025.07.31.08.01.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 31 Jul 2025 08:01:40 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Florian Weimer , Simon Griebel Subject: [PATCH v4 3/3] nptl: Remove INVALID_TD_P Date: Thu, 31 Jul 2025 12:00:47 -0300 Message-ID: <20250731150130.2970892-4-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250731150130.2970892-1-adhemerval.zanella@linaro.org> References: <20250731150130.2970892-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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~patchwork=sourceware.org@sourceware.org And use 'joinstate' to get the thread state instead of 'tid'. The joinstate is set by the kernel when the thread exits. Checked on x86_64-linux-gnu. --- nptl/pthread_getcpuclockid.c | 2 +- nptl/pthread_getschedparam.c | 2 +- nptl/pthread_setschedparam.c | 2 +- nptl/pthread_setschedprio.c | 2 +- sysdeps/nptl/pthreadP.h | 5 ++ sysdeps/pthread/Makefile | 1 + sysdeps/pthread/tst-pthread-exited.c | 106 +++++++++++++++++++++++++++ 7 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 sysdeps/pthread/tst-pthread-exited.c diff --git a/nptl/pthread_getcpuclockid.c b/nptl/pthread_getcpuclockid.c index 0cd9f77bea..70677d0138 100644 --- a/nptl/pthread_getcpuclockid.c +++ b/nptl/pthread_getcpuclockid.c @@ -29,7 +29,7 @@ __pthread_getcpuclockid (pthread_t threadid, clockid_t *clockid) struct pthread *pd = (struct pthread *) threadid; /* Make sure the descriptor is valid. */ - if (INVALID_TD_P (pd)) + if (!__pthread_descriptor_valid (pd)) /* Not a valid thread handle. */ return ESRCH; diff --git a/nptl/pthread_getschedparam.c b/nptl/pthread_getschedparam.c index bb74ed959c..aab0d836ce 100644 --- a/nptl/pthread_getschedparam.c +++ b/nptl/pthread_getschedparam.c @@ -28,7 +28,7 @@ __pthread_getschedparam (pthread_t threadid, int *policy, struct pthread *pd = (struct pthread *) threadid; /* Make sure the descriptor is valid. */ - if (INVALID_TD_P (pd)) + if (!__pthread_descriptor_valid (pd)) /* Not a valid thread handle. */ return ESRCH; diff --git a/nptl/pthread_setschedparam.c b/nptl/pthread_setschedparam.c index 1b43eb1f71..b057f8228f 100644 --- a/nptl/pthread_setschedparam.c +++ b/nptl/pthread_setschedparam.c @@ -29,7 +29,7 @@ __pthread_setschedparam (pthread_t threadid, int policy, struct pthread *pd = (struct pthread *) threadid; /* Make sure the descriptor is valid. */ - if (INVALID_TD_P (pd)) + if (!__pthread_descriptor_valid (pd)) /* Not a valid thread handle. */ return ESRCH; diff --git a/nptl/pthread_setschedprio.c b/nptl/pthread_setschedprio.c index c355716593..8cf7f5380f 100644 --- a/nptl/pthread_setschedprio.c +++ b/nptl/pthread_setschedprio.c @@ -29,7 +29,7 @@ __pthread_setschedprio (pthread_t threadid, int prio) struct pthread *pd = (struct pthread *) threadid; /* Make sure the descriptor is valid. */ - if (INVALID_TD_P (pd)) + if (!__pthread_descriptor_valid (pd)) /* Not a valid thread handle. */ return ESRCH; diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h index 9012bcc09b..4e14f60a05 100644 --- a/sysdeps/nptl/pthreadP.h +++ b/sysdeps/nptl/pthreadP.h @@ -233,6 +233,11 @@ libc_hidden_proto (__pthread_current_priority) nothing. And if the test triggers the thread descriptor is guaranteed to be invalid. */ #define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0) +static inline bool +__pthread_descriptor_valid (struct pthread *pd) +{ + return atomic_load_relaxed (&pd->joinstate) != THREAD_STATE_EXITED; +} extern void __pthread_unwind (__pthread_unwind_buf_t *__buf) __cleanup_fct_attribute __attribute ((__noreturn__)) diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index 7572f62cc0..3d6d815882 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -216,6 +216,7 @@ tests += \ tst-pt-vfork1 \ tst-pt-vfork2 \ tst-pthread-exit-signal \ + tst-pthread-exited \ tst-pthread-mutexattr \ tst-pthread-mutexattr-2 \ tst-pthread-raise-blocked-self \ diff --git a/sysdeps/pthread/tst-pthread-exited.c b/sysdeps/pthread/tst-pthread-exited.c new file mode 100644 index 0000000000..a951d09040 --- /dev/null +++ b/sysdeps/pthread/tst-pthread-exited.c @@ -0,0 +1,106 @@ +/* Test pthread interface which should return ESRCH when issued + with a terminated pthread_t. + + Copyright (C) 2025 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 +#include +#include +#include + +static void * +noop_thread (void *closure) +{ + return NULL; +} + +enum { nthreads = 1 }; + +static int +do_test_default (void) +{ + pthread_t thrs[nthreads]; + for (int i = 0; i < nthreads; i++) + thrs[i] = xpthread_create (NULL, noop_thread, NULL); + + support_wait_for_thread_exit (); + + for (int i = 0; i < nthreads; i++) + { + clockid_t clk; + TEST_COMPARE (pthread_getcpuclockid (thrs[i], &clk), ESRCH); + + struct sched_param sch = { 0 }; + int policy; + TEST_COMPARE (pthread_getschedparam (thrs[i], &policy, &sch), ESRCH); + + TEST_COMPARE (pthread_setschedparam (thrs[i], SCHED_FIFO, &sch), ESRCH); + + TEST_COMPARE (pthread_setschedprio (thrs[i], 0), ESRCH); + } + + for (int i = 0; i < nthreads; i++) + xpthread_join (thrs[i]); + + return 0; +} + + +static void * +detached_pause_thread (void *closure) +{ + pthread_detach (pthread_self ()); + pause (); + return NULL; +} + +static void +do_test_detached (void) +{ + pthread_t thrs[nthreads]; + for (int i = 0; i < nthreads; i++) + thrs[i] = xpthread_create (NULL, detached_pause_thread, NULL); + + for (int i = 0; i < nthreads; i++) + { + clockid_t clk; + TEST_COMPARE (pthread_getcpuclockid (thrs[i], &clk), 0); + + struct sched_param sch = { 0 }; + int policy; + TEST_COMPARE (pthread_getschedparam (thrs[i], &policy, &sch), 0); + + sch.sched_priority = 8; + TEST_COMPARE (pthread_setschedparam (thrs[i], SCHED_FIFO, &sch), EPERM); + + TEST_COMPARE (pthread_setschedprio (thrs[i], 0), 0); + } +} + +static int +do_test (void) +{ + do_test_default (); + do_test_detached (); + + return 0; +} + +#include