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 <n54%gmx.com@localhost> 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 <sys/cdefs.h>
>> -__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 <sys/types.h>
>> #include <sys/lwpctl.h>
>> @@ -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(&attr) != 0) {
> return true;
>
> }
> pthread_mutexattr_settype(&attr, MALLOC_MUTEX_TYPE);
> if (pthread_mutex_init(&mutex->lock, &attr) != 0) {
> pthread_mutexattr_destroy(&attr);
> return true;
> }
> pthread_mutexattr_destroy(&attr);
>
>
> 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 0x0000000000000246 in ?? ()
> #2 0x00007f7fffffe600 in ?? ()
> #3 0x00007f7ff70000d0 in ?? ()
> #4 0x00007f7ff72d94a0 in je_malloc_mutex_init (mutex=0x7f7fffffe600,
> mutex@entry=0x7f7ff70000d0, 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 0x00007f7ff731834f in je_base_new (tsdn=tsdn@entry=0x0,
> ind=ind@entry=0, extent_hooks=0x7f7ff75ca120 <je_extent_hooks_default>)
> at /usr/src/external/bsd/jemalloc/lib/../dist/src/base.c:366
> #6 0x00007f7ff73185d2 in je_base_boot (tsdn=tsdn@entry=0x0) at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/base.c:512
> #7 0x00007f7ff731011d in malloc_init_hard_a0_locked () at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1327
> #8 0x00007f7ff73107f5 in malloc_init_hard () at
> /usr/src/external/bsd/jemalloc/lib/../dist/src/jemalloc.c:1549
> #9 0x00007f7ff723f924 in ?? () from /usr/lib/libc.so.12
> #10 0x00007f7ff7ef9800 in ?? ()
> #11 0x00007f7ff723ac09 in _init () from /usr/lib/libc.so.12
> #12 0x0000000000000000 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...
>
>
> <sanitizer.log>
Attachment:
signature.asc
Description: Message signed with OpenPGP