TS-1891: Add double-free checking for reclaimable freelist The double-free checking for reclaimable freelist is very useful for us to validate the correctness of memory usage by upper layer code.
This feature is effective only when in debug mode. Signed-off-by: Yunkai Zhang <[email protected]> Signed-off-by: Zhao Yongming <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/7d9aaf90 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/7d9aaf90 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/7d9aaf90 Branch: refs/heads/3.3.x Commit: 7d9aaf902424d49784ec10071189421ec3896929 Parents: 912e8c1 Author: Yunkai Zhang <[email protected]> Authored: Sun May 12 17:31:37 2013 +0800 Committer: Zhao Yongming <[email protected]> Committed: Fri May 31 08:15:40 2013 +0800 ---------------------------------------------------------------------- CHANGES | 3 + lib/ts/ink_queue_ext.cc | 101 ++++++++++++++++++++++++++++++++++++++--- lib/ts/ink_queue_ext.h | 8 +++ 3 files changed, 104 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7d9aaf90/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 127f637..b2c902b 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,9 @@ Changes with Apache Traffic Server 3.3.3 + *) [TS-1891] Add double-free checking for reclaimable freelist + Author: Yunkai Zhang <[email protected]> + *) [TS-1926] Require Lua v5.1, by checking for lua_getfenv(). This is necessary since we are incompatible with Lua v5.2 (for now). http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7d9aaf90/lib/ts/ink_queue_ext.cc ---------------------------------------------------------------------- diff --git a/lib/ts/ink_queue_ext.cc b/lib/ts/ink_queue_ext.cc index f9e3f60..9cf7ba1 100644 --- a/lib/ts/ink_queue_ext.cc +++ b/lib/ts/ink_queue_ext.cc @@ -47,6 +47,7 @@ #define CEIL(x,y) (((x) + (y) - 1L) / (y)) #define ROUND(x,l) (((x) + ((l) - 1L)) & ~((l) - 1L)) +#define ITEM_MAGIC 0xFF #define MAX_NUM_FREELIST 1024 @@ -108,12 +109,20 @@ static inline void memory_alignment_init(InkFreeList *f, uint32_t type_size, uint32_t chunk_size, uint32_t alignment) { - uint32_t chunk_byte_size, user_alignment; + uint32_t chunk_byte_size, user_alignment, user_type_size; f->chunk_size_base = chunk_size; user_alignment = alignment; + user_type_size = type_size; chunk_size = 1; +#ifdef DEBUG + /* + * enlarge type_size to hold a item_magic + */ + type_size += sizeof(unsigned char); +#endif + /* * limit the size of each chunk and resize alignment. * 1) when size of chunk > MAX_CHUNK_BYTE_SIZE: @@ -152,7 +161,7 @@ memory_alignment_init(InkFreeList *f, uint32_t type_size, uint32_t chunk_size, ink_release_assert(alignment <= MAX_CHUNK_BYTE_SIZE); f->alignment = alignment; - f->type_size = type_size; + f->type_size = user_type_size; f->chunk_size = chunk_size; f->chunk_addr_mask = ~((uintptr_t)(alignment - 1)); f->chunk_byte_size = chunk_byte_size; @@ -183,7 +192,11 @@ mmap_align(size_t size, size_t alignment) { -1, 0); if (result == MAP_FAILED) { ink_stack_trace_dump(); - ink_fatal(1, "Failed to mmap %zu bytes, %s", size, strerror(errno)); + const char *err_str = "Out of memory, or the process's maximum number of " + "mappings would have been exceeded(if so, you can " + "enlarge 'vm.max_map_count' by sysctl in linux)."; + ink_fatal(1, "Failed to mmap %zu bytes, %s", size, + (errno == ENOMEM) ? err_str : strerror(errno)); } /* adjust the return memory so it is aligned */ @@ -206,6 +219,62 @@ mmap_align(size_t size, size_t alignment) { return (void*)ptr; } +#ifdef DEBUG +static inline uint32_t +get_chunk_item_magic_idx(InkFreeList *f, void *item, InkChunkInfo **ppChunk, + bool do_check = false) +{ + uint32_t idx; + uintptr_t chunk_addr; + + if (f->chunk_size > 1) + chunk_addr = (uintptr_t)item & f->chunk_addr_mask; + else + chunk_addr = (uintptr_t)item; + + if (*ppChunk == NULL) + *ppChunk = (InkChunkInfo *)(chunk_addr + f->type_size * f->chunk_size); + + idx = ((uintptr_t)item - chunk_addr) / f->type_size; + + if (do_check && (idx >= f->chunk_size + || ((uintptr_t)item - chunk_addr) % f->type_size)) { + ink_stack_trace_dump(); + ink_fatal(1, "Invalid address:%p, chunk_addr:%p, type_size:%d, chunk_size:%u, idx:%u", + item, (void *)chunk_addr, f->type_size, f->chunk_size, idx); + } + + return idx; +} + +static inline void +set_chunk_item_magic(InkFreeList *f, InkChunkInfo *pChunk, void *item) +{ + uint32_t idx; + + idx = get_chunk_item_magic_idx(f, item, &pChunk); + + ink_release_assert(pChunk->item_magic[idx] == 0); + + pChunk->item_magic[idx] = ITEM_MAGIC; +} + +static inline void +clear_chunk_item_magic(InkFreeList *f, InkChunkInfo *pChunk, void *item) +{ + uint32_t idx; + + idx = get_chunk_item_magic_idx(f, item, &pChunk, true); + + ink_release_assert(pChunk->item_magic[idx] == ITEM_MAGIC); + + pChunk->item_magic[idx] = 0; +} +#else +#define set_chunk_item_magic(a, b, c) +#define clear_chunk_item_magic(a, b, c) +#endif + static inline InkChunkInfo * get_chunk_info_addr(InkFreeList *f, void *item) { @@ -242,6 +311,17 @@ ink_chunk_create(InkFreeList *f, InkThreadCache *pCache) pChunk->pThreadCache = pCache; pChunk->link = Link<InkChunkInfo>(); +#ifdef DEBUG + /* + * The content will be initialized to zero when + * calls mmap() with MAP_ANONYMOUS flag on linux + * platform. + */ +#if !defined(linux) + memset(pChunk->item_magic, 0, chunk_size * sizeof(unsigned char)); +#endif +#endif + curr = pChunk->head; pChunk->inner_free_list = curr; for (i = 1; i < chunk_size; i++) { @@ -446,7 +526,7 @@ reclaimable_freelist_init(InkFreeList **fl, const char *name, * ink_freelist_init() is only called from single-threaded * initialization code. */ while (fll) { - /* Reuse InkFreeList if it has the same aligned type_size. */ + /* Reuse InkFreeList if it has the same type_size. */ if (fll->fl->type_size == type_size) { fll->fl->refcnt++; *fl = fll->fl; @@ -528,6 +608,7 @@ reclaimable_freelist_new(InkFreeList *f) ink_mutex_release(&f->lock); ptr = malloc_whole_chunk(f, pCache, pChunk); + set_chunk_item_magic(f, pChunk, ptr); return ptr; } @@ -538,6 +619,7 @@ reclaimable_freelist_new(InkFreeList *f) old_value = ink_atomic_increment((int *)&pCache->nr_free, -1); ink_release_assert(old_value > 0); ink_atomic_increment(&pCache->nr_malloc, 1); + set_chunk_item_magic(f, NULL, ptr); return ptr; } @@ -545,10 +627,11 @@ reclaimable_freelist_new(InkFreeList *f) pNextCache = pCache->next; while (pNextCache != pCache) { if ((ptr = ink_atomiclist_pop(&pNextCache->outer_free_list))) { - old_value = ink_atomic_increment((int *)&pNextCache->nr_free, -1); - ink_release_assert(old_value > 0); - ink_atomic_increment(&pNextCache->nr_malloc, 1); - return ptr; + old_value = ink_atomic_increment((int *)&pNextCache->nr_free, -1); + ink_release_assert(old_value > 0); + ink_atomic_increment(&pNextCache->nr_malloc, 1); + set_chunk_item_magic(f, NULL, ptr); + return ptr; } pNextCache = pNextCache->next; } @@ -582,6 +665,7 @@ reclaimable_freelist_new(InkFreeList *f) refresh_average_info(pCache); ink_atomic_increment(&pCache->nr_malloc, 1); + set_chunk_item_magic(f, NULL, ptr); return ptr; } @@ -595,6 +679,7 @@ reclaimable_freelist_free(InkFreeList *f, void *item) return; pChunk = get_chunk_info_addr(f, item); + clear_chunk_item_magic(f, pChunk, item); pCache = pChunk->pThreadCache; ink_atomic_increment((int *)&pCache->nr_malloc, -1); http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7d9aaf90/lib/ts/ink_queue_ext.h ---------------------------------------------------------------------- diff --git a/lib/ts/ink_queue_ext.h b/lib/ts/ink_queue_ext.h index fbb2704..cfe91fa 100644 --- a/lib/ts/ink_queue_ext.h +++ b/lib/ts/ink_queue_ext.h @@ -60,6 +60,14 @@ extern "C" struct _InkThreadCache *pThreadCache; LINK(_InkChunkInfo, link); + +#ifdef DEBUG + /* + * magic code for each item, + * it's used to check double-free issue. + */ + unsigned char item_magic[0]; +#endif } InkChunkInfo; typedef struct _InkThreadCache
