diff mbox series

[1/5] nptl: Add <thread_pointer.h> for defining __thread_pointer

Message ID aa65894a9829ba546c4672d3597e90ca29adff4f.1638798186.git.fweimer@redhat.com
State Superseded
Delegated to: Szabolcs Nagy
Headers
Series Extensible rseq support for glibc |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Florian Weimer Dec. 6, 2021, 1:46 p.m. UTC 
  <tls.h> already contains a definition that is quite similar,
but it is not readily accessible.
---
 sysdeps/csky/nptl/thread_pointer.h       | 25 ++++++++++++++++++
 sysdeps/hppa/nptl/thread_pointer.h       | 25 ++++++++++++++++++
 sysdeps/ia64/nptl/thread_pointer.h       | 19 ++++++++++++++
 sysdeps/m68k/nptl/thread_pointer.h       | 20 ++++++++++++++
 sysdeps/microblaze/nptl/thread_pointer.h | 19 ++++++++++++++
 sysdeps/nptl/thread_pointer.h            | 19 ++++++++++++++
 sysdeps/powerpc/nptl/thread_pointer.h    | 23 +++++++++++++++++
 sysdeps/sparc/nptl/thread_pointer.h      | 19 ++++++++++++++
 sysdeps/x86/nptl/thread_pointer.h        | 33 ++++++++++++++++++++++++
 9 files changed, 202 insertions(+)
 create mode 100644 sysdeps/csky/nptl/thread_pointer.h
 create mode 100644 sysdeps/hppa/nptl/thread_pointer.h
 create mode 100644 sysdeps/ia64/nptl/thread_pointer.h
 create mode 100644 sysdeps/m68k/nptl/thread_pointer.h
 create mode 100644 sysdeps/microblaze/nptl/thread_pointer.h
 create mode 100644 sysdeps/nptl/thread_pointer.h
 create mode 100644 sysdeps/powerpc/nptl/thread_pointer.h
 create mode 100644 sysdeps/sparc/nptl/thread_pointer.h
 create mode 100644 sysdeps/x86/nptl/thread_pointer.h

Comments

Mathieu Desnoyers Dec. 6, 2021, 4:44 p.m. UTC | #1 
----- On Dec 6, 2021, at 8:46 AM, Florian Weimer fweimer@redhat.com wrote:

> <tls.h> already contains a definition that is quite similar,
> but it is not readily accessible.
[...]

> +++ b/sysdeps/csky/nptl/thread_pointer.h
[...]
> +static inline void *
> +__thread_pointer (void)
> +{
> +  void *__result;
> +  __asm__ volatile ("mov %0, r31" : "=r" (__result));

Do we want volatile here ?

> +  return __result;
> +}
[...]

> +++ b/sysdeps/hppa/nptl/thread_pointer.h
[...]
> +
> +static inline void *
> +__thread_pointer (void)
> +{
> +  void *__result;
> +  __asm__ ("mfctl %%cr27, %0" : "=r" (__result));
> +  return __result;
> +}
[...]

> +++ b/sysdeps/ia64/nptl/thread_pointer.h
[...]
> +
> +#define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
[...]

Why do we find a mix of macros and static inline across architectures ?
I would expect that we stick to one or the other.

Static inline have some advantages in terms of type-awareness. Arguably here
everything is a (void *), so there is little benefit from it.

Defining macros end up clobbering the preprocessor namespace (output of "gcc -dM -E"),
which is one reason why static inlines are sometimes preferred. Or do we intend users
to use this for feature detection ? e.g. #ifdef __thread_pointer ?

OTOH, static inline functions will not be inlined when compiling with "-fno-inline"
(default at -O0). An attribute "always_inline" is needed for this.


> +++ b/sysdeps/m68k/nptl/thread_pointer.h
[...]
> +extern void * __m68k_read_tp (void);
> +#define __thread_pointer() __m68k_read_tp ()
[...]

> +++ b/sysdeps/microblaze/nptl/thread_pointer.h
[...]
> +#define __thread_pointer() ({ register void *__reg asm ("r21"); __reg; })

Not that applications are supposed to use variable names with a leading
"__" prefix (AFAIU this is reserved to libc), but one downside of these
statement expressions is that they would shadow any "__reg" variables.

One example where things could theoretically be a problem is found in
sysdeps/x86/sys/platform/x86.h within glibc, which defines a __reg
variable.

[...]

> +++ b/sysdeps/nptl/thread_pointer.h
[...]
> +#define __thread_pointer() __builtin_thread_pointer ()
[...]

> +++ b/sysdeps/powerpc/nptl/thread_pointer.h
[...]
> +#ifdef __powerpc64__
> +# define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
> +#else
> +# define __thread_pointer() ({ register void *__reg asm ("r2"); __reg; })
> +#endif
[ ...]

> +++ b/sysdeps/sparc/nptl/thread_pointer.h
[...]
> +#define __thread_pointer() ({ register void *__reg asm ("%g7"); __reg; })
[...]

> +++ b/sysdeps/x86/nptl/thread_pointer.h
[...]
> +#if __GNUC_PREREQ (11, 1)

I suspect this requires a #include "features.h" ?

> +# define __thread_pointer() __builtin_thread_pointer ()
> +#else
> +static inline void *
> +__thread_pointer (void)
> +{
> +  void *__result;
> +# ifdef __x86_64__
> +  __asm__ volatile ("mov %%fs:0, %0" : "=r" (__result));

Just out of curiosity, is x32 supported by glibc, and is this the
expected behavior ?

> +# else
> +  __asm__ volatile ("mov %%gs:0, %0" : "=r" (__result));

Do we really want a "volatile" asm here ? E.g. see:

sysdeps/x86_64/nptl/tls.h:

/* Return the thread descriptor for the current thread.

   The contained asm must *not* be marked volatile since otherwise
   assignments like
        pthread_descr self = thread_self();
   do not get optimized away.  */
# if __GNUC_PREREQ (6, 0)
#  define THREAD_SELF \
  (*(struct pthread *__seg_fs *) offsetof (struct pthread, header.self))
# else
#  define THREAD_SELF \
  ({ struct pthread *__self;                                                  \
     asm ("mov %%fs:%c1,%0" : "=r" (__self)                                   \
          : "i" (offsetof (struct pthread, header.self)));                    \
     __self;})
# endif


Is this expected to be a complete list of all architectures ? I notably identify
that arm, aarch64, and s390 are missing. Or is it that all the other architectures
map to "sysdeps/nptl/thread_pointers.h" ? Maybe it would be good to document this
in the commit message.

Thanks,

Mathieu

> +#endif
> +  return __result;
> +}
> +#endif
Florian Weimer Dec. 6, 2021, 5:01 p.m. UTC | #2 
* Mathieu Desnoyers:

> Why do we find a mix of macros and static inline across architectures ?

It's following the existing snippets in <tls.h> for those architectures
which cannot use __builtin_thread_pointer.

> Defining macros end up clobbering the preprocessor namespace (output
> of "gcc -dM -E"), which is one reason why static inlines are sometimes
> preferred. Or do we intend users to use this for feature detection ?
> e.g. #ifdef __thread_pointer ?

It's currently not an installed header, so this isn't a concern.

>
> OTOH, static inline functions will not be inlined when compiling with "-fno-inline"
> (default at -O0). An attribute "always_inline" is needed for this.
>
>
>> +++ b/sysdeps/m68k/nptl/thread_pointer.h
> [...]
>> +extern void * __m68k_read_tp (void);
>> +#define __thread_pointer() __m68k_read_tp ()
> [...]
>
>> +++ b/sysdeps/microblaze/nptl/thread_pointer.h
> [...]
>> +#define __thread_pointer() ({ register void *__reg asm ("r21"); __reg; })
>
> Not that applications are supposed to use variable names with a leading
> "__" prefix (AFAIU this is reserved to libc), but one downside of these
> statement expressions is that they would shadow any "__reg" variables.

It doesn't matter in this context because there are no macro arguments.
The only thing that would break this would be a __reg macro.

>> +++ b/sysdeps/x86/nptl/thread_pointer.h
> [...]
>> +#if __GNUC_PREREQ (11, 1)
>
> I suspect this requires a #include "features.h" ?

It's an internal header.  We'd also need include guards if we made this
into an installed header.

>> +# define __thread_pointer() __builtin_thread_pointer ()
>> +#else
>> +static inline void *
>> +__thread_pointer (void)
>> +{
>> +  void *__result;
>> +# ifdef __x86_64__
>> +  __asm__ volatile ("mov %%fs:0, %0" : "=r" (__result));
>
> Just out of curiosity, is x32 supported by glibc, and is this the
> expected behavior ?

x32 is supported and uses %fs.

>> +# else
>> +  __asm__ volatile ("mov %%gs:0, %0" : "=r" (__result));
>
> Do we really want a "volatile" asm here ? E.g. see:

With sufficiently separate compilation, we do not need it.  It's
currently not needed.

THREAD_SELF should not use it because it is often used in ways that
isn't needed after all, e.g. if THREAD_SELF is passed to THREAD_GETMEM
on an architecture that does not need a materialized thread pointer to
change TCB fields.

> Is this expected to be a complete list of all architectures ? I
> notably identify that arm, aarch64, and s390 are missing. Or is it
> that all the other architectures map to
> "sysdeps/nptl/thread_pointers.h" ? Maybe it would be good to document
> this in the commit message.

Yes, those can use __builtin_thread_pointer.  The patch as is builds on
all targets with GCC 11.

Thanks,
Florian
Florian Weimer Dec. 6, 2021, 7:55 p.m. UTC | #3 
* Florian Weimer:

> * Mathieu Desnoyers:
>
>> Why do we find a mix of macros and static inline across architectures ?
>
> It's following the existing snippets in <tls.h> for those architectures
> which cannot use __builtin_thread_pointer.

I'm reposting this separately.  I was able to restrict the set of
architectures that need this.

Thanks,
Florian
diff mbox series

Patch

diff --git a/sysdeps/csky/nptl/thread_pointer.h b/sysdeps/csky/nptl/thread_pointer.h
new file mode 100644
index 0000000000..c8be0b1e84
--- /dev/null
+++ b/sysdeps/csky/nptl/thread_pointer.h
@@ -0,0 +1,25 @@ 
+/* __thread_pointer definition.  csky version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+static inline void *
+__thread_pointer (void)
+{
+  void *__result;
+  __asm__ volatile ("mov %0, r31" : "=r" (__result));
+  return __result;
+}
diff --git a/sysdeps/hppa/nptl/thread_pointer.h b/sysdeps/hppa/nptl/thread_pointer.h
new file mode 100644
index 0000000000..118b7f9385
--- /dev/null
+++ b/sysdeps/hppa/nptl/thread_pointer.h
@@ -0,0 +1,25 @@ 
+/* __thread_pointer definition.  hppa version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+static inline void *
+__thread_pointer (void)
+{
+  void *__result;
+  __asm__ ("mfctl %%cr27, %0" : "=r" (__result));
+  return __result;
+}
diff --git a/sysdeps/ia64/nptl/thread_pointer.h b/sysdeps/ia64/nptl/thread_pointer.h
new file mode 100644
index 0000000000..412b9f0460
--- /dev/null
+++ b/sysdeps/ia64/nptl/thread_pointer.h
@@ -0,0 +1,19 @@ 
+/* __thread_pointer definition.  ia64 version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
diff --git a/sysdeps/m68k/nptl/thread_pointer.h b/sysdeps/m68k/nptl/thread_pointer.h
new file mode 100644
index 0000000000..932567fddb
--- /dev/null
+++ b/sysdeps/m68k/nptl/thread_pointer.h
@@ -0,0 +1,20 @@ 
+/* __thread_pointer definition.  m68k version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+extern void * __m68k_read_tp (void);
+#define __thread_pointer() __m68k_read_tp ()
diff --git a/sysdeps/microblaze/nptl/thread_pointer.h b/sysdeps/microblaze/nptl/thread_pointer.h
new file mode 100644
index 0000000000..020c73113a
--- /dev/null
+++ b/sysdeps/microblaze/nptl/thread_pointer.h
@@ -0,0 +1,19 @@ 
+/* __thread_pointer definition.  microblaze version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#define __thread_pointer() ({ register void *__reg asm ("r21"); __reg; })
diff --git a/sysdeps/nptl/thread_pointer.h b/sysdeps/nptl/thread_pointer.h
new file mode 100644
index 0000000000..6368b1083e
--- /dev/null
+++ b/sysdeps/nptl/thread_pointer.h
@@ -0,0 +1,19 @@ 
+/* __thread_pointer definition.  Generic version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#define __thread_pointer() __builtin_thread_pointer ()
diff --git a/sysdeps/powerpc/nptl/thread_pointer.h b/sysdeps/powerpc/nptl/thread_pointer.h
new file mode 100644
index 0000000000..69943a2f24
--- /dev/null
+++ b/sysdeps/powerpc/nptl/thread_pointer.h
@@ -0,0 +1,23 @@ 
+/* __thread_pointer definition.  powerpc version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#ifdef __powerpc64__
+# define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
+#else
+# define __thread_pointer() ({ register void *__reg asm ("r2"); __reg; })
+#endif
diff --git a/sysdeps/sparc/nptl/thread_pointer.h b/sysdeps/sparc/nptl/thread_pointer.h
new file mode 100644
index 0000000000..6a176cba28
--- /dev/null
+++ b/sysdeps/sparc/nptl/thread_pointer.h
@@ -0,0 +1,19 @@ 
+/* __thread_pointer definition.  sparc version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#define __thread_pointer() ({ register void *__reg asm ("%g7"); __reg; })
diff --git a/sysdeps/x86/nptl/thread_pointer.h b/sysdeps/x86/nptl/thread_pointer.h
new file mode 100644
index 0000000000..d636839ecf
--- /dev/null
+++ b/sysdeps/x86/nptl/thread_pointer.h
@@ -0,0 +1,33 @@ 
+/* __thread_pointer definition.  x86 version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#if __GNUC_PREREQ (11, 1)
+# define __thread_pointer() __builtin_thread_pointer ()
+#else
+static inline void *
+__thread_pointer (void)
+{
+  void *__result;
+# ifdef __x86_64__
+  __asm__ volatile ("mov %%fs:0, %0" : "=r" (__result));
+# else
+  __asm__ volatile ("mov %%gs:0, %0" : "=r" (__result));
+#endif
+  return __result;
+}
+#endif