This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] malloc: simplify and fix calloc
- From: Will Newton <will dot newton at gmail dot com>
- To: Joern Engel <joern at purestorage dot com>
- Cc: "GNU C. Library" <libc-alpha at sourceware dot org>, Siddhesh Poyarekar <siddhesh dot poyarekar at gmail dot com>
- Date: Tue, 26 Jan 2016 10:32:19 +0000
- Subject: Re: [PATCH] malloc: simplify and fix calloc
- Authentication-results: sourceware.org; auth=none
- References: <1453767942-19369-1-git-send-email-joern at purestorage dot com> <1453767942-19369-48-git-send-email-joern at purestorage dot com>
On Tue, Jan 26, 2016 at 12:25 AM, Joern Engel <joern@purestorage.com> wrote:
> Calloc was essentially a copy of malloc that tried to skip the memset if
> the memory was just returned from mmap() and is already cleared. It was
> also buggy and a suitable test could trigger a segfault.
Would it be possible to add such a test to the testsuite?
> New code only does the overflow check, malloc and memset. Runtime cost
> is lower than maintenance cost of already-buggy code.
>
> JIRA: PURE-27597
> JIRA: PURE-36718
> ---
> tpc/malloc2.13/malloc.c | 106 +++---------------------------------------------
> 1 file changed, 5 insertions(+), 101 deletions(-)
>
> diff --git a/tpc/malloc2.13/malloc.c b/tpc/malloc2.13/malloc.c
> index d9fecfe3f921..190c1d24b082 100644
> --- a/tpc/malloc2.13/malloc.c
> +++ b/tpc/malloc2.13/malloc.c
> @@ -3506,13 +3506,8 @@ Void_t *public_pVALLOc(size_t bytes)
>
> Void_t *public_cALLOc(size_t n, size_t elem_size)
> {
> - struct malloc_state *av;
> - mchunkptr oldtop, p;
> - INTERNAL_SIZE_T bytes, sz, csz, oldtopsize;
> + INTERNAL_SIZE_T bytes;
> Void_t *mem;
> - unsigned long clearsize;
> - unsigned long nclears;
> - INTERNAL_SIZE_T *d;
>
> /* size_t is unsigned so the behavior on overflow is defined. */
> bytes = n * elem_size;
> @@ -3525,101 +3520,10 @@ Void_t *public_cALLOc(size_t n, size_t elem_size)
> }
> }
>
> - __malloc_ptr_t(*hook) __MALLOC_PMT((size_t, __const __malloc_ptr_t)) = force_reg(dlmalloc_hook);
> - if (hook != NULL) {
> - sz = bytes;
> - mem = (*hook) (sz, RETURN_ADDRESS(0));
> - if (mem == 0)
> - return 0;
> -#ifdef HAVE_MEMCPY
> - return memset(mem, 0, sz);
> -#else
> - while (sz > 0)
> - ((char *)mem)[--sz] = 0; /* rather inefficient */
> - return mem;
> -#endif
> - }
> -
> - sz = bytes;
> -
> - av = arena_get(sz);
> - if (!av)
> - return 0;
> -
> - /* Check if we hand out the top chunk, in which case there may be no
> - need to clear. */
> -#if MORECORE_CLEARS
> - oldtop = top(av);
> - oldtopsize = chunksize(top(av));
> -#if MORECORE_CLEARS < 2
> - /* Only newly allocated memory is guaranteed to be cleared. */
> - if (av == &main_arena && oldtopsize < mp_.sbrk_base + av->max_system_mem - (char *)oldtop)
> - oldtopsize = (mp_.sbrk_base + av->max_system_mem - (char *)oldtop);
> -#endif
> - if (av != &main_arena) {
> - heap_info *heap = heap_for_ptr(oldtop);
> - if (oldtopsize < (char *)heap + heap->mprotect_size - (char *)oldtop)
> - oldtopsize = (char *)heap + heap->mprotect_size - (char *)oldtop;
> - }
> -#endif
> - mem = _int_malloc(av, sz);
> - if (mem == 0) {
> - av = get_backup_arena(av, bytes);
> - mem = _int_malloc(&main_arena, sz);
> - }
> - arena_unlock(av);
> -
> - assert(!mem || chunk_is_mmapped(mem2chunk(mem)) || av == arena_for_chunk(mem2chunk(mem)));
> - if (mem == 0)
> - return 0;
> - p = mem2chunk(mem);
> -
> - /* Two optional cases in which clearing not necessary */
> - if (chunk_is_mmapped(p)) {
> - if (perturb_byte)
> - MALLOC_ZERO(mem, sz);
> - return mem;
> - }
> -
> - csz = chunksize(p);
> -
> -#if MORECORE_CLEARS
> - if (perturb_byte == 0 && (p == oldtop && csz > oldtopsize)) {
> - /* clear only the bytes from non-freshly-sbrked memory */
> - csz = oldtopsize;
> - }
> -#endif
> -
> - /* Unroll clear of <= 36 bytes (72 if 8byte sizes). We know that
> - contents have an odd number of INTERNAL_SIZE_T-sized words;
> - minimally 3. */
> - d = (INTERNAL_SIZE_T *) mem;
> - clearsize = csz - SIZE_SZ;
> - nclears = clearsize / sizeof(INTERNAL_SIZE_T);
> - assert(nclears >= 3);
> -
> - if (nclears > 9)
> - MALLOC_ZERO(d, clearsize);
> -
> - else {
> - *(d + 0) = 0;
> - *(d + 1) = 0;
> - *(d + 2) = 0;
> - if (nclears > 4) {
> - *(d + 3) = 0;
> - *(d + 4) = 0;
> - if (nclears > 6) {
> - *(d + 5) = 0;
> - *(d + 6) = 0;
> - if (nclears > 8) {
> - *(d + 7) = 0;
> - *(d + 8) = 0;
> - }
> - }
> - }
> - }
> -
> - return mem;
> + mem = public_mALLOc(bytes);
> + if (!mem)
> + return NULL;
> + return memset(mem, 0, bytes);
> }
>
>
> --
> 2.7.0.rc3
>