From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 2EB643857823 for ; Wed, 11 Nov 2020 17:24:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2EB643857823 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-320-ujljkc_0Nui5vN_jvEs3wA-1; Wed, 11 Nov 2020 12:24:44 -0500 X-MC-Unique: ujljkc_0Nui5vN_jvEs3wA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 85C7C1017DCC; Wed, 11 Nov 2020 17:24:43 +0000 (UTC) Received: from localhost (unknown [10.33.36.62]) by smtp.corp.redhat.com (Postfix) with ESMTP id 217B35DA6A; Wed, 11 Nov 2020 17:24:42 +0000 (UTC) Date: Wed, 11 Nov 2020 17:24:42 +0000 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Cc: Jakub Jelinek , Florian Weimer Subject: [PATCH] libstdc++: Ensure __gthread_self doesn't call undefined weak symbol [PR 95989] Message-ID: <20201111172442.GA163354@redhat.com> MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="6TrnltStXW4iwmi0" Content-Disposition: inline X-Spam-Status: No, score=-14.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Nov 2020 17:24:51 -0000 --6TrnltStXW4iwmi0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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. --6TrnltStXW4iwmi0 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" commit f01eaa49afab0cbd88a7e2d177d6b416ce1b78c6 Author: Jonathan Wakely 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()); } --6TrnltStXW4iwmi0--