unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org
Subject: [PATCH v2 04/21] nptl: x86_64: Fix Race conditions in pthread cancellation (BZ#12683)
Date: Mon, 26 Feb 2018 18:03:19 -0300	[thread overview]
Message-ID: <1519679016-12241-5-git-send-email-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <1519679016-12241-1-git-send-email-adhemerval.zanella@linaro.org>

This patches adds the x86_64 modification required for the BZ#12683.
It basically provide the required ucontext_get_pc symbol and remove
the arch-specific libc-cancellation implementations.

It also remove bogus arch-specific THREAD_ATOMIC_BIT_SET where it
always reference to current thread instead of the one referenced by
input 'descr' argument. It works as long the input is the self thread
pointer, however it creates wrong code is it used along with a
description to a different one (as on nptl/pthread_cancel.c).

The code generated create an additional load to reference to TLS segment,
for instance the code:

  THREAD_ATOMIC_BIT_SET (THREAD_SELF, cancelhandling, CANCELED_BIT);

Compiles to:

  lock;orl $4, %fs:776

Where with patch changes it now compiles to:

  mov %fs:16,%rax
  lock;orl $4, 776(%rax)

If some usage indeed proves to be a hotspot we can add an extra macro
with a more descriptive name (THREAD_ATOMIC_BIT_SET_SELF for instance)
where x86_64 might optimize it.  In fact all x86_64 THREAD_ATOMIC_* macros
do not respect the input descr and possible will fail when used with
a 'descr' difference than THREAD_SELF.

Checked on x86_64-linux-gnu.

	[BZ #12683]
	* sysdeps/unix/sysv/linux/x86_64/cancellation.S: Remove file.
	* sysdeps/unix/sysv/linux/x86_64/libc-cancellation.S: Remove file.
	* sysdeps/unix/sysv/linux/x86_64/librt-cancellation.S: Remove file.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (lll_wait_tid):
	Use cancellable futex wait call.
	* sysdeps/unix/sysv/linux/x86_64/sigcontextinfo.h (ucontext_get_pc):
	New function.
	* sysdeps/x86_64/nptl/tcb-offsets.sym (TCB_CANCELING_BITMASK):
	Remove.
	* sysdeps/x86_64/nptl/tls.h (THREAD_ATOMIC_CMPXCHG_VAL,
	THREAD_ATOMIC_BIT_SET): Remove macros.
---
 ChangeLog                                          |  13 +++
 sysdeps/unix/sysv/linux/x86_64/cancellation.S      | 115 ---------------------
 sysdeps/unix/sysv/linux/x86_64/libc-cancellation.S |  21 ----
 .../unix/sysv/linux/x86_64/librt-cancellation.S    |  21 ----
 sysdeps/unix/sysv/linux/x86_64/lowlevellock.h      |   8 +-
 sysdeps/unix/sysv/linux/x86_64/sigcontextinfo.h    |  11 ++
 sysdeps/x86_64/nptl/tcb-offsets.sym                |   1 -
 sysdeps/x86_64/nptl/tls.h                          |  11 --
 8 files changed, 28 insertions(+), 173 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/cancellation.S
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/libc-cancellation.S
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/librt-cancellation.S

diff --git a/sysdeps/unix/sysv/linux/x86_64/cancellation.S b/sysdeps/unix/sysv/linux/x86_64/cancellation.S
deleted file mode 100644
index f3fb19d..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/cancellation.S
+++ /dev/null
@@ -1,115 +0,0 @@
-/* Copyright (C) 2009-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@redhat.com>, 2009.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sysdep.h>
-#include <tcb-offsets.h>
-#include <kernel-features.h>
-#include "lowlevellock.h"
-
-#define PTHREAD_UNWIND JUMPTARGET(__pthread_unwind)
-#if IS_IN (libpthread)
-# if defined SHARED && !defined NO_HIDDEN
-#  undef PTHREAD_UNWIND
-#  define PTHREAD_UNWIND __GI___pthread_unwind
-# endif
-#else
-# ifndef SHARED
-	.weak __pthread_unwind
-# endif
-#endif
-
-
-#ifdef __ASSUME_PRIVATE_FUTEX
-# define LOAD_PRIVATE_FUTEX_WAIT(reg) \
-	movl	$(FUTEX_WAIT | FUTEX_PRIVATE_FLAG), reg
-#else
-# if FUTEX_WAIT == 0
-#  define LOAD_PRIVATE_FUTEX_WAIT(reg) \
-	movl	%fs:PRIVATE_FUTEX, reg
-# else
-#  define LOAD_PRIVATE_FUTEX_WAIT(reg) \
-	movl	%fs:PRIVATE_FUTEX, reg ; \
-	orl	$FUTEX_WAIT, reg
-# endif
-#endif
-
-/* It is crucial that the functions in this file don't modify registers
-   other than %rax and %r11.  The syscall wrapper code depends on this
-   because it doesn't explicitly save the other registers which hold
-   relevant values.  */
-	.text
-
-	.hidden __pthread_enable_asynccancel
-ENTRY(__pthread_enable_asynccancel)
-	movl	%fs:CANCELHANDLING, %eax
-2:	movl	%eax, %r11d
-	orl	$TCB_CANCELTYPE_BITMASK, %r11d
-	cmpl	%eax, %r11d
-	je	1f
-
-	lock
-	cmpxchgl %r11d, %fs:CANCELHANDLING
-	jnz	2b
-
-	andl	$(TCB_CANCELSTATE_BITMASK|TCB_CANCELTYPE_BITMASK|TCB_CANCELED_BITMASK|TCB_EXITING_BITMASK|TCB_CANCEL_RESTMASK|TCB_TERMINATED_BITMASK), %r11d
-	cmpl	$(TCB_CANCELTYPE_BITMASK|TCB_CANCELED_BITMASK), %r11d
-	je	3f
-
-1:	ret
-
-3:	subq	$8, %rsp
-	cfi_adjust_cfa_offset(8)
-	LP_OP(mov) $TCB_PTHREAD_CANCELED, %fs:RESULT
-	lock
-	orl	$TCB_EXITING_BITMASK, %fs:CANCELHANDLING
-	mov	%fs:CLEANUP_JMP_BUF, %RDI_LP
-	call	PTHREAD_UNWIND
-	hlt
-END(__pthread_enable_asynccancel)
-
-
-	.hidden __pthread_disable_asynccancel
-ENTRY(__pthread_disable_asynccancel)
-	testl	$TCB_CANCELTYPE_BITMASK, %edi
-	jnz	1f
-
-	movl	%fs:CANCELHANDLING, %eax
-2:	movl	%eax, %r11d
-	andl	$~TCB_CANCELTYPE_BITMASK, %r11d
-	lock
-	cmpxchgl %r11d, %fs:CANCELHANDLING
-	jnz	2b
-
-	movl	%r11d, %eax
-3:	andl	$(TCB_CANCELING_BITMASK|TCB_CANCELED_BITMASK), %eax
-	cmpl	$TCB_CANCELING_BITMASK, %eax
-	je	4f
-1:	ret
-
-	/* Performance doesn't matter in this loop.  We will
-	   delay until the thread is canceled.  And we will unlikely
-	   enter the loop twice.  */
-4:	mov	%fs:0, %RDI_LP
-	movl	$__NR_futex, %eax
-	xorq	%r10, %r10
-	addq	$CANCELHANDLING, %rdi
-	LOAD_PRIVATE_FUTEX_WAIT (%esi)
-	syscall
-	movl	%fs:CANCELHANDLING, %eax
-	jmp	3b
-END(__pthread_disable_asynccancel)
diff --git a/sysdeps/unix/sysv/linux/x86_64/libc-cancellation.S b/sysdeps/unix/sysv/linux/x86_64/libc-cancellation.S
deleted file mode 100644
index ed7af0d..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/libc-cancellation.S
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Copyright (C) 2009-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@redhat.com>, 2009.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#define __pthread_enable_asynccancel __libc_enable_asynccancel
-#define __pthread_disable_asynccancel __libc_disable_asynccancel
-#include "cancellation.S"
diff --git a/sysdeps/unix/sysv/linux/x86_64/librt-cancellation.S b/sysdeps/unix/sysv/linux/x86_64/librt-cancellation.S
deleted file mode 100644
index d0f0ee4..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/librt-cancellation.S
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Copyright (C) 2009-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Ulrich Drepper <drepper@redhat.com>, 2009.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#define __pthread_enable_asynccancel __librt_enable_asynccancel
-#define __pthread_disable_asynccancel __librt_disable_asynccancel
-#include "cancellation.S"
diff --git a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index eedb6fc..957d4c1 100644
--- a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -231,10 +231,10 @@ extern int __lll_timedlock_elision (int *futex, short *adapt_count,
    afterwards.  The kernel up to version 3.16.3 does not use the private futex
    operations for futex wake-up when the clone terminates.  */
 #define lll_wait_tid(tid) \
-  do {					\
-    __typeof (tid) __tid;		\
-    while ((__tid = (tid)) != 0)	\
-      lll_futex_wait (&(tid), __tid, LLL_SHARED);\
+  do {									      \
+    __typeof (tid) __tid;						      \
+    while ((__tid = (tid)) != 0)					      \
+      lll_futex_wait_cancel (&(tid), __tid, LLL_SHARED);		      \
   } while (0)
 
 extern int __lll_timedwait_tid (int *, const struct timespec *)
diff --git a/sysdeps/unix/sysv/linux/x86_64/sigcontextinfo.h b/sysdeps/unix/sysv/linux/x86_64/sigcontextinfo.h
index 41ace60..3cc7336 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sigcontextinfo.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sigcontextinfo.h
@@ -15,6 +15,9 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef _SIGCONTEXTINFO_H
+#define _SIGCONTEXTINFO_H
+
 #include <stdint.h>
 
 #define SIGCONTEXT siginfo_t *_si, ucontext_t *
@@ -28,3 +31,11 @@
 
 #define CALL_SIGHANDLER(handler, signo, ctx) \
   (handler)((signo), SIGCONTEXT_EXTRA_ARGS (ctx))
+
+static inline
+uintptr_t ucontext_get_pc (const ucontext_t *uc)
+{
+  return uc->uc_mcontext.gregs[REG_RIP];
+}
+
+#endif
diff --git a/sysdeps/x86_64/nptl/tcb-offsets.sym b/sysdeps/x86_64/nptl/tcb-offsets.sym
index 8a25c48..b225e5b 100644
--- a/sysdeps/x86_64/nptl/tcb-offsets.sym
+++ b/sysdeps/x86_64/nptl/tcb-offsets.sym
@@ -19,7 +19,6 @@ PRIVATE_FUTEX		offsetof (tcbhead_t, private_futex)
 -- Not strictly offsets, but these values are also used in the TCB.
 TCB_CANCELSTATE_BITMASK	 CANCELSTATE_BITMASK
 TCB_CANCELTYPE_BITMASK	 CANCELTYPE_BITMASK
-TCB_CANCELING_BITMASK	 CANCELING_BITMASK
 TCB_CANCELED_BITMASK	 CANCELED_BITMASK
 TCB_EXITING_BITMASK	 EXITING_BITMASK
 TCB_CANCEL_RESTMASK	 CANCEL_RESTMASK
diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index bdd0237..ed0c65b 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -315,17 +315,6 @@ typedef struct
 	      abort (); })
 
 
-/* Atomic set bit.  */
-# define THREAD_ATOMIC_BIT_SET(descr, member, bit) \
-  (void) ({ if (sizeof ((descr)->member) == 4)				      \
-	      asm volatile (LOCK_PREFIX "orl %1, %%fs:%P0"		      \
-			    :: "i" (offsetof (struct pthread, member)),	      \
-			       "ir" (1 << (bit)));			      \
-	    else							      \
-	      /* Not necessary for other sizes in the moment.  */	      \
-	      abort (); })
-
-
 /* Set the stack guard field in TCB head.  */
 # define THREAD_SET_STACK_GUARD(value) \
     THREAD_SETMEM (THREAD_SELF, header.stack_guard, value)
-- 
2.7.4



  parent reply	other threads:[~2018-02-26 21:02 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-26 21:03 [PATCH v2 00/21] nptl: Fix Race conditions in pthread cancellation (BZ#12683) Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 01/21] powerpc: Create stackframe information on syscall Adhemerval Zanella
2018-02-26 21:41   ` Andreas Schwab
2018-02-27 12:05     ` Adhemerval Zanella
2018-05-07  2:49   ` Zack Weinberg
2018-05-07 13:57     ` Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 02/21] nptl: Fix testcases for new pthread cancellation mechanism Adhemerval Zanella
2018-02-26 21:43   ` Andreas Schwab
2018-02-27 12:05     ` Adhemerval Zanella
2018-05-07  2:49   ` Zack Weinberg
2018-05-07 17:13     ` Adhemerval Zanella
2018-05-08 13:35       ` Zack Weinberg
2018-05-08 17:26         ` Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 03/21] nptl: Fix Race conditions in pthread cancellation (BZ#12683) Adhemerval Zanella
2018-04-27 12:20   ` Zack Weinberg
2018-04-27 12:25     ` Adhemerval Zanella
2018-05-07  2:48       ` Zack Weinberg
2018-05-07  2:49   ` Zack Weinberg
2018-05-08 17:11     ` Adhemerval Zanella
2018-02-26 21:03 ` Adhemerval Zanella [this message]
2018-05-07  2:49   ` [PATCH v2 04/21] nptl: x86_64: " Zack Weinberg
2018-05-08 17:55     ` Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 05/21] nptl: x32: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 06/21] nptl: i386: " Adhemerval Zanella
2018-05-07  2:49   ` Zack Weinberg
2018-05-08 17:56     ` Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 07/21] nptl: powerpc: " Adhemerval Zanella
2018-05-07 19:25   ` Tulio Magno Quites Machado Filho
2018-05-08 18:07     ` Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 08/21] nptl: aarch64: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 09/21] nptl: arm: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 10/21] nptl: s390: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 11/21] nptl: ia64: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 12/21] nptl: alpha: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 13/21] nptl: m68k: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 14/21] nptl: microblaze: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 15/21] nptl: tile: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 16/21] nptl: sparc: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 17/21] nptl: nios2: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 18/21] nptl: sh: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 19/21] nptl: mips: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 20/21] nptl: hppa: " Adhemerval Zanella
2018-02-26 21:03 ` [PATCH v2 21/21] nptl: riscv: " Adhemerval Zanella
2018-02-27  1:16   ` DJ Delorie
2018-02-27 13:03     ` Adhemerval Zanella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1519679016-12241-5-git-send-email-adhemerval.zanella@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).