public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Ensure __gthread_self doesn't call undefined weak symbol [PR 95989]
@ 2020-11-11 17:24 Jonathan Wakely
  2020-11-11 18:08 ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2020-11-11 17:24 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Jakub Jelinek, Florian Weimer

[-- Attachment #1: Type: text/plain, Size: 2447 bytes --]

Since glibc 2.27 the pthread_self symbol has been defined in libc rather
than libpthread. Because we only call pthread_self through a weak alias
it's possible for statically linked executables to end up without a
definition of pthread_self. This crashes when trying to call an
undefined weak symbol.

We can use the __GLIBC_PREREQ version check to detect the version of
glibc where pthread_self is no longer in libpthread, and call it
directly rather than through the weak reference.

It would be better to check for pthread_self in libc during configure
instead of hardcoding the __GLIBC_PREREQ check. That would be somewhat
complicated by the fact that prior to glibc 2.27 only libc.so.6
contained the pthread_self symbol. The configure checks would need to
try to link both statically and dynamically, and the result would depend
on whether the static libc.a happens to be installed during configure
(which could vary between different systems using the same version of
glibc). Doing it properly is left for a future date, as it will be
needed anyway after glibc moves all pthread symbols from libpthread to
libc. When that happens we should revisit the whole approach of using
weak symbols for pthread symbols.

An undesirable consequence of this change is that code compiled prior to
the change might inline the old definition of this_thread::get_id()
which always returns (__gthread_t)1 in a program that isn't linked to
libpthread. Code compiled after the change will use pthread_self() and
so get a real TID. That could result in the main thread having different
thread::id values in different translation units. This seems acceptable,
as there are not expected to be many uses of thread::id in programs
that aren't linked to libpthread.

libgcc/ChangeLog:

	PR libstdc++/95989
	* gthr-posix.h (__gthread_self) [__GLIBC_PREREQ(2, 27)]: Call
	pthread_self directly rather than using weak alias.

libstdc++-v3/ChangeLog:

	PR libstdc++/95989
	* include/std/thread (this_thread::get_id): Add explicit cast
	from int to __gthread_t. Use __gthread_self for glibc 2.27 and
	newer.

Tested powerpc64le-linux (glibc 2.17) and x86_64-linux (glibc 2.31).

I can't approve the libgcc/gthr-posix.h part.

OK for trunk?

If the libgcc/gthr-posix.h change is not acceptable I will just change
the two places libstdc++ uses __gthread_self() so that they call
pthread_self() directly instead. But it seems worth fixing
gthr-posix.h to avoid the problem.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4281 bytes --]

commit f01eaa49afab0cbd88a7e2d177d6b416ce1b78c6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jul 9 10:11:57 2020

    libstdc++: Ensure __gthread_self doesn't call undefined weak symbol [PR 95989]
    
    Since glibc 2.27 the pthread_self symbol has been defined in libc rather
    than libpthread. Because we only call pthread_self through a weak alias
    it's possible for statically linked executables to end up without a
    definition of pthread_self. This crashes when trying to call an
    undefined weak symbol.
    
    We can use the __GLIBC_PREREQ version check to detect the version of
    glibc where pthread_self is no longer in libpthread, and call it
    directly rather than through the weak reference.
    
    It would be better to check for pthread_self in libc during configure
    instead of hardcoding the __GLIBC_PREREQ check. That would be somewhat
    complicated by the fact that prior to glibc 2.27 only libc.so.6
    contained the pthread_self symbol. The configure checks would need to
    try to link both statically and dynamically, and the result would depend
    on whether the static libc.a happens to be installed during configure
    (which could vary between different systems using the same version of
    glibc). Doing it properly is left for a future date, as it will be
    needed anyway after glibc moves all pthread symbols from libpthread to
    libc. When that happens we should revisit the whole approach of using
    weak symbols for pthread symbols.
    
    An undesirable consequence of this change is that code compiled prior to
    the change might inline the old definition of this_thread::get_id()
    which always returns (__gthread_t)1 in a program that isn't linked to
    libpthread. Code compiled after the change will use pthread_self() and
    so get a real TID. That could result in the main thread having different
    thread::id values in different translation units. This seems acceptable,
    as there are not expected to be many uses of thread::id in programs
    that aren't linked to libpthread.
    
    libgcc/ChangeLog:
    
            PR libstdc++/95989
            * gthr-posix.h (__gthread_self) [__GLIBC_PREREQ(2, 27)]: Call
            pthread_self directly rather than using weak alias.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/95989
            * include/std/thread (this_thread::get_id): Add explicit cast
            from int to __gthread_t. Use __gthread_self for glibc 2.27 and
            newer.

diff --git a/libgcc/gthr-posix.h b/libgcc/gthr-posix.h
index 965247602acf..5699e091c85b 100644
--- a/libgcc/gthr-posix.h
+++ b/libgcc/gthr-posix.h
@@ -684,7 +684,14 @@ __gthread_equal (__gthread_t __t1, __gthread_t __t2)
 static inline __gthread_t
 __gthread_self (void)
 {
+#if __GLIBC_PREREQ(2, 27)
+  /* Since Glibc 2.27, pthread_self is defined in libc not libpthread.
+   * Call it directly so that we get a non-weak reference and won't call
+   * an undefined weak symbol when linked to the libc.a static lib.  */
+  return pthread_self ();
+#else
   return __gthrw_(pthread_self) ();
+#endif
 }
 
 static inline int
diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
index 080036e26097..ac50729c41a6 100644
--- a/libstdc++-v3/include/std/thread
+++ b/libstdc++-v3/include/std/thread
@@ -350,13 +350,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     inline thread::id
     get_id() noexcept
     {
-#ifdef __GLIBC__
+#if defined __GLIBC__ && ! __GLIBC_PREREQ(2, 27)
       // For the GNU C library pthread_self() is usable without linking to
-      // libpthread.so but returns 0, so we cannot use it in single-threaded
-      // programs, because this_thread::get_id() != thread::id{} must be true.
-      // We know that pthread_t is an integral type in the GNU C library.
+      // libpthread, but prior to version 2.27 the version in libc returns 0,
+      // which breaks the invariant this_thread::get_id() != thread::id{}.
+      // We know that pthread_t is a scalar type in the GNU C library,
+      // so just use (__gthread_t)1 as the ID of the main (and only) thread.
       if (!__gthread_active_p())
-	return thread::id(1);
+	return thread::id((__gthread_t)1);
 #endif
       return thread::id(__gthread_self());
     }

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-24 15:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 17:24 [PATCH] libstdc++: Ensure __gthread_self doesn't call undefined weak symbol [PR 95989] Jonathan Wakely
2020-11-11 18:08 ` Jakub Jelinek
2020-11-11 19:24   ` Jonathan Wakely
2020-11-12 17:34   ` Jonathan Wakely
2020-11-19 21:42     ` Jonathan Wakely
2020-11-20 13:50       ` Jonathan Wakely
2020-11-24 15:06       ` Jonathan Wakely

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).