Re: [committed] libstdc++: Fix std::jthread assertion and re-enable skipped test

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 5/19/21 3:37 PM, Bernd Edlinger wrote:
> On 5/19/21 3:27 PM, Jonathan Wakely wrote:
>> On 18/05/21 13:58 +0200, Bernd Edlinger wrote:
>>> On 5/18/21 1:55 PM, Bernd Edlinger wrote:
>>>> On 5/17/21 7:13 PM, Jonathan Wakely via Gcc-patches wrote:
>>>>> libstdc++-v3/ChangeLog:
>>>>>
>>>>>     * include/std/thread (jthread::_S_create): Fix static assert
>>>>>     message.
>>>>>     * testsuite/30_threads/jthread/95989.cc: Re-enable test.
>>>>>     * testsuite/30_threads/jthread/jthread.cc: Do not require
>>>>>     pthread effective target.
>>>>>     * testsuite/30_threads/jthread/2.cc: Moved to...
>>>>>     * testsuite/30_threads/jthread/version.cc: ...here.
>>>>>
>>>>> Tested powerpc64le-linux. Committed to trunk.
>>>>>
>>>>> Let's see if this test is actually fixed, or if it still causes
>>>>> failures on some targets.
>>>>>
>>>>>
>>>>
>>>> Yes, indeed it is failing on x86_64-pc-linux-gnu.
>>>>
>>>
>>> that means only this one:
>>>
>>> FAIL: 30_threads/jthread/95989.cc execution test
>>
>> What's your glibc version?
>>
>>
> 
> that is ubuntu 20.04 with latest patches:
> 
> $ ldd --version
> ldd (Ubuntu GLIBC 2.31-0ubuntu9.3) 2.31
> Copyright (C) 2020 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> Written by Roland McGrath and Ulrich Drepper.
> 

The glibc version is probably not the reason,
I've got the same effect with Ubuntu EGLIBC 2.19-0ubuntu6.15 as well.

But I use binutils-2.36.1.

I think the problem is here:

libstdc++-v3/src/c++11/thread.cc:      __e = __gthread_join(_M_id._M_thread, 0);

which calls

libgcc/gthr-posix.h:  return __gthrw_(pthread_join) (__threadid, __value_ptr);

but this is only a weak reference to pthread_join:

669	  return __gthrw_(pthread_join) (__threadid, __value_ptr);
(gdb) disass
Dump of assembler code for function std::thread::join():
   0x0000000000405e20 <+0>:	endbr64 
   0x0000000000405e24 <+4>:	push   %rbx
   0x0000000000405e25 <+5>:	mov    %rdi,%rbx
   0x0000000000405e28 <+8>:	mov    (%rdi),%rdi
   0x0000000000405e2b <+11>:	test   %rdi,%rdi
   0x0000000000405e2e <+14>:	je     0x405e44 <std::thread::join()+36>
=> 0x0000000000405e30 <+16>:	xor    %esi,%esi
   0x0000000000405e32 <+18>:	callq  0x0
   0x0000000000405e37 <+23>:	test   %eax,%eax
   0x0000000000405e39 <+25>:	jne    0x405e49 <std::thread::join()+41>
   0x0000000000405e3b <+27>:	movq   $0x0,(%rbx)
   0x0000000000405e42 <+34>:	pop    %rbx
   0x0000000000405e43 <+35>:	retq   
   0x0000000000405e44 <+36>:	mov    $0x16,%eax
   0x0000000000405e49 <+41>:	mov    %eax,%edi
   0x0000000000405e4b <+43>:	callq  0x4012a9 <std::__throw_system_error(int)>
End of assembler dump.

but the pthread_join symbol is not checked for potentially being null.

The test case passes if I change this:

diff --git a/libstdc++-v3/testsuite/30_threads/jthread/95989.cc b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
index fb3f43bc722..61a3ff76aa1 100644
--- a/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
+++ b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
@@ -52,4 +52,5 @@ main()
   test01();
   test02();
   test03();
+  pthread_join(NULL, NULL);
 }


Thanks
Bernd.