svn commit: r343566 - in head/lib/libthr: . thread
Michal Meloun
meloun.michal at gmail.com
Mon Feb 4 08:10:20 UTC 2019
On 04.02.2019 8:00, Konstantin Belousov wrote:
> On Mon, Feb 04, 2019 at 07:31:19AM +0100, Michal Meloun wrote:
>> On 29.01.2019 23:46, Konstantin Belousov wrote:
>>> Author: kib
>>> Date: Tue Jan 29 22:46:44 2019
>>> New Revision: 343566
>>> URL: https://svnweb.freebsd.org/changeset/base/343566
>>>
>>> Log:
>>> Untangle jemalloc and mutexes initialization.
>>>
>>> The need to use libc malloc(3) from some places in libthr always
>>> caused issues. For instance, per-thread key allocation was switched to
>>> use plain mmap(2) to get storage, because some third party mallocs
>>> used keys for implementation of calloc(3).
>>>
>>> Even more important, libthr calls calloc(3) during initialization of
>>> pthread mutexes, and jemalloc uses pthread mutexes. Jemalloc provides
>>> some way to both postpone the initialization, and to make
>>> initialization to use specialized allocator, but this is very fragile
>>> and often breaks. See the referenced PR for another example.
>>>
>>> Add the small malloc implementation used by rtld, to libthr. Use it in
>>> thr_spec.c and for mutexes initialization. This avoids the issues with
>>> mutual dependencies between malloc and libthr in principle. The
>>> drawback is that some more allocations are not interceptable for
>>> alternate malloc implementations. There should be not too much memory
>>> use from this allocator, and the alternative, direct use of mmap(2) is
>>> obviously worse.
>>>
>>> PR: 235211
>>> MFC after: 2 weeks
>>> Sponsored by: The FreeBSD Foundation
>>> Differential revision: https://reviews.freebsd.org/D18988
>>>
>> This broke ARM static binaries (at least rescue/rescue). From first
>> look it seems that __pthread_mutex_init() invoked by atomic_init() is
>> called before curthread is set (before _thread_init_hack()).
>>
>>
>> root at tegra124:/usr/src # gdb --args
>> /usr/obj/usr/src/arm.armv7/rescue/rescue/rescue sh
>> ...
>> Reading symbols from /usr/obj/usr/src/arm.armv7/rescue/rescue/rescue...done.
>> (gdb) b _thread_init_hack
>> Breakpoint 1 at 0x67cad0: file /usr/src/lib/libthr/thread/thr_init.c,
>> line 296.
>> (gdb) r
>> Starting program: /usr/obj/usr/src/arm.armv7/rescue/rescue/rescue sh
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> __thr_calloc (num=1, size=<optimized out>) at
>> /usr/src/lib/libthr/thread/thr_malloc.c:82
>> 82 thr_malloc_lock(curthread);
>> (gdb) bt
>> #0 __thr_calloc (num=1, size=<optimized out>) at
>> /usr/src/lib/libthr/thread/thr_malloc.c:82
>> #1 0x00676e1c in mutex_init (mutex=<optimized out>,
>> mutex_attr=<optimized out>, calloc_cb=<optimized out>)
>> at /usr/src/lib/libthr/thread/thr_mutex.c:294
>> #2 __pthread_mutex_init (mutex=0xc6e948 <atomic_mtx>,
>> mutex_attr=<optimized out>) at /usr/src/lib/libthr/thread/thr_mutex.c:393
>> #3 0x001d41f0 in handle_static_init (argc=2, argv=<optimized out>,
>> env=<optimized out>) at /usr/src/lib/csu/common/ignore_init.c:124
>> #4 0x001d40e8 in __start (argc=2, argv=<optimized out>, env=<optimized
>> out>, ps_strings=<optimized out>, obj=0x0, cleanup=0x0)
>> at /usr/src/lib/csu/arm/crt1.c:112
>> #5 0x001d4000 in ?? ()
>> Backtrace stopped: previous frame identical to this frame (corrupt stack?)
>> (gdb) f 3
>> #3 0x001d41f0 in handle_static_init (argc=2, argv=<optimized out>,
>> env=<optimized out>) at /usr/src/lib/csu/common/ignore_init.c:124
>> 124 fn(argc, argv, env);
>> (gdb) list
>> 119 _init();
>> 120 array_size = __init_array_end - __init_array_start;
>> 121 for (n = 0; n < array_size; n++) {
>> 122 fn = __init_array_start[n];
>> 123 if ((uintptr_t)fn != 0 && (uintptr_t)fn != 1)
>> 124 fn(argc, argv, env);
>> 125 }
>> 126 }
>> (gdb) p n
>> $1 = 20
>> (gdb) p fn
>> $6 = (void (*)(int, char **, char **)) 0x63f21c <atomic_init>
>> (gdb) p (void *)&__init_array_start
>> $19 = (void *) 0xa7ab60
>> (gdb) p (void *)&__init_array_end
>> $20 = (void *) 0xa7abc0
>> (gdb) x/24a &__init_array_start
>> 0xa7ab60: 0x1d4270 <register_classes>
>> 0x2d926c <_$$hide$$ ifconfig.lo ifconfig_ctor>
>> 0x2d98e0 <_$$hide$$ ifconfig.lo link_ctor>
>> 0x2d9cb4 <_$$hide$$ ifconfig.lo inet_ctor>
>> 0xa7ab70: 0x2da2e0 <_$$hide$$ ifconfig.lo inet6_ctor>
>> 0x2db790 <_$$hide$$ ifconfig.lo clone_ctor>
>> 0x2dbb8c <_$$hide$$ ifconfig.lo mac_ctor>
>> 0x2dbe70 <_$$hide$$ ifconfig.lo ifmedia_ctor>
>> 0xa7ab80: 0x2dced0 <_$$hide$$ ifconfig.lo fib_ctor>
>> 0x2dd118 <_$$hide$$ ifconfig.lo vlan_ctor>
>> 0x2dd668 <_$$hide$$ ifconfig.lo vxlan_ctor>
>> 0x2df45c <_$$hide$$ ifconfig.lo gre_ctor>
>> 0xa7ab90: 0x2df6d8 <_$$hide$$ ifconfig.lo gif_ctor>
>> 0x2df874 <_$$hide$$ ifconfig.lo ipsec_ctor>
>> 0x2e2144 <_$$hide$$ ifconfig.lo ieee80211_ctor>
>> 0x2f0a10 <_$$hide$$ ifconfig.lo carp_ctor>
>> 0xa7aba0: 0x2f0e6c <_$$hide$$ ifconfig.lo group_ctor>
>> 0x2f199c <_$$hide$$ ifconfig.lo pfsync_ctor>
>> 0x2f1a04 <_$$hide$$ ifconfig.lo bridge_ctor>
>> 0x2f35dc <_$$hide$$ ifconfig.lo lagg_ctor>
>> 0xa7abb0: 0x63f21c <atomic_init>
>> 0x67cad0 <_thread_init_hack>
>> 0x90e5b4 <OPENSSL_cpuid_setup>
>> 0x9a6b98 <jemalloc_constructor>
>>
>> So it's clear that order of static constructors is invalid (moreover, I
>> thing that we are inside undefined area here).
>> Any idea/hint how to fix this.
>
> Try the following (I did not even compiled). It might require additional
> handling of NULL curthread in thr_malloc.c, in which case the locking
> should be elided.
>
> diff --git a/lib/libthr/thread/thr_malloc.c b/lib/libthr/thread/thr_malloc.c
> index 157c72f10d6..8b72a1840f7 100644
> --- a/lib/libthr/thread/thr_malloc.c
> +++ b/lib/libthr/thread/thr_malloc.c
> @@ -46,6 +46,8 @@ void
> __thr_malloc_init(void)
> {
>
> + if (npagesizes != 0)
> + return;
> npagesizes = getpagesizes(pagesizes_d, nitems(pagesizes_d));
> if (npagesizes == -1) {
> npagesizes = 1;
> diff --git a/lib/libthr/thread/thr_mutex.c b/lib/libthr/thread/thr_mutex.c
> index f6f37c1264e..4db65384331 100644
> --- a/lib/libthr/thread/thr_mutex.c
> +++ b/lib/libthr/thread/thr_mutex.c
> @@ -390,6 +390,7 @@ __pthread_mutex_init(pthread_mutex_t * __restrict mutex,
> }
> if (mutex_attr == NULL ||
> (*mutex_attr)->m_pshared == PTHREAD_PROCESS_PRIVATE) {
> + __thr_malloc_init();
> return (mutex_init(mutex, mutex_attr ? *mutex_attr : NULL,
> __thr_calloc));
> }
>
Thanks for fast response.
It works, but yes, additional handling of NULL curthread is required.
The following patch fixes this issue for me (tested only on ARMv7).
Can you, please, check it and eventually commit it?
Thanks,
Michal
-------------- next part --------------
Index: lib/libthr/thread/thr_malloc.c
===================================================================
--- lib/libthr/thread/thr_malloc.c (revision 343732)
+++ lib/libthr/thread/thr_malloc.c (working copy)
@@ -45,6 +45,8 @@
void
__thr_malloc_init(void)
{
+ if (npagesizes != 0)
+ return;
npagesizes = getpagesizes(pagesizes_d, nitems(pagesizes_d));
if (npagesizes == -1) {
@@ -59,6 +61,8 @@
thr_malloc_lock(struct pthread *curthread)
{
+ if (curthread == NULL)
+ return;
curthread->locklevel++;
_thr_umutex_lock(&thr_malloc_umtx, TID(curthread));
}
@@ -67,6 +71,8 @@
thr_malloc_unlock(struct pthread *curthread)
{
+ if (curthread == NULL)
+ return;
_thr_umutex_unlock(&thr_malloc_umtx, TID(curthread));
curthread->locklevel--;
_thr_ast(curthread);
Index: lib/libthr/thread/thr_mutex.c
===================================================================
--- lib/libthr/thread/thr_mutex.c (revision 343732)
+++ lib/libthr/thread/thr_mutex.c (working copy)
@@ -390,6 +390,7 @@
}
if (mutex_attr == NULL ||
(*mutex_attr)->m_pshared == PTHREAD_PROCESS_PRIVATE) {
+ __thr_malloc_init();
return (mutex_init(mutex, mutex_attr ? *mutex_attr : NULL,
__thr_calloc));
}
More information about the svn-src-head
mailing list