Re: CVS commit: src/lib/libpthread

[Andrew Doran]

On Thu, Jun 04, 2020 at 06:26:07PM +, Martin Husemann wrote:

> On Wed, Jun 03, 2020 at 10:10:24PM +, Andrew Doran wrote:
> > Module Name:src
> > Committed By:   ad
> > Date:   Wed Jun  3 22:10:24 UTC 2020
> > 
> > Modified Files:
> > src/lib/libpthread: pthread.c pthread_cond.c pthread_mutex.c
> > 
> > Log Message:
> > Deal with a couple of problems with threads being awoken early due to
> > timeouts or cancellation where:
> 
> Not sure if it is caused by this commit or joergs TSD/malloc, but today
> most of the libpthread tests time out on aarch64, while everything but
> a few minor nits was fine on May 30.

I tried with everything updated and it seems to be hanging on exit in
jemalloc.  Haven't looked further yet.

http://www.netbsd.org/~ad/2020/barrier.txt

Andrew


 
> Martin
> 
> (this is on a hummingboard pulse 4-core board)
> lib/libpthread/t_barrier (398/848): 1 test cases
> barrier: [300.034098s] Failed: Test case timed out after 300 seconds
> [300.034568s]
> 
> lib/libpthread/t_cond (399/848): 9 test cases
> bogus_timedwaits: [300.016677s] Failed: Test case timed out after 300 
> seconds
> broadcast: [300.023040s] Failed: Test case timed out after 300 seconds
> cond_timedwait_race: [300.034567s] Failed: Test case timed out after 300 
> seconds
> condattr: [0.013756s] Passed.
> destroy_after_cancel: [300.025649s] Failed: Test case timed out after 300 
> seconds
> signal_before_unlock: [300.023630s] Failed: Test case timed out after 300 
> seconds
> signal_before_unlock_static_init: [300.022909s] Failed: Test case timed 
> out after 300 seconds
> signal_delay_wait: [300.031310s] Failed: Test case timed out after 300 
> seconds
> signal_wait_race: [300.025552s] Failed: Test case timed out after 300 
> seconds
> [2400.221956s]
> 
> lib/libpthread/t_condwait (400/848): 2 test cases
> cond_wait_mono: [2.023083s] Passed.
> cond_wait_real: [2.017972s] Passed.
> [4.042078s]
> 
> lib/libpthread/t_detach (401/848): 1 test cases
> pthread_detach: [300.021242s] Failed: Test case timed out after 300 
> seconds
> [300.021708s]
> 
> lib/libpthread/t_equal (402/848): 1 test cases
> pthread_equal: [300.027646s] Failed: Test case timed out after 300 seconds
> [300.028295s]
> 
> lib/libpthread/t_fork (403/848): 1 test cases
> fork: [300.018077s] Failed: Test case timed out after 300 seconds
> [300.018975s]
> 
> lib/libpthread/t_fpu (404/848): 1 test cases
> fpu: [0.020578s] Passed.
> [0.021314s]
> 
> lib/libpthread/t_join (405/848): 1 test cases
> pthread_join: [0.021632s] Passed.
> [0.022411s]
> 
> lib/libpthread/t_kill (406/848): 1 test cases
> simple: [300.033759s] Failed: Test case timed out after 300 seconds
> [300.034528s]
> 
> lib/libpthread/t_mutex (407/848): 7 test cases
> mutex1: [300.017542s] Failed: Test case timed out after 300 seconds
> mutex2: [300.023064s] Failed: Test case timed out after 300 seconds
> mutex3: [300.023259s] Failed: Test case timed out after 300 seconds
> mutex4: [300.023015s] Failed: Test case timed out after 300 seconds
> mutex5: [300.023495s] Failed: Test case timed out after 300 seconds
> mutexattr1: [0.013769s] Passed.
> mutexattr2: [0.025483s] Passed.
> [1500.154160s]
> 
> lib/libpthread/t_name (408/848): 1 test cases
> name: [0.014262s] Passed.
> [0.014951s]
> 
> ...

Re: CVS commit: src/lib/libpthread

[Andrew Doran]

On Thu, Jun 04, 2020 at 09:28:09PM +, Andrew Doran wrote:

> On Thu, Jun 04, 2020 at 06:26:07PM +, Martin Husemann wrote:
> 
> > On Wed, Jun 03, 2020 at 10:10:24PM +, Andrew Doran wrote:
> > > Module Name:  src
> > > Committed By: ad
> > > Date: Wed Jun  3 22:10:24 UTC 2020
> > > 
> > > Modified Files:
> > >   src/lib/libpthread: pthread.c pthread_cond.c pthread_mutex.c
> > > 
> > > Log Message:
> > > Deal with a couple of problems with threads being awoken early due to
> > > timeouts or cancellation where:
> > 
> > Not sure if it is caused by this commit or joergs TSD/malloc, but today
> > most of the libpthread tests time out on aarch64, while everything but
> > a few minor nits was fine on May 30.
> 
> I'll try to reproduce it over the weekend.

This works OK for me on aarch64 with libpthread updated in isolation.  I'll
try updating more stuff tomorrow.

Andrew

> 
> Andrew
>  
> > Martin
> > 
> > (this is on a hummingboard pulse 4-core board)
> > lib/libpthread/t_barrier (398/848): 1 test cases
> > barrier: [300.034098s] Failed: Test case timed out after 300 seconds
> > [300.034568s]
> > 
> > lib/libpthread/t_cond (399/848): 9 test cases
> > bogus_timedwaits: [300.016677s] Failed: Test case timed out after 300 
> > seconds
> > broadcast: [300.023040s] Failed: Test case timed out after 300 seconds
> > cond_timedwait_race: [300.034567s] Failed: Test case timed out after 
> > 300 seconds
> > condattr: [0.013756s] Passed.
> > destroy_after_cancel: [300.025649s] Failed: Test case timed out after 
> > 300 seconds
> > signal_before_unlock: [300.023630s] Failed: Test case timed out after 
> > 300 seconds
> > signal_before_unlock_static_init: [300.022909s] Failed: Test case timed 
> > out after 300 seconds
> > signal_delay_wait: [300.031310s] Failed: Test case timed out after 300 
> > seconds
> > signal_wait_race: [300.025552s] Failed: Test case timed out after 300 
> > seconds
> > [2400.221956s]
> > 
> > lib/libpthread/t_condwait (400/848): 2 test cases
> > cond_wait_mono: [2.023083s] Passed.
> > cond_wait_real: [2.017972s] Passed.
> > [4.042078s]
> > 
> > lib/libpthread/t_detach (401/848): 1 test cases
> > pthread_detach: [300.021242s] Failed: Test case timed out after 300 
> > seconds
> > [300.021708s]
> > 
> > lib/libpthread/t_equal (402/848): 1 test cases
> > pthread_equal: [300.027646s] Failed: Test case timed out after 300 
> > seconds
> > [300.028295s]
> > 
> > lib/libpthread/t_fork (403/848): 1 test cases
> > fork: [300.018077s] Failed: Test case timed out after 300 seconds
> > [300.018975s]
> > 
> > lib/libpthread/t_fpu (404/848): 1 test cases
> > fpu: [0.020578s] Passed.
> > [0.021314s]
> > 
> > lib/libpthread/t_join (405/848): 1 test cases
> > pthread_join: [0.021632s] Passed.
> > [0.022411s]
> > 
> > lib/libpthread/t_kill (406/848): 1 test cases
> > simple: [300.033759s] Failed: Test case timed out after 300 seconds
> > [300.034528s]
> > 
> > lib/libpthread/t_mutex (407/848): 7 test cases
> > mutex1: [300.017542s] Failed: Test case timed out after 300 seconds
> > mutex2: [300.023064s] Failed: Test case timed out after 300 seconds
> > mutex3: [300.023259s] Failed: Test case timed out after 300 seconds
> > mutex4: [300.023015s] Failed: Test case timed out after 300 seconds
> > mutex5: [300.023495s] Failed: Test case timed out after 300 seconds
> > mutexattr1: [0.013769s] Passed.
> > mutexattr2: [0.025483s] Passed.
> > [1500.154160s]
> > 
> > lib/libpthread/t_name (408/848): 1 test cases
> > name: [0.014262s] Passed.
> > [0.014951s]
> > 
> > ...

Re: CVS commit: src/lib/libpthread

[Andrew Doran]

Hi,

On Thu, Jun 04, 2020 at 06:26:07PM +, Martin Husemann wrote:

> On Wed, Jun 03, 2020 at 10:10:24PM +, Andrew Doran wrote:
> > Module Name:src
> > Committed By:   ad
> > Date:   Wed Jun  3 22:10:24 UTC 2020
> > 
> > Modified Files:
> > src/lib/libpthread: pthread.c pthread_cond.c pthread_mutex.c
> > 
> > Log Message:
> > Deal with a couple of problems with threads being awoken early due to
> > timeouts or cancellation where:
> 
> Not sure if it is caused by this commit or joergs TSD/malloc, but today
> most of the libpthread tests time out on aarch64, while everything but
> a few minor nits was fine on May 30.

I'll try to reproduce it over the weekend.

Andrew
 
> Martin
> 
> (this is on a hummingboard pulse 4-core board)
> lib/libpthread/t_barrier (398/848): 1 test cases
> barrier: [300.034098s] Failed: Test case timed out after 300 seconds
> [300.034568s]
> 
> lib/libpthread/t_cond (399/848): 9 test cases
> bogus_timedwaits: [300.016677s] Failed: Test case timed out after 300 
> seconds
> broadcast: [300.023040s] Failed: Test case timed out after 300 seconds
> cond_timedwait_race: [300.034567s] Failed: Test case timed out after 300 
> seconds
> condattr: [0.013756s] Passed.
> destroy_after_cancel: [300.025649s] Failed: Test case timed out after 300 
> seconds
> signal_before_unlock: [300.023630s] Failed: Test case timed out after 300 
> seconds
> signal_before_unlock_static_init: [300.022909s] Failed: Test case timed 
> out after 300 seconds
> signal_delay_wait: [300.031310s] Failed: Test case timed out after 300 
> seconds
> signal_wait_race: [300.025552s] Failed: Test case timed out after 300 
> seconds
> [2400.221956s]
> 
> lib/libpthread/t_condwait (400/848): 2 test cases
> cond_wait_mono: [2.023083s] Passed.
> cond_wait_real: [2.017972s] Passed.
> [4.042078s]
> 
> lib/libpthread/t_detach (401/848): 1 test cases
> pthread_detach: [300.021242s] Failed: Test case timed out after 300 
> seconds
> [300.021708s]
> 
> lib/libpthread/t_equal (402/848): 1 test cases
> pthread_equal: [300.027646s] Failed: Test case timed out after 300 seconds
> [300.028295s]
> 
> lib/libpthread/t_fork (403/848): 1 test cases
> fork: [300.018077s] Failed: Test case timed out after 300 seconds
> [300.018975s]
> 
> lib/libpthread/t_fpu (404/848): 1 test cases
> fpu: [0.020578s] Passed.
> [0.021314s]
> 
> lib/libpthread/t_join (405/848): 1 test cases
> pthread_join: [0.021632s] Passed.
> [0.022411s]
> 
> lib/libpthread/t_kill (406/848): 1 test cases
> simple: [300.033759s] Failed: Test case timed out after 300 seconds
> [300.034528s]
> 
> lib/libpthread/t_mutex (407/848): 7 test cases
> mutex1: [300.017542s] Failed: Test case timed out after 300 seconds
> mutex2: [300.023064s] Failed: Test case timed out after 300 seconds
> mutex3: [300.023259s] Failed: Test case timed out after 300 seconds
> mutex4: [300.023015s] Failed: Test case timed out after 300 seconds
> mutex5: [300.023495s] Failed: Test case timed out after 300 seconds
> mutexattr1: [0.013769s] Passed.
> mutexattr2: [0.025483s] Passed.
> [1500.154160s]
> 
> lib/libpthread/t_name (408/848): 1 test cases
> name: [0.014262s] Passed.
> [0.014951s]
> 
> ...

Re: CVS commit: src/lib/libpthread

[Martin Husemann]

On Wed, Jun 03, 2020 at 10:10:24PM +, Andrew Doran wrote:
> Module Name:  src
> Committed By: ad
> Date: Wed Jun  3 22:10:24 UTC 2020
> 
> Modified Files:
>   src/lib/libpthread: pthread.c pthread_cond.c pthread_mutex.c
> 
> Log Message:
> Deal with a couple of problems with threads being awoken early due to
> timeouts or cancellation where:

Not sure if it is caused by this commit or joergs TSD/malloc, but today
most of the libpthread tests time out on aarch64, while everything but
a few minor nits was fine on May 30.

Martin

(this is on a hummingboard pulse 4-core board)
lib/libpthread/t_barrier (398/848): 1 test cases
barrier: [300.034098s] Failed: Test case timed out after 300 seconds
[300.034568s]

lib/libpthread/t_cond (399/848): 9 test cases
bogus_timedwaits: [300.016677s] Failed: Test case timed out after 300 
seconds
broadcast: [300.023040s] Failed: Test case timed out after 300 seconds
cond_timedwait_race: [300.034567s] Failed: Test case timed out after 300 
seconds
condattr: [0.013756s] Passed.
destroy_after_cancel: [300.025649s] Failed: Test case timed out after 300 
seconds
signal_before_unlock: [300.023630s] Failed: Test case timed out after 300 
seconds
signal_before_unlock_static_init: [300.022909s] Failed: Test case timed out 
after 300 seconds
signal_delay_wait: [300.031310s] Failed: Test case timed out after 300 
seconds
signal_wait_race: [300.025552s] Failed: Test case timed out after 300 
seconds
[2400.221956s]

lib/libpthread/t_condwait (400/848): 2 test cases
cond_wait_mono: [2.023083s] Passed.
cond_wait_real: [2.017972s] Passed.
[4.042078s]

lib/libpthread/t_detach (401/848): 1 test cases
pthread_detach: [300.021242s] Failed: Test case timed out after 300 seconds
[300.021708s]

lib/libpthread/t_equal (402/848): 1 test cases
pthread_equal: [300.027646s] Failed: Test case timed out after 300 seconds
[300.028295s]

lib/libpthread/t_fork (403/848): 1 test cases
fork: [300.018077s] Failed: Test case timed out after 300 seconds
[300.018975s]

lib/libpthread/t_fpu (404/848): 1 test cases
fpu: [0.020578s] Passed.
[0.021314s]

lib/libpthread/t_join (405/848): 1 test cases
pthread_join: [0.021632s] Passed.
[0.022411s]

lib/libpthread/t_kill (406/848): 1 test cases
simple: [300.033759s] Failed: Test case timed out after 300 seconds
[300.034528s]

lib/libpthread/t_mutex (407/848): 7 test cases
mutex1: [300.017542s] Failed: Test case timed out after 300 seconds
mutex2: [300.023064s] Failed: Test case timed out after 300 seconds
mutex3: [300.023259s] Failed: Test case timed out after 300 seconds
mutex4: [300.023015s] Failed: Test case timed out after 300 seconds
mutex5: [300.023495s] Failed: Test case timed out after 300 seconds
mutexattr1: [0.013769s] Passed.
mutexattr2: [0.025483s] Passed.
[1500.154160s]

lib/libpthread/t_name (408/848): 1 test cases
name: [0.014262s] Passed.
[0.014951s]

...

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

Kamil Rytarowski  writes:

> On 13.02.2020 20:03, Ryo ONODERA wrote:
>> Hi,
>> 
>> Kamil Rytarowski  writes:
>> 
>>> On 12.02.2020 15:01, Ryo ONODERA wrote:
 Hi,

 Kamil Rytarowski  writes:

> Hello,
>
> I will have a look at them.

 Thank you.
 Real fix is welcome.

 And multimedia/handbrake has workaround already.
 I have workaround patches for lang/mono6 (like your nspr patch).
 I will commit them after some tests.

>>>
>>> libblueray real fix patch is pending upstream.
>>>
>>> https://code.videolan.org/videolan/libbluray/merge_requests/17
>> 
>> Thank you very much!
>> I will apply this to multimedia/handbrake too.
>> 
>
> HandBrake is now patched upstream:
>
> https://github.com/HandBrake/HandBrake/commit/73e958d198aba4cab8f434be6cf0661da1a2f91b
>
>>> I will look into mono next.
>> 
>> Excellent.
>> 
> What is the reproducer for mono?
>

Remove the folowing patches:
lang/mono6/patches/patch-external_corert_src_Native_gc_env_gcenv.structs.h
lang/mono6/patches/patch-mono_metadata_w32mutex-unix.
lang/mono6/patches/patch-libgc_pthread__stop__world.c
lang/mono6/patches/patch-libgc_pthread__support.c
lang/mono6/patches/patch-mono_utils_mono-threads-posix.c

And run "make" in your pkgsrc/lang/mono6 directory.
During build, you will get SIGSEGVs.

Thank you.

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 13.02.2020 20:03, Ryo ONODERA wrote:
> Hi,
> 
> Kamil Rytarowski  writes:
> 
>> On 12.02.2020 15:01, Ryo ONODERA wrote:
>>> Hi,
>>>
>>> Kamil Rytarowski  writes:
>>>
 Hello,

 I will have a look at them.
>>>
>>> Thank you.
>>> Real fix is welcome.
>>>
>>> And multimedia/handbrake has workaround already.
>>> I have workaround patches for lang/mono6 (like your nspr patch).
>>> I will commit them after some tests.
>>>
>>
>> libblueray real fix patch is pending upstream.
>>
>> https://code.videolan.org/videolan/libbluray/merge_requests/17
> 
> Thank you very much!
> I will apply this to multimedia/handbrake too.
> 

HandBrake is now patched upstream:

https://github.com/HandBrake/HandBrake/commit/73e958d198aba4cab8f434be6cf0661da1a2f91b

>> I will look into mono next.
> 
> Excellent.
> 
What is the reproducer for mono?



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

Kamil Rytarowski  writes:

> On 12.02.2020 15:01, Ryo ONODERA wrote:
>> Hi,
>> 
>> Kamil Rytarowski  writes:
>> 
>>> Hello,
>>>
>>> I will have a look at them.
>> 
>> Thank you.
>> Real fix is welcome.
>> 
>> And multimedia/handbrake has workaround already.
>> I have workaround patches for lang/mono6 (like your nspr patch).
>> I will commit them after some tests.
>> 
>
> libblueray real fix patch is pending upstream.
>
> https://code.videolan.org/videolan/libbluray/merge_requests/17

Thank you very much!
I will apply this to multimedia/handbrake too.

> I will look into mono next.

Excellent.

>
>>> On 12.02.2020 14:02, Ryo ONODERA wrote:
 Hi,

 Kamil Rytarowski  writes:

> Please apple workaround (same like in NSPR) for now if fixing is 
> difficult.
>
> Such bugs can have security implications.

 Adding workarounds will not improve security problems.
 And I feel that such workarounds will not be accepted by upstream.
 I will add workarounds to some packages.
 However I feel that it is not meaningful...

> On 12.02.2020 09:49, Ryo ONODERA wrote:
>> Hi,
>>
>> I have two problematic pkgsrc packages at least.
>> Of course these programs have misuses and/or bugs, however I feel that
>> dealing pt_magic in pthread_equal() is too hasty for pkgsrc.
>>
>> multimedia/handbrake (internal libbluray):
>> The invalid thread pointer is not NULL.
>> pthread_equal t1: 0x
>> pthread_equal t2: 0x7073b25e2000
>>
>> Another one is lang/mono6:
>> The invalid thread pointer is not 0x.
>> pthread_equal t1: 0x7b066d4d7800
>> pthread_equal t2: 0x60f5f000
>>
>> Of course, it is desirable to fix every misuses and bugs in pkgsrc.
>> However it is impossible for now (at least for me).
>>
>> "Kamil Rytarowski"  writes:
>>
>>> Module Name:src
>>> Committed By:   kamil
>>> Date:   Sat Feb  8 17:06:03 UTC 2020
>>>
>>> Modified Files:
>>> src/lib/libpthread: pthread.c
>>>
>>> Log Message:
>>> Change the behavior of pthread_equal()
>>>
>>> On error when not aborting, do not return EINVAL as it has a side effect
>>> of being interpreted as matching threads. For invalid threads return
>>> unmatched.
>>>
>>> Check pthreads for NULL, before accessing pt_magic field. This avoids
>>> faults on comparision with a NULL pointer.
>>>
>>> This behavior is in the scope of UB, but should be easier to deal with
>>> buggy software.
>>>
>>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.163 -r1.164 src/lib/libpthread/pthread.c
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>>
>>> Modified files:
>>>
>>> Index: src/lib/libpthread/pthread.c
>>> diff -u src/lib/libpthread/pthread.c:1.163 
>>> src/lib/libpthread/pthread.c:1.164
>>> --- src/lib/libpthread/pthread.c:1.163  Wed Feb  5 14:56:04 2020
>>> +++ src/lib/libpthread/pthread.cSat Feb  8 17:06:03 2020
>>> @@ -1,4 +1,4 @@
>>> -/* $NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $  
>>> */
>>> +/* $NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $  
>>> */
>>>  
>>>  /*-
>>>   * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
>>> @@ -31,7 +31,7 @@
>>>   */
>>>  
>>>  #include 
>>> -__RCSID("$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $");
>>> +__RCSID("$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $");
>>>  
>>>  #define__EXPOSE_STACK  1
>>>  
>>> @@ -770,11 +770,11 @@ pthread_equal(pthread_t t1, pthread_t t2
>>> if (__predict_false(__uselibcstub))
>>> return __libc_thr_equal_stub(t1, t2);
>>>  
>>> -   pthread__error(EINVAL, "Invalid thread",
>>> -   t1->pt_magic == PT_MAGIC);
>>> +   pthread__error(0, "Invalid thread",
>>> +   (t1 != NULL) && (t1->pt_magic == PT_MAGIC));
>>>  
>>> -   pthread__error(EINVAL, "Invalid thread",
>>> -   t2->pt_magic == PT_MAGIC);
>>> +   pthread__error(0, "Invalid thread",
>>> +   (t2 != NULL) && (t2->pt_magic == PT_MAGIC));
>>>  
>>> /* Nothing special here. */
>>> return (t1 == t2);
>>>
>>
>
>

>>>
>>>
>> 
>
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 12.02.2020 15:01, Ryo ONODERA wrote:
> Hi,
> 
> Kamil Rytarowski  writes:
> 
>> Hello,
>>
>> I will have a look at them.
> 
> Thank you.
> Real fix is welcome.
> 
> And multimedia/handbrake has workaround already.
> I have workaround patches for lang/mono6 (like your nspr patch).
> I will commit them after some tests.
> 

libblueray real fix patch is pending upstream.

https://code.videolan.org/videolan/libbluray/merge_requests/17

I will look into mono next.

>> On 12.02.2020 14:02, Ryo ONODERA wrote:
>>> Hi,
>>>
>>> Kamil Rytarowski  writes:
>>>
 Please apple workaround (same like in NSPR) for now if fixing is difficult.

 Such bugs can have security implications.
>>>
>>> Adding workarounds will not improve security problems.
>>> And I feel that such workarounds will not be accepted by upstream.
>>> I will add workarounds to some packages.
>>> However I feel that it is not meaningful...
>>>
 On 12.02.2020 09:49, Ryo ONODERA wrote:
> Hi,
>
> I have two problematic pkgsrc packages at least.
> Of course these programs have misuses and/or bugs, however I feel that
> dealing pt_magic in pthread_equal() is too hasty for pkgsrc.
>
> multimedia/handbrake (internal libbluray):
> The invalid thread pointer is not NULL.
> pthread_equal t1: 0x
> pthread_equal t2: 0x7073b25e2000
>
> Another one is lang/mono6:
> The invalid thread pointer is not 0x.
> pthread_equal t1: 0x7b066d4d7800
> pthread_equal t2: 0x60f5f000
>
> Of course, it is desirable to fix every misuses and bugs in pkgsrc.
> However it is impossible for now (at least for me).
>
> "Kamil Rytarowski"  writes:
>
>> Module Name: src
>> Committed By:kamil
>> Date:Sat Feb  8 17:06:03 UTC 2020
>>
>> Modified Files:
>>  src/lib/libpthread: pthread.c
>>
>> Log Message:
>> Change the behavior of pthread_equal()
>>
>> On error when not aborting, do not return EINVAL as it has a side effect
>> of being interpreted as matching threads. For invalid threads return
>> unmatched.
>>
>> Check pthreads for NULL, before accessing pt_magic field. This avoids
>> faults on comparision with a NULL pointer.
>>
>> This behavior is in the scope of UB, but should be easier to deal with
>> buggy software.
>>
>>
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.163 -r1.164 src/lib/libpthread/pthread.c
>>
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
>>
>> Modified files:
>>
>> Index: src/lib/libpthread/pthread.c
>> diff -u src/lib/libpthread/pthread.c:1.163 
>> src/lib/libpthread/pthread.c:1.164
>> --- src/lib/libpthread/pthread.c:1.163   Wed Feb  5 14:56:04 2020
>> +++ src/lib/libpthread/pthread.c Sat Feb  8 17:06:03 2020
>> @@ -1,4 +1,4 @@
>> -/*  $NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $  
>> */
>> +/*  $NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $  
>> */
>>  
>>  /*-
>>   * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
>> @@ -31,7 +31,7 @@
>>   */
>>  
>>  #include 
>> -__RCSID("$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $");
>> +__RCSID("$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $");
>>  
>>  #define __EXPOSE_STACK  1
>>  
>> @@ -770,11 +770,11 @@ pthread_equal(pthread_t t1, pthread_t t2
>>  if (__predict_false(__uselibcstub))
>>  return __libc_thr_equal_stub(t1, t2);
>>  
>> -pthread__error(EINVAL, "Invalid thread",
>> -t1->pt_magic == PT_MAGIC);
>> +pthread__error(0, "Invalid thread",
>> +(t1 != NULL) && (t1->pt_magic == PT_MAGIC));
>>  
>> -pthread__error(EINVAL, "Invalid thread",
>> -t2->pt_magic == PT_MAGIC);
>> +pthread__error(0, "Invalid thread",
>> +(t2 != NULL) && (t2->pt_magic == PT_MAGIC));
>>  
>>  /* Nothing special here. */
>>  return (t1 == t2);
>>
>


>>>
>>
>>
> 




signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

Kamil Rytarowski  writes:

> Hello,
>
> I will have a look at them.

Thank you.
Real fix is welcome.

And multimedia/handbrake has workaround already.
I have workaround patches for lang/mono6 (like your nspr patch).
I will commit them after some tests.

> On 12.02.2020 14:02, Ryo ONODERA wrote:
>> Hi,
>> 
>> Kamil Rytarowski  writes:
>> 
>>> Please apple workaround (same like in NSPR) for now if fixing is difficult.
>>>
>>> Such bugs can have security implications.
>> 
>> Adding workarounds will not improve security problems.
>> And I feel that such workarounds will not be accepted by upstream.
>> I will add workarounds to some packages.
>> However I feel that it is not meaningful...
>> 
>>> On 12.02.2020 09:49, Ryo ONODERA wrote:
 Hi,

 I have two problematic pkgsrc packages at least.
 Of course these programs have misuses and/or bugs, however I feel that
 dealing pt_magic in pthread_equal() is too hasty for pkgsrc.

 multimedia/handbrake (internal libbluray):
 The invalid thread pointer is not NULL.
 pthread_equal t1: 0x
 pthread_equal t2: 0x7073b25e2000

 Another one is lang/mono6:
 The invalid thread pointer is not 0x.
 pthread_equal t1: 0x7b066d4d7800
 pthread_equal t2: 0x60f5f000

 Of course, it is desirable to fix every misuses and bugs in pkgsrc.
 However it is impossible for now (at least for me).

 "Kamil Rytarowski"  writes:

> Module Name:  src
> Committed By: kamil
> Date: Sat Feb  8 17:06:03 UTC 2020
>
> Modified Files:
>   src/lib/libpthread: pthread.c
>
> Log Message:
> Change the behavior of pthread_equal()
>
> On error when not aborting, do not return EINVAL as it has a side effect
> of being interpreted as matching threads. For invalid threads return
> unmatched.
>
> Check pthreads for NULL, before accessing pt_magic field. This avoids
> faults on comparision with a NULL pointer.
>
> This behavior is in the scope of UB, but should be easier to deal with
> buggy software.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.163 -r1.164 src/lib/libpthread/pthread.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
> Modified files:
>
> Index: src/lib/libpthread/pthread.c
> diff -u src/lib/libpthread/pthread.c:1.163 
> src/lib/libpthread/pthread.c:1.164
> --- src/lib/libpthread/pthread.c:1.163Wed Feb  5 14:56:04 2020
> +++ src/lib/libpthread/pthread.c  Sat Feb  8 17:06:03 2020
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $  
> */
> +/*   $NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $  
> */
>  
>  /*-
>   * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
> @@ -31,7 +31,7 @@
>   */
>  
>  #include 
> -__RCSID("$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $");
> +__RCSID("$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $");
>  
>  #define  __EXPOSE_STACK  1
>  
> @@ -770,11 +770,11 @@ pthread_equal(pthread_t t1, pthread_t t2
>   if (__predict_false(__uselibcstub))
>   return __libc_thr_equal_stub(t1, t2);
>  
> - pthread__error(EINVAL, "Invalid thread",
> - t1->pt_magic == PT_MAGIC);
> + pthread__error(0, "Invalid thread",
> + (t1 != NULL) && (t1->pt_magic == PT_MAGIC));
>  
> - pthread__error(EINVAL, "Invalid thread",
> - t2->pt_magic == PT_MAGIC);
> + pthread__error(0, "Invalid thread",
> + (t2 != NULL) && (t2->pt_magic == PT_MAGIC));
>  
>   /* Nothing special here. */
>   return (t1 == t2);
>

>>>
>>>
>> 
>
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

Hello,

I will have a look at them.

On 12.02.2020 14:02, Ryo ONODERA wrote:
> Hi,
> 
> Kamil Rytarowski  writes:
> 
>> Please apple workaround (same like in NSPR) for now if fixing is difficult.
>>
>> Such bugs can have security implications.
> 
> Adding workarounds will not improve security problems.
> And I feel that such workarounds will not be accepted by upstream.
> I will add workarounds to some packages.
> However I feel that it is not meaningful...
> 
>> On 12.02.2020 09:49, Ryo ONODERA wrote:
>>> Hi,
>>>
>>> I have two problematic pkgsrc packages at least.
>>> Of course these programs have misuses and/or bugs, however I feel that
>>> dealing pt_magic in pthread_equal() is too hasty for pkgsrc.
>>>
>>> multimedia/handbrake (internal libbluray):
>>> The invalid thread pointer is not NULL.
>>> pthread_equal t1: 0x
>>> pthread_equal t2: 0x7073b25e2000
>>>
>>> Another one is lang/mono6:
>>> The invalid thread pointer is not 0x.
>>> pthread_equal t1: 0x7b066d4d7800
>>> pthread_equal t2: 0x60f5f000
>>>
>>> Of course, it is desirable to fix every misuses and bugs in pkgsrc.
>>> However it is impossible for now (at least for me).
>>>
>>> "Kamil Rytarowski"  writes:
>>>
 Module Name:   src
 Committed By:  kamil
 Date:  Sat Feb  8 17:06:03 UTC 2020

 Modified Files:
src/lib/libpthread: pthread.c

 Log Message:
 Change the behavior of pthread_equal()

 On error when not aborting, do not return EINVAL as it has a side effect
 of being interpreted as matching threads. For invalid threads return
 unmatched.

 Check pthreads for NULL, before accessing pt_magic field. This avoids
 faults on comparision with a NULL pointer.

 This behavior is in the scope of UB, but should be easier to deal with
 buggy software.


 To generate a diff of this commit:
 cvs rdiff -u -r1.163 -r1.164 src/lib/libpthread/pthread.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

 Modified files:

 Index: src/lib/libpthread/pthread.c
 diff -u src/lib/libpthread/pthread.c:1.163 
 src/lib/libpthread/pthread.c:1.164
 --- src/lib/libpthread/pthread.c:1.163 Wed Feb  5 14:56:04 2020
 +++ src/lib/libpthread/pthread.c   Sat Feb  8 17:06:03 2020
 @@ -1,4 +1,4 @@
 -/*$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $  
 */
 +/*$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $  
 */
  
  /*-
   * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
 @@ -31,7 +31,7 @@
   */
  
  #include 
 -__RCSID("$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $");
 +__RCSID("$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $");
  
  #define   __EXPOSE_STACK  1
  
 @@ -770,11 +770,11 @@ pthread_equal(pthread_t t1, pthread_t t2
if (__predict_false(__uselibcstub))
return __libc_thr_equal_stub(t1, t2);
  
 -  pthread__error(EINVAL, "Invalid thread",
 -  t1->pt_magic == PT_MAGIC);
 +  pthread__error(0, "Invalid thread",
 +  (t1 != NULL) && (t1->pt_magic == PT_MAGIC));
  
 -  pthread__error(EINVAL, "Invalid thread",
 -  t2->pt_magic == PT_MAGIC);
 +  pthread__error(0, "Invalid thread",
 +  (t2 != NULL) && (t2->pt_magic == PT_MAGIC));
  
/* Nothing special here. */
return (t1 == t2);

>>>
>>
>>
> 




signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

Kamil Rytarowski  writes:

> Please apple workaround (same like in NSPR) for now if fixing is difficult.
>
> Such bugs can have security implications.

Adding workarounds will not improve security problems.
And I feel that such workarounds will not be accepted by upstream.
I will add workarounds to some packages.
However I feel that it is not meaningful...

> On 12.02.2020 09:49, Ryo ONODERA wrote:
>> Hi,
>> 
>> I have two problematic pkgsrc packages at least.
>> Of course these programs have misuses and/or bugs, however I feel that
>> dealing pt_magic in pthread_equal() is too hasty for pkgsrc.
>> 
>> multimedia/handbrake (internal libbluray):
>> The invalid thread pointer is not NULL.
>> pthread_equal t1: 0x
>> pthread_equal t2: 0x7073b25e2000
>> 
>> Another one is lang/mono6:
>> The invalid thread pointer is not 0x.
>> pthread_equal t1: 0x7b066d4d7800
>> pthread_equal t2: 0x60f5f000
>> 
>> Of course, it is desirable to fix every misuses and bugs in pkgsrc.
>> However it is impossible for now (at least for me).
>> 
>> "Kamil Rytarowski"  writes:
>> 
>>> Module Name:src
>>> Committed By:   kamil
>>> Date:   Sat Feb  8 17:06:03 UTC 2020
>>>
>>> Modified Files:
>>> src/lib/libpthread: pthread.c
>>>
>>> Log Message:
>>> Change the behavior of pthread_equal()
>>>
>>> On error when not aborting, do not return EINVAL as it has a side effect
>>> of being interpreted as matching threads. For invalid threads return
>>> unmatched.
>>>
>>> Check pthreads for NULL, before accessing pt_magic field. This avoids
>>> faults on comparision with a NULL pointer.
>>>
>>> This behavior is in the scope of UB, but should be easier to deal with
>>> buggy software.
>>>
>>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.163 -r1.164 src/lib/libpthread/pthread.c
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>>
>>> Modified files:
>>>
>>> Index: src/lib/libpthread/pthread.c
>>> diff -u src/lib/libpthread/pthread.c:1.163 
>>> src/lib/libpthread/pthread.c:1.164
>>> --- src/lib/libpthread/pthread.c:1.163  Wed Feb  5 14:56:04 2020
>>> +++ src/lib/libpthread/pthread.cSat Feb  8 17:06:03 2020
>>> @@ -1,4 +1,4 @@
>>> -/* $NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $  */
>>> +/* $NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $  */
>>>  
>>>  /*-
>>>   * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
>>> @@ -31,7 +31,7 @@
>>>   */
>>>  
>>>  #include 
>>> -__RCSID("$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $");
>>> +__RCSID("$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $");
>>>  
>>>  #define__EXPOSE_STACK  1
>>>  
>>> @@ -770,11 +770,11 @@ pthread_equal(pthread_t t1, pthread_t t2
>>> if (__predict_false(__uselibcstub))
>>> return __libc_thr_equal_stub(t1, t2);
>>>  
>>> -   pthread__error(EINVAL, "Invalid thread",
>>> -   t1->pt_magic == PT_MAGIC);
>>> +   pthread__error(0, "Invalid thread",
>>> +   (t1 != NULL) && (t1->pt_magic == PT_MAGIC));
>>>  
>>> -   pthread__error(EINVAL, "Invalid thread",
>>> -   t2->pt_magic == PT_MAGIC);
>>> +   pthread__error(0, "Invalid thread",
>>> +   (t2 != NULL) && (t2->pt_magic == PT_MAGIC));
>>>  
>>> /* Nothing special here. */
>>> return (t1 == t2);
>>>
>> 
>
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

Please apple workaround (same like in NSPR) for now if fixing is difficult.

Such bugs can have security implications.

On 12.02.2020 09:49, Ryo ONODERA wrote:
> Hi,
> 
> I have two problematic pkgsrc packages at least.
> Of course these programs have misuses and/or bugs, however I feel that
> dealing pt_magic in pthread_equal() is too hasty for pkgsrc.
> 
> multimedia/handbrake (internal libbluray):
> The invalid thread pointer is not NULL.
> pthread_equal t1: 0x
> pthread_equal t2: 0x7073b25e2000
> 
> Another one is lang/mono6:
> The invalid thread pointer is not 0x.
> pthread_equal t1: 0x7b066d4d7800
> pthread_equal t2: 0x60f5f000
> 
> Of course, it is desirable to fix every misuses and bugs in pkgsrc.
> However it is impossible for now (at least for me).
> 
> "Kamil Rytarowski"  writes:
> 
>> Module Name: src
>> Committed By:kamil
>> Date:Sat Feb  8 17:06:03 UTC 2020
>>
>> Modified Files:
>>  src/lib/libpthread: pthread.c
>>
>> Log Message:
>> Change the behavior of pthread_equal()
>>
>> On error when not aborting, do not return EINVAL as it has a side effect
>> of being interpreted as matching threads. For invalid threads return
>> unmatched.
>>
>> Check pthreads for NULL, before accessing pt_magic field. This avoids
>> faults on comparision with a NULL pointer.
>>
>> This behavior is in the scope of UB, but should be easier to deal with
>> buggy software.
>>
>>
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.163 -r1.164 src/lib/libpthread/pthread.c
>>
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
>>
>> Modified files:
>>
>> Index: src/lib/libpthread/pthread.c
>> diff -u src/lib/libpthread/pthread.c:1.163 src/lib/libpthread/pthread.c:1.164
>> --- src/lib/libpthread/pthread.c:1.163   Wed Feb  5 14:56:04 2020
>> +++ src/lib/libpthread/pthread.c Sat Feb  8 17:06:03 2020
>> @@ -1,4 +1,4 @@
>> -/*  $NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $  */
>> +/*  $NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $  */
>>  
>>  /*-
>>   * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
>> @@ -31,7 +31,7 @@
>>   */
>>  
>>  #include 
>> -__RCSID("$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $");
>> +__RCSID("$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $");
>>  
>>  #define __EXPOSE_STACK  1
>>  
>> @@ -770,11 +770,11 @@ pthread_equal(pthread_t t1, pthread_t t2
>>  if (__predict_false(__uselibcstub))
>>  return __libc_thr_equal_stub(t1, t2);
>>  
>> -pthread__error(EINVAL, "Invalid thread",
>> -t1->pt_magic == PT_MAGIC);
>> +pthread__error(0, "Invalid thread",
>> +(t1 != NULL) && (t1->pt_magic == PT_MAGIC));
>>  
>> -pthread__error(EINVAL, "Invalid thread",
>> -t2->pt_magic == PT_MAGIC);
>> +pthread__error(0, "Invalid thread",
>> +(t2 != NULL) && (t2->pt_magic == PT_MAGIC));
>>  
>>  /* Nothing special here. */
>>  return (t1 == t2);
>>
> 




signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

I have two problematic pkgsrc packages at least.
Of course these programs have misuses and/or bugs, however I feel that
dealing pt_magic in pthread_equal() is too hasty for pkgsrc.

multimedia/handbrake (internal libbluray):
The invalid thread pointer is not NULL.
pthread_equal t1: 0x
pthread_equal t2: 0x7073b25e2000

Another one is lang/mono6:
The invalid thread pointer is not 0x.
pthread_equal t1: 0x7b066d4d7800
pthread_equal t2: 0x60f5f000

Of course, it is desirable to fix every misuses and bugs in pkgsrc.
However it is impossible for now (at least for me).

"Kamil Rytarowski"  writes:

> Module Name:  src
> Committed By: kamil
> Date: Sat Feb  8 17:06:03 UTC 2020
>
> Modified Files:
>   src/lib/libpthread: pthread.c
>
> Log Message:
> Change the behavior of pthread_equal()
>
> On error when not aborting, do not return EINVAL as it has a side effect
> of being interpreted as matching threads. For invalid threads return
> unmatched.
>
> Check pthreads for NULL, before accessing pt_magic field. This avoids
> faults on comparision with a NULL pointer.
>
> This behavior is in the scope of UB, but should be easier to deal with
> buggy software.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.163 -r1.164 src/lib/libpthread/pthread.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
> Modified files:
>
> Index: src/lib/libpthread/pthread.c
> diff -u src/lib/libpthread/pthread.c:1.163 src/lib/libpthread/pthread.c:1.164
> --- src/lib/libpthread/pthread.c:1.163Wed Feb  5 14:56:04 2020
> +++ src/lib/libpthread/pthread.c  Sat Feb  8 17:06:03 2020
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $  */
> +/*   $NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $  */
>  
>  /*-
>   * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
> @@ -31,7 +31,7 @@
>   */
>  
>  #include 
> -__RCSID("$NetBSD: pthread.c,v 1.163 2020/02/05 14:56:04 ryoon Exp $");
> +__RCSID("$NetBSD: pthread.c,v 1.164 2020/02/08 17:06:03 kamil Exp $");
>  
>  #define  __EXPOSE_STACK  1
>  
> @@ -770,11 +770,11 @@ pthread_equal(pthread_t t1, pthread_t t2
>   if (__predict_false(__uselibcstub))
>   return __libc_thr_equal_stub(t1, t2);
>  
> - pthread__error(EINVAL, "Invalid thread",
> - t1->pt_magic == PT_MAGIC);
> + pthread__error(0, "Invalid thread",
> + (t1 != NULL) && (t1->pt_magic == PT_MAGIC));
>  
> - pthread__error(EINVAL, "Invalid thread",
> - t2->pt_magic == PT_MAGIC);
> + pthread__error(0, "Invalid thread",
> + (t2 != NULL) && (t2->pt_magic == PT_MAGIC));
>  
>   /* Nothing special here. */
>   return (t1 == t2);
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 03.02.2020 17:10, Ryo ONODERA wrote:
> Hi,
> 
> Kamil Rytarowski  writes:
> 
>> Please check this workaround:
>>
>> http://netbsd.org/~kamil/patch-00224-firefox-pthread_equal.txt
>>
>> It has to be applied on firefox's package.
> 
> Thank you very much.
> 
> This should be applied to pkgsrc/devel/nspr with slight change.
> (devel/nspr's C files cannot accept // comment.)
> This eliminates segfaults.
> 
> For the record, pthread_equal()'s first thread pointer, mon->owner, is
> invalid in PR_GetMonitorEntryCount() in nspr/pr/src/pthreads/ptsynch.c
> of devel/nspr.
> 

Please patch all affected packages in pkgsrc.

>>
>> The problem has to be reported upstream as a real bug.
> 
> Please file the bug report to Mozilla's bugzilla.
> 

I don't have a direct reproducer as of now myself. Please report a bug.

> Thank you very much.
> 




signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

Kamil Rytarowski  writes:

> On 03.02.2020 17:10, Ryo ONODERA wrote:
>> Hi,
>> 
>> Kamil Rytarowski  writes:
>> 
>>> Please check this workaround:
>>>
>>> http://netbsd.org/~kamil/patch-00224-firefox-pthread_equal.txt
>>>
>>> It has to be applied on firefox's package.
>> 
>> Thank you very much.
>> 
>> This should be applied to pkgsrc/devel/nspr with slight change.
>> (devel/nspr's C files cannot accept // comment.)
>> This eliminates segfaults.
>> 
>> For the record, pthread_equal()'s first thread pointer, mon->owner, is
>> invalid in PR_GetMonitorEntryCount() in nspr/pr/src/pthreads/ptsynch.c
>> of devel/nspr.
>> 
>
> Please patch all affected packages in pkgsrc.

I will commit this patch to devwl/nspr.

>>>
>>> The problem has to be reported upstream as a real bug.
>> 
>> Please file the bug report to Mozilla's bugzilla.
>> 
>
> I don't have a direct reproducer as of now myself. Please report a bug.

O.k. I will submit this to Mozilla.

Thank you very much.

>> Thank you very much.
>> 
>
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

Kamil Rytarowski  writes:

> Please check this workaround:
>
> http://netbsd.org/~kamil/patch-00224-firefox-pthread_equal.txt
>
> It has to be applied on firefox's package.

Thank you very much.

This should be applied to pkgsrc/devel/nspr with slight change.
(devel/nspr's C files cannot accept // comment.)
This eliminates segfaults.

For the record, pthread_equal()'s first thread pointer, mon->owner, is
invalid in PR_GetMonitorEntryCount() in nspr/pr/src/pthreads/ptsynch.c
of devel/nspr.

>
> The problem has to be reported upstream as a real bug.

Please file the bug report to Mozilla's bugzilla.

Thank you very much.

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

Please check this workaround:

http://netbsd.org/~kamil/patch-00224-firefox-pthread_equal.txt

It has to be applied on firefox's package.

The problem has to be reported upstream as a real bug.

On 03.02.2020 16:01, Ryo ONODERA wrote:
> Hi,
> 
> Ryo ONODERA  writes:
> 
>> Hi,
>>
>> I had tested with PTHREAD_DIAGASSERT however it did not produce any output.
>>
>> I am building current and pkgsrc packages from scratch now.
>>
>> I will reply my situation after this rebuild.
>>
>> Thank you.
> 
> With latest toolchain, kernel and userland,
> my firefox gets sefgault every run.
> 
> The core dump generated from PTHREAD_DIAGASSERT=ae does not show
> any useful things.
> See:
> Core was generated by `firefox'.
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0  0x743cb39860ca in _lwp_kill () from /usr/lib/libc.so.12
> [Current thread is 1 (process 4)]
> (gdb) bt
> #0  0x743cb39860ca in _lwp_kill () from /usr/lib/libc.so.12
> #1  0x743ca1943721 in ?? () from /usr/pkg/lib/firefox/libxul.so
> #2  0x743ca2172bee in ?? () from /usr/pkg/lib/firefox/libxul.so
> #3  0x743cb38b0140 in opendir () from /usr/lib/libc.so.12
> #4  0x0001000b in ?? ()
> #5  0x in ?? ()
> 
> 
> And I cannot get any text output to stdout.
> As far as I understand correctly, PTHREAD_DIAGASSERT=e enables the output
> to stdout.
> 
> 
> Still I feel that pthread_equal() is cause of my segfault.
> $ gdb /usr/pkg/lib/firefox/firefox
> GNU gdb (GDB) 8.3
> Copyright (C) 2019 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later 
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "x86_64--netbsd".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> .
> Find the GDB manual and other documentation resources online at:
> .
> 
> For help, type "help".
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from /usr/pkg/lib/firefox/firefox...
> (No debugging symbols found in /usr/pkg/lib/firefox/firefox)
> (gdb) r
> Starting program: /usr/pkg/lib/firefox/firefox
> [New process 18477]
> [Detaching after fork from child process 17695]
> [New LWP 2 of process 18477]
> [New LWP 3 of process 18477]
> [New LWP 4 of process 18477]
> [New LWP 5 of process 18477]
> [New LWP 6 of process 18477]
> [New LWP 7 of process 18477]
> [New LWP 8 of process 18477]
> [New LWP 9 of process 18477]
> [New LWP 10 of process 18477]
> [New LWP 11 of process 18477]
> [New LWP 12 of process 18477]
> [New LWP 13 of process 18477]
> [New LWP 14 of process 18477]
> [New LWP 15 of process 18477]
> [New LWP 16 of process 18477]
> JavaScript error: , line 0: UnknownError: The operation failed for reasons 
> unrelated to the database itself and not covered by any other error code.
> process 18477 is executing new program: /usr/pkg/lib/firefox/firefox
> [New process 18477]
> [Detaching after fork from child process 17526]
> [New LWP 2 of process 18477]
> [New LWP 3 of process 18477]
> [New LWP 4 of process 18477]
> [New LWP 5 of process 18477]
> [New LWP 6 of process 18477]
> [New LWP 7 of process 18477]
> [New LWP 8 of process 18477]
> [New LWP 9 of process 18477]
> [New LWP 10 of process 18477]
> [New LWP 11 of process 18477]
> [New LWP 12 of process 18477]
> [New LWP 13 of process 18477]
> [New LWP 14 of process 18477]
> [New LWP 15 of process 18477]
> [New LWP 16 of process 18477]
> [New LWP 17 of process 18477]
> [New LWP 18 of process 18477]
> [New LWP 19 of process 18477]
> [New LWP 20 of process 18477]
> [New LWP 21 of process 18477]
> [New LWP 22 of process 18477]
> [New LWP 23 of process 18477]
> [New LWP 24 of process 18477]
> [New LWP 25 of process 18477]
> [New LWP 26 of process 18477]
> [New LWP 27 of process 18477]
> [New LWP 28 of process 18477]
> [New LWP 29 of process 18477]
> [New LWP 14 of process 18477]
> [New LWP 30 of process 18477]
> [New LWP 31 of process 18477]
> [New LWP 32 of process 18477]
> [New LWP 33 of process 18477]
> [New LWP 34 of process 18477]
> [New LWP 35 of process 18477]
> [New LWP 36 of process 18477]
> [New LWP 37 of process 18477]
> [New LWP 38 of process 18477]
> [New LWP 39 of process 18477]
> [New process 18477]
> [Detaching after fork from child process 17454]
> [New LWP 39 of process 18477]
> [New LWP 41 of process 18477]
> [New LWP 42 of process 18477]
> [New LWP 43 of process 18477]
> [New LWP 44 of process 18477]
> [New LWP 45 of process 18477]
> [New process 18477]
> [Detaching after fork from child process 15097]
> [New LWP 47 of process 18477]
> [New LWP 48 of process 18477]
> [New LWP 49 of process 18477]
> [New LWP 50 of process 18477]
> [New LWP 51 of process 18477]
> [New LWP 45 of process 18477]

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

Ryo ONODERA  writes:

> Hi,
>
> I had tested with PTHREAD_DIAGASSERT however it did not produce any output.
>
> I am building current and pkgsrc packages from scratch now.
>
> I will reply my situation after this rebuild.
>
> Thank you.

With latest toolchain, kernel and userland,
my firefox gets sefgault every run.

The core dump generated from PTHREAD_DIAGASSERT=ae does not show
any useful things.
See:
Core was generated by `firefox'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x743cb39860ca in _lwp_kill () from /usr/lib/libc.so.12
[Current thread is 1 (process 4)]
(gdb) bt
#0  0x743cb39860ca in _lwp_kill () from /usr/lib/libc.so.12
#1  0x743ca1943721 in ?? () from /usr/pkg/lib/firefox/libxul.so
#2  0x743ca2172bee in ?? () from /usr/pkg/lib/firefox/libxul.so
#3  0x743cb38b0140 in opendir () from /usr/lib/libc.so.12
#4  0x0001000b in ?? ()
#5  0x in ?? ()


And I cannot get any text output to stdout.
As far as I understand correctly, PTHREAD_DIAGASSERT=e enables the output
to stdout.


Still I feel that pthread_equal() is cause of my segfault.
$ gdb /usr/pkg/lib/firefox/firefox
GNU gdb (GDB) 8.3
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64--netbsd".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/pkg/lib/firefox/firefox...
(No debugging symbols found in /usr/pkg/lib/firefox/firefox)
(gdb) r
Starting program: /usr/pkg/lib/firefox/firefox
[New process 18477]
[Detaching after fork from child process 17695]
[New LWP 2 of process 18477]
[New LWP 3 of process 18477]
[New LWP 4 of process 18477]
[New LWP 5 of process 18477]
[New LWP 6 of process 18477]
[New LWP 7 of process 18477]
[New LWP 8 of process 18477]
[New LWP 9 of process 18477]
[New LWP 10 of process 18477]
[New LWP 11 of process 18477]
[New LWP 12 of process 18477]
[New LWP 13 of process 18477]
[New LWP 14 of process 18477]
[New LWP 15 of process 18477]
[New LWP 16 of process 18477]
JavaScript error: , line 0: UnknownError: The operation failed for reasons 
unrelated to the database itself and not covered by any other error code.
process 18477 is executing new program: /usr/pkg/lib/firefox/firefox
[New process 18477]
[Detaching after fork from child process 17526]
[New LWP 2 of process 18477]
[New LWP 3 of process 18477]
[New LWP 4 of process 18477]
[New LWP 5 of process 18477]
[New LWP 6 of process 18477]
[New LWP 7 of process 18477]
[New LWP 8 of process 18477]
[New LWP 9 of process 18477]
[New LWP 10 of process 18477]
[New LWP 11 of process 18477]
[New LWP 12 of process 18477]
[New LWP 13 of process 18477]
[New LWP 14 of process 18477]
[New LWP 15 of process 18477]
[New LWP 16 of process 18477]
[New LWP 17 of process 18477]
[New LWP 18 of process 18477]
[New LWP 19 of process 18477]
[New LWP 20 of process 18477]
[New LWP 21 of process 18477]
[New LWP 22 of process 18477]
[New LWP 23 of process 18477]
[New LWP 24 of process 18477]
[New LWP 25 of process 18477]
[New LWP 26 of process 18477]
[New LWP 27 of process 18477]
[New LWP 28 of process 18477]
[New LWP 29 of process 18477]
[New LWP 14 of process 18477]
[New LWP 30 of process 18477]
[New LWP 31 of process 18477]
[New LWP 32 of process 18477]
[New LWP 33 of process 18477]
[New LWP 34 of process 18477]
[New LWP 35 of process 18477]
[New LWP 36 of process 18477]
[New LWP 37 of process 18477]
[New LWP 38 of process 18477]
[New LWP 39 of process 18477]
[New process 18477]
[Detaching after fork from child process 17454]
[New LWP 39 of process 18477]
[New LWP 41 of process 18477]
[New LWP 42 of process 18477]
[New LWP 43 of process 18477]
[New LWP 44 of process 18477]
[New LWP 45 of process 18477]
[New process 18477]
[Detaching after fork from child process 15097]
[New LWP 47 of process 18477]
[New LWP 48 of process 18477]
[New LWP 49 of process 18477]
[New LWP 50 of process 18477]
[New LWP 51 of process 18477]
[New LWP 45 of process 18477]
[New LWP 52 of process 18477]
[New LWP 53 of process 18477]
[New LWP 54 of process 18477]
[New LWP 55 of process 18477]
[New LWP 56 of process 18477]
[New LWP 57 of process 18477]
[New LWP 58 of process 18477]
[New LWP 59 of process 18477]
[New LWP 60 of process 18477]

Thread 21 "Socket Thread" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 4 of process 18477]
0x71a59bc0c2eb in pthread_equal () from /usr/lib/libpthread.so.1
(gdb) bt
#0  0x71a59bc0c2eb in pthread_equal 

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

I had tested with PTHREAD_DIAGASSERT however it did not produce any output.

I am building current and pkgsrc packages from scratch now.

I will reply my situation after this rebuild.

Thank you.

On February 3, 2020 7:23:33 AM GMT+09:00, Kamil Rytarowski  wrote:
>Hello,
>
>I've checked with NetBSD-current from today (2020-02-02) and
>pkgsrc-current (2020-02-02) and package firefox-72.0.2.
>
>I'm not reproducing any crash due to pthread_equal(3) misuse.
>Everything
>I tested, worked for me.
>
>Please try PTHREAD_DIAGASSERT=ae and debug the culprit crash with a
>core(5) file.
>
>On 01.02.2020 22:20, Kamil Rytarowski wrote:
>> Good idea. It could be checked quicker... however I presume that
>> t1->pt_magic + t1->pt_magic already crash on invalid t1/t2 pointers
>as
>> the argument with condition is evaluated.
>> 
>> Ryo, you might check:
>> $ export PTHREAD_DIAGASSERT=ae
>> $ firefox
>> 
>> It should create a coredump for investigation.
>> 
>> According to POSIX
>>
>(https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_equal.html)
>> passing invalid parameters is UB.
>> 
>> GLIBC, Illumos and all other BSDs (+ older NetBSD) have no sanity
>check
>> in pthread_equal(3). Apparently we are the first ones to notice the
>bug.
>> 
>> On 01.02.2020 21:18, Andrew Doran wrote:
>>> Hmm.  Was there not originally an environment variable to control
>this
>>> behaviour, since many applications are buggy?
>>>
>>> Andrew
>>>
>>> On Sun, Feb 02, 2020 at 01:01:49AM +0900, Ryo ONODERA wrote:
 Hi,

 pthread__error()s in pthread_equal() cause segfault
 during start of pkgsrc/www/firefox-72.0.2.

 Without pthread__error()s, www/firefox works fine
 like as follows.
 However I have no idea why I get segfaults.

 Could you take a look at this problem?

 Index: lib/libpthread/pthread.c
 ===
 RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
 retrieving revision 1.162
 diff -u -r1.162 pthread.c
 --- lib/libpthread/pthread.c   29 Jan 2020 17:11:57 -  1.162
 +++ lib/libpthread/pthread.c   1 Feb 2020 15:58:03 -
 @@ -770,11 +770,13 @@
if (__predict_false(__uselibcstub))
return __libc_thr_equal_stub(t1, t2);
  
 +#if 0
pthread__error(EINVAL, "Invalid thread",
t1->pt_magic == PT_MAGIC);
  
pthread__error(EINVAL, "Invalid thread",
t2->pt_magic == PT_MAGIC);
 +#endif
  
/* Nothing special here. */
return (t1 == t2);
 @@ -1108,7 +1110,7 @@
  {
char buf[1024];
size_t len;
 -  
 +
if (pthread__diagassert == 0)
return;
  


 "Kamil Rytarowski"  writes:

> Module Name:  src
> Committed By: kamil
> Date: Wed Jan 29 16:03:44 UTC 2020
>
> Modified Files:
>   src/lib/libpthread: pthread.c pthread_getcpuclockid.c
>
> Log Message:
> Chack thread->pt_magic with PT_MAGIC promptly
>
> Rearrange some checks to avoid verifying pthread_t after using it.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.160 -r1.161 src/lib/libpthread/pthread.c
> cvs rdiff -u -r1.2 -r1.3
>src/lib/libpthread/pthread_getcpuclockid.c
>
> Please note that diffs are not public domain; they are subject to
>the
> copyright notices on the relevant files.
>

 -- 
 Ryo ONODERA // r...@tetera.org
 PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1
>15F3
>> 
>> 

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

Hello,

I've checked with NetBSD-current from today (2020-02-02) and
pkgsrc-current (2020-02-02) and package firefox-72.0.2.

I'm not reproducing any crash due to pthread_equal(3) misuse. Everything
I tested, worked for me.

Please try PTHREAD_DIAGASSERT=ae and debug the culprit crash with a
core(5) file.

On 01.02.2020 22:20, Kamil Rytarowski wrote:
> Good idea. It could be checked quicker... however I presume that
> t1->pt_magic + t1->pt_magic already crash on invalid t1/t2 pointers as
> the argument with condition is evaluated.
> 
> Ryo, you might check:
> $ export PTHREAD_DIAGASSERT=ae
> $ firefox
> 
> It should create a coredump for investigation.
> 
> According to POSIX
> (https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_equal.html)
> passing invalid parameters is UB.
> 
> GLIBC, Illumos and all other BSDs (+ older NetBSD) have no sanity check
> in pthread_equal(3). Apparently we are the first ones to notice the bug.
> 
> On 01.02.2020 21:18, Andrew Doran wrote:
>> Hmm.  Was there not originally an environment variable to control this
>> behaviour, since many applications are buggy?
>>
>> Andrew
>>
>> On Sun, Feb 02, 2020 at 01:01:49AM +0900, Ryo ONODERA wrote:
>>> Hi,
>>>
>>> pthread__error()s in pthread_equal() cause segfault
>>> during start of pkgsrc/www/firefox-72.0.2.
>>>
>>> Without pthread__error()s, www/firefox works fine
>>> like as follows.
>>> However I have no idea why I get segfaults.
>>>
>>> Could you take a look at this problem?
>>>
>>> Index: lib/libpthread/pthread.c
>>> ===
>>> RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
>>> retrieving revision 1.162
>>> diff -u -r1.162 pthread.c
>>> --- lib/libpthread/pthread.c29 Jan 2020 17:11:57 -  1.162
>>> +++ lib/libpthread/pthread.c1 Feb 2020 15:58:03 -
>>> @@ -770,11 +770,13 @@
>>> if (__predict_false(__uselibcstub))
>>> return __libc_thr_equal_stub(t1, t2);
>>>  
>>> +#if 0
>>> pthread__error(EINVAL, "Invalid thread",
>>> t1->pt_magic == PT_MAGIC);
>>>  
>>> pthread__error(EINVAL, "Invalid thread",
>>> t2->pt_magic == PT_MAGIC);
>>> +#endif
>>>  
>>> /* Nothing special here. */
>>> return (t1 == t2);
>>> @@ -1108,7 +1110,7 @@
>>>  {
>>> char buf[1024];
>>> size_t len;
>>> -   
>>> +
>>> if (pthread__diagassert == 0)
>>> return;
>>>  
>>>
>>>
>>> "Kamil Rytarowski"  writes:
>>>
 Module Name:   src
 Committed By:  kamil
 Date:  Wed Jan 29 16:03:44 UTC 2020

 Modified Files:
src/lib/libpthread: pthread.c pthread_getcpuclockid.c

 Log Message:
 Chack thread->pt_magic with PT_MAGIC promptly

 Rearrange some checks to avoid verifying pthread_t after using it.


 To generate a diff of this commit:
 cvs rdiff -u -r1.160 -r1.161 src/lib/libpthread/pthread.c
 cvs rdiff -u -r1.2 -r1.3 src/lib/libpthread/pthread_getcpuclockid.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>>>
>>> -- 
>>> Ryo ONODERA // r...@tetera.org
>>> PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3
> 
> 




signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

Good idea. It could be checked quicker... however I presume that
t1->pt_magic + t1->pt_magic already crash on invalid t1/t2 pointers as
the argument with condition is evaluated.

Ryo, you might check:
$ export PTHREAD_DIAGASSERT=ae
$ firefox

It should create a coredump for investigation.

According to POSIX
(https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_equal.html)
passing invalid parameters is UB.

GLIBC, Illumos and all other BSDs (+ older NetBSD) have no sanity check
in pthread_equal(3). Apparently we are the first ones to notice the bug.

On 01.02.2020 21:18, Andrew Doran wrote:
> Hmm.  Was there not originally an environment variable to control this
> behaviour, since many applications are buggy?
> 
> Andrew
> 
> On Sun, Feb 02, 2020 at 01:01:49AM +0900, Ryo ONODERA wrote:
>> Hi,
>>
>> pthread__error()s in pthread_equal() cause segfault
>> during start of pkgsrc/www/firefox-72.0.2.
>>
>> Without pthread__error()s, www/firefox works fine
>> like as follows.
>> However I have no idea why I get segfaults.
>>
>> Could you take a look at this problem?
>>
>> Index: lib/libpthread/pthread.c
>> ===
>> RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
>> retrieving revision 1.162
>> diff -u -r1.162 pthread.c
>> --- lib/libpthread/pthread.c 29 Jan 2020 17:11:57 -  1.162
>> +++ lib/libpthread/pthread.c 1 Feb 2020 15:58:03 -
>> @@ -770,11 +770,13 @@
>>  if (__predict_false(__uselibcstub))
>>  return __libc_thr_equal_stub(t1, t2);
>>  
>> +#if 0
>>  pthread__error(EINVAL, "Invalid thread",
>>  t1->pt_magic == PT_MAGIC);
>>  
>>  pthread__error(EINVAL, "Invalid thread",
>>  t2->pt_magic == PT_MAGIC);
>> +#endif
>>  
>>  /* Nothing special here. */
>>  return (t1 == t2);
>> @@ -1108,7 +1110,7 @@
>>  {
>>  char buf[1024];
>>  size_t len;
>> -
>> +
>>  if (pthread__diagassert == 0)
>>  return;
>>  
>>
>>
>> "Kamil Rytarowski"  writes:
>>
>>> Module Name:src
>>> Committed By:   kamil
>>> Date:   Wed Jan 29 16:03:44 UTC 2020
>>>
>>> Modified Files:
>>> src/lib/libpthread: pthread.c pthread_getcpuclockid.c
>>>
>>> Log Message:
>>> Chack thread->pt_magic with PT_MAGIC promptly
>>>
>>> Rearrange some checks to avoid verifying pthread_t after using it.
>>>
>>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.160 -r1.161 src/lib/libpthread/pthread.c
>>> cvs rdiff -u -r1.2 -r1.3 src/lib/libpthread/pthread_getcpuclockid.c
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>>
>>
>> -- 
>> Ryo ONODERA // r...@tetera.org
>> PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3




signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Andrew Doran]

Hmm.  Was there not originally an environment variable to control this
behaviour, since many applications are buggy?

Andrew

On Sun, Feb 02, 2020 at 01:01:49AM +0900, Ryo ONODERA wrote:
> Hi,
> 
> pthread__error()s in pthread_equal() cause segfault
> during start of pkgsrc/www/firefox-72.0.2.
> 
> Without pthread__error()s, www/firefox works fine
> like as follows.
> However I have no idea why I get segfaults.
> 
> Could you take a look at this problem?
> 
> Index: lib/libpthread/pthread.c
> ===
> RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
> retrieving revision 1.162
> diff -u -r1.162 pthread.c
> --- lib/libpthread/pthread.c  29 Jan 2020 17:11:57 -  1.162
> +++ lib/libpthread/pthread.c  1 Feb 2020 15:58:03 -
> @@ -770,11 +770,13 @@
>   if (__predict_false(__uselibcstub))
>   return __libc_thr_equal_stub(t1, t2);
>  
> +#if 0
>   pthread__error(EINVAL, "Invalid thread",
>   t1->pt_magic == PT_MAGIC);
>  
>   pthread__error(EINVAL, "Invalid thread",
>   t2->pt_magic == PT_MAGIC);
> +#endif
>  
>   /* Nothing special here. */
>   return (t1 == t2);
> @@ -1108,7 +1110,7 @@
>  {
>   char buf[1024];
>   size_t len;
> - 
> +
>   if (pthread__diagassert == 0)
>   return;
>  
> 
> 
> "Kamil Rytarowski"  writes:
> 
> > Module Name:src
> > Committed By:   kamil
> > Date:   Wed Jan 29 16:03:44 UTC 2020
> >
> > Modified Files:
> > src/lib/libpthread: pthread.c pthread_getcpuclockid.c
> >
> > Log Message:
> > Chack thread->pt_magic with PT_MAGIC promptly
> >
> > Rearrange some checks to avoid verifying pthread_t after using it.
> >
> >
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.160 -r1.161 src/lib/libpthread/pthread.c
> > cvs rdiff -u -r1.2 -r1.3 src/lib/libpthread/pthread_getcpuclockid.c
> >
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.
> >
> 
> -- 
> Ryo ONODERA // r...@tetera.org
> PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Kamil Rytarowski  writes:

> On 01.02.2020 17:01, Ryo ONODERA wrote:
>> Hi,
>> 
>> pthread__error()s in pthread_equal() cause segfault
>> during start of pkgsrc/www/firefox-72.0.2.
>> 
>> Without pthread__error()s, www/firefox works fine
>> like as follows.
>> However I have no idea why I get segfaults.
>> 
>> Could you take a look at this problem?
>> 
>> Index: lib/libpthread/pthread.c
>> ===
>> RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
>> retrieving revision 1.162
>> diff -u -r1.162 pthread.c
>> --- lib/libpthread/pthread.c 29 Jan 2020 17:11:57 -  1.162
>> +++ lib/libpthread/pthread.c 1 Feb 2020 15:58:03 -
>> @@ -770,11 +770,13 @@
>>  if (__predict_false(__uselibcstub))
>>  return __libc_thr_equal_stub(t1, t2);
>>  
>> +#if 0
>>  pthread__error(EINVAL, "Invalid thread",
>>  t1->pt_magic == PT_MAGIC);
>>  
>>  pthread__error(EINVAL, "Invalid thread",
>>  t2->pt_magic == PT_MAGIC);
>> +#endif
>>  
>>  /* Nothing special here. */
>>  return (t1 == t2);
>> @@ -1108,7 +1110,7 @@
>>  {
>>  char buf[1024];
>>  size_t len;
>> -
>> +
>>  if (pthread__diagassert == 0)
>>  return;
>>  
>> 
>> 
>> "Kamil Rytarowski"  writes:
>> 
>>> Module Name:src
>>> Committed By:   kamil
>>> Date:   Wed Jan 29 16:03:44 UTC 2020
>>>
>>> Modified Files:
>>> src/lib/libpthread: pthread.c pthread_getcpuclockid.c
>>>
>>> Log Message:
>>> Chack thread->pt_magic with PT_MAGIC promptly
>>>
>>> Rearrange some checks to avoid verifying pthread_t after using it.
>>>
>>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.160 -r1.161 src/lib/libpthread/pthread.c
>>> cvs rdiff -u -r1.2 -r1.3 src/lib/libpthread/pthread_getcpuclockid.c
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>>
>> 
>
> I will have a look, but it will take a while to build firefox for me.

Thank you.
Of course I can wait.

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 01.02.2020 17:01, Ryo ONODERA wrote:
> Hi,
> 
> pthread__error()s in pthread_equal() cause segfault
> during start of pkgsrc/www/firefox-72.0.2.
> 
> Without pthread__error()s, www/firefox works fine
> like as follows.
> However I have no idea why I get segfaults.
> 
> Could you take a look at this problem?
> 
> Index: lib/libpthread/pthread.c
> ===
> RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
> retrieving revision 1.162
> diff -u -r1.162 pthread.c
> --- lib/libpthread/pthread.c  29 Jan 2020 17:11:57 -  1.162
> +++ lib/libpthread/pthread.c  1 Feb 2020 15:58:03 -
> @@ -770,11 +770,13 @@
>   if (__predict_false(__uselibcstub))
>   return __libc_thr_equal_stub(t1, t2);
>  
> +#if 0
>   pthread__error(EINVAL, "Invalid thread",
>   t1->pt_magic == PT_MAGIC);
>  
>   pthread__error(EINVAL, "Invalid thread",
>   t2->pt_magic == PT_MAGIC);
> +#endif
>  
>   /* Nothing special here. */
>   return (t1 == t2);
> @@ -1108,7 +1110,7 @@
>  {
>   char buf[1024];
>   size_t len;
> - 
> +
>   if (pthread__diagassert == 0)
>   return;
>  
> 
> 
> "Kamil Rytarowski"  writes:
> 
>> Module Name: src
>> Committed By:kamil
>> Date:Wed Jan 29 16:03:44 UTC 2020
>>
>> Modified Files:
>>  src/lib/libpthread: pthread.c pthread_getcpuclockid.c
>>
>> Log Message:
>> Chack thread->pt_magic with PT_MAGIC promptly
>>
>> Rearrange some checks to avoid verifying pthread_t after using it.
>>
>>
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.160 -r1.161 src/lib/libpthread/pthread.c
>> cvs rdiff -u -r1.2 -r1.3 src/lib/libpthread/pthread_getcpuclockid.c
>>
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
>>
> 

I will have a look, but it will take a while to build firefox for me.



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Ryo ONODERA]

Hi,

pthread__error()s in pthread_equal() cause segfault
during start of pkgsrc/www/firefox-72.0.2.

Without pthread__error()s, www/firefox works fine
like as follows.
However I have no idea why I get segfaults.

Could you take a look at this problem?

Index: lib/libpthread/pthread.c
===
RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
retrieving revision 1.162
diff -u -r1.162 pthread.c
--- lib/libpthread/pthread.c29 Jan 2020 17:11:57 -  1.162
+++ lib/libpthread/pthread.c1 Feb 2020 15:58:03 -
@@ -770,11 +770,13 @@
if (__predict_false(__uselibcstub))
return __libc_thr_equal_stub(t1, t2);
 
+#if 0
pthread__error(EINVAL, "Invalid thread",
t1->pt_magic == PT_MAGIC);
 
pthread__error(EINVAL, "Invalid thread",
t2->pt_magic == PT_MAGIC);
+#endif
 
/* Nothing special here. */
return (t1 == t2);
@@ -1108,7 +1110,7 @@
 {
char buf[1024];
size_t len;
-   
+
if (pthread__diagassert == 0)
return;
 


"Kamil Rytarowski"  writes:

> Module Name:  src
> Committed By: kamil
> Date: Wed Jan 29 16:03:44 UTC 2020
>
> Modified Files:
>   src/lib/libpthread: pthread.c pthread_getcpuclockid.c
>
> Log Message:
> Chack thread->pt_magic with PT_MAGIC promptly
>
> Rearrange some checks to avoid verifying pthread_t after using it.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.160 -r1.161 src/lib/libpthread/pthread.c
> cvs rdiff -u -r1.2 -r1.3 src/lib/libpthread/pthread_getcpuclockid.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>

-- 
Ryo ONODERA // r...@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

Actually it happened that modifiying pthread_atfork() to stop
malloc()ing is enough to address the problem.

I have landed the changes and removed '#if 0' kludge.

Thanks!

On 01.02.2020 13:59, Kamil Rytarowski wrote:
> On 31.01.2020 22:10, Andrew Doran wrote:
>> On Fri, Jan 31, 2020 at 06:55:00PM -, Christos Zoulas wrote:
>>
>>> In article <724af477-010b-9ddf-6ece-e23d7cf59...@gmx.com>,
>>> Kamil Rytarowski   wrote:
 -=-=-=-=-=-
 -=-=-=-=-=-

 On 31.01.2020 03:38, Christos Zoulas wrote:
> And it is fixed now.
>
> christos
>

 OK. I am going to submit a bug report upstream and get some feedback
 what is the way forward here, delaying initialization.
>>>
>>> I think that the way forward (on our side) is to do away with libpthread,
>>> merge it with libc and kill all the stub nonsense.
>>
>> Agreed.
>>
>> pthread__init() does some expensive stuff like _lwp_ctl().  I think we can
>> safely & without hacks defer a lot of that till the first pthread_create().
>>
>> Andrew
>>
> 
> This libc-libpthread split/merge is a red herring.
> 
> The problem here is with a mutual dependencies between POSIX threads
> library and malloc library.
> 
> I did some investigation and here are my findings:
> 
> 1. jemalloc abuses initialization and initializes self very early, with
> a constructor:
> 
> /*
>  * If an application creates a thread before doing any allocation in the
> main
>  * thread, then calls fork(2) in the main thread followed by memory
> allocation
>  * in the child process, a race can occur that results in deadlock
> within the
>  * child: the main thread may have forked while the created thread had
>  * partially initialized the allocator.  Ordinarily jemalloc prevents
>  * fork/malloc races via the following functions it registers during
>  * initialization using pthread_atfork(), but of course that does no good if
>  * the allocator isn't fully initialized at fork time.  The following
> library
>  * constructor is a partial solution to this problem.  It may still be
> possible
>  * to trigger the deadlock described above, but doing so would involve
> forking
>  * via a library constructor that runs before jemalloc's runs.
>  */
> #ifndef JEMALLOC_JET
> JEMALLOC_ATTR(constructor)
> static void
> 
> jemalloc_constructor(void) {
> malloc_init();
> }
> #endif
> 
> Relevant commit:
> 
> commit 20f1fc95adb35ea63dc61f47f2b0ffbd37d39f32
> Author: Jason Evans 
> Date:   Tue Oct 9 14:46:22 2012 -0700
> 
> Fix fork(2)-related deadlocks.
> 
> Add a library constructor for jemalloc that initializes the allocator.
> This fixes a race that could occur if threads were created by the main
> thread prior to any memory allocation, followed by fork(2), and then
> memory allocation in the child process.
> 
> Fix the prefork/postfork functions to acquire/release the ctl, prof, and
> rtree mutexes.  This fixes various fork() child process deadlocks, but
> one possible deadlock remains (intentionally) unaddressed: prof
> backtracing can acquire runtime library mutexes, so deadlock is still
> possible if heap profiling is enabled during fork().  This deadlock is
> known to be a real issue in at least the case of libgcc-based
> backtracing.
> 
> Reported by tfengjun.
> 
> 2. FreeBSD added a hack and an internal pthread_mutex_init() version
> called: _pthread_mutex_init_calloc_cb().. it passes a callback pointer
> to jemalloc's tiny calloc().
> 
> This is very ugly and I consider it as the wrong way of boostraping malloc.
> 
> 3. There is a problem inside libpthread. It as designed to not malloc()
> early to not trigger malloc initialization, however it was broken as it
> calls at_fork functions:
> 
> #0  malloc (size=size@entry=16) at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:2052
> #1  0x776b71d71a44 in af_alloc () at
> /usr/src/lib/libc/gen/pthread_atfork.c:80
> #2  af_alloc () at /usr/src/lib/libc/gen/pthread_atfork.c:74
> #3  _pthread_atfork (prepare=prepare@entry=0x0, parent=parent@entry=0x0,
> child=child@entry=0x776b7220b3e5 )
> at /usr/src/lib/libc/gen/pthread_atfork.c:121
> #4  0x776b7220cacd in pthread__init () at
> /usr/src/lib/libpthread/pthread.c:260
> #5  0x776b71d7a585 in _libc_init () at
> /usr/src/lib/libc/misc/initfini.c:128
> 
> These at_fork routines caused also issues with false-positives in Leak
> Sanitizer. I had to pacify the sanitizer and disable tracking of its
> allocations.
> 
> 
> This patch removes '#if 0' hack from src/lib/libpthread and switches
> at_fork to mmap()+munmap().
> 
> http://netbsd.org/~kamil/patch-00219-libpthread-libc-jemalloc.txt
> 
> This test disabled the constructor hack:
> 
> http://netbsd.org/~kamil/patch-00220-jemalloc-disable-constructor.txt
> 
> With these changes everything seems to work.
> 
> In order to avoid the FreeBSD specific hack with the constructor and
> initialize jemalloc always during libc bootstrap I propose the 

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 31.01.2020 22:10, Andrew Doran wrote:
> On Fri, Jan 31, 2020 at 06:55:00PM -, Christos Zoulas wrote:
> 
>> In article <724af477-010b-9ddf-6ece-e23d7cf59...@gmx.com>,
>> Kamil Rytarowski   wrote:
>>> -=-=-=-=-=-
>>> -=-=-=-=-=-
>>>
>>> On 31.01.2020 03:38, Christos Zoulas wrote:
 And it is fixed now.

 christos

>>>
>>> OK. I am going to submit a bug report upstream and get some feedback
>>> what is the way forward here, delaying initialization.
>>
>> I think that the way forward (on our side) is to do away with libpthread,
>> merge it with libc and kill all the stub nonsense.
> 
> Agreed.
> 
> pthread__init() does some expensive stuff like _lwp_ctl().  I think we can
> safely & without hacks defer a lot of that till the first pthread_create().
> 
> Andrew
> 

This libc-libpthread split/merge is a red herring.

The problem here is with a mutual dependencies between POSIX threads
library and malloc library.

I did some investigation and here are my findings:

1. jemalloc abuses initialization and initializes self very early, with
a constructor:

/*
 * If an application creates a thread before doing any allocation in the
main
 * thread, then calls fork(2) in the main thread followed by memory
allocation
 * in the child process, a race can occur that results in deadlock
within the
 * child: the main thread may have forked while the created thread had
 * partially initialized the allocator.  Ordinarily jemalloc prevents
 * fork/malloc races via the following functions it registers during
 * initialization using pthread_atfork(), but of course that does no good if
 * the allocator isn't fully initialized at fork time.  The following
library
 * constructor is a partial solution to this problem.  It may still be
possible
 * to trigger the deadlock described above, but doing so would involve
forking
 * via a library constructor that runs before jemalloc's runs.
 */
#ifndef JEMALLOC_JET
JEMALLOC_ATTR(constructor)
static void

jemalloc_constructor(void) {
malloc_init();
}
#endif

Relevant commit:

commit 20f1fc95adb35ea63dc61f47f2b0ffbd37d39f32
Author: Jason Evans 
Date:   Tue Oct 9 14:46:22 2012 -0700

Fix fork(2)-related deadlocks.

Add a library constructor for jemalloc that initializes the allocator.
This fixes a race that could occur if threads were created by the main
thread prior to any memory allocation, followed by fork(2), and then
memory allocation in the child process.

Fix the prefork/postfork functions to acquire/release the ctl, prof, and
rtree mutexes.  This fixes various fork() child process deadlocks, but
one possible deadlock remains (intentionally) unaddressed: prof
backtracing can acquire runtime library mutexes, so deadlock is still
possible if heap profiling is enabled during fork().  This deadlock is
known to be a real issue in at least the case of libgcc-based
backtracing.

Reported by tfengjun.

2. FreeBSD added a hack and an internal pthread_mutex_init() version
called: _pthread_mutex_init_calloc_cb().. it passes a callback pointer
to jemalloc's tiny calloc().

This is very ugly and I consider it as the wrong way of boostraping malloc.

3. There is a problem inside libpthread. It as designed to not malloc()
early to not trigger malloc initialization, however it was broken as it
calls at_fork functions:

#0  malloc (size=size@entry=16) at
/usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:2052
#1  0x776b71d71a44 in af_alloc () at
/usr/src/lib/libc/gen/pthread_atfork.c:80
#2  af_alloc () at /usr/src/lib/libc/gen/pthread_atfork.c:74
#3  _pthread_atfork (prepare=prepare@entry=0x0, parent=parent@entry=0x0,
child=child@entry=0x776b7220b3e5 )
at /usr/src/lib/libc/gen/pthread_atfork.c:121
#4  0x776b7220cacd in pthread__init () at
/usr/src/lib/libpthread/pthread.c:260
#5  0x776b71d7a585 in _libc_init () at
/usr/src/lib/libc/misc/initfini.c:128

These at_fork routines caused also issues with false-positives in Leak
Sanitizer. I had to pacify the sanitizer and disable tracking of its
allocations.


This patch removes '#if 0' hack from src/lib/libpthread and switches
at_fork to mmap()+munmap().

http://netbsd.org/~kamil/patch-00219-libpthread-libc-jemalloc.txt

This test disabled the constructor hack:

http://netbsd.org/~kamil/patch-00220-jemalloc-disable-constructor.txt

With these changes everything seems to work.

In order to avoid the FreeBSD specific hack with the constructor and
initialize jemalloc always during libc bootstrap I propose the following
approach:

 - add __libc_malloc_init() call in _libc_init()
 - redirect __libc_malloc_init() to jemalloc__init() with jemalloc
 - otherwise redirect to an empty stub

Here is a patch that does everything and works fine for me.

http://netbsd.org/~kamil/patch-00222-jemalloc-enhancements.txt

There are no longer jemalloc calls before being ready and jemalloc is
still initialized always but in its proper time.

(gdb) r
The program being 

Re: CVS commit: src/lib/libpthread

[Andrew Doran]

On Fri, Jan 31, 2020 at 06:55:00PM -, Christos Zoulas wrote:

> In article <724af477-010b-9ddf-6ece-e23d7cf59...@gmx.com>,
> Kamil Rytarowski   wrote:
> >-=-=-=-=-=-
> >-=-=-=-=-=-
> >
> >On 31.01.2020 03:38, Christos Zoulas wrote:
> >> And it is fixed now.
> >> 
> >> christos
> >> 
> >
> >OK. I am going to submit a bug report upstream and get some feedback
> >what is the way forward here, delaying initialization.
> 
> I think that the way forward (on our side) is to do away with libpthread,
> merge it with libc and kill all the stub nonsense.

Agreed.

pthread__init() does some expensive stuff like _lwp_ctl().  I think we can
safely & without hacks defer a lot of that till the first pthread_create().

Andrew

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 31.01.2020 19:55, Christos Zoulas wrote:
> In article <724af477-010b-9ddf-6ece-e23d7cf59...@gmx.com>,
> Kamil Rytarowski   wrote:
>> -=-=-=-=-=-
>> -=-=-=-=-=-
>>
>> On 31.01.2020 03:38, Christos Zoulas wrote:
>>> And it is fixed now.
>>>
>>> christos
>>>
>>
>> OK. I am going to submit a bug report upstream and get some feedback
>> what is the way forward here, delaying initialization.
> 
> I think that the way forward (on our side) is to do away with libpthread,
> merge it with libc and kill all the stub nonsense.
> 
> christos
> 

I was thinking about implementing custom mutexes inside jemalloc with
futexes. A potential implementation could be really thin. There is just
a problem that futexes still need some more time to land the sources.

https://locklessinc.com/articles/mutex_cv_futex/

My thread: https://github.com/jemalloc/jemalloc/issues/1753

I have got no opinion on stubs and separated libpthread.



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

In article <724af477-010b-9ddf-6ece-e23d7cf59...@gmx.com>,
Kamil Rytarowski   wrote:
>-=-=-=-=-=-
>-=-=-=-=-=-
>
>On 31.01.2020 03:38, Christos Zoulas wrote:
>> And it is fixed now.
>> 
>> christos
>> 
>
>OK. I am going to submit a bug report upstream and get some feedback
>what is the way forward here, delaying initialization.

I think that the way forward (on our side) is to do away with libpthread,
merge it with libc and kill all the stub nonsense.

christos

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 31.01.2020 03:38, Christos Zoulas wrote:
> And it is fixed now.
> 
> christos
> 

OK. I am going to submit a bug report upstream and get some feedback
what is the way forward here, delaying initialization.



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

And it is fixed now.

christos


signature.asc
Description: Message signed with OpenPGP

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

At this point I'd say it is simpler to just initialize the mutexattr_t fields 
in a new
libc stub for the attribute init.

christos

> On Jan 30, 2020, at 8:05 PM, Kamil Rytarowski  wrote:
> 
> Signed PGP part
> On 05.03.2019 23:49, Christos Zoulas wrote:
>> Module Name: src
>> Committed By:christos
>> Date:Tue Mar  5 22:49:38 UTC 2019
>> 
>> Modified Files:
>>  src/lib/libpthread: pthread_mutex.c
>> 
>> Log Message:
>> Jemalloc initializes mutexes before we become threaded and expects to use
>> them later.
>> 
>> 
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.64 -r1.65 src/lib/libpthread/pthread_mutex.c
>> 
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
>> 
>> 
>> Modified files:
>> 
>> Index: src/lib/libpthread/pthread_mutex.c
>> diff -u src/lib/libpthread/pthread_mutex.c:1.64 
>> src/lib/libpthread/pthread_mutex.c:1.65
>> --- src/lib/libpthread/pthread_mutex.c:1.64  Fri Dec  8 04:24:31 2017
>> +++ src/lib/libpthread/pthread_mutex.c   Tue Mar  5 17:49:38 2019
>> @@ -1,4 +1,4 @@
>> -/*  $NetBSD: pthread_mutex.c,v 1.64 2017/12/08 09:24:31 kre Exp $   */
>> +/*  $NetBSD: pthread_mutex.c,v 1.65 2019/03/05 22:49:38 christos Exp $  
>> */
>> 
>> /*-
>>  * Copyright (c) 2001, 2003, 2006, 2007, 2008 The NetBSD Foundation, Inc.
>> @@ -47,7 +47,7 @@
>>  */
>> 
>> #include 
>> -__RCSID("$NetBSD: pthread_mutex.c,v 1.64 2017/12/08 09:24:31 kre Exp $");
>> +__RCSID("$NetBSD: pthread_mutex.c,v 1.65 2019/03/05 22:49:38 christos Exp 
>> $");
>> 
>> #include 
>> #include 
>> @@ -122,8 +122,14 @@ pthread_mutex_init(pthread_mutex_t *ptm,
>> {
>>  uintptr_t type, proto, val, ceil;
>> 
>> +#if 0
>> +/*
>> + * Always initialize the mutex structure, maybe be used later
>> + * and the cost should be minimal.
>> + */
>>  if (__predict_false(__uselibcstub))
>>  return __libc_mutex_init_stub(ptm, attr);
>> +#endif
>> 
>>  if (attr == NULL) {
>>  type = PTHREAD_MUTEX_NORMAL;
>> 
> 
> This change looks questionable.
> 
> Inside external/bsd/jemalloc/lib/../dist/src/mutex.c:
> 
>pthread_mutexattr_t attr;
> 
>if (pthread_mutexattr_init() != 0) {
>return true;
> 
>}
>pthread_mutexattr_settype(, MALLOC_MUTEX_TYPE);
>if (pthread_mutex_init(>lock, ) != 0) {
>pthread_mutexattr_destroy();
>return true;
>}
>pthread_mutexattr_destroy();
> 
> 
> We initialize attr with garbage as we use libc stubs and later pass
> garbage to pthread_mutex_init().
> 
> I want to add this sanity check inside pthread_mutex_init()
> 
> http://netbsd.org/~kamil/patch-00218-pthread_mutex_init-check-attr.txt
> 
> Unfortunately this causes jemalloc to deadlock.
> 
> (gdb) bt
> #0  pthread_rwlock_destroy (ptr=0x0) at
> /usr/src/lib/libpthread/pthread_rwlock.c:112
> #1  0x0246 in ?? ()
> #2  0x7f7fe600 in ?? ()
> #3  0x7f7ff7d0 in ?? ()
> #4  0x7f7ff72d94a0 in je_malloc_mutex_init (mutex=0x7f7fe600,
> mutex@entry=0x7f7ff7d0, name=name@entry=0x7f7ff7387106 "base",
>rank=rank@entry=18,
> lock_order=lock_order@entry=malloc_mutex_rank_exclusive) at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/mutex.c:167
> #5  0x7f7ff731834f in je_base_new (tsdn=tsdn@entry=0x0,
> ind=ind@entry=0, extent_hooks=0x7f7ff75ca120 )
>at /usr/src/external/bsd/jemalloc/lib/../dist/src/base.c:366
> #6  0x7f7ff73185d2 in je_base_boot (tsdn=tsdn@entry=0x0) at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/base.c:512
> #7  0x7f7ff731011d in malloc_init_hard_a0_locked () at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1327
> #8  0x7f7ff73107f5 in malloc_init_hard () at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1549
> #9  0x7f7ff723f924 in ?? () from /usr/lib/libc.so.12
> #10 0x7f7ff7ef9800 in ?? ()
> #11 0x7f7ff723ac09 in _init () from /usr/lib/libc.so.12
> #12 0x in ?? ()
> 
> Could we please fix it properly?
> 
> I am not sure what is the proper way here. We probably need to delay
> usage of pthread(3) but it is not that trivial.
> 
> Personally, I preferred the old jemalloc...
> 
> 
> 



signature.asc
Description: Message signed with OpenPGP

Re: CVS commit: src/lib/libpthread

[Kamil Rytarowski]

On 05.03.2019 23:49, Christos Zoulas wrote:
> Module Name:  src
> Committed By: christos
> Date: Tue Mar  5 22:49:38 UTC 2019
> 
> Modified Files:
>   src/lib/libpthread: pthread_mutex.c
> 
> Log Message:
> Jemalloc initializes mutexes before we become threaded and expects to use
> them later.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.64 -r1.65 src/lib/libpthread/pthread_mutex.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 
> 
> Modified files:
> 
> Index: src/lib/libpthread/pthread_mutex.c
> diff -u src/lib/libpthread/pthread_mutex.c:1.64 
> src/lib/libpthread/pthread_mutex.c:1.65
> --- src/lib/libpthread/pthread_mutex.c:1.64   Fri Dec  8 04:24:31 2017
> +++ src/lib/libpthread/pthread_mutex.cTue Mar  5 17:49:38 2019
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: pthread_mutex.c,v 1.64 2017/12/08 09:24:31 kre Exp $   */
> +/*   $NetBSD: pthread_mutex.c,v 1.65 2019/03/05 22:49:38 christos Exp $  
> */
>  
>  /*-
>   * Copyright (c) 2001, 2003, 2006, 2007, 2008 The NetBSD Foundation, Inc.
> @@ -47,7 +47,7 @@
>   */
>  
>  #include 
> -__RCSID("$NetBSD: pthread_mutex.c,v 1.64 2017/12/08 09:24:31 kre Exp $");
> +__RCSID("$NetBSD: pthread_mutex.c,v 1.65 2019/03/05 22:49:38 christos Exp 
> $");
>  
>  #include 
>  #include 
> @@ -122,8 +122,14 @@ pthread_mutex_init(pthread_mutex_t *ptm,
>  {
>   uintptr_t type, proto, val, ceil;
>  
> +#if 0
> + /*
> +  * Always initialize the mutex structure, maybe be used later
> +  * and the cost should be minimal.
> +  */
>   if (__predict_false(__uselibcstub))
>   return __libc_mutex_init_stub(ptm, attr);
> +#endif
>  
>   if (attr == NULL) {
>   type = PTHREAD_MUTEX_NORMAL;
> 

This change looks questionable.

Inside external/bsd/jemalloc/lib/../dist/src/mutex.c:

pthread_mutexattr_t attr;

if (pthread_mutexattr_init() != 0) {
return true;

}
pthread_mutexattr_settype(, MALLOC_MUTEX_TYPE);
if (pthread_mutex_init(>lock, ) != 0) {
pthread_mutexattr_destroy();
return true;
}
pthread_mutexattr_destroy();


We initialize attr with garbage as we use libc stubs and later pass
garbage to pthread_mutex_init().

I want to add this sanity check inside pthread_mutex_init()

http://netbsd.org/~kamil/patch-00218-pthread_mutex_init-check-attr.txt

Unfortunately this causes jemalloc to deadlock.

(gdb) bt
#0  pthread_rwlock_destroy (ptr=0x0) at
/usr/src/lib/libpthread/pthread_rwlock.c:112
#1  0x0246 in ?? ()
#2  0x7f7fe600 in ?? ()
#3  0x7f7ff7d0 in ?? ()
#4  0x7f7ff72d94a0 in je_malloc_mutex_init (mutex=0x7f7fe600,
mutex@entry=0x7f7ff7d0, name=name@entry=0x7f7ff7387106 "base",
rank=rank@entry=18,
lock_order=lock_order@entry=malloc_mutex_rank_exclusive) at
/usr/src/external/bsd/jemalloc/lib/../dist/src/mutex.c:167
#5  0x7f7ff731834f in je_base_new (tsdn=tsdn@entry=0x0,
ind=ind@entry=0, extent_hooks=0x7f7ff75ca120 )
at /usr/src/external/bsd/jemalloc/lib/../dist/src/base.c:366
#6  0x7f7ff73185d2 in je_base_boot (tsdn=tsdn@entry=0x0) at
/usr/src/external/bsd/jemalloc/lib/../dist/src/base.c:512
#7  0x7f7ff731011d in malloc_init_hard_a0_locked () at
/usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1327
#8  0x7f7ff73107f5 in malloc_init_hard () at
/usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1549
#9  0x7f7ff723f924 in ?? () from /usr/lib/libc.so.12
#10 0x7f7ff7ef9800 in ?? ()
#11 0x7f7ff723ac09 in _init () from /usr/lib/libc.so.12
#12 0x in ?? ()

Could we please fix it properly?

I am not sure what is the proper way here. We probably need to delay
usage of pthread(3) but it is not that trivial.

Personally, I preferred the old jemalloc...



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Tue, May 07, 2019 at 06:14:09PM +, m...@netbsd.org wrote:
> Pre-emptively making a thread where people can call this an ugly hack

It is, please put a note into doc/HACKS at least.

Joerg

Re: CVS commit: src/lib/libpthread

[maya]

Pre-emptively making a thread where people can call this an ugly hack

On Tue, May 07, 2019 at 06:12:54PM +, Maya Rashish wrote:
> Module Name:  src
> Committed By: maya
> Date: Tue May  7 18:12:53 UTC 2019
> 
> Modified Files:
>   src/lib/libpthread: Makefile
> 
> Log Message:
> Replace the link command for libpthread.a so that we create a single section
> with all the libpthread symbols in it.
> This makes -lpthread behave like to -Wl,--whole-archive -lpthread.
> 
> This avoids a situation where threaded static binaries use some libc thread
> stubs, which are racy.
> 
> Fixes PR lib/54001: call_once2_32, call_once2_static test cases failing on
> amd64 since gcc7 import.
> 
> Suggested by Jonathan Wakely, thanks!
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.92 -r1.93 src/lib/libpthread/Makefile
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

> Modified files:
> 
> Index: src/lib/libpthread/Makefile
> diff -u src/lib/libpthread/Makefile:1.92 src/lib/libpthread/Makefile:1.93
> --- src/lib/libpthread/Makefile:1.92  Wed Apr 24 11:43:19 2019
> +++ src/lib/libpthread/Makefile   Tue May  7 18:12:53 2019
> @@ -1,4 +1,4 @@
> -#$NetBSD: Makefile,v 1.92 2019/04/24 11:43:19 kamil Exp $
> +#$NetBSD: Makefile,v 1.93 2019/05/07 18:12:53 maya Exp $
>  #
>  
>  NOSANITIZER= # defined
> @@ -269,6 +269,20 @@ MLINKS+= tss.3 tss_set.3
>  
>  INCS+=   threads.h
>  
> +# PR lib/54001: create libpthread.a as a single large object, with all the
> +# symbols in one section. ensures that if any libpthread function is used,
> +# you get all of them from libpthread, and not the libc stubs.
> +#
> +# This makes -lpthread equivalent to -Wl,--whole-archive -lpthread
> +
> +__archivebuild: .USE
> + ${_MKTARGET_BUILD}
> + @rm -f ${.TARGET}
> + ${LD} -r -o ${.TARGET}.o `NM=${NM} ${LORDER} ${.ALLSRC:M*o} | ${TSORT}`
> + ${AR} ${_ARFL} ${.TARGET} ${.TARGET}.o
> +
> +CLEANFILES+= ${.TARGET}.o
> +
>  .include 
>  
>  .else
> 

Re: CVS commit: src/lib/libpthread

[maya]

netbsd-8 needs a web browser too

On Tue, Aug 01, 2017 at 12:31:45PM +, Martin Husemann wrote:
> Module Name:  src
> Committed By: martin
> Date: Tue Aug  1 12:31:45 UTC 2017
> 
> Modified Files:
>   src/lib/libpthread: pthread_attr.c
> 
> Log Message:
> pthread__attr_init_private:
> malloc+memset -> calloc. Also initialize all values to the proper
> defaults.
> This fixes the "rustc panic" discussed on pkgsrc-users.
> OK: joerg
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.17 -r1.18 src/lib/libpthread/pthread_attr.c

Re: CVS commit: src/lib/libpthread

[Taylor R Campbell]

> Date: Tue, 28 Mar 2017 20:10:24 +0200
> From: Joerg Sonnenberger 
> 
> On Tue, Mar 28, 2017 at 05:42:53PM +, Maya Rashish wrote:
> > Remove outdated CAVEATS.
> > 
> > Not sure everything is standards compliant, but I've been told non-default
> > values are supported and pshared exists.
> 
> pshared is not supported.

That is addressed in the BUGS section, which was preserved.

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Tue, Mar 28, 2017 at 05:42:53PM +, Maya Rashish wrote:
> Module Name:  src
> Committed By: maya
> Date: Tue Mar 28 17:42:52 UTC 2017
> 
> Modified Files:
>   src/lib/libpthread: pthread_condattr.3
> 
> Log Message:
> Remove outdated CAVEATS.
> 
> Not sure everything is standards compliant, but I've been told non-default
> values are supported and pshared exists.

pshared is not supported.

Joerg

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Mon, Aug 24, 2015 at 12:45:15PM +, Antti Kantee wrote:
 I started work on this again by doing a web search for the error, and found
 this:
 http://bugs.musicpd.org/view.php?id=4110
 
 There you argue that using constexpr for PTHREAD_MUTEX_INITIALIZER is
 broken.  So if we assume that you are correct, shouldn't we remove the
 constexprs from libc++/dist/libcxx/include/__mutex_base?

Except that the standard requires it.

Joerg

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

Returning to this pickle.

On 26/06/15 18:24, Joerg Sonnenberger wrote:

On Fri, Jun 26, 2015 at 12:49:09PM +, Antti Kantee wrote:

This is the simplest program to repeat the problem with g++ 5.1 and NetBSD's
pthread_types.h:

#include pthread.h
class foo {
pthread_mutex_t m;
public:
constexpr foo() : m(PTHREAD_MUTEX_INITIALIZER) {}
};

(plus or minus typos since I typed it by hand from the virtual machine
console)

If there's some way to say pthread_types.h is even more of a system header
than with -isystem and compilers should not generate errors, then I'd like
to hear it.


You don't understand. The system header is not pthread_types.h, it is
mutex.


Also, if this is really the correct DR, I'm not sure how a flaw in the
standard translates to gcc bug
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1911
(especially when there's no fix for the defect in the standard)


The fix is to apply the C++14 rules, which is more relaxed.


So, I really don't see how to sweep the issue under the rug except by a)
figuring out a workaround b) labeling all C++ compilers which happen to
implement the current standard as incompatible with NetBSD.


DRs are retroactive, e.g. bug fixes and corrections. As such they are
part of the current standard (and of course, it is not the current
standard, since that would be C++14).


I started work on this again by doing a web search for the error, and 
found this:

http://bugs.musicpd.org/view.php?id=4110

There you argue that using constexpr for PTHREAD_MUTEX_INITIALIZER is 
broken.  So if we assume that you are correct, shouldn't we remove the 
constexprs from libc++/dist/libcxx/include/__mutex_base?

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Mon, Aug 24, 2015 at 02:20:55PM +, Antti Kantee wrote:
 On 24/08/15 13:26, Joerg Sonnenberger wrote:
 On Mon, Aug 24, 2015 at 12:45:15PM +, Antti Kantee wrote:
 I started work on this again by doing a web search for the error, and found
 this:
 http://bugs.musicpd.org/view.php?id=4110
 
 There you argue that using constexpr for PTHREAD_MUTEX_INITIALIZER is
 broken.  So if we assume that you are correct, shouldn't we remove the
 constexprs from libc++/dist/libcxx/include/__mutex_base?
 
 Except that the standard requires it.
 
 So are you now saying that that what you said in the above URL is wrong and
 that the NetBSD headers are broken?  Let me quote the relevant part of what
 you said:

Stop inventing things, please.

 
 Nothing in ISO C11, ISO C++11 or POSIX requires PTHREAD_MUTEX_INITIALIZER
 to qualify for constexpr.
 
 But now you are saying that the standard does require
 PTHREAD_MUTEX_INITIALIZER to qualify for constexpr.

A mutex has to be constexpr constructable. That's in the standard. How a
mutex is implemented is not. Nothing in mpd requires the constexpr
property, so requiring it just makes things more difficult for no good
reason.

Joerg

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On 24/08/15 13:26, Joerg Sonnenberger wrote:

On Mon, Aug 24, 2015 at 12:45:15PM +, Antti Kantee wrote:

I started work on this again by doing a web search for the error, and found
this:
http://bugs.musicpd.org/view.php?id=4110

There you argue that using constexpr for PTHREAD_MUTEX_INITIALIZER is
broken.  So if we assume that you are correct, shouldn't we remove the
constexprs from libc++/dist/libcxx/include/__mutex_base?


Except that the standard requires it.


So are you now saying that that what you said in the above URL is wrong 
and that the NetBSD headers are broken?  Let me quote the relevant part 
of what you said:


Nothing in ISO C11, ISO C++11 or POSIX requires 
PTHREAD_MUTEX_INITIALIZER to qualify for constexpr.


But now you are saying that the standard does require 
PTHREAD_MUTEX_INITIALIZER to qualify for constexpr.


Feel free to reply in a fashion where you actually explain your stance 
instead of just dropping some facts.  Otherwise I'll have to guess what 
you mean, and that prevents me from fixing things the right way.

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On 24/08/15 14:36, Joerg Sonnenberger wrote:

On Mon, Aug 24, 2015 at 02:20:55PM +, Antti Kantee wrote:

On 24/08/15 13:26, Joerg Sonnenberger wrote:

On Mon, Aug 24, 2015 at 12:45:15PM +, Antti Kantee wrote:

I started work on this again by doing a web search for the error, and found
this:
http://bugs.musicpd.org/view.php?id=4110

There you argue that using constexpr for PTHREAD_MUTEX_INITIALIZER is
broken.  So if we assume that you are correct, shouldn't we remove the
constexprs from libc++/dist/libcxx/include/__mutex_base?


Except that the standard requires it.


So are you now saying that that what you said in the above URL is wrong and
that the NetBSD headers are broken?  Let me quote the relevant part of what
you said:


Stop inventing things, please.


A simple yes/no[, because ...] is more effective than evading the 
question.  I'm still not sure which one it is.



Nothing in ISO C11, ISO C++11 or POSIX requires PTHREAD_MUTEX_INITIALIZER
to qualify for constexpr.

But now you are saying that the standard does require
PTHREAD_MUTEX_INITIALIZER to qualify for constexpr.


A mutex has to be constexpr constructable. That's in the standard. How a
mutex is implemented is not. Nothing in mpd requires the constexpr
property, so requiring it just makes things more difficult for no good
reason.


What I understand from that response: the same code is wrong in mpd (and 
presumably every other application) but right in libc++; it's 
standard-compliant to implement mutex in a way which does not compile.

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Mon, Aug 24, 2015 at 03:03:28PM +, Antti Kantee wrote:
 On 24/08/15 14:36, Joerg Sonnenberger wrote:
 On Mon, Aug 24, 2015 at 02:20:55PM +, Antti Kantee wrote:
 On 24/08/15 13:26, Joerg Sonnenberger wrote:
 On Mon, Aug 24, 2015 at 12:45:15PM +, Antti Kantee wrote:
 I started work on this again by doing a web search for the error, and 
 found
 this:
 http://bugs.musicpd.org/view.php?id=4110
 
 There you argue that using constexpr for PTHREAD_MUTEX_INITIALIZER is
 broken.  So if we assume that you are correct, shouldn't we remove the
 constexprs from libc++/dist/libcxx/include/__mutex_base?
 
 Except that the standard requires it.
 
 So are you now saying that that what you said in the above URL is wrong and
 that the NetBSD headers are broken?  Let me quote the relevant part of what
 you said:
 
 Stop inventing things, please.
 
 A simple yes/no[, because ...] is more effective than evading the
 question.  I'm still not sure which one it is.
 
 Nothing in ISO C11, ISO C++11 or POSIX requires PTHREAD_MUTEX_INITIALIZER
 to qualify for constexpr.
 
 But now you are saying that the standard does require
 PTHREAD_MUTEX_INITIALIZER to qualify for constexpr.
 
 A mutex has to be constexpr constructable. That's in the standard. How a
 mutex is implemented is not. Nothing in mpd requires the constexpr
 property, so requiring it just makes things more difficult for no good
 reason.
 
 What I understand from that response: the same code is wrong in mpd (and
 presumably every other application) but right in libc++; it's
 standard-compliant to implement mutex in a way which does not compile.

Whether or not all the rules apply to system headers is a question
outside the scope of what I am willing to discuss. If you build a system
with strange flags that explicitly disable the normal system header
logic, you are on your own.

The code in mpd depends on the language DR to be resolved without having
a good reason to do so. Because I don't trust a compiler to behave
certainly doesn't qualify.

There are two approaches to implement a mutex in a standard-compliant
way with a PTHREAD_MUTEX_INITIALIZER using volatile elements:
(1) Assume that the compiler has a fix for the DR.
(2) Defer the actual mutex setup until a later point.

Clang implements the DR behavior in so far as skipping the check for
system headers. I have no idea what GCC is doing. The second option
comes with a significant penalty and simple is not acceptable.

Joerg

Re: CVS commit: src/lib/libpthread

[Martin Husemann]

On Fri, Jun 26, 2015 at 01:38:45AM +, Antti Kantee wrote:
 On Fri, Jun 26, 2015 at 01:33:09AM +, Antti Kantee wrote:
  Module Name:src
  Committed By:   pooka
  Date:   Fri Jun 26 01:33:09 UTC 2015
  
  Modified Files:
  src/lib/libpthread: pthread_types.h
  
  Log Message:
  C++ (namely libc++) expects to be using PTHREAD_FOO_INITIALIZER as a
  member initializer. This does not work for volatile types. Since C++
  does not touch the guts of those types, redefine them as non-volatile.
  
  Fixes libc++ compilation with g++ 5.1, as reported in PR lib/49989.
 
 Forgot to mention: the patch is from christos (thanks!)

Please document this in doc/HACKS, it is an evil hack and IMHO working
around a g++ bug.

Martin

Re: CVS commit: src/lib/libpthread

[Nick Hudson]

On 06/26/15 02:33, Antti Kantee wrote:

+# ifdef __CPU_SIMPLE_LOCK_PAD
+#  define __pthread_spin_t unsigned char
+# else
+#  define __pthread_spin_t unsigned int
+# endif


Are you sure this works for hppa which has funky __cpu_simple_lock_t?

Nick

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On 26/06/15 06:02, Martin Husemann wrote:

On Fri, Jun 26, 2015 at 01:38:45AM +, Antti Kantee wrote:

On Fri, Jun 26, 2015 at 01:33:09AM +, Antti Kantee wrote:

Module Name:src
Committed By:   pooka
Date:   Fri Jun 26 01:33:09 UTC 2015

Modified Files:
src/lib/libpthread: pthread_types.h

Log Message:
C++ (namely libc++) expects to be using PTHREAD_FOO_INITIALIZER as a
member initializer. This does not work for volatile types. Since C++
does not touch the guts of those types, redefine them as non-volatile.

Fixes libc++ compilation with g++ 5.1, as reported in PR lib/49989.


Forgot to mention: the patch is from christos (thanks!)


Please document this in doc/HACKS, it is an evil hack and IMHO working
around a g++ bug.


Done, rev 1.160 of doc/HACKS.

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

This is the simplest program to repeat the problem with g++ 5.1 and 
NetBSD's pthread_types.h:


#include pthread.h
class foo {
pthread_mutex_t m;
public:
constexpr foo() : m(PTHREAD_MUTEX_INITIALIZER) {}
};

(plus or minus typos since I typed it by hand from the virtual machine 
console)


If there's some way to say pthread_types.h is even more of a system 
header than with -isystem and compilers should not generate errors, 
then I'd like to hear it.


Also, if this is really the correct DR, I'm not sure how a flaw in the 
standard translates to gcc bug

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1911
(especially when there's no fix for the defect in the standard)

So, I really don't see how to sweep the issue under the rug except by a) 
figuring out a workaround b) labeling all C++ compilers which happen to 
implement the current standard as incompatible with NetBSD.

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Fri, Jun 26, 2015 at 12:07:36PM +, Antti Kantee wrote:
 On 26/06/15 11:53, Joerg Sonnenberger wrote:
 That said, have
 you verified why it doesn't happen with libstdc++ itself? I would
 somewhat suspect that the threatment of the header as system header
 hides the problem for libstdc++, I can't imagine that it can correctly
 implement the constexpr constructor without performance penalties
 otherwise...
 
 I don't speak C++, but a grep-based guess coupled with common sense suggests
 that libstdc++ is fine because it doesn't use PTHREAD_MUTEX_INITIALIZER.

It is spelled __GTHREAD_MUTEX_INIT or something like that and goes via
another layer of indirection. Without it, the constexpr version would
need another flag bit, see above about runtime penalty.

Joerg

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Fri, Jun 26, 2015 at 12:49:09PM +, Antti Kantee wrote:
 This is the simplest program to repeat the problem with g++ 5.1 and NetBSD's
 pthread_types.h:
 
 #include pthread.h
 class foo {
   pthread_mutex_t m;
 public:
   constexpr foo() : m(PTHREAD_MUTEX_INITIALIZER) {}
 };
 
 (plus or minus typos since I typed it by hand from the virtual machine
 console)
 
 If there's some way to say pthread_types.h is even more of a system header
 than with -isystem and compilers should not generate errors, then I'd like
 to hear it.

You don't understand. The system header is not pthread_types.h, it is
mutex.

 Also, if this is really the correct DR, I'm not sure how a flaw in the
 standard translates to gcc bug
 http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1911
 (especially when there's no fix for the defect in the standard)

The fix is to apply the C++14 rules, which is more relaxed.

 So, I really don't see how to sweep the issue under the rug except by a)
 figuring out a workaround b) labeling all C++ compilers which happen to
 implement the current standard as incompatible with NetBSD.

DRs are retroactive, e.g. bug fixes and corrections. As such they are
part of the current standard (and of course, it is not the current
standard, since that would be C++14).

Joerg

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On 26/06/15 07:04, Nick Hudson wrote:

On 06/26/15 02:33, Antti Kantee wrote:

+# ifdef __CPU_SIMPLE_LOCK_PAD
+#  define __pthread_spin_t unsigned char
+# else
+#  define __pthread_spin_t unsigned int
+# endif


Are you sure this works for hppa which has funky __cpu_simple_lock_t?


You're right, it doesn't, I should've checked more carefully before 
commit.  I improved the definition to not depend on 
__CPU_SIMPLE_LOCK_PAD and use sizeof+alignof instead.  That will keep 
all ports happy, regardless of how they define __cpu_simple_lock_t and 
assume it to be aligned.


Thanks for catching it.

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On 26/06/15 09:14, Joerg Sonnenberger wrote:

On Fri, Jun 26, 2015 at 01:33:09AM +, Antti Kantee wrote:

Module Name:src
Committed By:   pooka
Date:   Fri Jun 26 01:33:09 UTC 2015

Modified Files:
src/lib/libpthread: pthread_types.h

Log Message:
C++ (namely libc++) expects to be using PTHREAD_FOO_INITIALIZER as a
member initializer. This does not work for volatile types. Since C++
does not touch the guts of those types, redefine them as non-volatile.

Fixes libc++ compilation with g++ 5.1, as reported in PR lib/49989.


Please revert this. It is ugly, it breaks the abstraction of the system
headers and it is a hackaround for a GCC bug. This should be handled by
GCC and only by GCC. The relevant DR against C++11 is core issue #1911.


As long as NetBSD's goals include interoperates well with other 
systems and conforms to open systems standards as much as is 
practical, the workaround is mandated.  It fixes source level 
interoperation with a contemporary release of a major compiler (fact) 
and given that it's a few lines to a single file, it's practical 
(subjective).  Of course, I'd *like* to go back in time to fix all bugs, 
but I'm not high on hopes with that being possible.

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Fri, Jun 26, 2015 at 01:33:09AM +, Antti Kantee wrote:
 Module Name:  src
 Committed By: pooka
 Date: Fri Jun 26 01:33:09 UTC 2015
 
 Modified Files:
   src/lib/libpthread: pthread_types.h
 
 Log Message:
 C++ (namely libc++) expects to be using PTHREAD_FOO_INITIALIZER as a
 member initializer. This does not work for volatile types. Since C++
 does not touch the guts of those types, redefine them as non-volatile.
 
 Fixes libc++ compilation with g++ 5.1, as reported in PR lib/49989.

Please revert this. It is ugly, it breaks the abstraction of the system
headers and it is a hackaround for a GCC bug. This should be handled by
GCC and only by GCC. The relevant DR against C++11 is core issue #1911.

Joerg

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On 26/06/15 07:04, Nick Hudson wrote:

On 06/26/15 02:33, Antti Kantee wrote:

+# ifdef __CPU_SIMPLE_LOCK_PAD
+#  define __pthread_spin_t unsigned char
+# else
+#  define __pthread_spin_t unsigned int
+# endif


Are you sure this works for hppa which has funky __cpu_simple_lock_t?


Slightly related.  arch/arm has:

/*
 * This should have always been an 8-bit type, but since it's been exposed
 * to user-space, we don't want ABI breakage there.
 */
#if defined(_KERNEL)
typedef volatile unsigned char  __cpu_simple_lock_t;
#else
typedef volatile int__cpu_simple_lock_t;
#endif /* _KERNEL */


Should that be no ifdef  __CPU_SIMPLE_LOCK_PAD like everywhere else?

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On 26/06/15 11:53, Joerg Sonnenberger wrote:

That said, have
you verified why it doesn't happen with libstdc++ itself? I would
somewhat suspect that the threatment of the header as system header
hides the problem for libstdc++, I can't imagine that it can correctly
implement the constexpr constructor without performance penalties
otherwise...


I don't speak C++, but a grep-based guess coupled with common sense 
suggests that libstdc++ is fine because it doesn't use 
PTHREAD_MUTEX_INITIALIZER.  Not that you can use common sense to reason 
about C++, but anyway.  Also, most likely nobody has tried compiling 
libstdc++ for NetBSD with the g++ 5 series, and therefore we don't know 
if it fails or not.  Finally, I don't understand why this would be an 
issue with just libstdc++/libc++ as opposed to any C++ compiled code 
which uses PTHREAD_X_INITIALIZER (lack of understanding again based on 
common sense instead of C++ sense).

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Fri, Jun 26, 2015 at 10:14:41AM +, Antti Kantee wrote:
 As long as NetBSD's goals include interoperates well with other systems
 and conforms to open systems standards as much as is practical, the
 workaround is mandated.  It fixes source level interoperation with a
 contemporary release of a major compiler (fact) and given that it's a few
 lines to a single file, it's practical (subjective).  Of course, I'd *like*
 to go back in time to fix all bugs, but I'm not high on hopes with that
 being possible.

We generally don't workaround standard compliance bugs in compilers,
especially .0 releases with a history of being buggy. That said, have
you verified why it doesn't happen with libstdc++ itself? I would
somewhat suspect that the threatment of the header as system header
hides the problem for libstdc++, I can't imagine that it can correctly
implement the constexpr constructor without performance penalties
otherwise...

Joerg

Re: CVS commit: src/lib/libpthread

[Antti Kantee]

On Fri, Jun 26, 2015 at 01:33:09AM +, Antti Kantee wrote:
 Module Name:  src
 Committed By: pooka
 Date: Fri Jun 26 01:33:09 UTC 2015
 
 Modified Files:
   src/lib/libpthread: pthread_types.h
 
 Log Message:
 C++ (namely libc++) expects to be using PTHREAD_FOO_INITIALIZER as a
 member initializer. This does not work for volatile types. Since C++
 does not touch the guts of those types, redefine them as non-volatile.
 
 Fixes libc++ compilation with g++ 5.1, as reported in PR lib/49989.

Forgot to mention: the patch is from christos (thanks!)

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

In article 20141217014908.359b...@cvs.netbsd.org,
Antti Kantee source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  pooka
Date:  Wed Dec 17 01:49:08 UTC 2014

Modified Files:
   src/lib/libpthread: pthread_makelwp_netbsd.c

Log Message:
include correct header for last minute just-in-case defensive addition
that's too trivial to check

Please rename the file pthread_makelwp.c; calling a file _netbsd.c in
a NetBSD tree for a NetBSD implementation is bogus.

christos

Re: CVS commit: src/lib/libpthread

[Izumi Tsutsui]

Will this workaround fix for PR lib/47703 be pulled to 6.1 release?
Does releng have any plan for all libpthread issue (including dlopen one)?

IMO ticket #722 (pthread_condattr_setclock(3) addition) should be
reverted before 6.1 if it requires syscall bump for the real fix.

Furthermore, was it safe to add a new pthread_condattr_setclock(3) function
without libpthread minor bump?  I'm afraid pkgsrc ruby193 (and other)
binaries built on 6.1 silently fails on 6.0.1.

Currently ruby193 built on 6.0.1 works but fails on ruby193 built
on netbsd-6 because the former doesn't use pthread_condattr_setclock(3).
---
Izumi Tsutsui

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Apr 1,  9:18pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

| Will this workaround fix for PR lib/47703 be pulled to 6.1 release?

Yes,

| Does releng have any plan for all libpthread issue (including dlopen one)?

This is an open question to core.

| IMO ticket #722 (pthread_condattr_setclock(3) addition) should be
| reverted before 6.1 if it requires syscall bump for the real fix.

It does not.

| Furthermore, was it safe to add a new pthread_condattr_setclock(3) function
| without libpthread minor bump?  I'm afraid pkgsrc ruby193 (and other)
| binaries built on 6.1 silently fails on 6.0.1.

It was a mistake, we should have bumped.

| Currently ruby193 built on 6.0.1 works but fails on ruby193 built
| on netbsd-6 because the former doesn't use pthread_condattr_setclock(3).

Right.

christos

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

In article 20130401132606.8cc2997...@rebar.astron.com,
Christos Zoulas chris...@zoulas.com wrote:

| Furthermore, was it safe to add a new pthread_condattr_setclock(3) function
| without libpthread minor bump?  I'm afraid pkgsrc ruby193 (and other)
| binaries built on 6.1 silently fails on 6.0.1.

It was a mistake, we should have bumped.

Note, that bumping would have just changed the nature of the failure, would
not have fixed the problem.

christos

Re: CVS commit: src/lib/libpthread

[Izumi Tsutsui]

 | Furthermore, was it safe to add a new pthread_condattr_setclock(3) function
 | without libpthread minor bump?  I'm afraid pkgsrc ruby193 (and other)
 | binaries built on 6.1 silently fails on 6.0.1.
 
 It was a mistake, we should have bumped.
 
 Note, that bumping would have just changed the nature of the failure, would
 not have fixed the problem.

Is it generally acceptable to bump minor
(i.e. adding new public functions in libs)
in release branches?

I'm afraid not all third party programs use proper configure script.

---
Izumi Tsutsui

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Apr 1, 11:43pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

|  | Furthermore, was it safe to add a new pthread_condattr_setclock(3) 
function
|  | without libpthread minor bump?  I'm afraid pkgsrc ruby193 (and other)
|  | binaries built on 6.1 silently fails on 6.0.1.
|  
|  It was a mistake, we should have bumped.
|  
|  Note, that bumping would have just changed the nature of the failure, would
|  not have fixed the problem.
| 
| Is it generally acceptable to bump minor
| (i.e. adding new public functions in libs)
| in release branches?

It depends on where we build 3rd party binaries. If we build on
the earliest branch there is no problem. But if we build in the
latest one there is. We should write the rules down, so that we
don't repeat the same problem on each release. Perhaps the best
solution is to add the missing function on all branches, but that
does not help those who did not upgrade.

| I'm afraid not all third party programs use proper configure script.

That can always cause issues.

Re: CVS commit: src/lib/libpthread

[Izumi Tsutsui]

 |  | Furthermore, was it safe to add a new pthread_condattr_setclock(3) 
 function
 |  | without libpthread minor bump?  I'm afraid pkgsrc ruby193 (and other)
 |  | binaries built on 6.1 silently fails on 6.0.1.
 |  
 |  It was a mistake, we should have bumped.
 |  
 |  Note, that bumping would have just changed the nature of the failure, 
 would
 |  not have fixed the problem.
 | 
 | Is it generally acceptable to bump minor
 | (i.e. adding new public functions in libs)
 | in release branches?
 
 It depends on where we build 3rd party binaries. If we build on
 the earliest branch there is no problem. But if we build in the
 latest one there is. We should write the rules down, so that we
 don't repeat the same problem on each release. Perhaps the best
 solution is to add the missing function on all branches, but that
 does not help those who did not upgrade.

Can't we simply revert ticket #722 (pthread_condattr_setclock(3) addition)?
Is that function really necessary for 6.1?

---
Izumi Tsutsui

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Apr 2,  1:22am, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

|  It depends on where we build 3rd party binaries. If we build on
|  the earliest branch there is no problem. But if we build in the
|  latest one there is. We should write the rules down, so that we
|  don't repeat the same problem on each release. Perhaps the best
|  solution is to add the missing function on all branches, but that
|  does not help those who did not upgrade.
| 
| Can't we simply revert ticket #722 (pthread_condattr_setclock(3) addition)?
| Is that function really necessary for 6.1?

I think it is too late now.

christos

Re: CVS commit: src/lib/libpthread

[Izumi Tsutsui]

 | Can't we simply revert ticket #722 (pthread_condattr_setclock(3) addition)?
 | Is that function really necessary for 6.1?
 
 I think it is too late now.

Hmm. If core and releng think so, I have no further comment.
(but I'm afraid we need 6.1_RC4 anyway since RC3 doesn't include pthread fixes)

---
Izumi Tsutsui

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Apr 2,  1:46am, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

|  I think it is too late now.
| 
| Hmm. If core and releng think so, I have no further comment.
| (but I'm afraid we need 6.1_RC4 anyway since RC3 doesn't include pthread 
fixes)

I don't know if it is too late, I think it is too late. Releng knows best.

christos

Re: CVS commit: src/lib/libpthread

[YAMAMOTO Takashi]

 In article 20130329032804.23b1514a...@mail.netbsd.org,
 YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote:

mono escaping its scope?
 
 Yes, I already added it to lwp_park to avoid the hack but not using
 the syscall yet until we decide what we need to pass to lwp_park
 to avoid priority inversion and fix the races in pthread_cond_*.
 
 christos

i meant the following code.
struct timespec mono will be used after the end of the block.
is it safe?

if (pthread_cond_getclock(cond) == CLOCK_MONOTONIC) {
struct timespec mono, real;
if (clock_gettime(CLOCK_REALTIME, real) == -1 ||
clock_gettime(CLOCK_MONOTONIC, mono) == -1)
return errno;
timespecsub(abstime, mono, mono);
timespecadd(mono, real, mono);
abstime = mono;
}

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

In article 20130329032804.23b1514a...@mail.netbsd.org,
YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote:

mono escaping its scope?

Yes, I already added it to lwp_park to avoid the hack but not using
the syscall yet until we decide what we need to pass to lwp_park
to avoid priority inversion and fix the races in pthread_cond_*.

christos

Re: CVS commit: src/lib/libpthread

[YAMAMOTO Takashi]

 Module Name:  src
 Committed By: christos
 Date: Thu Mar 28 18:07:12 UTC 2013
 
 Modified Files:
   src/lib/libpthread: pthread_cond.c
 
 Log Message:
 PR/47703: Yasushi Oshima: pthread_cond_timedwait() does not wait
 after call pthread_condattr_setclock(CLOCK_MONOTONIC)
 
 _lwp_park(2) expects a realtime clock, and it gets passed a monotonic
 one.  Since monotonic  real, it never sleeps. This patch adjusts
 the monotonic clock to be a real one before it passes is to
 _lwp_park(2). This is the minimal hacky fix and it will be fixed
 properly in _lwp_park(2) in the future.
 
 XXX: pullup to 6.

mono escaping its scope?

YAMAMOTO Takashi

 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.59 -r1.60 src/lib/libpthread/pthread_cond.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Re: CVS commit: src/lib/libpthread

[Martin Husemann]

On Wed, Nov 21, 2012 at 07:35:29PM -0500, Christos Zoulas wrote:
 Ok, that's simple to change. But where is the documentation for how this
 is supposed to work, so I can put a cross-reference to it.

http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_key_delete.html

  It is the responsibility of the application to free any application
  storage or perform any cleanup actions for data structures related to
  the deleted key or associated thread-specific data in any threads; this
  cleanup can be done either before or after pthread_key_delete() is
  called. Any attempt to use key following the call to
  pthread_key_delete() results in undefined behavior.

and later in the Rationale:

  No such cleanup is done by pthread_key_delete(). In particular,
  destructor functions are not called.

Martin

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Nov 22,  9:14am, mar...@duskware.de (Martin Husemann) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

| On Wed, Nov 21, 2012 at 07:35:29PM -0500, Christos Zoulas wrote:
|  Ok, that's simple to change. But where is the documentation for how this
|  is supposed to work, so I can put a cross-reference to it.
| 
| 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_key_delete.html
| 
|   It is the responsibility of the application to free any application
|   storage or perform any cleanup actions for data structures related to
|   the deleted key or associated thread-specific data in any threads; this
|   cleanup can be done either before or after pthread_key_delete() is
|   called. Any attempt to use key following the call to
|   pthread_key_delete() results in undefined behavior.
| 
| and later in the Rationale:
| 
|   No such cleanup is done by pthread_key_delete(). In particular,
|   destructor functions are not called.

Fixed, thanks!

christos

Re: CVS commit: src/lib/libpthread

[YAMAMOTO Takashi]

hi,

 Module Name:  src
 Committed By: christos
 Date: Wed Nov 21 19:19:25 UTC 2012
 
 Modified Files:
   src/lib/libpthread: pthread_int.h pthread_specific.c pthread_tsd.c
 
 Log Message:
 Replace the simple implementation of pthread_key_{create,destroy}
 and pthread_{g,s}etspecific functions, to one that invalidates
 values of keys in other threads when pthread_key_delete() is called.
 This fixes chromium, which expects pthread_key_delete() to do
 cleanup in all threads.

i think pthread_key_delete should not call dtor.

YAMAMOTO Takashi

 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.87 -r1.88 src/lib/libpthread/pthread_int.h
 cvs rdiff -u -r1.23 -r1.24 src/lib/libpthread/pthread_specific.c
 cvs rdiff -u -r1.8 -r1.9 src/lib/libpthread/pthread_tsd.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Nov 21, 10:51pm, y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

|  Replace the simple implementation of pthread_key_{create,destroy}
|  and pthread_{g,s}etspecific functions, to one that invalidates
|  values of keys in other threads when pthread_key_delete() is called.
|  This fixes chromium, which expects pthread_key_delete() to do
|  cleanup in all threads.
| 
| i think pthread_key_delete should not call dtor.

Ok, that's simple to change. But where is the documentation for how this
is supposed to work, so I can put a cross-reference to it.

christos

Re: CVS commit: src/lib/libpthread

[David Laight]

On Wed, Sep 12, 2012 at 02:55:48PM +, Matt Thomas wrote:
 Module Name:  src
 Committed By: matt
 Date: Wed Sep 12 14:55:48 UTC 2012
 
 Modified Files:
   src/lib/libpthread: pthread_specific.c
 
 Log Message:
 Only copy the ucontext_t in pthread_setcontext if _UC_TLSBASE is set.
 Conditionalize the test on _UC_TLSBASE being defined.

Why not just define it to be zero if it isn't needed ?

David

-- 
David Laight: da...@l8s.co.uk

Re: CVS commit: src/lib/libpthread

[Matt Thomas]

On Sep 12, 2012, at 9:45 AM, David Laight wrote:

 On Wed, Sep 12, 2012 at 02:55:48PM +, Matt Thomas wrote:
 Module Name: src
 Committed By:matt
 Date:Wed Sep 12 14:55:48 UTC 2012
 
 Modified Files:
  src/lib/libpthread: pthread_specific.c
 
 Log Message:
 Only copy the ucontext_t in pthread_setcontext if _UC_TLSBASE is set.
 Conditionalize the test on _UC_TLSBASE being defined.
 
 Why not just define it to be zero if it isn't needed ?

lint will complain. :(

Re: CVS commit: src/lib/libpthread

[Joerg Sonnenberger]

On Sat, Mar 10, 2012 at 06:01:10PM +, Joerg Sonnenberger wrote:
 cvs rdiff -u -r1.23 -r1.24 src/lib/libpthread/sem.c

This part wasn't originally intended, so I changed the commit message to
Fix error handling. It doesn't hurt either.

Joerg

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Aug 6, 12:26pm, m.droch...@fz-juelich.de (Matthias Drochner) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

| 
|  Module Name:src
|  Committed By:   christos
|  Date:   Fri Aug 6 05:25:02 UTC 2010
| 
|  Modified Files:
|  src/lib/libpthread: pthread.h pthread_attr.c
| 
|  Log Message: Add pthread_getattr_np()
| 
| I think this needs a weak_alias for pthread_attr_get_np().

They are different and pthread_attr_get_np() is not weak...

christos

Re: CVS commit: src/lib/libpthread

[Christos Zoulas]

On Aug 6, 12:59pm, m.droch...@fz-juelich.de (Matthias Drochner) wrote:
-- Subject: Re: CVS commit: src/lib/libpthread

| chris...@zoulas.com said:
|  They are different and pthread_attr_get_np() is not weak...
| 
| But pthread_attr_get_np() is now used internally.
| The rule is that functions which are used internally
| and not part of standard C need to be protected.
| 
| best regards
| Matthias
| 

Thanks. I had forgotten about that.

christos

Re: CVS commit: src/lib/libpthread

[David Holland]

On Tue, May 18, 2010 at 11:04:12AM +0300, Jukka Ruohonen wrote:
  In my opinion we could have more introductory sections that connect the
  pages in a more logical manner.[1] Just listing the manual pages in the
  intro-sections is not enough.
  
  For example [...]
  
  Another option could be a book-like index, which I already mentioned on
  d...@.

These things serve different purposes and ISTM that we should try to
establish both.

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/lib/libpthread

[Jukka Ruohonen]

On Mon, May 17, 2010 at 07:35:37PM +0200, Matthias Drochner wrote:
 Also, the xrefs are not sorted strictly alphabetically but
 in some logical order which is good imo.

The Xrefs are generally a symptom of the manual page format. As D. Holland
well said, the man -k is so 1980s.

In my opinion we could have more introductory sections that connect the
pages in a more logical manner.[1] Just listing the manual pages in the
intro-sections is not enough.

For example, I think memoryallocators(9) and atomic_ops(3) are good
examples. The latter highlights also nicely and briefly the rationale, which
is often entirely missing in the man-pages. (Often the question particularly
in the kernel is: why? Not how.)

So in the long run we could have e.g. libc(3) or kernel(9).

Another option could be a book-like index, which I already mentioned on
d...@.

- Jukka.

[1] It would be interesting to use e.g. Graphviz to see what kind of a
spider web the Xrefs really are.

Re: CVS commit: src/lib/libpthread

[Marc Balmer]

Am 16.05.10 14:23, schrieb Jukka Ruohonen:
 Module Name:  src
 Committed By: jruoho
 Date: Sun May 16 12:23:32 UTC 2010
 
 Modified Files:
   src/lib/libpthread: pthread.3
 
 Log Message:
 Add the Butenhof's book to SEE ALSO. (It was decent enough when I read it
 years ago, but if there are better ones, please feel free to add those.)

Is the SEE ALSO section meant to refer to external sources?  I always
thought it was only to reference other manpages.

Butenhof's book is a must read for serious pthread users, of course...

 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.13 -r1.14 src/lib/libpthread/pthread.3
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.
 
 
 
 
 Modified files:
 
 Index: src/lib/libpthread/pthread.3
 diff -u src/lib/libpthread/pthread.3:1.13 src/lib/libpthread/pthread.3:1.14
 --- src/lib/libpthread/pthread.3:1.13 Sun May 16 12:20:00 2010
 +++ src/lib/libpthread/pthread.3  Sun May 16 12:23:32 2010
 @@ -1,4 +1,4 @@
 -.\  $NetBSD: pthread.3,v 1.13 2010/05/16 12:20:00 jruoho Exp $
 +.\  $NetBSD: pthread.3,v 1.14 2010/05/16 12:23:32 jruoho Exp $
  .\
  .\ Copyright (c) 2003, 2007, 2009 The NetBSD Foundation, Inc.
  .\ All rights reserved.
 @@ -156,6 +156,13 @@
  for
  .Xr sh 1 ) .
  .El
 +.Sh SEE ALSO
 +.Rs
 +.%A David R. Butenhof
 +.%T Programming with POSIX(R) Threads
 +.%D 1997
 +.%I Addison-Wesley
 +.Re
  .Sh STANDARDS
  The
  .Nm
 

Re: CVS commit: src/lib/libpthread

[Jukka Ruohonen]

On Mon, May 17, 2010 at 09:21:18AM +0200, Marc Balmer wrote:
 Is the SEE ALSO section meant to refer to external sources?  I always
 thought it was only to reference other manpages.

I don't know if there has ever been a strict rule about this; even some of
the really old manual pages refer to external sources.

As manual pages are kind of bulletins or snapshots, I believe references to
good books or other useful material may be useful to someone. These also
signal a message that we don't try to document it all here; if you are
interested, there is better and more detailed documentation elsewhere.

- Jukka.

Re: CVS commit: src/lib/libpthread

[Matthias Drochner]

 Is the SEE ALSO section meant to refer to external sources?

Just as a data point, OSF/1 does this.
To cite eg inet(7):

RELATED INFORMATION
  Functions: ioctl(2), socket(2).
  Network Information: netintro(7), tcp(7), udp(7), ip(7), icmp(7).
  Network Programmer's Guide
  Technical Overview
  RFC 2373, IP Version 6 Addressing Architecture, July 1998

Or pthread_create(3);

RELATED INFORMATION
  Functions: pthread_atfork(3), pthread_attr_destroy(3),
  pthread_attr_init(3), pthread_attr_setdetachstate(3),
  pthread_attr_setinheritsched(3), pthread_attr_setschedparam(3),
  pthread_attr_setschedpolicy(3), pthread_attr_setstacksize(3),
  pthread_cancel(3), pthread_detach(3), pthread_exit(3), pthread_join(3)
  Manuals: Guide to DECthreads and Programmer's Guide

Also, the xrefs are not sorted strictly alphabetically but
in some logical order which is good imo.

best regards
Matthias





Forschungszentrum Juelich GmbH
52425 Juelich
Sitz der Gesellschaft: Juelich
Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498
Vorsitzende des Aufsichtsrats: MinDir'in Baerbel Brumme-Bothe
Geschaeftsfuehrung: Prof. Dr. Achim Bachem (Vorsitzender),
Dr. Ulrich Krafft (stellv. Vorsitzender), Prof. Dr.-Ing. Harald Bolt,
Prof. Dr. Sebastian M. Schmidt

Re: CVS commit: src/lib/libpthread

[Michael Graff]

On 3/24/10 7:32 PM, enami tsugutomo wrote:

 The _lwp_ctl() call also need to be called with self-pt_lwpctl
 doesn't it?

I thought pthread__first would be the same as pthread__self() here, but
when I just used pthread__first rather than calling pthread__self() I
got a different value.

I don't know if this means pthread__first() is no longer correct either
and needs to be reset, or I had some other bug in this very simple change.

--Michael

 
 enami.
 
 @@ -235,11 +235,14 @@
  static void
  pthread__fork_callback(void)
  {
 +   struct __pthread_st *self;
  
 /* lwpctl state is not copied across fork. */
 if (_lwp_ctl(LWPCTL_FEATURE_CURCPU, pthread__first-pt_lwpctl)) {
 err(1, _lwp_ctl);
 }
 +   self = pthread__self();
 +   self-pt_lid = _lwp_self();
  }
  
  static void
 

Re: CVS commit: src/lib/libpthread

[enami tsugutomo]

 Modified Files:
   src/lib/libpthread: pthread.c
 
 Log Message:
 fix the pthread pt_lid in the fork callback function that runs in the child=
  instead of a function that may be going away.  KNFify
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.114 -r1.115 src/lib/libpthread/pthread.c

The _lwp_ctl() call also need to be called with self-pt_lwpctl
doesn't it?

enami.

@@ -235,11 +235,14 @@
 static void
 pthread__fork_callback(void)
 {
+   struct __pthread_st *self;
 
/* lwpctl state is not copied across fork. */
if (_lwp_ctl(LWPCTL_FEATURE_CURCPU, pthread__first-pt_lwpctl)) {
err(1, _lwp_ctl);
}
+   self = pthread__self();
+   self-pt_lid = _lwp_self();
 }
 
 static void