From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22541 invoked by alias); 6 Dec 2013 19:43:20 -0000 Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org Received: (qmail 22480 invoked by uid 89); 6 Dec 2013 19:43:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,SPF_NEUTRAL autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: popelka.ms.mff.cuni.cz Received: from Unknown (HELO popelka.ms.mff.cuni.cz) (195.113.20.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Dec 2013 19:43:17 +0000 Received: from domone.kolej.mff.cuni.cz (popelka.ms.mff.cuni.cz [195.113.20.131]) by popelka.ms.mff.cuni.cz (Postfix) with ESMTPS id A83DB6BF4A; Fri, 6 Dec 2013 20:43:05 +0100 (CET) Received: by domone.kolej.mff.cuni.cz (Postfix, from userid 1000) id 82DBD5F767; Fri, 6 Dec 2013 20:43:05 +0100 (CET) Date: Fri, 06 Dec 2013 19:43:00 -0000 From: =?utf-8?B?T25kxZllaiBCw61sa2E=?= To: Atsushi Nemoto Cc: libc-alpha@sourceware.org, libc-ports@sourceware.org Subject: [RFC][BZ #13690] Always read private before lll_unlock. Message-ID: <20131206194305.GA26401@domone.podge> References: <20131206190159.GA25502@domone.podge> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20131206190159.GA25502@domone.podge> User-Agent: Mutt/1.5.20 (2009-06-14) X-SW-Source: 2013-12/txt/msg00003.txt.bz2 On Fri, Dec 06, 2013 at 08:01:59PM +0100, Ondřej Bílka wrote: > Hi, a related issue to semaphore races is a race in mutex unlocking. > from bugzilla: > > On most platforms, lll_unlock() is defined as a macro like this: > #define lll_unlock(lock, private) \ > ((void) ({ \ > int *__futex = &(lock); \ > int __val = atomic_exchange_rel (__futex, 0); \ > if (__builtin_expect (__val > 1, 0)) \ > lll_futex_wake (__futex, 1, private); \ > })) > Which causes this problem that could be avoided by changing macro to #define lll_unlock(lock, private) \ ((void) ({ \ int *__futex = &(lock); \ int __private = private \ I wrote a prelimitary patch for that, most of lll_unlock macros are duplicates so I added a file include/futex_unlock.h with common implementation. We should check these for more duplicates and if other functions need similar wrapper. Comments? --- include/futex_unlock.h | 15 +++++++++++++++ nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h | 8 +------- nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h | 6 +++++- nptl/sysdeps/unix/sysv/linux/sh/lowlevellock.h | 8 +++++++- nptl/sysdeps/unix/sysv/linux/sparc/lowlevellock.h | 6 ++++++ ports/sysdeps/unix/sysv/linux/aarch64/nptl/lowlevellock.h | 11 +---------- ports/sysdeps/unix/sysv/linux/alpha/nptl/lowlevellock.h | 10 +--------- ports/sysdeps/unix/sysv/linux/arm/nptl/lowlevellock.h | 11 +---------- ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.h | 8 +------- ports/sysdeps/unix/sysv/linux/ia64/nptl/lowlevellock.h | 12 +----------- ports/sysdeps/unix/sysv/linux/m68k/nptl/lowlevellock.h | 11 +---------- .../unix/sysv/linux/microblaze/nptl/lowlevellock.h | 10 +--------- ports/sysdeps/unix/sysv/linux/mips/nptl/lowlevellock.h | 11 +---------- ports/sysdeps/unix/sysv/linux/tile/nptl/lowlevellock.h | 11 +---------- 14 files changed, 43 insertions(+), 95 deletions(-) create mode 100644 include/futex_unlock.h diff --git a/include/futex_unlock.h b/include/futex_unlock.h new file mode 100644 index 0000000..cd497c2 --- /dev/null +++ b/include/futex_unlock.h @@ -0,0 +1,15 @@ +#define lll_unlock(lock, private) \ + ((void)) ({ \ + int __private = private; \ + __lll_unlock (lock, __private); \ + }) + +#define lll_unlock(lock, private) \ + ((void) ({ \ + int *__futex = &(lock); \ + int __val = atomic_exchange_rel (__futex, 0); \ + if (__builtin_expect (__val > 1, 0)) \ + lll_futex_wake (__futex, 1, private); \ + })) + + diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h index f33f703..84dea3d 100644 --- a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h @@ -302,13 +302,7 @@ extern int __lll_robust_timedlock_wait __val; \ }) -#define lll_unlock(lock, private) \ - ((void) ({ \ - int *__futex = &(lock); \ - int __val = atomic_exchange_rel (__futex, 0); \ - if (__builtin_expect (__val > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - })) +#include #define lll_robust_unlock(lock, private) \ ((void) ({ \ diff --git a/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h index 3dab05e..911ff74 100644 --- a/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h +++ b/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h @@ -299,6 +299,11 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, #define lll_robust_timedlock(futex, abstime, id, private) \ __lll_robust_timedlock (&(futex), abstime, id, private) +#define lll_unlock(futex, private) \ + ((void)) ({ \ + int __private = private; \ + __lll_unlock (&(futex), __private); \ + }) #define __lll_unlock(futex, private) \ (void) \ @@ -310,7 +315,6 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, if (__builtin_expect (__oldval > 1, 0)) \ lll_futex_wake (__futexp, 1, private); \ }) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) #define __lll_robust_unlock(futex, private) \ diff --git a/nptl/sysdeps/unix/sysv/linux/sh/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/sh/lowlevellock.h index 486e02c..e24cdf2 100644 --- a/nptl/sysdeps/unix/sysv/linux/sh/lowlevellock.h +++ b/nptl/sysdeps/unix/sysv/linux/sh/lowlevellock.h @@ -284,7 +284,13 @@ extern int __lll_unlock_wake (int *__futex, int private) attribute_hidden; timeout, private); \ __result; }) -#define lll_unlock(futex, private) \ +#define lll_unlock(lock, private) \ + ((void)) ({ \ + int __private = private; \ + __lll_unlock (lock, __private); \ + }) + +#define __lll_unlock(futex, private) \ (void) ({ int __result, *__futex = &(futex); \ __asm __volatile ("\ .align 2\n\ diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/sparc/lowlevellock.h index 5ee8f6d..e435f4b 100644 --- a/nptl/sysdeps/unix/sysv/linux/sparc/lowlevellock.h +++ b/nptl/sysdeps/unix/sysv/linux/sparc/lowlevellock.h @@ -293,6 +293,12 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, __lll_robust_timedlock (&(futex), abstime, id, private) #define lll_unlock(lock, private) \ + ((void)) ({ \ + int __private = private; \ + __lll_unlock (lock, __private); \ + }) + +#define __lll_unlock(lock, private) \ ((void) ({ \ int *__futex = &(lock); \ int __val = atomic_exchange_24_rel (__futex, 0); \ diff --git a/ports/sysdeps/unix/sysv/linux/aarch64/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/aarch64/nptl/lowlevellock.h index 52f8a7a..0256f81 100644 --- a/ports/sysdeps/unix/sysv/linux/aarch64/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/aarch64/nptl/lowlevellock.h @@ -232,16 +232,7 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, __lll_robust_timedlock (&(futex), abstime, id, private) -#define __lll_unlock(futex, private) \ - (void) \ - ({ int *__futex = (futex); \ - int __oldval = atomic_exchange_rel (__futex, 0); \ - if (__builtin_expect (__oldval > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - }) - -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) - +#include #define __lll_robust_unlock(futex, private) \ (void) \ diff --git a/ports/sysdeps/unix/sysv/linux/alpha/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/alpha/nptl/lowlevellock.h index 567f8ab..a1f1143 100644 --- a/ports/sysdeps/unix/sysv/linux/alpha/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/alpha/nptl/lowlevellock.h @@ -268,15 +268,7 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, __lll_robust_timedlock (&(futex), abstime, id, private) -#define __lll_unlock(futex, private) \ - (void) \ - ({ int *__futex = (futex); \ - int __oldval = atomic_exchange_rel (__futex, 0); \ - if (__builtin_expect (__oldval > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - }) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) - +#include #define __lll_robust_unlock(futex, private) \ (void) \ diff --git a/ports/sysdeps/unix/sysv/linux/arm/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/arm/nptl/lowlevellock.h index a29593a..c9b3335 100644 --- a/ports/sysdeps/unix/sysv/linux/arm/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/arm/nptl/lowlevellock.h @@ -258,16 +258,7 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, #define lll_robust_timedlock(futex, abstime, id, private) \ __lll_robust_timedlock (&(futex), abstime, id, private) - -#define __lll_unlock(futex, private) \ - (void) \ - ({ int *__futex = (futex); \ - int __oldval = atomic_exchange_rel (__futex, 0); \ - if (__builtin_expect (__oldval > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - }) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) - +#include #define __lll_robust_unlock(futex, private) \ (void) \ diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.h index 4cf8468..d76ef90 100644 --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/lowlevellock.h @@ -288,13 +288,7 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, #define lll_robust_timedlock(futex, abstime, id, private) \ __lll_robust_timedlock (&(futex), abstime, id, private) -#define __lll_unlock(futex, private) \ - (void) \ - ({ int val = atomic_exchange_rel (futex, 0); \ - if (__builtin_expect (val > 1, 0)) \ - lll_futex_wake (futex, 1, private); \ - }) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) +#include #define __lll_robust_unlock(futex,private) \ (void) \ diff --git a/ports/sysdeps/unix/sysv/linux/ia64/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/ia64/nptl/lowlevellock.h index cd36f95..3140447 100644 --- a/ports/sysdeps/unix/sysv/linux/ia64/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/ia64/nptl/lowlevellock.h @@ -236,17 +236,7 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, #define lll_robust_timedlock(futex, abstime, id, private) \ __lll_robust_timedlock (&(futex), abstime, id, private) - -#define __lll_unlock(futex, private) \ - ((void) ({ \ - int *__futex = (futex); \ - int __val = atomic_exchange_rel (__futex, 0); \ - \ - if (__builtin_expect (__val > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - })) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) - +#include #define __lll_robust_unlock(futex, private) \ ((void) ({ \ diff --git a/ports/sysdeps/unix/sysv/linux/m68k/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/m68k/nptl/lowlevellock.h index 0df6604..5f683c2 100644 --- a/ports/sysdeps/unix/sysv/linux/m68k/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/m68k/nptl/lowlevellock.h @@ -228,16 +228,7 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, #define lll_robust_timedlock(futex, abstime, id, private) \ __lll_robust_timedlock (&(futex), abstime, id, private) - -#define __lll_unlock(futex, private) \ - (void) \ - ({ int *__futex = (futex); \ - int __oldval = atomic_exchange_rel (__futex, 0); \ - if (__builtin_expect (__oldval > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - }) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) - +#include #define __lll_robust_unlock(futex, private) \ (void) \ diff --git a/ports/sysdeps/unix/sysv/linux/microblaze/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/microblaze/nptl/lowlevellock.h index 70f5537..a601233 100644 --- a/ports/sysdeps/unix/sysv/linux/microblaze/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/microblaze/nptl/lowlevellock.h @@ -256,15 +256,7 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, #define lll_robust_timedlock(futex, abstime, id, private) \ __lll_robust_timedlock (&(futex), abstime, id, private) -#define __lll_unlock(futex, private) \ - ((void) ({ \ - int *__futex = (futex); \ - int __val = atomic_exchange_rel (__futex, 0); \ - \ - if (__builtin_expect (__val > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - })) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) +#include #define __lll_robust_unlock(futex, private) \ ((void) ({ \ diff --git a/ports/sysdeps/unix/sysv/linux/mips/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/mips/nptl/lowlevellock.h index 208df8d..001f5f4 100644 --- a/ports/sysdeps/unix/sysv/linux/mips/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/mips/nptl/lowlevellock.h @@ -271,16 +271,7 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, __lll_robust_timedlock (&(futex), abstime, id, private) -#define __lll_unlock(futex, private) \ - ((void) ({ \ - int *__futex = (futex); \ - int __val = atomic_exchange_rel (__futex, 0); \ - \ - if (__builtin_expect (__val > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - })) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) - +#include #define __lll_robust_unlock(futex, private) \ ((void) ({ \ diff --git a/ports/sysdeps/unix/sysv/linux/tile/nptl/lowlevellock.h b/ports/sysdeps/unix/sysv/linux/tile/nptl/lowlevellock.h index a9822ec..a8844f3 100644 --- a/ports/sysdeps/unix/sysv/linux/tile/nptl/lowlevellock.h +++ b/ports/sysdeps/unix/sysv/linux/tile/nptl/lowlevellock.h @@ -266,16 +266,7 @@ __lll_robust_timedlock (int *futex, const struct timespec *abstime, #define lll_robust_timedlock(futex, abstime, id, private) \ __lll_robust_timedlock (&(futex), abstime, id, private) - -#define __lll_unlock(futex, private) \ - (void) \ - ({ int *__futex = (futex); \ - int __oldval = atomic_exchange_rel (__futex, 0); \ - if (__builtin_expect (__oldval > 1, 0)) \ - lll_futex_wake (__futex, 1, private); \ - }) -#define lll_unlock(futex, private) __lll_unlock(&(futex), private) - +#include #define __lll_robust_unlock(futex, private) \ (void) \ -- 1.8.4.rc3