From e13400c919e6b6730284ff011875bbcdd6821f1c Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Tue, 22 Jan 2019 13:59:23 -0800 Subject: [PATCH] Sanity check szind on tcache flush. This adds some overhead to the tcache flush path (which is one of the popular paths). Guard it behind a config option. --- configure.ac | 16 +++++++ .../internal/jemalloc_internal_defs.h.in | 3 ++ src/tcache.c | 42 ++++++++++++++++++- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index c0911db1..8049ded3 100644 --- a/configure.ac +++ b/configure.ac @@ -1403,6 +1403,22 @@ if test "x$enable_readlinkat" = "x1" ; then fi AC_SUBST([enable_readlinkat]) +dnl Avoid the extra size checking by default +AC_ARG_ENABLE([extra-size-check], + [AS_HELP_STRING([--enable-extra-size-check], + [Perform additonal size related sanity checks])], +[if test "x$enable_extra_size_check" = "xno" ; then + enable_extra_size_check="0" +else + enable_extra_size_check="1" +fi +], +[enable_extra_size_check=="0"] +) +if test "x$enable_extra_size_check" = "x1" ; then + AC_DEFINE([JEMALLOC_EXTRA_SIZE_CHECK], [ ]) +fi +AC_SUBST([enable_extra_size_check]) JE_COMPILABLE([a program using __builtin_unreachable], [ void foo (void) { diff --git a/include/jemalloc/internal/jemalloc_internal_defs.h.in b/include/jemalloc/internal/jemalloc_internal_defs.h.in index 3e94c023..4f0359a8 100644 --- a/include/jemalloc/internal/jemalloc_internal_defs.h.in +++ b/include/jemalloc/internal/jemalloc_internal_defs.h.in @@ -372,4 +372,7 @@ */ #undef JEMALLOC_STRERROR_R_RETURNS_CHAR_WITH_GNU_SOURCE +/* Performs additional size-matching sanity checks when defined. */ +#undef JEMALLOC_EXTRA_SIZE_CHECK + #endif /* JEMALLOC_INTERNAL_DEFS_H_ */ diff --git a/src/tcache.c b/src/tcache.c index 9125179a..be4fb878 100644 --- a/src/tcache.c +++ b/src/tcache.c @@ -100,6 +100,34 @@ tcache_alloc_small_hard(tsdn_t *tsdn, arena_t *arena, tcache_t *tcache, return ret; } +/* Enabled with --enable-extra-size-check. */ +#ifdef JEMALLOC_EXTRA_SIZE_CHECK +static void +tbin_extents_lookup_size_check(tsdn_t *tsdn, cache_bin_t *tbin, szind_t binind, + size_t nflush, extent_t **extents){ + rtree_ctx_t rtree_ctx_fallback; + rtree_ctx_t *rtree_ctx = tsdn_rtree_ctx(tsdn, &rtree_ctx_fallback); + + /* + * Verify that the items in the tcache all have the correct size; this + * is useful for catching sized deallocation bugs, also to fail early + * instead of corrupting metadata. Since this can be turned on for opt + * builds, avoid the branch in the loop. + */ + szind_t szind; + size_t sz_sum = binind * nflush; + for (unsigned i = 0 ; i < nflush; i++) { + rtree_extent_szind_read(tsdn, &extents_rtree, + rtree_ctx, (uintptr_t)*(tbin->avail - 1 - i), true, + &extents[i], &szind); + sz_sum -= szind; + } + if (sz_sum != 0) { + abort(); + } +} +#endif + void tcache_bin_flush_small(tsd_t *tsd, tcache_t *tcache, cache_bin_t *tbin, szind_t binind, unsigned rem) { @@ -112,11 +140,16 @@ tcache_bin_flush_small(tsd_t *tsd, tcache_t *tcache, cache_bin_t *tbin, assert(arena != NULL); unsigned nflush = tbin->ncached - rem; VARIABLE_ARRAY(extent_t *, item_extent, nflush); + +#ifndef JEMALLOC_EXTRA_SIZE_CHECK /* Look up extent once per item. */ for (unsigned i = 0 ; i < nflush; i++) { item_extent[i] = iealloc(tsd_tsdn(tsd), *(tbin->avail - 1 - i)); } - +#else + tbin_extents_lookup_size_check(tsd_tsdn(tsd), tbin, binind, nflush, + item_extent); +#endif while (nflush > 0) { /* Lock the arena bin associated with the first object. */ extent_t *extent = item_extent[0]; @@ -202,11 +235,16 @@ tcache_bin_flush_large(tsd_t *tsd, cache_bin_t *tbin, szind_t binind, assert(tcache_arena != NULL); unsigned nflush = tbin->ncached - rem; VARIABLE_ARRAY(extent_t *, item_extent, nflush); + +#ifndef JEMALLOC_EXTRA_SIZE_CHECK /* Look up extent once per item. */ for (unsigned i = 0 ; i < nflush; i++) { item_extent[i] = iealloc(tsd_tsdn(tsd), *(tbin->avail - 1 - i)); } - +#else + tbin_extents_lookup_size_check(tsd_tsdn(tsd), tbin, binind, nflush, + item_extent); +#endif while (nflush > 0) { /* Lock the arena associated with the first object. */ extent_t *extent = item_extent[0];