From f3b2668b3219e108348b9a28d00c4f805a1b5ab6 Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Fri, 5 Feb 2021 16:47:09 -0800 Subject: [PATCH] Report the offending pointer on sized dealloc bug detection. --- include/jemalloc/internal/arena_inlines_b.h | 9 +++++---- include/jemalloc/internal/safety_check.h | 2 +- src/jemalloc.c | 5 +++-- src/safety_check.c | 8 ++++---- src/tcache.c | 21 ++++++++++++++++++--- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/include/jemalloc/internal/arena_inlines_b.h b/include/jemalloc/internal/arena_inlines_b.h index 13e6eb52..5df8e858 100644 --- a/include/jemalloc/internal/arena_inlines_b.h +++ b/include/jemalloc/internal/arena_inlines_b.h @@ -211,7 +211,7 @@ arena_vsalloc(tsdn_t *tsdn, const void *ptr) { } JEMALLOC_ALWAYS_INLINE bool -large_dalloc_safety_checks(edata_t *edata, szind_t szind) { +large_dalloc_safety_checks(edata_t *edata, void *ptr, szind_t szind) { if (!config_opt_safety_checks) { return false; } @@ -229,7 +229,8 @@ large_dalloc_safety_checks(edata_t *edata, szind_t szind) { return true; } if (unlikely(sz_index2size(szind) != edata_usize_get(edata))) { - safety_check_fail_sized_dealloc(/* current_dealloc */ true); + safety_check_fail_sized_dealloc(/* current_dealloc */ true, + ptr); return true; } @@ -243,7 +244,7 @@ arena_dalloc_large_no_tcache(tsdn_t *tsdn, void *ptr, szind_t szind) { } else { edata_t *edata = emap_edata_lookup(tsdn, &arena_emap_global, ptr); - if (large_dalloc_safety_checks(edata, szind)) { + if (large_dalloc_safety_checks(edata, ptr, szind)) { /* See the comment in isfree. */ return; } @@ -287,7 +288,7 @@ arena_dalloc_large(tsdn_t *tsdn, void *ptr, tcache_t *tcache, szind_t szind, } else { edata_t *edata = emap_edata_lookup(tsdn, &arena_emap_global, ptr); - if (large_dalloc_safety_checks(edata, szind)) { + if (large_dalloc_safety_checks(edata, ptr, szind)) { /* See the comment in isfree. */ return; } diff --git a/include/jemalloc/internal/safety_check.h b/include/jemalloc/internal/safety_check.h index a7a44338..b27ac088 100644 --- a/include/jemalloc/internal/safety_check.h +++ b/include/jemalloc/internal/safety_check.h @@ -1,7 +1,7 @@ #ifndef JEMALLOC_INTERNAL_SAFETY_CHECK_H #define JEMALLOC_INTERNAL_SAFETY_CHECK_H -void safety_check_fail_sized_dealloc(bool current_dealloc); +void safety_check_fail_sized_dealloc(bool current_dealloc, const void *ptr); void safety_check_fail(const char *format, ...); /* Can set to NULL for a default. */ void safety_check_set_abort(void (*abort_fn)(const char *)); diff --git a/src/jemalloc.c b/src/jemalloc.c index dc3c98b6..9d038806 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -2751,7 +2751,7 @@ maybe_check_alloc_ctx(tsd_t *tsd, void *ptr, emap_alloc_ctx_t *alloc_ctx) { &dbg_ctx); if (alloc_ctx->szind != dbg_ctx.szind) { safety_check_fail_sized_dealloc( - /* current_dealloc */ true); + /* current_dealloc */ true, ptr); return true; } if (alloc_ctx->slab != dbg_ctx.slab) { @@ -2801,7 +2801,8 @@ isfree(tsd_t *tsd, void *ptr, size_t usize, tcache_t *tcache, bool slow_path) { if (config_opt_safety_checks) { /* Small alloc may have !slab (sampled). */ if (alloc_ctx.szind != sz_size2index(usize)) { - safety_check_fail_sized_dealloc(true); + safety_check_fail_sized_dealloc(true, + ptr); } } } else { diff --git a/src/safety_check.c b/src/safety_check.c index c692835a..0dff9348 100644 --- a/src/safety_check.c +++ b/src/safety_check.c @@ -3,14 +3,14 @@ static void (*safety_check_abort)(const char *message); -void safety_check_fail_sized_dealloc(bool current_dealloc) { +void safety_check_fail_sized_dealloc(bool current_dealloc, const void *ptr) { char *src = current_dealloc ? "the current pointer being freed" : "in thread cache, possibly from previous deallocations"; safety_check_fail(": size mismatch detected, likely caused by" - " application sized deallocation bugs (source: %s). Suggest building" - "with --enable-debug or address sanitizer for debugging. Abort.\n", - src); + " application sized deallocation bugs (source address: %p, %s). " + "Suggest building with --enable-debug or address sanitizer for " + "debugging. Abort.\n", ptr, src); } void safety_check_set_abort(void (*abort_fn)(const char *)) { diff --git a/src/tcache.c b/src/tcache.c index 7c4047f4..3489e724 100644 --- a/src/tcache.c +++ b/src/tcache.c @@ -250,6 +250,20 @@ tcache_bin_flush_metadata_visitor(void *szind_sum_ctx, util_prefetch_write_range(alloc_ctx->edata, sizeof(edata_t)); } +JEMALLOC_NOINLINE static void +tcache_bin_flush_size_check_fail(cache_bin_ptr_array_t *arr, szind_t szind, + size_t nptrs, emap_batch_lookup_result_t *edatas) { + bool found_mismatch = false; + for (size_t i = 0; i < nptrs; i++) { + if (edata_szind_get(edatas[i].edata) != szind) { + found_mismatch = true; + safety_check_fail_sized_dealloc(false, + tcache_bin_flush_ptr_getter(arr, i)); + } + } + assert(found_mismatch); +} + static void tcache_bin_flush_edatas_lookup(tsd_t *tsd, cache_bin_ptr_array_t *arr, szind_t binind, size_t nflush, emap_batch_lookup_result_t *edatas) { @@ -264,8 +278,8 @@ tcache_bin_flush_edatas_lookup(tsd_t *tsd, cache_bin_ptr_array_t *arr, &tcache_bin_flush_ptr_getter, (void *)arr, &tcache_bin_flush_metadata_visitor, (void *)&szind_sum, edatas); - if (config_opt_safety_checks && szind_sum != 0) { - safety_check_fail_sized_dealloc(false); + if (config_opt_safety_checks && unlikely(szind_sum != 0)) { + tcache_bin_flush_size_check_fail(arr, binind, nflush, edatas); } } @@ -435,7 +449,8 @@ tcache_bin_flush_impl(tsd_t *tsd, tcache_t *tcache, cache_bin_t *cache_bin, dalloc_count++; } } else { - if (large_dalloc_safety_checks(edata, binind)) { + if (large_dalloc_safety_checks(edata, ptr, + binind)) { /* See the comment in isfree. */ continue; }