From ad505e0ec622883fbb0650763ea8b54f64a770c9 Mon Sep 17 00:00:00 2001 From: "Jory A. Pratt" Date: Sun, 11 Aug 2013 09:44:59 -0500 Subject: [PATCH 01/15] Allow toolchain to determine ar --- Makefile.in | 4 +++- configure.ac | 10 +++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Makefile.in b/Makefile.in index 74810472..478becbe 100644 --- a/Makefile.in +++ b/Makefile.in @@ -55,6 +55,8 @@ PIC_CFLAGS = @PIC_CFLAGS@ CTARGET = @CTARGET@ LDTARGET = @LDTARGET@ MKLIB = @MKLIB@ +AR = @AR@ +ARFLAGS = crus CC_MM = @CC_MM@ ifeq (macho, $(ABI)) @@ -185,7 +187,7 @@ $(objroot)lib/$(LIBJEMALLOC)_s.$(A) : $(COBJS) $(STATIC_LIBS): @mkdir -p $(@D) - $(MKLIB) $+ + $(AR) $(ARFLAGS) $@ $+ $(objroot)test/bitmap$(EXE): $(objroot)src/bitmap.$(O) diff --git a/configure.ac b/configure.ac index c270662b..f4b4c21f 100644 --- a/configure.ac +++ b/configure.ac @@ -226,9 +226,13 @@ PIC_CFLAGS='-fPIC -DPIC' CTARGET='-o $@' LDTARGET='-o $@' EXTRA_LDFLAGS= -MKLIB='ar crus $@' CC_MM=1 +AN_MAKEVAR([AR], [AC_PROG_AR]) +AN_PROGRAM([ar], [AC_PROG_AR]) +AC_DEFUN([AC_PROG_AR], [AC_CHECK_TOOL(AR, ar, :)]) +AC_PROG_AR + dnl Platform-specific settings. abi and RPATH can probably be determined dnl programmatically, but doing so is error-prone, which makes it generally dnl not worth the trouble. @@ -310,7 +314,8 @@ case "${host}" in EXTRA_LDFLAGS="-link -DEBUG" CTARGET='-Fo$@' LDTARGET='-Fe$@' - MKLIB='lib -nologo -out:$@' + AR='lib' + ARFLAGS='-nologo -out:' CC_MM= else importlib="${so}" @@ -403,7 +408,6 @@ AC_SUBST([enable_autogen]) AC_PROG_INSTALL AC_PROG_RANLIB -AC_PATH_PROG([AR], [ar], [false], [$PATH]) AC_PATH_PROG([LD], [ld], [false], [$PATH]) AC_PATH_PROG([AUTOCONF], [autoconf], [false], [$PATH]) From 80ddf498eb166cad45c8592973eb4f949f176688 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Tue, 20 Aug 2013 11:48:19 +0100 Subject: [PATCH 02/15] Fix build break for MSVC. Introduce AROUT to control whether there is space between ARFLAGS and $@. This regression was introduced by ad505e0ec622883fbb0650763ea8b54f64a770c9. Reported by Mike Hommey. --- Makefile.in | 4 ++-- configure.ac | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Makefile.in b/Makefile.in index 478becbe..5909416e 100644 --- a/Makefile.in +++ b/Makefile.in @@ -56,7 +56,7 @@ CTARGET = @CTARGET@ LDTARGET = @LDTARGET@ MKLIB = @MKLIB@ AR = @AR@ -ARFLAGS = crus +ARFLAGS = @ARFLAGS@ CC_MM = @CC_MM@ ifeq (macho, $(ABI)) @@ -187,7 +187,7 @@ $(objroot)lib/$(LIBJEMALLOC)_s.$(A) : $(COBJS) $(STATIC_LIBS): @mkdir -p $(@D) - $(AR) $(ARFLAGS) $@ $+ + $(AR) $(ARFLAGS)@AROUT@ $+ $(objroot)test/bitmap$(EXE): $(objroot)src/bitmap.$(O) diff --git a/configure.ac b/configure.ac index f4b4c21f..73d3f94f 100644 --- a/configure.ac +++ b/configure.ac @@ -226,6 +226,8 @@ PIC_CFLAGS='-fPIC -DPIC' CTARGET='-o $@' LDTARGET='-o $@' EXTRA_LDFLAGS= +ARFLAGS='crus' +AROUT=' $@' CC_MM=1 AN_MAKEVAR([AR], [AC_PROG_AR]) @@ -316,6 +318,7 @@ case "${host}" in LDTARGET='-Fe$@' AR='lib' ARFLAGS='-nologo -out:' + AROUT='$@' CC_MM= else importlib="${so}" @@ -348,6 +351,8 @@ AC_SUBST([PIC_CFLAGS]) AC_SUBST([CTARGET]) AC_SUBST([LDTARGET]) AC_SUBST([MKLIB]) +AC_SUBST([ARFLAGS]) +AC_SUBST([AROUT]) AC_SUBST([CC_MM]) if test "x$abi" != "xpecoff"; then From a33488d648ebe6e56b266210fc8d468fbf48a6a2 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Thu, 3 Oct 2013 14:38:39 -0700 Subject: [PATCH 03/15] Fix typo. --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 8ab88487..d758be17 100644 --- a/ChangeLog +++ b/ChangeLog @@ -60,7 +60,7 @@ found in the git revision history: Bug fixes: - Fix "arenas.extend" mallctl to output the number of arenas. - - Fix chunk_recycyle() to unconditionally inform Valgrind that returned memory + - Fix chunk_recycle() to unconditionally inform Valgrind that returned memory is undefined. - Fix build break on FreeBSD related to alloca.h. From dd6ef0302f3980200ed602ec600e211f55e58694 Mon Sep 17 00:00:00 2001 From: Alexandre Perrin Date: Fri, 20 Sep 2013 19:58:11 +0200 Subject: [PATCH 04/15] malloc_conf_init: revert errno value when readlink(2) fail. --- src/jemalloc.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/jemalloc.c b/src/jemalloc.c index bc350ed9..e3991da2 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -436,8 +436,9 @@ malloc_conf_init(void) } break; case 1: { + int linklen = 0; #ifndef _WIN32 - int linklen; + int saved_errno = errno; const char *linkname = # ifdef JEMALLOC_PREFIX "/etc/"JEMALLOC_PREFIX"malloc.conf" @@ -446,21 +447,20 @@ malloc_conf_init(void) # endif ; - if ((linklen = readlink(linkname, buf, - sizeof(buf) - 1)) != -1) { - /* - * Use the contents of the "/etc/malloc.conf" - * symbolic link's name. - */ - buf[linklen] = '\0'; - opts = buf; - } else -#endif - { + /* + * Try to use the contents of the "/etc/malloc.conf" + * symbolic link's name. + */ + linklen = readlink(linkname, buf, sizeof(buf) - 1); + if (linklen == -1) { /* No configuration specified. */ - buf[0] = '\0'; - opts = buf; + linklen = 0; + /* restore errno */ + set_errno(saved_errno); } +#endif + buf[linklen] = '\0'; + opts = buf; break; } case 2: { const char *envname = From 3ab682d341f033017d042e8498578c2332eacd69 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sat, 19 Oct 2013 17:19:49 -0700 Subject: [PATCH 05/15] Silence an unused variable warning. Reported by Ricardo Nabinger Sanchez. --- src/ctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctl.c b/src/ctl.c index f2ef4e60..f278105a 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -1109,7 +1109,7 @@ epoch_ctl(const size_t *mib, size_t miblen, void *oldp, size_t *oldlenp, void *newp, size_t newlen) { int ret; - uint64_t newval; + UNUSED uint64_t newval; malloc_mutex_lock(&ctl_mtx); WRITE(newval, uint64_t); From 543abf7e6c7de06fe9654e91190b5c44a11b065e Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sat, 19 Oct 2013 17:20:18 -0700 Subject: [PATCH 06/15] Fix inlining warning. Add the JEMALLOC_ALWAYS_INLINE_C macro and use it for always-inlined functions declared in .c files. This fixes a function attribute inconsistency for debug builds that resulted in (harmless) compiler warnings about functions not being inlinable. Reported by Ricardo Nabinger Sanchez. --- include/jemalloc/internal/jemalloc_internal.h.in | 12 ++++++++++++ src/jemalloc.c | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/include/jemalloc/internal/jemalloc_internal.h.in b/include/jemalloc/internal/jemalloc_internal.h.in index e46ac544..53c135c2 100644 --- a/include/jemalloc/internal/jemalloc_internal.h.in +++ b/include/jemalloc/internal/jemalloc_internal.h.in @@ -232,9 +232,18 @@ static const bool config_ivsalloc = # define __DECONST(type, var) ((type)(uintptr_t)(const void *)(var)) #endif +/* + * JEMALLOC_ALWAYS_INLINE is used within header files for functions that are + * static inline functions if inlining is enabled, and single-definition + * library-private functions if inlining is disabled. + * + * JEMALLOC_ALWAYS_INLINE_C is for use in .c files, in which case the denoted + * functions are always static, regardless of whether inlining is enabled. + */ #ifdef JEMALLOC_DEBUG /* Disable inlining to make debugging easier. */ # define JEMALLOC_ALWAYS_INLINE +# define JEMALLOC_ALWAYS_INLINE_C static # define JEMALLOC_INLINE # define inline #else @@ -242,8 +251,11 @@ static const bool config_ivsalloc = # ifdef JEMALLOC_HAVE_ATTR # define JEMALLOC_ALWAYS_INLINE \ static inline JEMALLOC_ATTR(unused) JEMALLOC_ATTR(always_inline) +# define JEMALLOC_ALWAYS_INLINE_C \ + static inline JEMALLOC_ATTR(always_inline) # else # define JEMALLOC_ALWAYS_INLINE static inline +# define JEMALLOC_ALWAYS_INLINE_C static inline # endif # define JEMALLOC_INLINE static inline # ifdef _MSC_VER diff --git a/src/jemalloc.c b/src/jemalloc.c index e3991da2..ae56db6b 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -282,7 +282,7 @@ arenas_cleanup(void *arg) malloc_mutex_unlock(&arenas_lock); } -static JEMALLOC_ATTR(always_inline) void +JEMALLOC_ALWAYS_INLINE_C void malloc_thread_init(void) { @@ -299,7 +299,7 @@ malloc_thread_init(void) quarantine_alloc_hook(); } -static JEMALLOC_ATTR(always_inline) bool +JEMALLOC_ALWAYS_INLINE_C bool malloc_init(void) { @@ -1402,7 +1402,7 @@ je_mallctlbymib(const size_t *mib, size_t miblen, void *oldp, size_t *oldlenp, */ #ifdef JEMALLOC_EXPERIMENTAL -static JEMALLOC_ATTR(always_inline) void * +JEMALLOC_ALWAYS_INLINE_C void * iallocm(size_t usize, size_t alignment, bool zero, bool try_tcache, arena_t *arena) { From 87a02d2bb18dbcb2955541b849bc95862e864803 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sat, 19 Oct 2013 21:40:20 -0700 Subject: [PATCH 07/15] Fix a Valgrind integration flaw. Fix a Valgrind integration flaw that caused Valgrind warnings about reads of uninitialized memory in arena chunk headers. --- include/jemalloc/internal/arena.h | 66 ++++++++++++------- include/jemalloc/internal/private_namespace.h | 2 + src/arena.c | 21 ++++-- 3 files changed, 57 insertions(+), 32 deletions(-) diff --git a/include/jemalloc/internal/arena.h b/include/jemalloc/internal/arena.h index f2c18f43..bbcfedac 100644 --- a/include/jemalloc/internal/arena.h +++ b/include/jemalloc/internal/arena.h @@ -441,6 +441,7 @@ void arena_postfork_child(arena_t *arena); #ifndef JEMALLOC_ENABLE_INLINE arena_chunk_map_t *arena_mapp_get(arena_chunk_t *chunk, size_t pageind); size_t *arena_mapbitsp_get(arena_chunk_t *chunk, size_t pageind); +size_t arena_mapbitsp_read(size_t *mapbitsp); size_t arena_mapbits_get(arena_chunk_t *chunk, size_t pageind); size_t arena_mapbits_unallocated_size_get(arena_chunk_t *chunk, size_t pageind); @@ -451,6 +452,7 @@ size_t arena_mapbits_dirty_get(arena_chunk_t *chunk, size_t pageind); size_t arena_mapbits_unzeroed_get(arena_chunk_t *chunk, size_t pageind); size_t arena_mapbits_large_get(arena_chunk_t *chunk, size_t pageind); size_t arena_mapbits_allocated_get(arena_chunk_t *chunk, size_t pageind); +void arena_mapbitsp_write(size_t *mapbitsp, size_t mapbits); void arena_mapbits_unallocated_set(arena_chunk_t *chunk, size_t pageind, size_t size, size_t flags); void arena_mapbits_unallocated_size_set(arena_chunk_t *chunk, size_t pageind, @@ -497,11 +499,18 @@ arena_mapbitsp_get(arena_chunk_t *chunk, size_t pageind) return (&arena_mapp_get(chunk, pageind)->bits); } +JEMALLOC_ALWAYS_INLINE size_t +arena_mapbitsp_read(size_t *mapbitsp) +{ + + return (*mapbitsp); +} + JEMALLOC_ALWAYS_INLINE size_t arena_mapbits_get(arena_chunk_t *chunk, size_t pageind) { - return (*arena_mapbitsp_get(chunk, pageind)); + return (arena_mapbitsp_read(arena_mapbitsp_get(chunk, pageind))); } JEMALLOC_ALWAYS_INLINE size_t @@ -584,83 +593,90 @@ arena_mapbits_allocated_get(arena_chunk_t *chunk, size_t pageind) return (mapbits & CHUNK_MAP_ALLOCATED); } +JEMALLOC_ALWAYS_INLINE void +arena_mapbitsp_write(size_t *mapbitsp, size_t mapbits) +{ + + *mapbitsp = mapbits; +} + JEMALLOC_ALWAYS_INLINE void arena_mapbits_unallocated_set(arena_chunk_t *chunk, size_t pageind, size_t size, size_t flags) { - size_t *mapbitsp; + size_t *mapbitsp = arena_mapbitsp_get(chunk, pageind); - mapbitsp = arena_mapbitsp_get(chunk, pageind); assert((size & PAGE_MASK) == 0); assert((flags & ~CHUNK_MAP_FLAGS_MASK) == 0); assert((flags & (CHUNK_MAP_DIRTY|CHUNK_MAP_UNZEROED)) == flags); - *mapbitsp = size | CHUNK_MAP_BININD_INVALID | flags; + arena_mapbitsp_write(mapbitsp, size | CHUNK_MAP_BININD_INVALID | flags); } JEMALLOC_ALWAYS_INLINE void arena_mapbits_unallocated_size_set(arena_chunk_t *chunk, size_t pageind, size_t size) { - size_t *mapbitsp; + size_t *mapbitsp = arena_mapbitsp_get(chunk, pageind); + size_t mapbits = arena_mapbitsp_read(mapbitsp); - mapbitsp = arena_mapbitsp_get(chunk, pageind); assert((size & PAGE_MASK) == 0); - assert((*mapbitsp & (CHUNK_MAP_LARGE|CHUNK_MAP_ALLOCATED)) == 0); - *mapbitsp = size | (*mapbitsp & PAGE_MASK); + assert((mapbits & (CHUNK_MAP_LARGE|CHUNK_MAP_ALLOCATED)) == 0); + arena_mapbitsp_write(mapbitsp, size | (mapbits & PAGE_MASK)); } JEMALLOC_ALWAYS_INLINE void arena_mapbits_large_set(arena_chunk_t *chunk, size_t pageind, size_t size, size_t flags) { - size_t *mapbitsp; + size_t *mapbitsp = arena_mapbitsp_get(chunk, pageind); + size_t mapbits = arena_mapbitsp_read(mapbitsp); size_t unzeroed; - mapbitsp = arena_mapbitsp_get(chunk, pageind); assert((size & PAGE_MASK) == 0); assert((flags & CHUNK_MAP_DIRTY) == flags); - unzeroed = *mapbitsp & CHUNK_MAP_UNZEROED; /* Preserve unzeroed. */ - *mapbitsp = size | CHUNK_MAP_BININD_INVALID | flags | unzeroed | - CHUNK_MAP_LARGE | CHUNK_MAP_ALLOCATED; + unzeroed = mapbits & CHUNK_MAP_UNZEROED; /* Preserve unzeroed. */ + arena_mapbitsp_write(mapbitsp, size | CHUNK_MAP_BININD_INVALID | flags + | unzeroed | CHUNK_MAP_LARGE | CHUNK_MAP_ALLOCATED); } JEMALLOC_ALWAYS_INLINE void arena_mapbits_large_binind_set(arena_chunk_t *chunk, size_t pageind, size_t binind) { - size_t *mapbitsp; + size_t *mapbitsp = arena_mapbitsp_get(chunk, pageind); + size_t mapbits = arena_mapbitsp_read(mapbitsp); assert(binind <= BININD_INVALID); - mapbitsp = arena_mapbitsp_get(chunk, pageind); assert(arena_mapbits_large_size_get(chunk, pageind) == PAGE); - *mapbitsp = (*mapbitsp & ~CHUNK_MAP_BININD_MASK) | (binind << - CHUNK_MAP_BININD_SHIFT); + arena_mapbitsp_write(mapbitsp, (mapbits & ~CHUNK_MAP_BININD_MASK) | + (binind << CHUNK_MAP_BININD_SHIFT)); } JEMALLOC_ALWAYS_INLINE void arena_mapbits_small_set(arena_chunk_t *chunk, size_t pageind, size_t runind, size_t binind, size_t flags) { - size_t *mapbitsp; + size_t *mapbitsp = arena_mapbitsp_get(chunk, pageind); + size_t mapbits = arena_mapbitsp_read(mapbitsp); size_t unzeroed; assert(binind < BININD_INVALID); - mapbitsp = arena_mapbitsp_get(chunk, pageind); assert(pageind - runind >= map_bias); assert((flags & CHUNK_MAP_DIRTY) == flags); - unzeroed = *mapbitsp & CHUNK_MAP_UNZEROED; /* Preserve unzeroed. */ - *mapbitsp = (runind << LG_PAGE) | (binind << CHUNK_MAP_BININD_SHIFT) | - flags | unzeroed | CHUNK_MAP_ALLOCATED; + unzeroed = mapbits & CHUNK_MAP_UNZEROED; /* Preserve unzeroed. */ + arena_mapbitsp_write(mapbitsp, (runind << LG_PAGE) | (binind << + CHUNK_MAP_BININD_SHIFT) | flags | unzeroed | CHUNK_MAP_ALLOCATED); } JEMALLOC_ALWAYS_INLINE void arena_mapbits_unzeroed_set(arena_chunk_t *chunk, size_t pageind, size_t unzeroed) { - size_t *mapbitsp; + size_t *mapbitsp = arena_mapbitsp_get(chunk, pageind); + size_t mapbits = arena_mapbitsp_read(mapbitsp); - mapbitsp = arena_mapbitsp_get(chunk, pageind); - *mapbitsp = (*mapbitsp & ~CHUNK_MAP_UNZEROED) | unzeroed; + arena_mapbitsp_write(mapbitsp, (mapbits & ~CHUNK_MAP_UNZEROED) | + unzeroed); } JEMALLOC_INLINE bool diff --git a/include/jemalloc/internal/private_namespace.h b/include/jemalloc/internal/private_namespace.h index 65de3163..cdb0b0eb 100644 --- a/include/jemalloc/internal/private_namespace.h +++ b/include/jemalloc/internal/private_namespace.h @@ -33,6 +33,8 @@ #define arena_mapbits_unzeroed_get JEMALLOC_N(arena_mapbits_unzeroed_get) #define arena_mapbits_unzeroed_set JEMALLOC_N(arena_mapbits_unzeroed_set) #define arena_mapbitsp_get JEMALLOC_N(arena_mapbitsp_get) +#define arena_mapbitsp_read JEMALLOC_N(arena_mapbitsp_read) +#define arena_mapbitsp_write JEMALLOC_N(arena_mapbitsp_write) #define arena_mapp_get JEMALLOC_N(arena_mapp_get) #define arena_maxclass JEMALLOC_N(arena_maxclass) #define arena_new JEMALLOC_N(arena_new) diff --git a/src/arena.c b/src/arena.c index 05a787f8..0a73be2a 100644 --- a/src/arena.c +++ b/src/arena.c @@ -569,17 +569,24 @@ arena_chunk_alloc(arena_t *arena) * unless the chunk is not zeroed. */ if (zero == false) { + VALGRIND_MAKE_MEM_UNDEFINED( + (void *)arena_mapp_get(chunk, map_bias+1), + (size_t)((uintptr_t) arena_mapp_get(chunk, + chunk_npages-1) - (uintptr_t)arena_mapp_get(chunk, + map_bias+1))); for (i = map_bias+1; i < chunk_npages-1; i++) arena_mapbits_unzeroed_set(chunk, i, unzeroed); - } else if (config_debug) { + } else { VALGRIND_MAKE_MEM_DEFINED( (void *)arena_mapp_get(chunk, map_bias+1), - (void *)((uintptr_t) - arena_mapp_get(chunk, chunk_npages-1) - - (uintptr_t)arena_mapp_get(chunk, map_bias+1))); - for (i = map_bias+1; i < chunk_npages-1; i++) { - assert(arena_mapbits_unzeroed_get(chunk, i) == - unzeroed); + (size_t)((uintptr_t) arena_mapp_get(chunk, + chunk_npages-1) - (uintptr_t)arena_mapp_get(chunk, + map_bias+1))); + if (config_debug) { + for (i = map_bias+1; i < chunk_npages-1; i++) { + assert(arena_mapbits_unzeroed_get(chunk, + i) == unzeroed); + } } } arena_mapbits_unallocated_set(chunk, chunk_npages-1, From ff08ef7046563ed3a2bf2bfb2acdcf91218df88e Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sat, 19 Oct 2013 21:41:10 -0700 Subject: [PATCH 08/15] Update ChangeLog. --- ChangeLog | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ChangeLog b/ChangeLog index d758be17..36614273 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,16 @@ found in the git revision history: http://www.canonware.com/cgi-bin/gitweb.cgi?p=jemalloc.git git://canonware.com/jemalloc.git +* 3.4.1 (XXX) + + Bug fixes: + - Fix a Valgrind integration flaw that caused Valgrind warnings about reads of + uninitialized memory in arena chunk headers. + - Preserve errno during the first allocation. A readlink(2) call during + initialization fails unless /etc/malloc.conf exists, so errno was typically + set during the first allocation prior to this fix. + - Fix compilation warnings reported by gcc 4.8.1. + * 3.4.0 (June 2, 2013) This version is essentially a small bugfix release, but the addition of From dda90f59e2b67903668a2799970f64df163e9ccf Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sat, 19 Oct 2013 23:48:40 -0700 Subject: [PATCH 09/15] Fix a Valgrind integration flaw. Fix a Valgrind integration flaw that caused Valgrind warnings about reads of uninitialized memory in internal zero-initialized data structures (relevant to tcache and prof code). --- ChangeLog | 7 +++++-- include/jemalloc/internal/tcache.h | 4 ++-- src/arena.c | 23 ++++++++++++++++++----- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index 36614273..9b51fd57 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,8 +9,11 @@ found in the git revision history: * 3.4.1 (XXX) Bug fixes: - - Fix a Valgrind integration flaw that caused Valgrind warnings about reads of - uninitialized memory in arena chunk headers. + - Fix Valgrind integration flaws that caused Valgrind warnings about reads of + uninitialized memory in: + + arena chunk headers + + internal zero-initialized data structures (relevant to tcache and prof + code) - Preserve errno during the first allocation. A readlink(2) call during initialization fails unless /etc/malloc.conf exists, so errno was typically set during the first allocation prior to this fix. diff --git a/include/jemalloc/internal/tcache.h b/include/jemalloc/internal/tcache.h index ba36204f..d4eecdee 100644 --- a/include/jemalloc/internal/tcache.h +++ b/include/jemalloc/internal/tcache.h @@ -313,6 +313,7 @@ tcache_alloc_small(tcache_t *tcache, size_t size, bool zero) } else if (opt_zero) memset(ret, 0, size); } + VALGRIND_MAKE_MEM_UNDEFINED(ret, size); } else { if (config_fill && opt_junk) { arena_alloc_junk_small(ret, &arena_bin_info[binind], @@ -321,7 +322,6 @@ tcache_alloc_small(tcache_t *tcache, size_t size, bool zero) VALGRIND_MAKE_MEM_UNDEFINED(ret, size); memset(ret, 0, size); } - VALGRIND_MAKE_MEM_UNDEFINED(ret, size); if (config_stats) tbin->tstats.nrequests++; @@ -368,11 +368,11 @@ tcache_alloc_large(tcache_t *tcache, size_t size, bool zero) else if (opt_zero) memset(ret, 0, size); } + VALGRIND_MAKE_MEM_UNDEFINED(ret, size); } else { VALGRIND_MAKE_MEM_UNDEFINED(ret, size); memset(ret, 0, size); } - VALGRIND_MAKE_MEM_UNDEFINED(ret, size); if (config_stats) tbin->tstats.nrequests++; diff --git a/src/arena.c b/src/arena.c index 0a73be2a..d28b629a 100644 --- a/src/arena.c +++ b/src/arena.c @@ -368,14 +368,21 @@ arena_run_zero(arena_chunk_t *chunk, size_t run_ind, size_t npages) (npages << LG_PAGE)); } +static inline void +arena_run_page_mark_zeroed(arena_chunk_t *chunk, size_t run_ind) +{ + + VALGRIND_MAKE_MEM_DEFINED((void *)((uintptr_t)chunk + (run_ind << + LG_PAGE)), PAGE); +} + static inline void arena_run_page_validate_zeroed(arena_chunk_t *chunk, size_t run_ind) { size_t i; UNUSED size_t *p = (size_t *)((uintptr_t)chunk + (run_ind << LG_PAGE)); - VALGRIND_MAKE_MEM_DEFINED((void *)((uintptr_t)chunk + (run_ind << - LG_PAGE)), PAGE); + arena_run_page_mark_zeroed(chunk, run_ind); for (i = 0; i < PAGE / sizeof(size_t); i++) assert(p[i] == 0); } @@ -458,6 +465,9 @@ arena_run_split(arena_t *arena, arena_run_t *run, size_t size, bool large, } else if (config_debug) { arena_run_page_validate_zeroed( chunk, run_ind+i); + } else { + arena_run_page_mark_zeroed( + chunk, run_ind+i); } } } else { @@ -467,6 +477,9 @@ arena_run_split(arena_t *arena, arena_run_t *run, size_t size, bool large, */ arena_run_zero(chunk, run_ind, need_pages); } + } else { + VALGRIND_MAKE_MEM_UNDEFINED((void *)((uintptr_t)chunk + + (run_ind << LG_PAGE)), (need_pages << LG_PAGE)); } /* @@ -508,9 +521,9 @@ arena_run_split(arena_t *arena, arena_run_t *run, size_t size, bool large, arena_run_page_validate_zeroed(chunk, run_ind+need_pages-1); } + VALGRIND_MAKE_MEM_UNDEFINED((void *)((uintptr_t)chunk + + (run_ind << LG_PAGE)), (need_pages << LG_PAGE)); } - VALGRIND_MAKE_MEM_UNDEFINED((void *)((uintptr_t)chunk + (run_ind << - LG_PAGE)), (need_pages << LG_PAGE)); } static arena_chunk_t * @@ -1465,6 +1478,7 @@ arena_malloc_small(arena_t *arena, size_t size, bool zero) } else if (opt_zero) memset(ret, 0, size); } + VALGRIND_MAKE_MEM_UNDEFINED(ret, size); } else { if (config_fill && opt_junk) { arena_alloc_junk_small(ret, &arena_bin_info[binind], @@ -1473,7 +1487,6 @@ arena_malloc_small(arena_t *arena, size_t size, bool zero) VALGRIND_MAKE_MEM_UNDEFINED(ret, size); memset(ret, 0, size); } - VALGRIND_MAKE_MEM_UNDEFINED(ret, size); return (ret); } From 8edaf86b67579f480343f720b89704456a20d1d6 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 20 Oct 2013 14:07:18 -0700 Subject: [PATCH 10/15] Fix dangerous casts in tests. Fix dangerous casts of int variables to pointers in thread join function calls. On LP64 systems, int and pointers are different sizes, so writes can corrupt memory. --- test/allocated.c | 4 ++-- test/thread_arena.c | 8 ++++++-- test/thread_tcache_enabled.c | 4 ++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/test/allocated.c b/test/allocated.c index 9884905d..b1a9cfd9 100644 --- a/test/allocated.c +++ b/test/allocated.c @@ -104,12 +104,12 @@ main(void) je_thread_start(NULL); je_thread_create(&thread, je_thread_start, NULL); - je_thread_join(thread, (void *)&ret); + je_thread_join(thread, NULL); je_thread_start(NULL); je_thread_create(&thread, je_thread_start, NULL); - je_thread_join(thread, (void *)&ret); + je_thread_join(thread, NULL); je_thread_start(NULL); diff --git a/test/thread_arena.c b/test/thread_arena.c index c5a21fa0..6b9bc9cf 100644 --- a/test/thread_arena.c +++ b/test/thread_arena.c @@ -72,8 +72,12 @@ main(void) (void *)&arena_ind); } - for (i = 0; i < NTHREADS; i++) - je_thread_join(threads[i], (void *)&ret); + for (i = 0; i < NTHREADS; i++) { + intptr_t join_ret; + je_thread_join(threads[i], (void *)&join_ret); + if (join_ret != 0) + ret = 1; + } label_return: malloc_printf("Test end\n"); diff --git a/test/thread_tcache_enabled.c b/test/thread_tcache_enabled.c index 2061b7bb..586b5330 100644 --- a/test/thread_tcache_enabled.c +++ b/test/thread_tcache_enabled.c @@ -77,12 +77,12 @@ main(void) je_thread_start(NULL); je_thread_create(&thread, je_thread_start, NULL); - je_thread_join(thread, (void *)&ret); + je_thread_join(thread, NULL); je_thread_start(NULL); je_thread_create(&thread, je_thread_start, NULL); - je_thread_join(thread, (void *)&ret); + je_thread_join(thread, NULL); je_thread_start(NULL); From 0f1d8ec300f746d5c9618904aa1d5568a6f524b5 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 20 Oct 2013 14:09:01 -0700 Subject: [PATCH 11/15] Fix an off-by-one flaw in a test. --- test/ALLOCM_ARENA.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/ALLOCM_ARENA.c b/test/ALLOCM_ARENA.c index 2c52485e..ca91b621 100644 --- a/test/ALLOCM_ARENA.c +++ b/test/ALLOCM_ARENA.c @@ -23,7 +23,8 @@ je_thread_start(void *arg) size_t mib[3]; size_t miblen = sizeof(mib) / sizeof(size_t); const char *dss_precs[] = {"disabled", "primary", "secondary"}; - const char *dss = dss_precs[thread_ind % 4]; + const char *dss = dss_precs[thread_ind % + (sizeof(dss_precs)/sizeof(char*))]; if (mallctlnametomib("arena.0.dss", mib, &miblen) != 0) { malloc_printf("Error in mallctlnametomib()\n"); abort(); From 7b65180b32558fc4f2bc7b6ac5602f306ed3a014 Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 20 Oct 2013 14:09:54 -0700 Subject: [PATCH 12/15] Fix a race condition in the "arenas.extend" mallctl. Fix a race condition in the "arenas.extend" mallctl that could lead to internal data structure corruption. The race could be hit if one thread called the "arenas.extend" mallctl while another thread concurrently triggered initialization of one of the lazily created arenas. --- ChangeLog | 2 ++ src/ctl.c | 84 ++++++++++++++++++++++++++++++------------------------- 2 files changed, 48 insertions(+), 38 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9b51fd57..def7685e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,8 @@ found in the git revision history: * 3.4.1 (XXX) Bug fixes: + - Fix a race in the "arenas.extend" mallctl that could cause memory corruption + of internal data structures and subsequent crashes. - Fix Valgrind integration flaws that caused Valgrind warnings about reads of uninitialized memory in: + arena chunk headers diff --git a/src/ctl.c b/src/ctl.c index f278105a..ebba7c25 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -546,43 +546,30 @@ ctl_arena_refresh(arena_t *arena, unsigned i) static bool ctl_grow(void) { - size_t astats_size; ctl_arena_stats_t *astats; arena_t **tarenas; - /* Extend arena stats and arenas arrays. */ - astats_size = (ctl_stats.narenas + 2) * sizeof(ctl_arena_stats_t); - if (ctl_stats.narenas == narenas_auto) { - /* ctl_stats.arenas and arenas came from base_alloc(). */ - astats = (ctl_arena_stats_t *)imalloc(astats_size); - if (astats == NULL) - return (true); - memcpy(astats, ctl_stats.arenas, (ctl_stats.narenas + 1) * - sizeof(ctl_arena_stats_t)); - - tarenas = (arena_t **)imalloc((ctl_stats.narenas + 1) * - sizeof(arena_t *)); - if (tarenas == NULL) { - idalloc(astats); - return (true); - } - memcpy(tarenas, arenas, ctl_stats.narenas * sizeof(arena_t *)); - } else { - astats = (ctl_arena_stats_t *)iralloc(ctl_stats.arenas, - astats_size, 0, 0, false, false); - if (astats == NULL) - return (true); - - tarenas = (arena_t **)iralloc(arenas, (ctl_stats.narenas + 1) * - sizeof(arena_t *), 0, 0, false, false); - if (tarenas == NULL) - return (true); - } - /* Initialize the new astats and arenas elements. */ - memset(&astats[ctl_stats.narenas + 1], 0, sizeof(ctl_arena_stats_t)); - if (ctl_arena_init(&astats[ctl_stats.narenas + 1])) + /* Allocate extended arena stats and arenas arrays. */ + astats = (ctl_arena_stats_t *)imalloc((ctl_stats.narenas + 2) * + sizeof(ctl_arena_stats_t)); + if (astats == NULL) return (true); - tarenas[ctl_stats.narenas] = NULL; + tarenas = (arena_t **)imalloc((ctl_stats.narenas + 1) * + sizeof(arena_t *)); + if (tarenas == NULL) { + idalloc(astats); + return (true); + } + + /* Initialize the new astats element. */ + memcpy(astats, ctl_stats.arenas, (ctl_stats.narenas + 1) * + sizeof(ctl_arena_stats_t)); + memset(&astats[ctl_stats.narenas + 1], 0, sizeof(ctl_arena_stats_t)); + if (ctl_arena_init(&astats[ctl_stats.narenas + 1])) { + idalloc(tarenas); + idalloc(astats); + return (true); + } /* Swap merged stats to their new location. */ { ctl_arena_stats_t tstats; @@ -593,13 +580,34 @@ ctl_grow(void) memcpy(&astats[ctl_stats.narenas + 1], &tstats, sizeof(ctl_arena_stats_t)); } + /* Initialize the new arenas element. */ + tarenas[ctl_stats.narenas] = NULL; + { + arena_t **arenas_old = arenas; + /* + * Swap extended arenas array into place. Although ctl_mtx + * protects this function from other threads extending the + * array, it does not protect from other threads mutating it + * (i.e. initializing arenas and setting array elements to + * point to them). Therefore, array copying must happen under + * the protection of arenas_lock. + */ + malloc_mutex_lock(&arenas_lock); + arenas = tarenas; + memcpy(arenas, arenas_old, ctl_stats.narenas * + sizeof(arena_t *)); + narenas_total++; + arenas_extend(narenas_total - 1); + malloc_mutex_unlock(&arenas_lock); + /* + * Deallocate arenas_old only if it came from imalloc() (not + * base_alloc()). + */ + if (ctl_stats.narenas != narenas_auto) + idalloc(arenas_old); + } ctl_stats.arenas = astats; ctl_stats.narenas++; - malloc_mutex_lock(&arenas_lock); - arenas = tarenas; - narenas_total++; - arenas_extend(narenas_total - 1); - malloc_mutex_unlock(&arenas_lock); return (false); } From d504477935151ed7befb77930f3ca64fa4d4102b Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 20 Oct 2013 15:11:01 -0700 Subject: [PATCH 13/15] Fix a compiler warning. Fix a compiler warning in chunk_record() that was due to reading node rather than xnode. In practice this did not cause any correctness issue, but dataflow analysis in some compilers cannot tell that node and xnode are always equal in cases that the read is reached. --- src/chunk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chunk.c b/src/chunk.c index aef3fede..b17f43f0 100644 --- a/src/chunk.c +++ b/src/chunk.c @@ -294,7 +294,7 @@ label_return: if (xnode != NULL) base_node_dealloc(xnode); if (xprev != NULL) - base_node_dealloc(prev); + base_node_dealloc(xprev); } void From 7a9c8d10b6c412338b39342026831bc444e15c7d Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 20 Oct 2013 19:38:19 -0700 Subject: [PATCH 14/15] Update README. --- README | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/README b/README index 7661683b..9b268f42 100644 --- a/README +++ b/README @@ -1,10 +1,14 @@ -jemalloc is a general-purpose scalable concurrent malloc(3) implementation. -This distribution is a "portable" implementation that currently targets -FreeBSD, Linux, Apple OS X, and MinGW. jemalloc is included as the default -allocator in the FreeBSD and NetBSD operating systems, and it is used by the -Mozilla Firefox web browser on Microsoft Windows-related platforms. Depending -on your needs, one of the other divergent versions may suit your needs better -than this distribution. +jemalloc is a general purpose malloc(3) implementation that emphasizes +fragmentation avoidance and scalable concurrency support. jemalloc first came +into use as the FreeBSD libc allocator in 2005, and since then it has found its +way into numerous applications that rely on its predictable behavior. In 2010 +jemalloc development efforts broadened to include developer support features +such as heap profiling, Valgrind integration, and extensive monitoring/tuning +hooks. Modern jemalloc releases continue to be integrated back into FreeBSD, +and therefore versatility remains critical. Ongoing development efforts trend +toward making jemalloc among the best allocators for a broad range of demanding +applications, and eliminating/mitigating weaknesses that have practical +repercussions for real world applications. The COPYING file contains copyright and licensing information. From 0f7ba3ff2a3f05c990b369bbf67b8bcc9bfbf35b Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 20 Oct 2013 19:40:09 -0700 Subject: [PATCH 15/15] Update ChangeLog for 3.4.1. --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index def7685e..0efc7426 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,7 +6,7 @@ found in the git revision history: http://www.canonware.com/cgi-bin/gitweb.cgi?p=jemalloc.git git://canonware.com/jemalloc.git -* 3.4.1 (XXX) +* 3.4.1 (October 20, 2013) Bug fixes: - Fix a race in the "arenas.extend" mallctl that could cause memory corruption