From: "Ondřej Bílka" <neleai@seznam.cz>
To: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Cc: libc-alpha@sourceware.org, libc-ports@sourceware.org
Subject: [RFC][BZ #13690] Always read private before lll_unlock.
Date: Fri, 06 Dec 2013 19:43:00 -0000 [thread overview]
Message-ID: <20131206194305.GA26401@domone.podge> (raw)
In-Reply-To: <20131206190159.GA25502@domone.podge>
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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#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 <futex_unlock.h>
#define __lll_robust_unlock(futex, private) \
(void) \
--
1.8.4.rc3
next parent reply other threads:[~2013-12-06 19:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20131206190159.GA25502@domone.podge>
2013-12-06 19:43 ` Ondřej Bílka [this message]
2013-12-06 21:12 ` Mike Frysinger
2013-12-06 21:52 ` Ondřej Bílka
2013-12-18 20:43 ` Torvald Riegel
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131206194305.GA26401@domone.podge \
--to=neleai@seznam.cz \
--cc=anemo@mba.ocn.ne.jp \
--cc=libc-alpha@sourceware.org \
--cc=libc-ports@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).