From 9dcad2dfd11762ea3b542ef45a20bd8297c6f18c Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 13 Feb 2011 18:11:54 -0800 Subject: [PATCH] Fix "thread.{de,}allocatedp" mallctl. For the non-TLS case (as on OS X), if the "thread.{de,}allocatedp" mallctl was called before any allocation occurred for that thread, the TSD was still NULL, thus putting the application at risk of dereferencing NULL. Fix this by refactoring the initialization code, and making it part of the conditional logic for all per thread allocation counter accesses. --- jemalloc/Makefile.in | 2 + .../jemalloc/internal/jemalloc_internal.h.in | 66 ++++++++++--------- jemalloc/src/ctl.c | 4 +- jemalloc/src/jemalloc.c | 22 +++++++ jemalloc/test/allocated.c | 37 +++++++++++ 5 files changed, 99 insertions(+), 32 deletions(-) diff --git a/jemalloc/Makefile.in b/jemalloc/Makefile.in index ee674b33..eb8a21f6 100644 --- a/jemalloc/Makefile.in +++ b/jemalloc/Makefile.in @@ -72,6 +72,8 @@ CTESTS := @srcroot@test/allocated.c @srcroot@test/allocm.c \ .PHONY: install_html install_man install_doc install .PHONY: tests check clean distclean relclean +.SECONDARY : $(CTESTS:@srcroot@%.c=@objroot@%.o) + # Default target. all: $(DSOS) diff --git a/jemalloc/include/jemalloc/internal/jemalloc_internal.h.in b/jemalloc/include/jemalloc/internal/jemalloc_internal.h.in index 0680b43e..d9bac508 100644 --- a/jemalloc/include/jemalloc/internal/jemalloc_internal.h.in +++ b/jemalloc/include/jemalloc/internal/jemalloc_internal.h.in @@ -240,6 +240,13 @@ extern void (*JEMALLOC_P(malloc_message))(void *wcbopaque, const char *s); #endif #include "jemalloc/internal/prof.h" +#ifdef JEMALLOC_STATS +typedef struct { + uint64_t allocated; + uint64_t deallocated; +} thread_allocated_t; +#endif + #undef JEMALLOC_H_STRUCTS /******************************************************************************/ #define JEMALLOC_H_EXTERNS @@ -295,45 +302,28 @@ extern arena_t **arenas; extern unsigned narenas; #ifdef JEMALLOC_STATS -typedef struct { - uint64_t allocated; - uint64_t deallocated; -} thread_allocated_t; # ifndef NO_TLS extern __thread thread_allocated_t thread_allocated_tls; -# define ALLOCATED_GET() thread_allocated_tls.allocated -# define DEALLOCATED_GET() thread_allocated_tls.deallocated +# define ALLOCATED_GET() (thread_allocated_tls.allocated) +# define ALLOCATEDP_GET() (&thread_allocated_tls.allocated) +# define DEALLOCATED_GET() (thread_allocated_tls.deallocated) +# define DEALLOCATEDP_GET() (&thread_allocated_tls.deallocated) # define ALLOCATED_ADD(a, d) do { \ thread_allocated_tls.allocated += a; \ thread_allocated_tls.deallocated += d; \ } while (0) # else extern pthread_key_t thread_allocated_tsd; -# define ALLOCATED_GET() \ - (uint64_t)((pthread_getspecific(thread_allocated_tsd) != NULL) \ - ? ((thread_allocated_t *) \ - pthread_getspecific(thread_allocated_tsd))->allocated : 0) -# define DEALLOCATED_GET() \ - (uint64_t)((pthread_getspecific(thread_allocated_tsd) != NULL) \ - ? ((thread_allocated_t \ - *)pthread_getspecific(thread_allocated_tsd))->deallocated : \ - 0) +thread_allocated_t *thread_allocated_get_hard(void); + +# define ALLOCATED_GET() (thread_allocated_get()->allocated) +# define ALLOCATEDP_GET() (&thread_allocated_get()->allocated) +# define DEALLOCATED_GET() (thread_allocated_get()->deallocated) +# define DEALLOCATEDP_GET() (&thread_allocated_get()->deallocated) # define ALLOCATED_ADD(a, d) do { \ - thread_allocated_t *thread_allocated = (thread_allocated_t *) \ - pthread_getspecific(thread_allocated_tsd); \ - if (thread_allocated != NULL) { \ - thread_allocated->allocated += (a); \ - thread_allocated->deallocated += (d); \ - } else { \ - thread_allocated = (thread_allocated_t *) \ - imalloc(sizeof(thread_allocated_t)); \ - if (thread_allocated != NULL) { \ - pthread_setspecific(thread_allocated_tsd, \ - thread_allocated); \ - thread_allocated->allocated = (a); \ - thread_allocated->deallocated = (d); \ - } \ - } \ + thread_allocated_t *thread_allocated = thread_allocated_get(); \ + thread_allocated->allocated += (a); \ + thread_allocated->deallocated += (d); \ } while (0) # endif #endif @@ -384,6 +374,9 @@ size_t s2u(size_t size); size_t sa2u(size_t size, size_t alignment, size_t *run_size_p); void malloc_write(const char *s); arena_t *choose_arena(void); +# ifdef NO_TLS +thread_allocated_t *thread_allocated_get(void); +# endif #endif #if (defined(JEMALLOC_ENABLE_INLINE) || defined(JEMALLOC_C_)) @@ -544,6 +537,19 @@ choose_arena(void) return (ret); } + +#ifdef NO_TLS +JEMALLOC_INLINE thread_allocated_t * +thread_allocated_get(void) +{ + thread_allocated_t *thread_allocated = (thread_allocated_t *) + pthread_getspecific(thread_allocated_tsd); + + if (thread_allocated == NULL) + return (thread_allocated_get_hard()); + return (thread_allocated); +} +#endif #endif #include "jemalloc/internal/rtree.h" diff --git a/jemalloc/src/ctl.c b/jemalloc/src/ctl.c index 0b8b06f3..c37b4e75 100644 --- a/jemalloc/src/ctl.c +++ b/jemalloc/src/ctl.c @@ -1151,9 +1151,9 @@ RETURN: #ifdef JEMALLOC_STATS CTL_RO_NL_GEN(thread_allocated, ALLOCATED_GET(), uint64_t); -CTL_RO_NL_GEN(thread_allocatedp, &ALLOCATED_GET(), uint64_t *); +CTL_RO_NL_GEN(thread_allocatedp, ALLOCATEDP_GET(), uint64_t *); CTL_RO_NL_GEN(thread_deallocated, DEALLOCATED_GET(), uint64_t); -CTL_RO_NL_GEN(thread_deallocatedp, &DEALLOCATED_GET(), uint64_t *); +CTL_RO_NL_GEN(thread_deallocatedp, DEALLOCATEDP_GET(), uint64_t *); #endif /******************************************************************************/ diff --git a/jemalloc/src/jemalloc.c b/jemalloc/src/jemalloc.c index f5434c7f..61a36c73 100644 --- a/jemalloc/src/jemalloc.c +++ b/jemalloc/src/jemalloc.c @@ -213,6 +213,28 @@ stats_print_atexit(void) JEMALLOC_P(malloc_stats_print)(NULL, NULL, NULL); } +#if (defined(JEMALLOC_STATS) && defined(NO_TLS)) +thread_allocated_t * +thread_allocated_get_hard(void) +{ + thread_allocated_t *thread_allocated = (thread_allocated_t *) + imalloc(sizeof(thread_allocated_t)); + if (thread_allocated == NULL) { + static thread_allocated_t static_thread_allocated = {0, 0}; + malloc_write(": Error allocating TSD;" + " mallctl(\"thread.{de,}allocated[p]\", ...)" + " will be inaccurate\n"); + if (opt_abort) + abort(); + return (&static_thread_allocated); + } + pthread_setspecific(thread_allocated_tsd, thread_allocated); + thread_allocated->allocated = 0; + thread_allocated->deallocated = 0; + return (thread_allocated); +} +#endif + /* * End miscellaneous support functions. */ diff --git a/jemalloc/test/allocated.c b/jemalloc/test/allocated.c index 64a17351..b1e40e47 100644 --- a/jemalloc/test/allocated.c +++ b/jemalloc/test/allocated.c @@ -16,6 +16,7 @@ thread_start(void *arg) int err; void *p; uint64_t a0, a1, d0, d1; + uint64_t *ap0, *ap1, *dp0, *dp1; size_t sz, usize; sz = sizeof(a0); @@ -31,6 +32,20 @@ thread_start(void *arg) strerror(err)); exit(1); } + sz = sizeof(ap0); + if ((err = JEMALLOC_P(mallctl)("thread.allocatedp", &ap0, &sz, NULL, + 0))) { + if (err == ENOENT) { +#ifdef JEMALLOC_STATS + assert(false); +#endif + goto RETURN; + } + fprintf(stderr, "%s(): Error in mallctl(): %s\n", __func__, + strerror(err)); + exit(1); + } + assert(*ap0 == a0); sz = sizeof(d0); if ((err = JEMALLOC_P(mallctl)("thread.deallocated", &d0, &sz, NULL, @@ -45,6 +60,20 @@ thread_start(void *arg) strerror(err)); exit(1); } + sz = sizeof(dp0); + if ((err = JEMALLOC_P(mallctl)("thread.deallocatedp", &dp0, &sz, NULL, + 0))) { + if (err == ENOENT) { +#ifdef JEMALLOC_STATS + assert(false); +#endif + goto RETURN; + } + fprintf(stderr, "%s(): Error in mallctl(): %s\n", __func__, + strerror(err)); + exit(1); + } + assert(*dp0 == d0); p = JEMALLOC_P(malloc)(1); if (p == NULL) { @@ -54,6 +83,10 @@ thread_start(void *arg) sz = sizeof(a1); JEMALLOC_P(mallctl)("thread.allocated", &a1, &sz, NULL, 0); + sz = sizeof(ap1); + JEMALLOC_P(mallctl)("thread.allocatedp", &ap1, &sz, NULL, 0); + assert(*ap1 == a1); + assert(ap0 == ap1); usize = JEMALLOC_P(malloc_usable_size)(p); assert(a0 + usize <= a1); @@ -62,6 +95,10 @@ thread_start(void *arg) sz = sizeof(d1); JEMALLOC_P(mallctl)("thread.deallocated", &d1, &sz, NULL, 0); + sz = sizeof(dp1); + JEMALLOC_P(mallctl)("thread.deallocatedp", &dp1, &sz, NULL, 0); + assert(*dp1 == d1); + assert(dp0 == dp1); assert(d0 + usize <= d1);