Re: [PATCH] libstdc++: Ensure __gthread_self doesn't call undefined weak symbol [PR 95989]

[Jonathan Wakely <jwakely@redhat.com>]

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

On 19/11/20 21:42 +0000, Jonathan Wakely wrote:
>On 12/11/20 17:34 +0000, Jonathan Wakely wrote:
>>On 11/11/20 19:08 +0100, Jakub Jelinek via Libstdc++ wrote:
>>>On Wed, Nov 11, 2020 at 05:24:42PM +0000, Jonathan Wakely wrote:
>>>>--- 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)
>>>
>>>What if it is a non-glibc system where __GLIBC_PREREQ macro isn't defined?
>>>I think you'd get then
>>>error: missing binary operator before token "("
>>>So I think you want
>>>#if defined __GLIBC__ && defined __GLIBC_PREREQ
>>>#if __GLIBC_PREREQ(2, 27)
>>>return pthread_self ();
>>>#else
>>>return __gthrw_(pthread_self) ();
>>>#else
>>>return __gthrw_(pthread_self) ();
>>>#endif
>>>or similar.
>>
>>
>>Here's a fixed version of the patch.
>>
>>I've moved the glibc-specific code in this_thread::get_id() into a new
>>macro defined in config/os/gnu-linux/os_defines.h (where we already
>>know we are dealing with glibc). That means we don't do the
>>__GLIBC_PREREQ check directly in <thread>, it's hidden away in a
>>target-specific header.
>>
>>Tested powerpc64le-linux (glibc 2.17 and 2.32), sparc-solaris2.11 and
>>powerpc-aix.
>
>I've committed this version which only fixes this_thread::get_id() in
>libstdc++, and doesn't change __gthread_self in gthr-posix.h
>
>Due to a recent change to replace other uses of __gthread_self with
>calls to this_thread::get_id(), fixing it there fixes all uses in
>libstdc++.
>
>Tested x86_64-linux, powerpc-aix, sparc-solaris2.11, committed to
>trunk.


>diff --git a/libstdc++-v3/testsuite/30_threads/jthread/95989.cc b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
>new file mode 100644
>index 000000000000..46444b5ccabc
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
>@@ -0,0 +1,54 @@
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library.  This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This 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 General Public License for more details.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-std=gnu++2a" }
>+// { dg-do run { target c++2a } }
>+// { dg-require-gthreads {} }
>+// { dg-additional-options "-pthread" { target pthread } }
>+// { dg-additional-options "-static" { target static } }
>+
>+#include <thread>
>+
>+// PR libstdc++/95989
>+// Segfault compiling with static libraries and using jthread::request_stop
>+
>+void
>+test01()
>+{
>+  std::jthread t{ [] () {} };
>+}
>+
>+void
>+test02()
>+{
>+  std::jthread t{ [] () {} };
>+  t.request_stop();
>+}
>+
>+void
>+test03()
>+{
>+  std::jthread t{ [] {} };
>+  std::stop_callback cb(t.get_stop_token(), [] () {});
>+}
>+
>+int
>+main()
>+{
>+  test01();
>+  test01();
>+}

This test runs test01 twice, which isn't what I meant to do.

Fixed by this patch (but the test is still failing, see PR 97944).

Committed to trunk.



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

commit 7e0078f8643f9204777152ed0f915b52072a05c8
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 24 13:11:13 2020

    libstdc++: Run all tests in file
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/30_threads/jthread/95989.cc: Run all three test
            functions, not just the first one twice.

diff --git a/libstdc++-v3/testsuite/30_threads/jthread/95989.cc b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
index 46444b5ccabc..c7a9430eee90 100644
--- a/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
+++ b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
@@ -50,5 +50,6 @@ int
 main()
 {
   test01();
-  test01();
+  test02();
+  test03();
 }