diff mbox

[review] nptl: Cleanup mutex internal offset tests

Message ID gerrit.1573242232000.I7a4e48cc91b4c4ada57e9a5d1b151fb702bfaa9f@gnutoolchain-gerrit.osci.io
State Superseded
Headers

Commit Message

Simon Marchi (Code Review) Nov. 8, 2019, 7:43 p.m. UTC 
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/581
......................................................................

nptl: Cleanup mutex internal offset tests

The offset of pthread_mutex_t __data.__nusers, __data.__spins,
__data.elision, __data.list are not required to be constant over
the releases.  Only the __data.__flags are used for static
initializers.

This patch also adds an additional size check for __data.__flags.

Check with a build against affected abis.

Change-Id: I7a4e48cc91b4c4ada57e9a5d1b151fb702bfaa9f
---
M nptl/pthreadP.h
M nptl/pthread_mutex_init.c
M sysdeps/aarch64/nptl/pthread-offsets.h
M sysdeps/alpha/nptl/pthread-offsets.h
M sysdeps/arm/nptl/pthread-offsets.h
M sysdeps/csky/nptl/pthread-offsets.h
M sysdeps/hppa/nptl/pthread-offsets.h
M sysdeps/i386/nptl/pthread-offsets.h
M sysdeps/ia64/nptl/pthread-offsets.h
M sysdeps/m68k/nptl/pthread-offsets.h
M sysdeps/microblaze/nptl/pthread-offsets.h
M sysdeps/mips/nptl/pthread-offsets.h
M sysdeps/nios2/nptl/pthread-offsets.h
M sysdeps/powerpc/nptl/pthread-offsets.h
M sysdeps/riscv/nptl/pthread-offsets.h
M sysdeps/s390/nptl/pthread-offsets.h
M sysdeps/sh/nptl/pthread-offsets.h
M sysdeps/sparc/nptl/pthread-offsets.h
M sysdeps/x86_64/nptl/pthread-offsets.h
19 files changed, 6 insertions(+), 94 deletions(-)

Comments

Simon Marchi (Code Review) Nov. 14, 2019, 2:34 p.m. UTC | #1 
Florian Weimer has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/581
......................................................................


Patch Set 1:

(5 comments)

I ran this through a full build-many-glibcs.py run, and the results look good.

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,18 @@ 
| +Parent:     31f000a8 (Remove hppa pthreadP.h)
| +Author:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +AuthorDate: 2019-11-07 20:58:41 +0000
| +Commit:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +CommitDate: 2019-11-08 16:41:53 -0300
| +
| +nptl: Cleanup mutex internal offset tests
| +
| +The offset of pthread_mutex_t __data.__nusers, __data.__spins,

PS1, Line 9:

“offsets” (I think)

| +__data.elision, __data.list are not required to be constant over
| +the releases.  Only the __data.__flags are used for static
| +initializers.

PS1, Line 12:

“Only __data.__kind is used for static initializers.”

| +
| +This patch also adds an additional size check for __data.__flags.

PS1, Line 14:

__data.__kind

| +
| +Check with a build against affected abis.

PS1, Line 16:

“Checked with a build against affected ABIs.”?

| +
| +Change-Id: I7a4e48cc91b4c4ada57e9a5d1b151fb702bfaa9f
| --- nptl/pthread_mutex_init.c
| +++ nptl/pthread_mutex_init.c
| @@ -51,17 +51,11 @@ int
|  __pthread_mutex_init (pthread_mutex_t *mutex,
|  		      const pthread_mutexattr_t *mutexattr)
|  {
|    const struct pthread_mutexattr *imutexattr;
|  
|    ASSERT_TYPE_SIZE (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T);
|  
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__nusers,
| -				  __PTHREAD_MUTEX_NUSERS_OFFSET);
| +  /* The __flags is the only field where its offset should be checked to

PS1, Line 58:

Probably: /* __kind is the only field
(without the definite article, and fixed field name).

| +     avoid ABI breakage with static initializers.  */
|    ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__kind,
|  				  __PTHREAD_MUTEX_KIND_OFFSET);
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__spins,
| -				  __PTHREAD_MUTEX_SPINS_OFFSET);
| -#if __PTHREAD_MUTEX_LOCK_ELISION
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__elision,
| -				  __PTHREAD_MUTEX_ELISION_OFFSET);
| -#endif
Simon Marchi (Code Review) Nov. 20, 2019, 2:49 p.m. UTC | #2 
Adhemerval Zanella has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/581
......................................................................


Patch Set 1:

(5 comments)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +1,18 @@ 
| +Parent:     31f000a8 (Remove hppa pthreadP.h)
| +Author:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +AuthorDate: 2019-11-07 20:58:41 +0000
| +Commit:     Adhemerval Zanella <adhemerval.zanella@linaro.org>
| +CommitDate: 2019-11-08 16:41:53 -0300
| +
| +nptl: Cleanup mutex internal offset tests
| +
| +The offset of pthread_mutex_t __data.__nusers, __data.__spins,

PS1, Line 9:

Ack

| +__data.elision, __data.list are not required to be constant over
| +the releases.  Only the __data.__flags are used for static
| +initializers.

PS1, Line 12:

Ack

| +
| +This patch also adds an additional size check for __data.__flags.

PS1, Line 14:

Ack

| +
| +Check with a build against affected abis.

PS1, Line 16:

Ack

| +
| +Change-Id: I7a4e48cc91b4c4ada57e9a5d1b151fb702bfaa9f
| --- nptl/pthread_mutex_init.c
| +++ nptl/pthread_mutex_init.c
| @@ -51,17 +51,11 @@ int
|  __pthread_mutex_init (pthread_mutex_t *mutex,
|  		      const pthread_mutexattr_t *mutexattr)
|  {
|    const struct pthread_mutexattr *imutexattr;
|  
|    ASSERT_TYPE_SIZE (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T);
|  
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__nusers,
| -				  __PTHREAD_MUTEX_NUSERS_OFFSET);
| +  /* The __flags is the only field where its offset should be checked to

PS1, Line 58:

Ack

| +     avoid ABI breakage with static initializers.  */
|    ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__kind,
|  				  __PTHREAD_MUTEX_KIND_OFFSET);
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__spins,
| -				  __PTHREAD_MUTEX_SPINS_OFFSET);
| -#if __PTHREAD_MUTEX_LOCK_ELISION
| -  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__elision,
| -				  __PTHREAD_MUTEX_ELISION_OFFSET);
| -#endif
Simon Marchi (Code Review) Nov. 22, 2019, 11:09 a.m. UTC | #3 
Florian Weimer has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/581
......................................................................


Patch Set 2: Code-Review+2
diff mbox

Patch

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index bbb2bf0..9c20067 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -617,5 +617,8 @@ 
   _Static_assert (offsetof (type, member) == offset,			\
 		  "offset of " #member " field of " #type " != "	\
 		  ASSERT_PTHREAD_STRING (offset))
+#define ASSERT_PTHREAD_INTERNAL_MEMBER_SIZE(type, member, mtype)	\
+  _Static_assert (sizeof (((type) { 0 }).member) != 8,	\
+		  "sizeof (" #type "." #member ") != sizeof (" #mtype "))")
 
 #endif	/* pthreadP.h */
diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
index 20800b8..d2a86ee 100644
--- a/nptl/pthread_mutex_init.c
+++ b/nptl/pthread_mutex_init.c
@@ -55,18 +55,11 @@ 
 
   ASSERT_TYPE_SIZE (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T);
 
-  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__nusers,
-				  __PTHREAD_MUTEX_NUSERS_OFFSET);
+  /* The __flags is the only field where its offset should be checked to
+     avoid ABI breakage with static initializers.  */
   ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__kind,
 				  __PTHREAD_MUTEX_KIND_OFFSET);
-  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__spins,
-				  __PTHREAD_MUTEX_SPINS_OFFSET);
-#if __PTHREAD_MUTEX_LOCK_ELISION
-  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__elision,
-				  __PTHREAD_MUTEX_ELISION_OFFSET);
-#endif
-  ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__list,
-				  __PTHREAD_MUTEX_LIST_OFFSET);
+  ASSERT_PTHREAD_INTERNAL_MEMBER_SIZE (pthread_mutex_t, __data.__kind, int);
 
   imutexattr = ((const struct pthread_mutexattr *) mutexattr
 		?: &default_mutexattr);
diff --git a/sysdeps/aarch64/nptl/pthread-offsets.h b/sysdeps/aarch64/nptl/pthread-offsets.h
index 16c6b0d..8208f6f 100644
--- a/sysdeps/aarch64/nptl/pthread-offsets.h
+++ b/sysdeps/aarch64/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 #define __PTHREAD_MUTEX_KIND_OFFSET     16
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     24
diff --git a/sysdeps/alpha/nptl/pthread-offsets.h b/sysdeps/alpha/nptl/pthread-offsets.h
index 16c6b0d..8208f6f 100644
--- a/sysdeps/alpha/nptl/pthread-offsets.h
+++ b/sysdeps/alpha/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 #define __PTHREAD_MUTEX_KIND_OFFSET     16
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     24
diff --git a/sysdeps/arm/nptl/pthread-offsets.h b/sysdeps/arm/nptl/pthread-offsets.h
index 9617354..e4e775d 100644
--- a/sysdeps/arm/nptl/pthread-offsets.h
+++ b/sysdeps/arm/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     20
diff --git a/sysdeps/csky/nptl/pthread-offsets.h b/sysdeps/csky/nptl/pthread-offsets.h
index 9617354..e4e775d 100644
--- a/sysdeps/csky/nptl/pthread-offsets.h
+++ b/sysdeps/csky/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     20
diff --git a/sysdeps/hppa/nptl/pthread-offsets.h b/sysdeps/hppa/nptl/pthread-offsets.h
index 8ae01b9..e4e775d 100644
--- a/sysdeps/hppa/nptl/pthread-offsets.h
+++ b/sysdeps/hppa/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   32
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    36
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     36
diff --git a/sysdeps/i386/nptl/pthread-offsets.h b/sysdeps/i386/nptl/pthread-offsets.h
index 9617354..e4e775d 100644
--- a/sysdeps/i386/nptl/pthread-offsets.h
+++ b/sysdeps/i386/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     20
diff --git a/sysdeps/ia64/nptl/pthread-offsets.h b/sysdeps/ia64/nptl/pthread-offsets.h
index 16c6b0d..8208f6f 100644
--- a/sysdeps/ia64/nptl/pthread-offsets.h
+++ b/sysdeps/ia64/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 #define __PTHREAD_MUTEX_KIND_OFFSET     16
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     24
diff --git a/sysdeps/m68k/nptl/pthread-offsets.h b/sysdeps/m68k/nptl/pthread-offsets.h
index 9617354..e4e775d 100644
--- a/sysdeps/m68k/nptl/pthread-offsets.h
+++ b/sysdeps/m68k/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     20
diff --git a/sysdeps/microblaze/nptl/pthread-offsets.h b/sysdeps/microblaze/nptl/pthread-offsets.h
index 9617354..e4e775d 100644
--- a/sysdeps/microblaze/nptl/pthread-offsets.h
+++ b/sysdeps/microblaze/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     20
diff --git a/sysdeps/mips/nptl/pthread-offsets.h b/sysdeps/mips/nptl/pthread-offsets.h
index 0ac3eda..e96d34a 100644
--- a/sysdeps/mips/nptl/pthread-offsets.h
+++ b/sysdeps/mips/nptl/pthread-offsets.h
@@ -1,13 +1,5 @@ 
 #if _MIPS_SIM == _ABI64
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 # define __PTHREAD_MUTEX_KIND_OFFSET     16
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     24
 #else
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 # define __PTHREAD_MUTEX_KIND_OFFSET     12
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     20
 #endif
diff --git a/sysdeps/nios2/nptl/pthread-offsets.h b/sysdeps/nios2/nptl/pthread-offsets.h
index 9617354..e4e775d 100644
--- a/sysdeps/nios2/nptl/pthread-offsets.h
+++ b/sysdeps/nios2/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     20
diff --git a/sysdeps/powerpc/nptl/pthread-offsets.h b/sysdeps/powerpc/nptl/pthread-offsets.h
index bdda1f1..25b5bf2 100644
--- a/sysdeps/powerpc/nptl/pthread-offsets.h
+++ b/sysdeps/powerpc/nptl/pthread-offsets.h
@@ -1,15 +1,7 @@ 
 #include <bits/wordsize.h>
 
 #if __WORDSIZE == 64
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 # define __PTHREAD_MUTEX_KIND_OFFSET     16
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     24
 #else
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 # define __PTHREAD_MUTEX_KIND_OFFSET     12
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     20
 #endif
diff --git a/sysdeps/riscv/nptl/pthread-offsets.h b/sysdeps/riscv/nptl/pthread-offsets.h
index f164004..6fc032f 100644
--- a/sysdeps/riscv/nptl/pthread-offsets.h
+++ b/sysdeps/riscv/nptl/pthread-offsets.h
@@ -17,8 +17,4 @@ 
    License along with the GNU C Library.  If not, see
    <https://www.gnu.org/licenses/>.  */
 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 #define __PTHREAD_MUTEX_KIND_OFFSET     16
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     24
diff --git a/sysdeps/s390/nptl/pthread-offsets.h b/sysdeps/s390/nptl/pthread-offsets.h
index bdda1f1..25b5bf2 100644
--- a/sysdeps/s390/nptl/pthread-offsets.h
+++ b/sysdeps/s390/nptl/pthread-offsets.h
@@ -1,15 +1,7 @@ 
 #include <bits/wordsize.h>
 
 #if __WORDSIZE == 64
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 # define __PTHREAD_MUTEX_KIND_OFFSET     16
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     24
 #else
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 # define __PTHREAD_MUTEX_KIND_OFFSET     12
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     20
 #endif
diff --git a/sysdeps/sh/nptl/pthread-offsets.h b/sysdeps/sh/nptl/pthread-offsets.h
index 9617354..e4e775d 100644
--- a/sysdeps/sh/nptl/pthread-offsets.h
+++ b/sysdeps/sh/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 #define __PTHREAD_MUTEX_KIND_OFFSET     12
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     20
diff --git a/sysdeps/sparc/nptl/pthread-offsets.h b/sysdeps/sparc/nptl/pthread-offsets.h
index bdda1f1..25b5bf2 100644
--- a/sysdeps/sparc/nptl/pthread-offsets.h
+++ b/sysdeps/sparc/nptl/pthread-offsets.h
@@ -1,15 +1,7 @@ 
 #include <bits/wordsize.h>
 
 #if __WORDSIZE == 64
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 # define __PTHREAD_MUTEX_KIND_OFFSET     16
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     24
 #else
-# define __PTHREAD_MUTEX_NUSERS_OFFSET   16
 # define __PTHREAD_MUTEX_KIND_OFFSET     12
-# define __PTHREAD_MUTEX_SPINS_OFFSET    20
-# define __PTHREAD_MUTEX_ELISION_OFFSET  22
-# define __PTHREAD_MUTEX_LIST_OFFSET     20
 #endif
diff --git a/sysdeps/x86_64/nptl/pthread-offsets.h b/sysdeps/x86_64/nptl/pthread-offsets.h
index 16c6b0d..8208f6f 100644
--- a/sysdeps/x86_64/nptl/pthread-offsets.h
+++ b/sysdeps/x86_64/nptl/pthread-offsets.h
@@ -1,5 +1 @@ 
-#define __PTHREAD_MUTEX_NUSERS_OFFSET   12
 #define __PTHREAD_MUTEX_KIND_OFFSET     16
-#define __PTHREAD_MUTEX_SPINS_OFFSET    20
-#define __PTHREAD_MUTEX_ELISION_OFFSET  22
-#define __PTHREAD_MUTEX_LIST_OFFSET     24