Date: Mon, 4 Feb 2019 07:31:19 +0100 From: Michal Meloun <meloun.michal@gmail.com> To: Konstantin Belousov <kib@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r343566 - in head/lib/libthr: . thread Message-ID: <c5f22e60-53fd-3677-9067-dd597218935c@freebsd.org> In-Reply-To: <201901292246.x0TMkjQH074121@repo.freebsd.org> References: <201901292246.x0TMkjQH074121@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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. Thanks, Michal > Added: > head/lib/libthr/thread/thr_malloc.c (contents, props changed) > Modified: > head/lib/libthr/Makefile > head/lib/libthr/thread/Makefile.inc > head/lib/libthr/thread/thr_fork.c > head/lib/libthr/thread/thr_init.c > head/lib/libthr/thread/thr_mutex.c > head/lib/libthr/thread/thr_private.h > head/lib/libthr/thread/thr_spec.c > > Modified: head/lib/libthr/Makefile > ============================================================================== > --- head/lib/libthr/Makefile Tue Jan 29 22:45:24 2019 (r343565) > +++ head/lib/libthr/Makefile Tue Jan 29 22:46:44 2019 (r343566) > @@ -27,6 +27,7 @@ CFLAGS+=-I${SRCTOP}/lib/libthread_db > CFLAGS+=-Winline > > CFLAGS.thr_stack.c+= -Wno-cast-align > +CFLAGS.malloc.c+= -Wno-cast-align > .include <bsd.compiler.mk> > .if !(${COMPILER_TYPE} == "gcc" && ${COMPILER_VERSION} < 40300) > CFLAGS.thr_symbols.c+= -Wno-missing-variable-declarations > @@ -50,12 +51,14 @@ CFLAGS+=-D_PTHREADS_INVARIANTS > PRECIOUSLIB= > > .PATH: ${.CURDIR}/arch/${MACHINE_CPUARCH}/${MACHINE_CPUARCH} > +.PATH: ${SRCTOP}/libexec/rtld-elf > > .if exists(${.CURDIR}/arch/${MACHINE_CPUARCH}/Makefile.inc) > .include "${.CURDIR}/arch/${MACHINE_CPUARCH}/Makefile.inc" > .endif > .include "${.CURDIR}/sys/Makefile.inc" > .include "${.CURDIR}/thread/Makefile.inc" > +SRCS+= malloc.c > > .if ${MK_INSTALLLIB} != "no" > SYMLINKS+=lib${LIB}.a ${LIBDIR}/libpthread.a > > Modified: head/lib/libthr/thread/Makefile.inc > ============================================================================== > --- head/lib/libthr/thread/Makefile.inc Tue Jan 29 22:45:24 2019 (r343565) > +++ head/lib/libthr/thread/Makefile.inc Tue Jan 29 22:46:44 2019 (r343566) > @@ -31,6 +31,7 @@ SRCS+= \ > thr_kern.c \ > thr_kill.c \ > thr_main_np.c \ > + thr_malloc.c \ > thr_multi_np.c \ > thr_mutex.c \ > thr_mutexattr.c \ > > Modified: head/lib/libthr/thread/thr_fork.c > ============================================================================== > --- head/lib/libthr/thread/thr_fork.c Tue Jan 29 22:45:24 2019 (r343565) > +++ head/lib/libthr/thread/thr_fork.c Tue Jan 29 22:46:44 2019 (r343566) > @@ -170,6 +170,7 @@ __thr_fork(void) > */ > if (_thr_isthreaded() != 0) { > was_threaded = 1; > + __thr_malloc_prefork(curthread); > _malloc_prefork(); > __thr_pshared_atfork_pre(); > _rtld_atfork_pre(rtld_locks); > @@ -197,6 +198,10 @@ __thr_fork(void) > */ > curthread->tlflags &= ~TLFLAGS_IN_TDLIST; > > + /* before thr_self() */ > + if (was_threaded) > + __thr_malloc_postfork(curthread); > + > /* child is a new kernel thread. */ > thr_self(&curthread->tid); > > @@ -241,6 +246,7 @@ __thr_fork(void) > _thr_signal_postfork(); > > if (was_threaded) { > + __thr_malloc_postfork(curthread); > _rtld_atfork_post(rtld_locks); > __thr_pshared_atfork_post(); > _malloc_postfork(); > > Modified: head/lib/libthr/thread/thr_init.c > ============================================================================== > --- head/lib/libthr/thread/thr_init.c Tue Jan 29 22:45:24 2019 (r343565) > +++ head/lib/libthr/thread/thr_init.c Tue Jan 29 22:46:44 2019 (r343566) > @@ -461,6 +461,7 @@ init_private(void) > */ > if (init_once == 0) { > __thr_pshared_init(); > + __thr_malloc_init(); > /* Find the stack top */ > mib[0] = CTL_KERN; > mib[1] = KERN_USRSTACK; > > Added: head/lib/libthr/thread/thr_malloc.c > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ head/lib/libthr/thread/thr_malloc.c Tue Jan 29 22:46:44 2019 (r343566) > @@ -0,0 +1,137 @@ > +/*- > + * SPDX-License-Identifier: BSD-2-Clause-FreeBSD > + * > + * Copyright (c) 2019 The FreeBSD Foundation > + * All rights reserved. > + * > + * This software was developed by Konstantin Belousov <kib@FreeBSD.org> > + * under sponsorship from the FreeBSD Foundation. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. > + */ > + > +#include <sys/cdefs.h> > +__FBSDID("$FreeBSD$"); > + > +#include <sys/types.h> > +#include <sys/mman.h> > +#include <rtld_malloc.h> > +#include "thr_private.h" > + > +int npagesizes; > +size_t *pagesizes; > +static size_t pagesizes_d[2]; > +static struct umutex thr_malloc_umtx; > + > +void > +__thr_malloc_init(void) > +{ > + > + npagesizes = getpagesizes(pagesizes_d, nitems(pagesizes_d)); > + if (npagesizes == -1) { > + npagesizes = 1; > + pagesizes_d[0] = PAGE_SIZE; > + } > + pagesizes = pagesizes_d; > + _thr_umutex_init(&thr_malloc_umtx); > +} > + > +static void > +thr_malloc_lock(struct pthread *curthread) > +{ > + > + curthread->locklevel++; > + _thr_umutex_lock(&thr_malloc_umtx, TID(curthread)); > +} > + > +static void > +thr_malloc_unlock(struct pthread *curthread) > +{ > + > + _thr_umutex_unlock(&thr_malloc_umtx, TID(curthread)); > + curthread->locklevel--; > + _thr_ast(curthread); > +} > + > +void * > +__thr_calloc(size_t num, size_t size) > +{ > + struct pthread *curthread; > + void *res; > + > + curthread = _get_curthread(); > + thr_malloc_lock(curthread); > + res = __crt_calloc(num, size); > + thr_malloc_unlock(curthread); > + return (res); > +} > + > +void > +__thr_free(void *cp) > +{ > + struct pthread *curthread; > + > + curthread = _get_curthread(); > + thr_malloc_lock(curthread); > + __crt_free(cp); > + thr_malloc_unlock(curthread); > +} > + > +void * > +__thr_malloc(size_t nbytes) > +{ > + struct pthread *curthread; > + void *res; > + > + curthread = _get_curthread(); > + thr_malloc_lock(curthread); > + res = __crt_malloc(nbytes); > + thr_malloc_unlock(curthread); > + return (res); > +} > + > +void * > +__thr_realloc(void *cp, size_t nbytes) > +{ > + struct pthread *curthread; > + void *res; > + > + curthread = _get_curthread(); > + thr_malloc_lock(curthread); > + res = __crt_realloc(cp, nbytes); > + thr_malloc_unlock(curthread); > + return (res); > +} > + > +void > +__thr_malloc_prefork(struct pthread *curthread) > +{ > + > + _thr_umutex_lock(&thr_malloc_umtx, TID(curthread)); > +} > + > +void > +__thr_malloc_postfork(struct pthread *curthread) > +{ > + > + _thr_umutex_unlock(&thr_malloc_umtx, TID(curthread)); > +} > > Modified: head/lib/libthr/thread/thr_mutex.c > ============================================================================== > --- head/lib/libthr/thread/thr_mutex.c Tue Jan 29 22:45:24 2019 (r343565) > +++ head/lib/libthr/thread/thr_mutex.c Tue Jan 29 22:46:44 2019 (r343566) > @@ -306,10 +306,11 @@ init_static(struct pthread *thread, pthread_mutex_t *m > THR_LOCK_ACQUIRE(thread, &_mutex_static_lock); > > if (*mutex == THR_MUTEX_INITIALIZER) > - ret = mutex_init(mutex, &_pthread_mutexattr_default, calloc); > + ret = mutex_init(mutex, &_pthread_mutexattr_default, > + __thr_calloc); > else if (*mutex == THR_ADAPTIVE_MUTEX_INITIALIZER) > ret = mutex_init(mutex, &_pthread_mutexattr_adaptive_default, > - calloc); > + __thr_calloc); > else > ret = 0; > THR_LOCK_RELEASE(thread, &_mutex_static_lock); > @@ -390,7 +391,7 @@ __pthread_mutex_init(pthread_mutex_t * __restrict mute > if (mutex_attr == NULL || > (*mutex_attr)->m_pshared == PTHREAD_PROCESS_PRIVATE) { > return (mutex_init(mutex, mutex_attr ? *mutex_attr : NULL, > - calloc)); > + __thr_calloc)); > } > pmtx = __thr_pshared_offpage(__DECONST(void *, mutex), 1); > if (pmtx == NULL) > @@ -483,7 +484,7 @@ _pthread_mutex_destroy(pthread_mutex_t *mutex) > } else { > *mutex = THR_MUTEX_DESTROYED; > mutex_assert_not_owned(_get_curthread(), m); > - free(m); > + __thr_free(m); > ret = 0; > } > } > > Modified: head/lib/libthr/thread/thr_private.h > ============================================================================== > --- head/lib/libthr/thread/thr_private.h Tue Jan 29 22:45:24 2019 (r343565) > +++ head/lib/libthr/thread/thr_private.h Tue Jan 29 22:46:44 2019 (r343566) > @@ -1003,6 +1003,14 @@ void __thr_pshared_destroy(void *key) __hidden; > void __thr_pshared_atfork_pre(void) __hidden; > void __thr_pshared_atfork_post(void) __hidden; > > +void *__thr_calloc(size_t num, size_t size); > +void __thr_free(void *cp); > +void *__thr_malloc(size_t nbytes); > +void *__thr_realloc(void *cp, size_t nbytes); > +void __thr_malloc_init(void); > +void __thr_malloc_prefork(struct pthread *curthread); > +void __thr_malloc_postfork(struct pthread *curthread); > + > __END_DECLS > __NULLABILITY_PRAGMA_POP > > > Modified: head/lib/libthr/thread/thr_spec.c > ============================================================================== > --- head/lib/libthr/thread/thr_spec.c Tue Jan 29 22:45:24 2019 (r343565) > +++ head/lib/libthr/thread/thr_spec.c Tue Jan 29 22:46:44 2019 (r343566) > @@ -155,8 +155,7 @@ _thread_cleanupspecific(void) > } > } > THR_LOCK_RELEASE(curthread, &_keytable_lock); > - munmap(curthread->specific, PTHREAD_KEYS_MAX * sizeof(struct > - pthread_specific_elem)); > + __thr_free(curthread->specific); > curthread->specific = NULL; > if (curthread->specific_data_count > 0) { > stderr_debug("Thread %p has exited with leftover " > @@ -179,10 +178,9 @@ _pthread_setspecific(pthread_key_t userkey, const void > > pthread = _get_curthread(); > if (pthread->specific == NULL) { > - tmp = mmap(NULL, PTHREAD_KEYS_MAX * > - sizeof(struct pthread_specific_elem), > - PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); > - if (tmp == MAP_FAILED) > + tmp = __thr_calloc(PTHREAD_KEYS_MAX, > + sizeof(struct pthread_specific_elem)); > + if (tmp == NULL) > return (ENOMEM); > pthread->specific = tmp; > } >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?c5f22e60-53fd-3677-9067-dd597218935c>